Skip to content

Commit

Permalink
Update LoadSecrets() (radius-project#7796)
Browse files Browse the repository at this point in the history 
# Description

1. Update LoadSecrets() to return data for all keys for corresponding
secret store when no keys filter is provided.
2. Update return type to include SecretData{}
3. Updated existing LoadSecrets() to return data collated for multiple
secret store ids
4. Update to helper function populateSecretData() and it's unit tests
which populates secret data map returned.
5. Update functional test to include envSecrets input

## Type of change
- This pull request adds or changes features of Radius and has an
approved issue (radius-project#6917).

Fixes: (Part of radius-project#6917)
  • Loading branch information
@lakshmimsft
lakshmimsft authored Aug 29, 2024
1 parent a58b8cb commit 84cb120
Show file tree
Hide file tree
Showing 17 changed files with 303 additions and 97 deletions.
11 changes: 6 additions & 5 deletions pkg/recipes/configloader/mock_secret_loader.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

67 changes: 49 additions & 18 deletions pkg/recipes/configloader/secrets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials"
"github.com/radius-project/radius/pkg/corerp/api/v20231001preview"
"github.com/radius-project/radius/pkg/recipes"
"github.com/radius-project/radius/pkg/ucp/resources"
)

Expand All @@ -35,10 +36,18 @@ type secretsLoader struct {
ArmClientOptions *arm.ClientOptions
}

// LoadSecrets loads secrets from secret stores based on input map of provided secret store IDs and secret keys.
// It 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, secretStoreIDResourceKeys map[string][]string) (secretData map[string]map[string]string, err error) {
for secretStoreID, secretKeys := range secretStoreIDResourceKeys {
// LoadSecrets loads secrets from secret stores based on input map of provided secret store IDs and secret keys filter.
// It returns a map with keys as secret store IDs and corresponding SecretData{}.
// 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 ID.
// Eg: secretStoreKeysFilter = {"SecretStoreID1": nil} or secretKeysFilter = {"SecretStoreID1": []} will retrieve data for all secrets from "SecretStoreID1".
// ---
// 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.
// Eg; secretStoreKeysFilter = {"SecretStoreID1": ["secretkey1", "secretkey2"]} will retrieve data for only "secretkey1" and "secretkey2" keys from "SecretStoreID1".
func (e *secretsLoader) LoadSecrets(ctx context.Context, secretStoreKeysFilter map[string][]string) (secretData map[string]recipes.SecretData, err error) {
secretData = make(map[string]recipes.SecretData)

for secretStoreID, secretKeysFilter := range secretStoreKeysFilter {
secretStoreResourceID, err := resources.ParseResource(secretStoreID)
if err != nil {
return nil, err
Expand All @@ -55,33 +64,55 @@ func (e *secretsLoader) LoadSecrets(ctx context.Context, secretStoreIDResourceKe
return nil, err
}

// Populate the secretData map.
secretData, err = populateSecretData(secretStoreID, secretKeys, &secrets)
// Populate secretStoreData with secret type and map of secret keys and values
secretStoreData, err := populateSecretData(secretStoreID, secretKeysFilter, &secrets)
if err != nil {
return nil, err
}

secretData[secretStoreID] = secretStoreData
}

return secretData, nil
}

// populateSecretData is a helper function to populate secret data from a secret store.
func populateSecretData(secretStoreID string, secretKeys []string, secrets *v20231001preview.SecretStoresClientListSecretsResponse) (map[string]map[string]string, error) {
secretData := make(map[string]map[string]string)

// It takes a secret store ID, a filter for secret keys, and a response containing the secret data.
// It returns SecretData{} populated with secret Type and a map with secret keys and their corresponding values.
// ---
// If the secret keys filter is nil or empty, it retrieves data for all keys in the secret store.
// Eg: secretKeysFilter = nil or secretKeysFilter = [] will retrieve data for all secrets for secretStoreID.
// ---
// If the secret keys filter is populated, it retrieves data for the specified keys.
// Eg: secretKeysFilter = [secretkey1] will retrieve data for only 'secretkey1' key.
func populateSecretData(secretStoreID string, secretKeysFilter []string, secrets *v20231001preview.SecretStoresClientListSecretsResponse) (recipes.SecretData, error) {
if secrets == nil {
return nil, fmt.Errorf("secrets not found for secret store ID '%s'", secretStoreID)
return recipes.SecretData{}, fmt.Errorf("secrets not found for secret store ID '%s'", secretStoreID)
}

if secrets.Type == nil {
return recipes.SecretData{}, fmt.Errorf("secret store type is not set for secret store ID '%s'", secretStoreID)
}

secretData := recipes.SecretData{
Type: string(*secrets.Type),
Data: make(map[string]string),
}

// If secretKeysFilter is nil or empty, populate secretKeysFilter with all keys
if len(secretKeysFilter) == 0 {
secretKeysFilter = make([]string, 0, len(secrets.Data))
for secretKey := range secrets.Data {
secretKeysFilter = append(secretKeysFilter, secretKey)
}
}

for _, secretKey := range secretKeys {
if secretDataValue, ok := secrets.Data[secretKey]; ok {
if secretData[secretStoreID] == nil {
secretData[secretStoreID] = make(map[string]string)
}
secretData[secretStoreID][secretKey] = *secretDataValue.Value
} else {
return nil, fmt.Errorf("a secret key was not found in secret store '%s'", secretStoreID)
for _, secretKey := range secretKeysFilter {
secretDataValue, ok := secrets.Data[secretKey]
if !ok {
return recipes.SecretData{}, fmt.Errorf("a secret key was not found in secret store '%s'", secretStoreID)
}
secretData.Data[secretKey] = *secretDataValue.Value
}

return secretData, nil
Expand Down
82 changes: 72 additions & 10 deletions pkg/recipes/configloader/secrets_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,28 @@ import (
"testing"

"github.com/radius-project/radius/pkg/corerp/api/v20231001preview"
"github.com/radius-project/radius/pkg/recipes"
"github.com/radius-project/radius/pkg/to"
"github.com/stretchr/testify/require"
)

func Test_populateSecretData(t *testing.T) {
secretStoreDataTypeGeneric := v20231001preview.SecretStoreDataTypeGeneric
tests := []struct {
name string
secretKeys []string
secrets *v20231001preview.SecretStoresClientListSecretsResponse
secretStoreID string
expectedSecrets map[string]map[string]string
expectedSecrets recipes.SecretData
expectError bool
expectedErrMsg string
}{
{
name: "success",
secretKeys: []string{"secretKey1", "secretKey2"},
name: "success - data for input secretKey1 returned",
secretKeys: []string{"secretKey1"},
secrets: &v20231001preview.SecretStoresClientListSecretsResponse{
SecretStoreListSecretsResult: v20231001preview.SecretStoreListSecretsResult{
Type: &secretStoreDataTypeGeneric,
Data: map[string]*v20231001preview.SecretValueProperties{
"secretKey1": {
Value: to.Ptr("secretValue1"),
Expand All @@ -33,34 +36,93 @@ func Test_populateSecretData(t *testing.T) {
}},
},
secretStoreID: "testSecretStore",
expectedSecrets: map[string]map[string]string{
"testSecretStore": {
expectedSecrets: recipes.SecretData{
Type: "generic",
Data: map[string]string{"secretKey1": "secretValue1"},
},
expectError: false,
},
{
name: "success - data for all keys returned with nil secretKeys input",
secretKeys: nil,
secrets: &v20231001preview.SecretStoresClientListSecretsResponse{
SecretStoreListSecretsResult: v20231001preview.SecretStoreListSecretsResult{
Type: &secretStoreDataTypeGeneric,
Data: map[string]*v20231001preview.SecretValueProperties{
"secretKey1": {
Value: to.Ptr("secretValue1"),
},
"secretKey2": {
Value: to.Ptr("secretValue2"),
},
}},
},
secretStoreID: "testSecretStore",
expectedSecrets: recipes.SecretData{
Type: "generic",
Data: map[string]string{
"secretKey1": "secretValue1",
"secretKey2": "secretValue2",
},
},
expectError: false,
},
{
name: "fail with nil secrets input",
name: "success - returned with nil secretKeys input when no secret data exist",
secretKeys: nil,
secrets: &v20231001preview.SecretStoresClientListSecretsResponse{
SecretStoreListSecretsResult: v20231001preview.SecretStoreListSecretsResult{
Type: &secretStoreDataTypeGeneric,
Data: nil},
},
secretStoreID: "testSecretStore",
expectedSecrets: recipes.SecretData{
Type: "generic",
Data: map[string]string{},
},
expectError: false,
},
{
name: "fail - nil secrets input",
secretKeys: []string{"secretKey1"},
secrets: nil,
secretStoreID: "testSecretStore",
expectedSecrets: nil,
expectedSecrets: recipes.SecretData{},
expectError: true,
expectedErrMsg: "secrets not found for secret store ID 'testSecretStore'",
},
{
name: "missing secret key",
name: "fail - missing secret key",
secretKeys: []string{"missingKey"},
secrets: &v20231001preview.SecretStoresClientListSecretsResponse{
SecretStoreListSecretsResult: v20231001preview.SecretStoreListSecretsResult{},
SecretStoreListSecretsResult: v20231001preview.SecretStoreListSecretsResult{
Type: &secretStoreDataTypeGeneric,
},
},
secretStoreID: "testSecretStore",
expectedSecrets: nil,
expectedSecrets: recipes.SecretData{},
expectError: true,
expectedErrMsg: "a secret key was not found in secret store 'testSecretStore'",
},
{
name: "fail - missing secret type",
secretKeys: []string{"secretKey1"},
secrets: &v20231001preview.SecretStoresClientListSecretsResponse{
SecretStoreListSecretsResult: v20231001preview.SecretStoreListSecretsResult{
Data: map[string]*v20231001preview.SecretValueProperties{
"secretKey1": {
Value: to.Ptr("secretValue1"),
},
"secretKey2": {
Value: to.Ptr("secretValue2"),
},
}},
},
secretStoreID: "testSecretStore",
expectedSecrets: recipes.SecretData{},
expectError: true,
expectedErrMsg: "secret store type is not set for secret store ID 'testSecretStore'",
},
}

for _, tt := range tests {
Expand Down
2 changes: 1 addition & 1 deletion pkg/recipes/configloader/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,5 @@ type ConfigurationLoader interface {

//go:generate mockgen -typed -destination=./mock_secret_loader.go -package=configloader -self_package github.com/radius-project/radius/pkg/recipes/configloader github.com/radius-project/radius/pkg/recipes/configloader SecretsLoader
type SecretsLoader interface {
LoadSecrets(ctx context.Context, secretStoreIDs map[string][]string) (secretData map[string]map[string]string, err error)
LoadSecrets(ctx context.Context, secretStoreIDs map[string][]string) (secretData map[string]recipes.SecretData, err error)
}
9 changes: 5 additions & 4 deletions pkg/recipes/driver/gitconfig.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"strings"

git "github.com/go-git/go-git/v5"
"github.com/radius-project/radius/pkg/recipes"
)

// getGitURLWithSecrets returns the git URL with secrets information added.
Expand Down Expand Up @@ -147,7 +148,7 @@ func GetGitURL(templatePath string) (*url.URL, error) {

// addSecretsToGitConfigIfApplicable adds secrets to the Git configuration file if applicable.
// It is a wrapper function to addSecretsToGitConfig()
func addSecretsToGitConfigIfApplicable(secretStoreID string, secretData map[string]map[string]string, requestDirPath string, templatePath string) error {
func addSecretsToGitConfigIfApplicable(secretStoreID string, secretData map[string]recipes.SecretData, requestDirPath string, templatePath string) error {
if secretStoreID == "" || secretData == nil {
return nil
}
Expand All @@ -157,7 +158,7 @@ func addSecretsToGitConfigIfApplicable(secretStoreID string, secretData map[stri
return fmt.Errorf("secrets not found for secret store ID %q", secretStoreID)
}

err := addSecretsToGitConfig(requestDirPath, secrets, templatePath)
err := addSecretsToGitConfig(requestDirPath, secrets.Data, templatePath)
if err != nil {
return err
}
Expand All @@ -167,7 +168,7 @@ func addSecretsToGitConfigIfApplicable(secretStoreID string, secretData map[stri

// unsetGitConfigForDir removes a conditional include directive from the global Git configuration if applicable.
// It is a wrapper function to unsetGitConfigForDir()
func unsetGitConfigForDirIfApplicable(secretStoreID string, secretData map[string]map[string]string, requestDirPath string, templatePath string) error {
func unsetGitConfigForDirIfApplicable(secretStoreID string, secretData map[string]recipes.SecretData, requestDirPath string, templatePath string) error {
if secretStoreID == "" || secretData == nil {
return nil
}
Expand All @@ -177,7 +178,7 @@ func unsetGitConfigForDirIfApplicable(secretStoreID string, secretData map[strin
return fmt.Errorf("secrets not found for secret store ID %q", secretStoreID)
}

err := unsetGitConfigForDir(requestDirPath, secrets, templatePath)
err := unsetGitConfigForDir(requestDirPath, secrets.Data, templatePath)
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit 84cb120

Please sign in to comment.