cli: don't scope TLS client certs to a specific tenant by default #97585
+24
−65
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
knz
requested review from
herkolategan and
renatolabs
and removed request for
a team
|
This change is |
This commit changes the default for `--tenant-scope` from "only the system tenant" to "cert valid for all tenants". Note that the scoping is generally useful for security, and it is used in CockroachCloud. However, CockroachCloud does not use our CLI code to generate certs and sets its cert tenant scopes on its own. Given that our CLI code is provided for convenience and developer productivity, and we don't expect certs generated here to be used in multi-tenant deployments where tenants are adversarial to each other, defaulting to certs that are valid on every tenant is a good choice. Release note: None
knz
force-pushed
the
20230223-tenant-scope
branch
from
0a62ea7 to
c50bd93
Compare
stevendanna
approved these changes
Feb 24, 2023
|
TFYR! bors r=stevendanna |
|
Build succeeded: |
healthy-pod
pushed a commit
to healthy-pod/cockroach
that referenced
this pull request
Mar 8, 2023
Previously, TLS client certs were scoped to the system tenant by default. After cockroachdb#97585, they became valid for all tenants by default (unless `--tenant-scope` is passed). This caused the multitenant-upgrade roachtest to fail when certs are generated using the latest roachprod binary from an old cockroach binary. This change ensures that such certs are not scoped to the system tenant only by passing `--tenant-scope`. Release note: None Epic: none Closes cockroachdb#97016
craig bot
pushed a commit
that referenced
this pull request
Jun 14, 2023
104772: roachtest: remove unused create tenant options r=srosenberg,stevendanna a=herkolategan The `otherTenantIDs` field on `createTenantOptions` is no longer used. Complimentary to this change: #97585 Epic: [CRDB-23559](https://cockroachlabs.atlassian.net/browse/CRDB-23559) 104777: roachprod: fix confusing start-up error r=srosenberg a=herkolategan Starting a secure cluster locally does a check for certificates. In the event the certificates are not found, which is a valid case, an error is printed. The start command works correctly, but the error can cause confusion: ```bash ./bin/roachprod start local --secure 12:47:56 cluster_synced.go:2475: 0: COMMAND_PROBLEM: exit status 1 (1) COMMAND_PROBLEM Wraps: (2) exit status 1 Error types: (1) errors.Cmd (2) *exec.ExitError: local: initializing certs 1/1 / local: distributing certs 2/2 12:47:59 cockroach.go:184: local: starting nodes ``` This change modifies the command to do a check without causing an error, and also propagates any real errors that could occur while doing the check. Epic: none Co-authored-by: Herko Lategan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Epic: CRDB-23559
Fixes: #97584
This commit changes the default for
--tenant-scopefrom "only the system tenant" to "cert valid for all tenants".Note that the scoping is generally useful for security, and it is used in CockroachCloud. However, CockroachCloud does not use our CLI code to generate certs and sets its cert tenant scopes on its own.
Given that our CLI code is provided for convenience and developer productivity, and we don't expect certs generated here to be used in multi-tenant deployments where tenants are adversarial to each other, defaulting to certs that are valid on every tenant is a good choice.
Release note: None