Add metadata overrides for sensitive connection string values (URL and DSN support)#3825
Conversation
ca9e8e6 to
c7fef3d
Compare
nelson-parente
changed the title
4d7d390 to
16901ee
Compare
| func (m *PostgresAuthMetadata) buildConnectionString() (string, error) { | ||
| if m.ConnectionString == "" { | ||
| return "", errors.New("connection string is required") | ||
| } |
There was a problem hiding this comment.
Is an error here intended? The below method buildDSNConnectionString appears to build a suitable connection string
There was a problem hiding this comment.
Initially, I didn't add all the props so you couldn't build a valid connection with metadata only. Since I've now added more props to the metadata we can allow to build only from meta.
|
So the logic here should be the broken out (potentially sensitive) values can be used along with values in the dns connection string. If the values are present in their own metadata property then they should overwrite the same value in the DNS connection string. Of course we hope the user won't provide them twice but the precedence should always be the individual value over the connection string one. Looking at the other potential connection string values, we should also add We also don't have to support this in v1, only v2 for State. |
Signed-off-by: nelson.parente <[email protected]>
a6e6bfe to
790af47
Compare
| } | ||
|
|
||
| // buildConnectionString builds the connection string from the metadata. | ||
| // It supports both DSN-style and URL-style connection strings. |
Signed-off-by: nelson.parente <[email protected]>
…metadata-options-for
…metadata-options-for
alicejgibbons
left a comment
alicejgibbons
left a comment
There was a problem hiding this comment.
Logic lgtm, only im unsure if we want to continue to support v1, would confirm with the community
Logic is the same for v1/v2 I don't think there is harm in adding this to v1 too |
|
@yaron2 can we merge this? I don't think merge queue is working so we need manual merge. |
|
@holopin-bot @nelson-parente Thank you! Here's a digital badge as a small token of appreciation. |
|
Congratulations @nelson-parente, the maintainer of this repository has issued you a badge! Here it is: https://holopin.io/claim/cmbs0d6sr1630907l5s58im4e2 This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account. |
…d DSN support) (dapr#3825) Signed-off-by: nelson.parente <[email protected]>
5 participants
Description
This introduces support for overriding sensitive connection string values via metadata. Both URL-style and DSN-style connection strings are supported.
Currently supported overrides asked in the issue:
Happy to add support for additional fields if needed.
Docs PR: dapr/docs#4664
Issue reference
#3809
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: