[DRAFT] server: allow multiple session cookies side-by-side #92063
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
Epic: CRDB-14537 This commit lays the groundwork for the ability to run tenant servers in-memory. It introduces a new `serverController` with two roles: - it maps tenant names to server instances. - it only starts servers the first time the tenant name is referenced, and only if that tenant is marked active. As of this commit, no subsystem inside CockroachDB refers to this new server controller; this means there is no signal hooked up to start these servers automatically yet. For testing, a debug HTTP endpoint has been added: `/debug/tickle?name=<tenantname>` Example use: 1. start a server. At this point no secondary tenant server is created yet. 2. create a test tenant, e.g. via `select crdb_internal.create_tenant(123, 'hello');`. At this point, the secondary tenant server is still not running. 3. Perform a HTTP request to the debug endpoint, e.g. to `/debug/tickle?name=hello` 4. Observe (e.g. in logs): the secondary server has been started. One can also observe that the controller also serves the name `system` to refer to the system tenant. For now, the secondary servers created this way use separate network listeners for SQL/HTTP/RPC. NB: This mechanism will be superseded when cockroachdb#84604 is addressed. The port number is assigned randomly. To derive a predictable port number for testing (until issue cockroachdb#84604 is addressed), the operator can pass e.g. `--secondary-tenant-port-offset=100` to the start command (for 100 past the base port number). Release note: None
Release note: None
Prior to this patch, we were using the same netutil.Server object to manage both `net.Conn` created to serve HTTP connections, and `net.Conn` created to serve SQL connections. This was confusing, because a lot of the complexity specific for HTTP connections (integration with the HTTP2 query handling, etc) is not required for raw TCP connections as used by pgwire. This commit clarifies this by separating the two roles. This also cleans up the server initialization. Release note: None
This commit introduces a HTTP (de)multiplexer for all in-memory tenant servers. By default, HTTP requests are routed to the system tenant server. Thsi can be overridden with the header `X-Cockroach-Tenant`. Release note: None
Prior to this commit, if there were multiple `session` cookies in the HTTP header, only the first one was considered. With this commit, login succeeds if _any_ of the presented `session` cookies is valid. Release note: None
|
This change is |
|
cc @Santamaura @dhartunian - this is not a serious PR, just a small change I was making to enable a visual demo of the change in #91744. (I'm not even sure the approach here is valid: is it even possible for a web browser to remember multiple cookies with the same key side-by-side, and present them back to the server? It worked with manual |
I've been testing this approach with the login fanout work and it seems like no; the web browser will just choose 1 session cookie based on path length or if equal, whichever was set last. To get around this we instead appended the extra sessions onto the current session cookie which seems to work fine. |
|
@Santamaura did this already. |
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.
All commits except for the last are from #91744 and previous PRs.
Prior to this commit, if there were multiple
sessioncookies in theHTTP header, only the first one was considered.
With this commit, login succeeds if any of the presented
sessioncookies is valid.