cloudspannerecosystem / spanner-cli

Interactive command line tool for Cloud Spanner
Apache License 2.0
230 stars 28 forks source link

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

Closed iwinux closed 1 month ago

iwinux commented 1 month ago

The --endpoint flag (added by https://github.com/cloudspannerecosystem/spanner-cli/pull/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.

google-cla[bot] commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

@yfuruyama I've signed it just now.

yfuruyama commented 1 month ago

@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 commented 1 month ago

@yfuruyama Done

yfuruyama commented 1 month ago

@iwinux Thank you!