From 84cb1208b266e65e34eefbe7c2eb71d8fdbcf898 Mon Sep 17 00:00:00 2001 From: Lakshmi Javadekar <103459615+lakshmimsft@users.noreply.github.com> Date: Thu, 29 Aug 2024 12:17:46 -0700 Subject: [PATCH] Update LoadSecrets() (#7796) # 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 (#6917). Fixes: (Part of #6917) --- .../configloader/mock_secret_loader.go | 11 ++- pkg/recipes/configloader/secrets.go | 67 ++++++++++---- pkg/recipes/configloader/secrets_test.go | 82 +++++++++++++++-- pkg/recipes/configloader/types.go | 2 +- pkg/recipes/driver/gitconfig.go | 9 +- pkg/recipes/driver/gitconfig_test.go | 38 ++++++-- pkg/recipes/driver/types.go | 2 +- pkg/recipes/engine/engine.go | 2 +- pkg/recipes/terraform/config/config.go | 4 +- .../terraform/config/providers/types.go | 6 +- .../terraform/config/providers/types_test.go | 91 ++++++++++++++----- pkg/recipes/terraform/execute.go | 4 +- pkg/recipes/terraform/execute_test.go | 37 +++++++- pkg/recipes/terraform/types.go | 5 +- pkg/recipes/types.go | 6 ++ .../resources/recipe_terraform_test.go | 2 +- .../corerp-resources-terraform-postgres.bicep | 32 +++++-- 17 files changed, 303 insertions(+), 97 deletions(-) diff --git a/pkg/recipes/configloader/mock_secret_loader.go b/pkg/recipes/configloader/mock_secret_loader.go index 0c57f9808f..f48ad4cdc9 100644 --- a/pkg/recipes/configloader/mock_secret_loader.go +++ b/pkg/recipes/configloader/mock_secret_loader.go @@ -13,6 +13,7 @@ import ( context "context" reflect "reflect" + recipes "github.com/radius-project/radius/pkg/recipes" gomock "go.uber.org/mock/gomock" ) @@ -40,10 +41,10 @@ func (m *MockSecretsLoader) EXPECT() *MockSecretsLoaderMockRecorder { } // LoadSecrets mocks base method. -func (m *MockSecretsLoader) LoadSecrets(arg0 context.Context, arg1 map[string][]string) (map[string]map[string]string, error) { +func (m *MockSecretsLoader) LoadSecrets(arg0 context.Context, arg1 map[string][]string) (map[string]recipes.SecretData, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "LoadSecrets", arg0, arg1) - ret0, _ := ret[0].(map[string]map[string]string) + ret0, _ := ret[0].(map[string]recipes.SecretData) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -61,19 +62,19 @@ type MockSecretsLoaderLoadSecretsCall struct { } // Return rewrite *gomock.Call.Return -func (c *MockSecretsLoaderLoadSecretsCall) Return(arg0 map[string]map[string]string, arg1 error) *MockSecretsLoaderLoadSecretsCall { +func (c *MockSecretsLoaderLoadSecretsCall) Return(arg0 map[string]recipes.SecretData, arg1 error) *MockSecretsLoaderLoadSecretsCall { c.Call = c.Call.Return(arg0, arg1) return c } // Do rewrite *gomock.Call.Do -func (c *MockSecretsLoaderLoadSecretsCall) Do(f func(context.Context, map[string][]string) (map[string]map[string]string, error)) *MockSecretsLoaderLoadSecretsCall { +func (c *MockSecretsLoaderLoadSecretsCall) Do(f func(context.Context, map[string][]string) (map[string]recipes.SecretData, error)) *MockSecretsLoaderLoadSecretsCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockSecretsLoaderLoadSecretsCall) DoAndReturn(f func(context.Context, map[string][]string) (map[string]map[string]string, error)) *MockSecretsLoaderLoadSecretsCall { +func (c *MockSecretsLoaderLoadSecretsCall) DoAndReturn(f func(context.Context, map[string][]string) (map[string]recipes.SecretData, error)) *MockSecretsLoaderLoadSecretsCall { c.Call = c.Call.DoAndReturn(f) return c } diff --git a/pkg/recipes/configloader/secrets.go b/pkg/recipes/configloader/secrets.go index f6e811bb79..821f58c879 100644 --- a/pkg/recipes/configloader/secrets.go +++ b/pkg/recipes/configloader/secrets.go @@ -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" ) @@ -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 @@ -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 diff --git a/pkg/recipes/configloader/secrets_test.go b/pkg/recipes/configloader/secrets_test.go index a0e2f18010..5587ff25d4 100644 --- a/pkg/recipes/configloader/secrets_test.go +++ b/pkg/recipes/configloader/secrets_test.go @@ -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"), @@ -33,8 +36,31 @@ 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", }, @@ -42,25 +68,61 @@ func Test_populateSecretData(t *testing.T) { 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 { diff --git a/pkg/recipes/configloader/types.go b/pkg/recipes/configloader/types.go index 65dff77a73..a6c2a3c825 100644 --- a/pkg/recipes/configloader/types.go +++ b/pkg/recipes/configloader/types.go @@ -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) } diff --git a/pkg/recipes/driver/gitconfig.go b/pkg/recipes/driver/gitconfig.go index 1996246755..c23524a3df 100644 --- a/pkg/recipes/driver/gitconfig.go +++ b/pkg/recipes/driver/gitconfig.go @@ -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. @@ -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 } @@ -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 } @@ -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 } @@ -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 } diff --git a/pkg/recipes/driver/gitconfig_test.go b/pkg/recipes/driver/gitconfig_test.go index 870e577bc9..84d24eb02f 100644 --- a/pkg/recipes/driver/gitconfig_test.go +++ b/pkg/recipes/driver/gitconfig_test.go @@ -22,6 +22,7 @@ import ( "path/filepath" "testing" + "github.com/radius-project/radius/pkg/recipes" "github.com/stretchr/testify/require" ) @@ -192,24 +193,31 @@ func Test_GetGitURL(t *testing.T) { // Additional tests exist for inner function call addSecretsToGitConfig() in TestAddConfig(). func Test_addSecretsToGitConfigIfApplicable(t *testing.T) { templatePath := "git::dev.azure.com/project/module" + secretDetails := map[string]recipes.SecretData{ + "existingID": { + Type: "generic", + Data: map[string]string{"key": "value"}, + }, + } + tests := []struct { name string secretStoreID string - secretData map[string]map[string]string + secretData map[string]recipes.SecretData expectError bool expectErrMsg string }{ { name: "Secrets not found for secret store ID", secretStoreID: "missingID", - secretData: map[string]map[string]string{"existingID": {"key": "value"}}, + secretData: secretDetails, expectError: true, expectErrMsg: "secrets not found for secret store ID \"missingID\"", }, { name: "Successful secrets addition", secretStoreID: "existingID", - secretData: map[string]map[string]string{"existingID": {"key": "value"}}, + secretData: secretDetails, expectError: false, }, { @@ -247,7 +255,7 @@ func Test_unsetGitConfigForDirIfApplicable(t *testing.T) { tests := []struct { name string secretStoreID string - secretData map[string]map[string]string + secretData map[string]recipes.SecretData expectError bool expectErrMsg string expectCallToFunc bool @@ -255,14 +263,24 @@ func Test_unsetGitConfigForDirIfApplicable(t *testing.T) { { name: "Secrets not found for secret store ID", secretStoreID: "missingID", - secretData: map[string]map[string]string{"existingID": {"key": "value"}}, - expectError: true, - expectErrMsg: "secrets not found for secret store ID \"missingID\"", + secretData: map[string]recipes.SecretData{ + "existingID": { + Type: "generic", + Data: map[string]string{"key": "value"}, + }, + }, + expectError: true, + expectErrMsg: "secrets not found for secret store ID \"missingID\"", }, { - name: "Successful call to unsetGitConfigForDir", - secretStoreID: "existingID", - secretData: map[string]map[string]string{"existingID": {"username": "test-user"}, "pat": {"pat": "ghp_token"}}, + name: "Successful call to unsetGitConfigForDir", + secretStoreID: "existingID", + secretData: map[string]recipes.SecretData{ + "existingID": { + Type: "generic", + Data: map[string]string{"username": "test-user", "pat": "ghp_token"}, + }, + }, expectError: false, expectCallToFunc: true, }, diff --git a/pkg/recipes/driver/types.go b/pkg/recipes/driver/types.go index 7adcc0e635..f61d46571a 100644 --- a/pkg/recipes/driver/types.go +++ b/pkg/recipes/driver/types.go @@ -82,7 +82,7 @@ type BaseOptions struct { // "secretKey": "secretKeyXYZ", // }, // } - Secrets map[string]map[string]string + Secrets map[string]recipes.SecretData } // ExecuteOptions is the options for the Execute method. diff --git a/pkg/recipes/engine/engine.go b/pkg/recipes/engine/engine.go index 7fd76b4ba9..5814c7bc5b 100644 --- a/pkg/recipes/engine/engine.go +++ b/pkg/recipes/engine/engine.go @@ -225,7 +225,7 @@ func (e *engine) getDriver(ctx context.Context, recipeMetadata recipes.ResourceM return definition, driver, nil } -func (e *engine) getRecipeConfigSecrets(ctx context.Context, driver recipedriver.Driver, configuration *recipes.Configuration, definition *recipes.EnvironmentDefinition) (secretData map[string]map[string]string, err error) { +func (e *engine) getRecipeConfigSecrets(ctx context.Context, driver recipedriver.Driver, configuration *recipes.Configuration, definition *recipes.EnvironmentDefinition) (secretData map[string]recipes.SecretData, err error) { driverWithSecrets, ok := driver.(recipedriver.DriverWithSecrets) if !ok { return nil, nil diff --git a/pkg/recipes/terraform/config/config.go b/pkg/recipes/terraform/config/config.go index 99839645b8..c9cae26868 100644 --- a/pkg/recipes/terraform/config/config.go +++ b/pkg/recipes/terraform/config/config.go @@ -111,7 +111,7 @@ func (cfg *TerraformConfig) Save(ctx context.Context, workingDir string) error { // It also updates module provider block if aliases exist and required_provider configuration to the file. // Save() must be called to save the generated providers config. requiredProviders contains a list of provider names // that are required for the module. -func (cfg *TerraformConfig) AddProviders(ctx context.Context, requiredProviders map[string]*RequiredProviderInfo, ucpConfiguredProviders map[string]providers.Provider, envConfig *recipes.Configuration, secrets map[string]map[string]string) error { +func (cfg *TerraformConfig) AddProviders(ctx context.Context, requiredProviders map[string]*RequiredProviderInfo, ucpConfiguredProviders map[string]providers.Provider, envConfig *recipes.Configuration, secrets map[string]recipes.SecretData) error { logger := ucplog.FromContextOrDiscard(ctx) providerConfigs, err := getProviderConfigs(ctx, requiredProviders, ucpConfiguredProviders, envConfig, secrets) if err != nil { @@ -230,7 +230,7 @@ func newModuleConfig(moduleSource string, moduleVersion string, params ...Recipe // The function returns a map where the keys are provider names and the values are slices of maps. // Each map in the slice represents a specific configuration for the corresponding provider. // This structure allows for multiple configurations per provider. -func getProviderConfigs(ctx context.Context, requiredProviders map[string]*RequiredProviderInfo, ucpConfiguredProviders map[string]providers.Provider, envConfig *recipes.Configuration, secrets map[string]map[string]string) (map[string][]map[string]any, error) { +func getProviderConfigs(ctx context.Context, requiredProviders map[string]*RequiredProviderInfo, ucpConfiguredProviders map[string]providers.Provider, envConfig *recipes.Configuration, secrets map[string]recipes.SecretData) (map[string][]map[string]any, error) { // Get recipe provider configurations from the environment configuration providerConfigs, err := providers.GetRecipeProviderConfigs(ctx, envConfig, secrets) if err != nil { diff --git a/pkg/recipes/terraform/config/providers/types.go b/pkg/recipes/terraform/config/providers/types.go index 5f55112c35..dace465f80 100644 --- a/pkg/recipes/terraform/config/providers/types.go +++ b/pkg/recipes/terraform/config/providers/types.go @@ -50,7 +50,7 @@ func GetUCPConfiguredTerraformProviders(ucpConn sdk.Connection, secretProvider * // GetRecipeProviderConfigs returns the Terraform provider configurations for Terraform providers // specified under the RecipeConfig/Terraform/Providers section under environment configuration. // The function also extracts secrets from the secrets data input and updates the provider configurations with secrets as applicable. -func GetRecipeProviderConfigs(ctx context.Context, envConfig *recipes.Configuration, secrets map[string]map[string]string) (map[string][]map[string]any, error) { +func GetRecipeProviderConfigs(ctx context.Context, envConfig *recipes.Configuration, secrets map[string]recipes.SecretData) (map[string][]map[string]any, error) { providerConfigs := make(map[string][]map[string]any) // If the provider is not configured, or has empty configuration, skip this iteration @@ -96,14 +96,14 @@ func GetRecipeProviderConfigs(ctx context.Context, envConfig *recipes.Configurat } // extractSecretsFromRecipeConfig extracts secrets for env recipe configuration from the secrets data input and updates the currentConfig map. -func extractSecretsFromRecipeConfig(recipeConfigSecrets map[string]datamodel.SecretReference, secrets map[string]map[string]string) (map[string]any, error) { +func extractSecretsFromRecipeConfig(recipeConfigSecrets map[string]datamodel.SecretReference, secrets map[string]recipes.SecretData) (map[string]any, error) { secretsConfig := make(map[string]any) // Extract secrets from configDetails if they are present for secretName, secretReference := range recipeConfigSecrets { // Extract secret value from the secrets data input if secretIDs, ok := secrets[secretReference.Source]; ok { - if secretValue, ok := secretIDs[secretReference.Key]; ok { + if secretValue, ok := secretIDs.Data[secretReference.Key]; ok { secretsConfig[secretName] = secretValue } else { return nil, fmt.Errorf("missing secret key in secret store id: %s", secretReference.Source) diff --git a/pkg/recipes/terraform/config/providers/types_test.go b/pkg/recipes/terraform/config/providers/types_test.go index f52d2bef87..9d577ef7aa 100644 --- a/pkg/recipes/terraform/config/providers/types_test.go +++ b/pkg/recipes/terraform/config/providers/types_test.go @@ -13,7 +13,7 @@ func Test_GetRecipeProviderConfigs(t *testing.T) { testCases := []struct { desc string envConfig *recipes.Configuration - secrets map[string]map[string]string + secrets map[string]recipes.SecretData expected map[string][]map[string]any }{ { @@ -132,9 +132,15 @@ func Test_GetRecipeProviderConfigs(t *testing.T) { }, }, }, - secrets: map[string]map[string]string{ - "secretstoreid1": {"secretkey1": "secretvalue1"}, - "secretstoreid2": {"secretkey2": "secretvalue2"}, + secrets: map[string]recipes.SecretData{ + "secretstoreid1": { + Type: "generic", + Data: map[string]string{"secretkey1": "secretvalue1"}, + }, + "secretstoreid2": { + Type: "generic", + Data: map[string]string{"secretkey2": "secretvalue2"}, + }, }, expected: map[string][]map[string]any{ "azurerm": { @@ -172,9 +178,15 @@ func Test_GetRecipeProviderConfigs(t *testing.T) { }, }, }, - secrets: map[string]map[string]string{ - "secretstoreid1": {"secretkey1": "secretvalue1"}, - "secretstoreid2": {"secretkey2": "secretvalue2"}, + secrets: map[string]recipes.SecretData{ + "secretstoreid1": { + Type: "generic", + Data: map[string]string{"secretkey1": "secretvalue1"}, + }, + "secretstoreid2": { + Type: "generic", + Data: map[string]string{"secretkey2": "secretvalue2"}, + }, }, expected: map[string][]map[string]any{ "azurerm": { @@ -218,10 +230,16 @@ func Test_GetRecipeProviderConfigs(t *testing.T) { }, }, }, - secrets: map[string]map[string]string{ - "secretstoreid1": {"secretkey1": "secretvalue1", - "secret-usedid-env": "secretvalue-usedid-env"}, - "secretstore-env": {"secretkey-env": "secretvalue-env"}, + secrets: map[string]recipes.SecretData{ + "secretstoreid1": { + Type: "generic", + Data: map[string]string{"secretkey1": "secretvalue1", + "secret-usedid-env": "secretvalue-usedid-env"}, + }, + "secretstore-env": { + Type: "generic", + Data: map[string]string{"secretkey-env": "secretvalue-env"}, + }, }, expected: map[string][]map[string]any{ "azurerm": { @@ -258,8 +276,11 @@ func Test_GetRecipeProviderConfigs(t *testing.T) { }, }, }, - secrets: map[string]map[string]string{ - "secretstoreid1": {"secretkey1": "secretvalue-clientid"}, + secrets: map[string]recipes.SecretData{ + "secretstoreid1": { + Type: "generic", + Data: map[string]string{"secretkey1": "secretvalue-clientid"}, + }, }, expected: map[string][]map[string]any{ "azurerm": { @@ -287,7 +308,7 @@ func Test_extractSecretsFromRecipeConfig(t *testing.T) { name string currentConfig map[string]any recipeConfigSecrets map[string]datamodel.SecretReference - secrets map[string]map[string]string + secrets map[string]recipes.SecretData expectedConfig map[string]any expectError bool expectedErrorMessage string @@ -297,8 +318,11 @@ func Test_extractSecretsFromRecipeConfig(t *testing.T) { recipeConfigSecrets: map[string]datamodel.SecretReference{ "password": {Source: "dbSecrets", Key: "dbPass"}, }, - secrets: map[string]map[string]string{ - "dbSecrets": {"dbPass": "secretPassword"}, + secrets: map[string]recipes.SecretData{ + "dbSecrets": { + Type: "generic", + Data: map[string]string{"dbPass": "secretPassword"}, + }, }, expectedConfig: map[string]any{ "password": "secretPassword", @@ -310,8 +334,11 @@ func Test_extractSecretsFromRecipeConfig(t *testing.T) { recipeConfigSecrets: map[string]datamodel.SecretReference{ "password": {Source: "missingSource", Key: "dbPass"}, }, - secrets: map[string]map[string]string{ - "dbSecrets": {"dbPass": "secretPassword"}, + secrets: map[string]recipes.SecretData{ + "dbSecrets": { + Type: "generic", + Data: map[string]string{"dbPass": "secretPassword"}, + }, }, expectError: true, expectedErrorMessage: "missing secret store id: missingSource", @@ -321,8 +348,11 @@ func Test_extractSecretsFromRecipeConfig(t *testing.T) { recipeConfigSecrets: map[string]datamodel.SecretReference{ "password": {Source: "dbSecrets", Key: "missingKey"}, }, - secrets: map[string]map[string]string{ - "dbSecrets": {"dbPass": "secretPassword"}, + secrets: map[string]recipes.SecretData{ + "dbSecrets": { + Type: "generic", + Data: map[string]string{"dbPass": "secretPassword"}, + }, }, expectError: true, expectedErrorMessage: "missing secret key in secret store id: dbSecrets", @@ -339,12 +369,29 @@ func Test_extractSecretsFromRecipeConfig(t *testing.T) { { name: "missing recipeConfigSecrets", recipeConfigSecrets: nil, - secrets: map[string]map[string]string{ - "dbSecrets": {"dbPass": "secretPassword"}, + secrets: map[string]recipes.SecretData{ + "dbSecrets": { + Type: "generic", + Data: map[string]string{"dbPass": "secretPassword"}, + }, }, expectedConfig: map[string]any{}, expectError: false, }, + { + name: "missing secrets data", + recipeConfigSecrets: map[string]datamodel.SecretReference{ + "password": {Source: "dbSecrets", Key: "missingKey"}, + }, + secrets: map[string]recipes.SecretData{ + "dbSecrets": { + Type: "generic", + }, + }, + expectedConfig: map[string]any{}, + expectError: true, + expectedErrorMessage: "missing secret key in secret store id: dbSecrets", + }, } for _, tt := range tests { diff --git a/pkg/recipes/terraform/execute.go b/pkg/recipes/terraform/execute.go index 1c36a022ff..4fd6709e2f 100644 --- a/pkg/recipes/terraform/execute.go +++ b/pkg/recipes/terraform/execute.go @@ -224,8 +224,8 @@ func (e executor) setEnvironmentVariables(tf *tfexec.Terraform, options Options) if recipeConfig != nil && recipeConfig.EnvSecrets != nil && len(recipeConfig.EnvSecrets) > 0 { 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 secretData, ok := options.Secrets[secretReference.Source]; ok { + if secretValue, ok := secretData.Data[secretReference.Key]; ok { envVarUpdate = true envVars[secretName] = secretValue } else { diff --git a/pkg/recipes/terraform/execute_test.go b/pkg/recipes/terraform/execute_test.go index 80db042c82..1a79932995 100644 --- a/pkg/recipes/terraform/execute_test.go +++ b/pkg/recipes/terraform/execute_test.go @@ -157,13 +157,16 @@ func TestSetEnvironmentVariables(t *testing.T) { }, }, }, - Secrets: map[string]map[string]string{ - "secretstoreid1": {"secretkey1": "secretvalue1"}, + Secrets: map[string]recipes.SecretData{ + "secretstoreid1": { + Type: "generic", + Data: map[string]string{"secretkey1": "secretvalue1"}, + }, }, }, }, { - name: "missing secret data", + name: "missing secret keys", opts: Options{ EnvConfig: &recipes.Configuration{ RecipeConfig: dm.RecipeConfigProperties{ @@ -181,8 +184,32 @@ func TestSetEnvironmentVariables(t *testing.T) { }, }, }, - Secrets: map[string]map[string]string{ - "secretstoreid2": {"secretkey2": "secretvalue2"}, + Secrets: map[string]recipes.SecretData{ + "secretstoreid2": { + Type: "generic", + Data: map[string]string{"secretkey2": "secretvalue2"}, + }, + }, + }, + wantErr: true, + }, + { + name: "missing secret data", + opts: Options{ + EnvConfig: &recipes.Configuration{ + RecipeConfig: dm.RecipeConfigProperties{ + EnvSecrets: map[string]dm.SecretReference{ + "TEST_ENV_VAR3": { + Source: "secretstoreid1", + Key: "secretkey1", + }, + }, + }, + }, + Secrets: map[string]recipes.SecretData{ + "secretstoreid2": { + Type: "generic", + }, }, }, wantErr: true, diff --git a/pkg/recipes/terraform/types.go b/pkg/recipes/terraform/types.go index 652b97a570..4d1959cb60 100644 --- a/pkg/recipes/terraform/types.go +++ b/pkg/recipes/terraform/types.go @@ -64,9 +64,8 @@ type Options struct { ResourceRecipe *recipes.ResourceMetadata // Secrets represents a map of secrets required for recipe execution. - // The outer map's key represents the secretStoreIDs while - // while the inner map's key-value pairs represent the [secretKey]secretValue. - Secrets map[string]map[string]string + // The map's key represents the secretStoreIDs while the value represents the secret data. + Secrets map[string]recipes.SecretData } // NewTerraform creates a working directory for Terraform execution and new Terraform executor with Terraform logs enabled. diff --git a/pkg/recipes/types.go b/pkg/recipes/types.go index 50da74df49..f61f202966 100644 --- a/pkg/recipes/types.go +++ b/pkg/recipes/types.go @@ -108,6 +108,12 @@ type RecipeOutput struct { Status *rpv1.RecipeStatus } +// SecretData represents secrets data and includes secret type and a map of secret keys to their values. +type SecretData struct { + Type string `json:"type"` + Data map[string]string `json:"data"` +} + // PrepareRecipeOutput populates the recipe output from the recipe deployment output stored in the "result" object. // outputs map is the value of "result" output from the recipe deployment response. func (ro *RecipeOutput) PrepareRecipeResponse(resultValue map[string]any) error { diff --git a/test/functional-portable/corerp/noncloud/resources/recipe_terraform_test.go b/test/functional-portable/corerp/noncloud/resources/recipe_terraform_test.go index d61145fa5e..20014e8e14 100644 --- a/test/functional-portable/corerp/noncloud/resources/recipe_terraform_test.go +++ b/test/functional-portable/corerp/noncloud/resources/recipe_terraform_test.go @@ -144,7 +144,7 @@ func Test_TerraformRecipe_KubernetesPostgres(t *testing.T) { appName := "corerp-resources-terraform-pg-app" envName := "corerp-resources-terraform-pg-env" extenderName := "pgs-resources-terraform-pgsapp" - secretName := "pgs-hostsecret" + secretName := "pgs-secretstore" secretResourceName := appName + "/" + secretName userName := "postgres" password := "abc-123-hgd-@#$'" diff --git a/test/functional-portable/corerp/noncloud/resources/testdata/corerp-resources-terraform-postgres.bicep b/test/functional-portable/corerp/noncloud/resources/testdata/corerp-resources-terraform-postgres.bicep index f102bdf6db..39336803a5 100644 --- a/test/functional-portable/corerp/noncloud/resources/testdata/corerp-resources-terraform-postgres.bicep +++ b/test/functional-portable/corerp/noncloud/resources/testdata/corerp-resources-terraform-postgres.bicep @@ -24,13 +24,15 @@ resource env 'Applications.Core/environments@2023-10-01-preview' = { providers: { postgresql: [ { alias: 'pgdb-test' - username: userName - password: password sslmode: 'disable' - secrets: { - host: { - source: pgshostsecret.id - key: 'host' + secrets: { + username: { + source: pgsecretstore.id + key: 'username' + } + password: { + source: pgsecretstore.id + key: 'password' } } } ] @@ -39,6 +41,12 @@ resource env 'Applications.Core/environments@2023-10-01-preview' = { env: { PGPORT: '5432' } + envSecrets: { + PGHOST: { + source: pgsecretstore.id + key: 'host' + } + } } recipes: { 'Applications.Core/extenders': { @@ -79,12 +87,18 @@ resource pgsapp 'Applications.Core/extenders@2023-10-01-preview' = { } } -resource pgshostsecret 'Applications.Core/secretStores@2023-10-01-preview' = { - name: 'pgs-hostsecret' +resource pgsecretstore 'Applications.Core/secretStores@2023-10-01-preview' = { + name: 'pgs-secretstore' properties: { - resource: 'corerp-resources-terraform-pg-app/pgs-hostsecret' + resource: 'corerp-resources-terraform-pg-app/pgs-secretstore' type: 'generic' data: { + username: { + value: userName + } + password: { + value: password + } host: { value: 'postgres.corerp-resources-terraform-pg-app.svc.cluster.local' }