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
release-21.2: sqlproxyccl: support secure connections to SQL backends #70290
Conversation
Previously, when establishing a TLS connection to the SQL backend, the sqlproxy failed to set .ServerName on the tls.Config. The result was the error `tls: either ServerName or InsecureSkipVerify must be specified in the tls.Config` whenever .SkipVerify was false. This behavior made it impossible to establish verified secure connections to SQL backends. This commit properly sets .ServerName based on the outgoingAddress returned by the tenantdir service. Release note: None Release justification: Having a verified TLS connection betweeen the SQLProxy and SQL Pods in Cockroach Serverless is a requirement for the beta release. This code change enables that secure connection without making any changes to CockroachDB, itself.
5e806c4
to
b94d0a1
Compare
Thanks for opening a backport. Please check the backport criteria before merging:
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
Add a brief release justification to the body of your PR to justify this backport. Some other things to consider:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @catj-cockroach, @chrisseto, and @darinpp)
Previously, the sqlproxy would parse incoming and outgoing addresses using golang's net.SplitHostPort function. This was inadequate because net.SplitHostPort does not know how to correctly parse IPv6 addresses. This commit correctly uses addr.SplitHostPort to ensure that both IPv4 and IPv6 are supported. This commit also includes an updated release note for #69959. Release note (bug fix, security update): cockroach mt start-proxy now appropriately sets the .ServerName member of outgoing TLS connections. This allows the proxy to function appropriately when the --insecure and --skip-verify CLI flags are ommitted. Release justification: Having a verified TLS connection betweeen the SQLProxy and SQL Pods in Cockroach Serverless is a requirement for the beta release. This code change enables that secure connection without making any changes to CockroachDB, itself.
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from b94d0a1 to blathers/backport-release-21.2-70290: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 21.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
Backport 1/1 commits from #69959 on behalf of @chrisseto.
/cc @cockroachdb/release
Release note: None
Release justification: Having a verified TLS connection betweeen the
SQLProxy and SQL Pods in Cockroach Serverless is a requirement for the
beta release. This code change enables that secure connection without
making any changes to CockroachDB, itself.