Add metadata overrides for sensitive connection string values (URL and DSN support) #3825
Conversation
nelson-parente
force-pushed
the
oss-125-postgresql-state-store-add-separate-metadata-options-for
branch
from
ca9e8e6 to
c7fef3d
Compare
nelson-parente
changed the title
nelson-parente
force-pushed
the
oss-125-postgresql-state-store-add-separate-metadata-options-for
branch
3 times, most recently
from
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]>
nelson-parente
force-pushed
the
oss-125-postgresql-state-store-add-separate-metadata-options-for
branch
from
a6e6bfe to
790af47
Compare
| @@ -87,6 +102,117 @@ func (m *PostgresAuthMetadata) InitWithMetadata(meta map[string]string, opts Ini | |||
| return nil | |||
| } | |||
|
|
|||
| // 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
Logic is the same for v1/v2 I don't think there is harm in adding this to v1 too |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
@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. |
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: