Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CLI flag --skip-tls-verify #194

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

iwinux
Copy link
Contributor

@iwinux iwinux commented Oct 15, 2024

The --endpoint flag (added by #162) allows spanner-cli to connects to a custom API endpoint.

However, gRPC will only send credentials (specified by --credentials or env GOOGLE_APPLICATION_CREDENTIALS) when TLS is enabled. To ease some use cases during local testing, this pull request adds --skip-tls-verify which implicitly enables TLS while skipping certificate verification.

Copy link

google-cla bot commented Oct 15, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@yfuruyama
Copy link
Collaborator

To ease some use cases during local testing

Could you elaborate on your use case where you have to skip certificate verification?

I think #162 was supposed to be used for some endpoints listed the following doc, but every endpoint uses proper TLS ceritification: https://cloud.google.com/spanner/docs/endpoints

@iwinux
Copy link
Contributor Author

iwinux commented Oct 17, 2024

The use case involves sending Spanner queries through a local gRPC proxy (for internal logging purpose). The proxy itself is using TLS with self-signed certificate. As of security concern, the usage is not exposed to public Internet, therefore skipping verification is acceptable.

@yfuruyama
Copy link
Collaborator

@iwinux Thank you for sharing that. That sounds a reasonable use case.

I want to merge this, but looks like CLA is not signed yet.

@iwinux
Copy link
Contributor Author

iwinux commented Oct 18, 2024

@yfuruyama I've signed it just now.

@yfuruyama
Copy link
Collaborator

@iwinux Thanks! Could you update README with a new option? https://github.com/cloudspannerecosystem/spanner-cli/blob/master/README.md?plain=1#L33-L57

@iwinux
Copy link
Contributor Author

iwinux commented Oct 21, 2024

@yfuruyama Done

@yfuruyama
Copy link
Collaborator

@iwinux Thank you!

@yfuruyama yfuruyama self-requested a review October 21, 2024 02:11
@yfuruyama yfuruyama merged commit dd056dc into cloudspannerecosystem:master Oct 21, 2024
2 checks passed
@iwinux iwinux deleted the skip-tls-verify branch October 21, 2024 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants