- Notifications
You must be signed in to change notification settings - Fork1.1k
feat: add support for reading db connection string from a file#20910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
c6a1ee8 to14b0604Compare
deansheather left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think a fake driver is what we need but maybe@f0ssel will have more knowledge. He wrote something similar for the AWS IAM database auth system.
| ifvals.PostgresURL!="" { | ||
| returnxerrors.Errorf("cannot specify both --postgres-url and --postgres-url-file") | ||
| } | ||
| postgresURL,err:=ReadPostgresURLFromFile(vals.PostgresURLFile.String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We need some sort of go sql driver BS so we can get this to be read on every single connection attempt. Reading it once on startup won't resolve the customer's problem.
cli/server_createadminuser.go Outdated
| ctx,cancel:=inv.SignalNotifyContext(ctx,StopSignals...) | ||
| defercancel() | ||
| // Read the postgres URL from a file, if specified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This code is duplicated 3 times. Maybe we should add a helper that reads both theurl andurl-file args, handles the mutual exclusion, and returns a driver string and URL string. (as well as registering the new driver).
9b982d9 tob9d0281Compare…tring by reading a file from disk
e0c4f0b todf7b098Compare
Uh oh!
There was an error while loading.Please reload this page.
This is a pr to add support for reading the db connection string from a file, which is an alternative option to reading the connection string from an environment variable.
This is the first step in addressing a lack of support for rotating db credentials, as at present a rollout restart must be performed in order for the updated credentials to be picked up by the pod, i.e. env vars are not updated in a pod when the underlying secret is updated. A
pgfileurlsql driver is also added to re-read the file on each connection attempt to support credential rotation.This would be used by populating values.yaml with:
With it referencing a secret created using:
created with mux!