From 21764da8cb1ca6950ac58c1dc99c0b9ae257184b Mon Sep 17 00:00:00 2001 From: Alex Barganier Date: Mon, 18 Jul 2022 13:40:24 -0400 Subject: [PATCH] tenant: use `-h` to check if tenant scoped client certs available Previously, whether the test server created tenant-scoped client certificates for tests was based on a hardcoded version gate. This was sufficient in the past, but as tenant-scoped client certs are now being backported to older cockroachdb versions, a more dynamic approach to determine whether or not these certificates are available is needed. This patch adds a mechanism to do so. The new approach runs the `cockroach cert create-client --help` command to view the available flags for the current cockroach binary. If the `--tenant-scope` flag is present in the help text, then we can say with confidence that tenant scoped client certificates are available. We can use this to signal the broader system to make use of these certificates when running tests in secure mode. This follows the approach used in: https://github.com/cockroachdb/cockroach/pull/83703 --- testserver/tenant.go | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/testserver/tenant.go b/testserver/tenant.go index 958f13d..e3bdfe4 100644 --- a/testserver/tenant.go +++ b/testserver/tenant.go @@ -15,6 +15,7 @@ package testserver import ( + "bytes" "database/sql" "errors" "fmt" @@ -36,6 +37,25 @@ func (ts *testServerImpl) isTenant() bool { return ts.curTenantID < firstTenantID } +// cockroachSupportsTenantScopeCert is a hack to figure out if the version of +// cockroach on the test server supports tenant scoped certificates. This is less +// brittle than a static version comparison as these tenant scoped certificates are +// subject to backports to older CRDB verions. +func (ts *testServerImpl) cockroachSupportsTenantScopeCert() (bool, error) { + certCmdArgs := []string{ + "cert", + "create-client", + "--help", + } + checkTenantScopeCertCmd := exec.Command(ts.serverArgs.cockroachBinary, certCmdArgs...) + var output bytes.Buffer + checkTenantScopeCertCmd.Stdout = &output + if err := checkTenantScopeCertCmd.Run(); err != nil { + return false, err + } + return strings.Contains(output.String(), "--tenant-scope"), nil +} + // NewTenantServer creates and returns a new SQL tenant pointed at the receiver, // which acts as a KV server, and starts it. // The SQL tenant is responsible for all SQL processing and does not store any @@ -87,7 +107,11 @@ func (ts *testServerImpl) NewTenantServer(proxy bool) (TestServer, error) { if err := createCertCmd.Run(); err != nil { return nil, fmt.Errorf("%s command %s failed: %w", tenantserverMessagePrefix, createCertCmd, err) } - if ts.version.AtLeast(version.MustParse("v22.2.0-alpha")) { + tenantScopeCertsAvailable, err := ts.cockroachSupportsTenantScopeCert() + if err != nil { + return nil, fmt.Errorf("failed to determine if tenant scoped certificates are available: %w", err) + } + if tenantScopeCertsAvailable { // Overwrite root client certificate scoped to the system and current tenant. // Tenant scoping is needed for client certificates used to access tenant servers. tenantScopedClientCertArgs := []string{