Database Access: decouple Auth from Session #43344
Merged
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
… share CloudClients instance, update tests.
github-actions
bot
added
database-access
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
Tener
added
the
no-changelog
|
I've dropped the |
greedy52
reviewed
Jun 21, 2024
greedy52
approved these changes
Jun 25, 2024
probakowski
approved these changes
Jun 25, 2024
This was referenced Jun 26, 2024
codingllama
reviewed
Jun 26, 2024
| // GetTLSConfig builds the client TLS configuration for the session. | ||
| GetTLSConfig(ctx context.Context, sessionCtx *Session) (*tls.Config, error) | ||
| GetTLSConfig(ctx context.Context, certExpiry time.Time, database types.Database, databaseUser string) (*tls.Config, error) |
There was a problem hiding this comment.
This signature change (and possibly others) broke the e/ build.
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.
This change decouples
lib/srv/db/Authfromlib/srv/db/Session, making it possible to use anAuthinstance without an active user session.Authused to depend on various fields fromSession, yet in practice very few fields were actually used. The updated interface is explicit about the data requirements, which improves the clarity.The updated interface no longer accepts session ID, but instead makes it possible to clone
Authwith overridden logger instance. This is used to inject session id and database name fields in a consistent fashion.The cloning is made easier by the fact that
Authno longer owns any resources: it no longer implementsfunc Close() error. The ownership of the only resource, theCloudClientsfield, was moved.Authwill no longer initialise an instance of that type on startup, expecting to be passed a copy instead. This change also means improved sharing ofCloudClientsinstance along with the associated benefits.The tests are updated accordingly.
While this change presents no immediate benefit, it will ultimately allow for periodic connections to databases for the purpose of healthchecks as well as pulling in database schema information.