rpc,server: use node certs for shared-process tenant RPCs #96553
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
Goroutine / heap / profile dump generation is non-deterministic. The reference test output should not include them. Release note: None
This will protect against programming errors. Release note: None
Release note: None
Release note: None
|
This change is |
knz
force-pushed
the
20230204-rpc-client
branch
2 times, most recently
from
e8048d7 to
f5dfd09
Compare
The security package was using pre-go 1.13 error types. This was crufty and also caused the error payloads to be unduly redacted in logs. This patch fixes it. Release note: None
This constructor makes an object that contains a mutex lock. We have linters that find it illegal to copy an object that contains a lock. So this patch fixes that by ensuring that struct is instantiated on the heap. Release note: None
Prior to this patch, `GetClientTLSConfig()` was used both to obtain a TLS config suitable for a standalone/CLI RPC client and also for the TLS config inside a KV server. This was confusing and error prone. We really have two cases and they are quite different: - a standalone RPC client (e.g. a CLI tool) connecting to a remote RPC server, under some non-node identity. This is what `GetClientTLSConfig()` is for. - a RPC server for a KV node that's also RPC client of another KV node. In that case, this server can know it has server (node) certs to work with. This is the new `GetNodeClientTLSConfig()` - a RPC server for a secondary tenant server that's also RPC client to something else (either another server for the secondary tenant, or a KV node). This is what `GetTenantTLSConfig()` is for. Release note: None
When a tenant server running in the same process as a KV node makes an outgoing RPC, it can use the same node certs as the KV node. This has two advantages: - it is simpler to orchestrate; this way we do not need to provision the tenant client certs manually; - it paves the road for using a single RPC server for all tenants on a server (future work). Release note: None
Now that the RPC code is able to perform tenant-to-tenant RPCs without tenant client certs, we can demonstrate this by disabling the auto-generation of a tenant client cert when shared-process servers are used. Release note: None
knz
force-pushed
the
20230204-rpc-client
branch
from
f5dfd09 to
fb76cc9
Compare
stevendanna
approved these changes
Feb 7, 2023
|
thank you! bors r=stevendanna |
|
Build failed: |
|
bors r=stevendanna |
|
Build succeeded: |
aadityasondhi
added a commit
to aadityasondhi/cockroach
that referenced
this pull request
Mar 16, 2023
This patch unskips and re-records the datadriven `TestTenantZip` as it was fixed in cockroachdb#96553, but was not unskipped or recorded. The test was run locally using `--stress` and did not flake: ``` 101 runs so far, 0 failures, over 5m0s ``` Fixes cockroachdb#87141 Release note: None
aadityasondhi
added a commit
to aadityasondhi/cockroach
that referenced
this pull request
Mar 20, 2023
This patch unskips and re-records the datadriven `TestTenantZip` as it was fixed in cockroachdb#96553, but was not unskipped or recorded. The test was run locally using `--stress` and did not flake: ``` 101 runs so far, 0 failures, over 5m0s ``` Fixes cockroachdb#87141 Release note: None
craig bot
pushed a commit
that referenced
this pull request
Mar 21, 2023
97677: tsearch: add stemming and stopword elimination for several languages r=jordanlewis a=jordanlewis Updates: #41288 Epic: CRDB-22357 First commit is #92966. This commit adds stopword elimination for text search. The languages supported are the same ones that Postgres does. The stopword lists were copied from Postgres commit e757080e041214cf6983e3e77ef01e83f1371d72. Also, add snowball stemming provided by the blevesearch snowball stemming library. Release note (sql change): add stemming and stopword eliminating text search configurations for English, Danish, Dutch, Finnish, French, German, Hungarian, Italian, Norwegian, Portuguese, Russian, Spanish, Swedish, and Turkish. 98778: cli: unskip test tenant zip test r=dhartunian a=aadityasondhi This patch unskips and re-records the datadriven `TestTenantZip` as it was fixed in #96553, but was not unskipped or recorded. The test was run locally using `--stress` and did not flake: ``` 101 runs so far, 0 failures, over 5m0s ``` Fixes #87141 Release note: None 98830: sqlinstance: add `binary_version` column to instances table r=knz,JeffSwenson a=healthy-pod This code change adds a `binary_version` column to the instances table. This is done by adding the column to the bootstrap schema for system.sql_instances, and piggy-backing on the existing code for the V23_1_SystemRbrReadNew migration that overwrites the live schema for this table by the bootstrap copy. This redefinition of the meaning of the V23_1_SystemRbrReadNew is backward-incompatible and is only possible because this commit is merged before the v23.1 branch is cut. Release note: None Epic: [CRDB-20860](https://cockroachlabs.atlassian.net/browse/CRDB-20860) 99100: kvserver: skip `TestReplicateQueueExpirationLeasesOnly` under deadlock r=erikgrinaker a=erikgrinaker I give up. Resolves #99015. Epic: none Release note: None 99106: server: avoid some log spam r=erikgrinaker a=knz This change removes the following log spam: ``` could not run claimed job 102: no resumer is available for AUTO CONFIG RUNNER ``` Epic: CRDB-23559 Release note: None Co-authored-by: Jordan Lewis <[email protected]> Co-authored-by: Aaditya Sondhi <[email protected]> Co-authored-by: healthy-pod <[email protected]> Co-authored-by: Erik Grinaker <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]>
abarganier
pushed a commit
to abarganier/cockroach
that referenced
this pull request
Sep 20, 2023
This patch unskips and re-records the datadriven `TestTenantZip` as it was fixed in cockroachdb#96553, but was not unskipped or recorded. The test was run locally using `--stress` and did not flake: ``` 101 runs so far, 0 failures, over 5m0s ``` Fixes cockroachdb#87141 Release note: None
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.
TLDR: This PR makes it possible to use regular node certs with shared-process multi-tenancy (i.e. skip creating client tenant certs). I think this will help @aadityasondhi 's work on
debug zip.Fixes #96215.
Epic: CRDB-23559
The PR also contains a few cleanup commits; it's a short change away from fixing #87141 and #77173, which I intend to do in a separate PR.