-
Notifications
You must be signed in to change notification settings - Fork 100
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update LoadSecrets() #7796
Update LoadSecrets() #7796
Conversation
pkg/recipes/configloader/secrets.go
Outdated
if secretData[secretStoreID] == nil { | ||
secretData[secretStoreID] = make(map[string]string) | ||
} | ||
secretData[secretStoreID][secretKey] = *secretDataValue.Value |
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.
ln96- ln105 is just ln76-ln82 refactored
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7796 +/- ##
==========================================
+ Coverage 61.05% 61.06% +0.01%
==========================================
Files 523 523
Lines 27466 27474 +8
==========================================
+ Hits 16770 16778 +8
- Misses 9211 9212 +1
+ Partials 1485 1484 -1 ☔ View full report in Codecov by Sentry. |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
e8e4385
to
5ed6808
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
5ed6808
to
6f13718
Compare
pkg/recipes/configloader/secrets.go
Outdated
// If the input secret keys filter for a secret store ID is nil or empty, it retrieves secret data for all keys for that secret store. | ||
// When the secret keys filter is populated for a secret store ID, it retrieves secret data for the specified keys for the associated secret store ID. | ||
// The function returns a map of secret data, where the keys are the secret store IDs and the values are maps of secret keys and their corresponding values. | ||
func (e *secretsLoader) LoadSecrets(ctx context.Context, secretStoreIDKeysFilter map[string][]string) (secretData map[string]map[string]string, err error) { |
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.
Why not just name this filter
and give an example in the doc above?
func (e *secretsLoader) LoadSecrets(ctx context.Context, secretStoreIDKeysFilter map[string][]string) (secretData map[string]map[string]string, err error) { | |
func (e *secretsLoader) LoadSecrets(ctx context.Context, filter map[string][]string) (secretData map[string]map[string]string, err error) { |
fbbad7c
to
1aec7a4
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
@@ -24,13 +24,15 @@ resource env 'Applications.Core/environments@2023-10-01-preview' = { | |||
providers: { |
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.
are these changes related to this PR?
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.
Discussed offline. The functional test needs to be updated with envSecrets block. This PR updates code that can affect running of functional test with envSecrets. We'll include this test in the PR.
@@ -225,7 +225,7 @@ func (e executor) setEnvironmentVariables(tf *tfexec.Terraform, options Options) | |||
for secretName, secretReference := range recipeConfig.EnvSecrets { | |||
// Extract secret value from the secrets input | |||
if secretIDs, ok := options.Secrets[secretReference.Source]; ok { | |||
if secretValue, ok := secretIDs[secretReference.Key]; ok { | |||
if secretValue, ok := secretIDs.Data[secretReference.Key]; ok { | |||
envVarUpdate = true |
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.
id it gonna throw nil pointer if secretIDs
is nil?
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.
secretIDs is a struct and will not be nil. Renamed variable to secretData. There exists a unit test to check for secretData.Data when nil.
bicep-types
Outdated
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.
are these changes related to this PR?
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.
removed from PR.
1aec7a4
to
93f111b
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
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.
lgtm
93f111b
to
c151322
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Description
Type of change
Fixes: (Part of #6917)