From 0a7d6a842e742a6c8d7a115db3999ff7ca18cc8b Mon Sep 17 00:00:00 2001 From: lakshmimsft Date: Sun, 25 Feb 2024 23:33:48 -0800 Subject: [PATCH] rebase cleanup --- pkg/recipes/terraform/config/config.go | 35 +++--- pkg/recipes/terraform/config/config_test.go | 101 +++++++++--------- .../terraform/config/providers/types.go | 32 +++++- .../providers-emptyazureconfig.tf.json | 8 +- .../providers-overridereqproviders.tf.json | 12 +-- .../config/testdata/providers-valid.tf.json | 26 ++--- pkg/recipes/terraform/execute.go | 2 +- 7 files changed, 117 insertions(+), 99 deletions(-) diff --git a/pkg/recipes/terraform/config/config.go b/pkg/recipes/terraform/config/config.go index e1f23143914..b0a5311c154 100644 --- a/pkg/recipes/terraform/config/config.go +++ b/pkg/recipes/terraform/config/config.go @@ -113,8 +113,8 @@ func (cfg *TerraformConfig) Save(ctx context.Context, workingDir string) error { // by Radius to generate custom provider configurations. 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 []string, ucpSupportedProviders map[string]providers.Provider, envConfig *recipes.Configuration) error { - providerConfigs, err := getProviderConfigs(ctx, requiredProviders, ucpSupportedProviders, envConfig) +func (cfg *TerraformConfig) AddProviders(ctx context.Context, requiredProviders []string, ucpConfiguredProviders map[string]providers.Provider, envConfig *recipes.Configuration) error { + providerConfigs, err := getProviderConfigs(ctx, requiredProviders, ucpConfiguredProviders, envConfig) if err != nil { return err } @@ -165,21 +165,21 @@ func newModuleConfig(moduleSource string, moduleVersion string, params ...Recipe return moduleConfig } -// getProviderConfigs generates the Terraform provider configurations for the required providers. -func getProviderConfigs(ctx context.Context, requiredProviders []string, ucpSupportedProviders map[string]providers.Provider, envConfig *recipes.Configuration) (map[string]any, error) { - providerConfigs := make(map[string]any) - +// getProviderConfigs generates the Terraform provider configurations. This is built from a combination of environment level recipe configuration for +// providers and the provider configurations registered with UCP. The environment level recipe configuration for providers takes precedence over UCP provider configurations. +func getProviderConfigs(ctx context.Context, requiredProviders []string, ucpConfiguredProviders map[string]providers.Provider, envConfig *recipes.Configuration) (map[string]any, error) { // Get recipe provider configurations from the environment configuration - providerConfigs = getRecipeProviderConfigs(ctx, envConfig) + providerConfigs := getRecipeProviderConfigs(ctx, envConfig) // Build provider configurations for required providers excluding the ones already present in providerConfigs for _, provider := range requiredProviders { if _, ok := providerConfigs[provider]; ok { - // If provider is in providerConfigs, skip this iteration + // Environment level recipe configuration for providers will take precedence over + // UCP provider configuration (currently these include azurerm, aws, kubernetes providers) continue } - builder, ok := ucpSupportedProviders[provider] + builder, ok := ucpConfiguredProviders[provider] if !ok { // No-op: For any other provider under required_providers, Radius doesn't generate any custom configuration. continue @@ -189,7 +189,6 @@ func getProviderConfigs(ctx context.Context, requiredProviders []string, ucpSupp if err != nil { return nil, err } - if len(config) > 0 { providerConfigs[provider] = config } @@ -198,15 +197,25 @@ func getProviderConfigs(ctx context.Context, requiredProviders []string, ucpSupp return providerConfigs, nil } -// getRecipeProviderConfigs returns the Terraform provider configurations for Terraform providers. +// getRecipeProviderConfigs returns the Terraform provider configurations for Terraform providers +// specified under the RecipeConfig/Terraform/Providers section under environment configuration. func getRecipeProviderConfigs(ctx context.Context, envConfig *recipes.Configuration) map[string]any { providerConfigs := make(map[string]any) // If the provider is not configured, or has empty configuration, skip this iteration - if envConfig.RecipeConfig.Terraform.Providers != nil { + if envConfig != nil && envConfig.RecipeConfig.Terraform.Providers != nil { for provider, config := range envConfig.RecipeConfig.Terraform.Providers { if len(config) > 0 { - providerConfigs[provider] = config + configList := make([]any, 0) + + for _, configDetails := range config { + //for _, additionalProperty := range configDetails.AdditionalProperties { + // configList = append(configList, additionalProperty) + //} + configList = append(configList, configDetails.AdditionalProperties) + } + + providerConfigs[provider] = configList } } } diff --git a/pkg/recipes/terraform/config/config_test.go b/pkg/recipes/terraform/config/config_test.go index 3cdfa377a47..cc2a8aa250d 100644 --- a/pkg/recipes/terraform/config/config_test.go +++ b/pkg/recipes/terraform/config/config_test.go @@ -18,6 +18,7 @@ package config import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -330,7 +331,7 @@ func Test_AddRecipeContext(t *testing.T) { } func Test_AddProviders(t *testing.T) { - mProvider, supportedUCPProviders, mBackend := setup(t) + mProvider, ucpConfiguredProviders, mBackend := setup(t) envRecipe, resourceRecipe := getTestInputs() expectedBackend := map[string]any{ "kubernetes": map[string]any{ @@ -341,17 +342,16 @@ func Test_AddProviders(t *testing.T) { } configTests := []struct { - desc string - envConfig recipes.Configuration - requiredProviders []string - expectedUCPSupportedProviders []map[string]any - expectedConfigFile string - Err error - times int + desc string + envConfig recipes.Configuration + requiredProviders []string + expectedUCPConfiguredProviders []map[string]any + expectedConfigFile string + Err error }{ { desc: "valid all supported providers", - expectedUCPSupportedProviders: []map[string]any{ + expectedUCPConfiguredProviders: []map[string]any{ { "region": "test-region", }, @@ -380,13 +380,12 @@ func Test_AddProviders(t *testing.T) { providers.KubernetesProviderName, "sql", }, - times: 1, expectedConfigFile: "testdata/providers-valid.tf.json", }, - /*{ - desc: "invalid aws scope", - expectedUCPSupportedProviders: nil, - Err: errors.New("Invalid AWS provider scope"), + { + desc: "invalid aws scope", + expectedUCPConfiguredProviders: nil, + Err: errors.New("Invalid AWS provider scope"), envConfig: recipes.Configuration{ Providers: datamodel.Providers{ AWS: datamodel.ProvidersAWS{ @@ -397,11 +396,10 @@ func Test_AddProviders(t *testing.T) { requiredProviders: []string{ providers.AWSProviderName, }, - times: 1, }, { desc: "empty aws provider config", - expectedUCPSupportedProviders: []map[string]any{ + expectedUCPConfiguredProviders: []map[string]any{ {}, }, Err: nil, @@ -409,12 +407,11 @@ func Test_AddProviders(t *testing.T) { requiredProviders: []string{ providers.AWSProviderName, }, - times: 1, expectedConfigFile: "testdata/providers-empty.tf.json", }, { desc: "empty aws scope", - expectedUCPSupportedProviders: []map[string]any{ + expectedUCPConfiguredProviders: []map[string]any{ nil, }, Err: nil, @@ -431,12 +428,11 @@ func Test_AddProviders(t *testing.T) { requiredProviders: []string{ providers.AWSProviderName, }, - times: 1, expectedConfigFile: "testdata/providers-empty.tf.json", }, { desc: "empty azure provider config", - expectedUCPSupportedProviders: []map[string]any{ + expectedUCPConfiguredProviders: []map[string]any{ { "features": map[string]any{}, }, @@ -446,13 +442,12 @@ func Test_AddProviders(t *testing.T) { requiredProviders: []string{ providers.AzureProviderName, }, - times: 1, expectedConfigFile: "testdata/providers-emptyazureconfig.tf.json", }, { - desc: "valid recipe providers", - expectedUCPSupportedProviders: nil, - Err: nil, + desc: "valid recipe providers in env config", + expectedUCPConfiguredProviders: nil, + Err: nil, envConfig: recipes.Configuration{ RecipeConfig: datamodel.RecipeConfigProperties{ Terraform: datamodel.TerraformConfigProperties{ @@ -477,18 +472,14 @@ func Test_AddProviders(t *testing.T) { }, }, requiredProviders: nil, - times: 1, expectedConfigFile: "testdata/providers-envrecipeproviders.tf.json", }, { - desc: "overridding required provider configs", - expectedUCPSupportedProviders: []map[string]any{ + desc: "recipe provider config overridding required provider configs", + expectedUCPConfiguredProviders: []map[string]any{ { "region": "test-region", }, - { - "config_path": "/home/radius/.kube/config", - }, }, Err: nil, envConfig: recipes.Configuration{ @@ -515,33 +506,45 @@ func Test_AddProviders(t *testing.T) { providers.AWSProviderName, providers.KubernetesProviderName, }, - times: -1, expectedConfigFile: "testdata/providers-overridereqproviders.tf.json", }, { - desc: "recipe providers not populated", - expectedUCPSupportedProviders: nil, - Err: nil, + desc: "recipe providers not populated", + expectedUCPConfiguredProviders: nil, + Err: nil, envConfig: recipes.Configuration{ RecipeConfig: datamodel.RecipeConfigProperties{ Terraform: datamodel.TerraformConfigProperties{}, }, }, requiredProviders: nil, - times: 1, expectedConfigFile: "testdata/providers-empty.tf.json", }, { - desc: "recipe providers not populated", - expectedUCPSupportedProviders: nil, - Err: nil, + desc: "recipe providers and tfconfigproperties not populated", + expectedUCPConfiguredProviders: nil, + Err: nil, envConfig: recipes.Configuration{ RecipeConfig: datamodel.RecipeConfigProperties{}, }, requiredProviders: nil, - times: 1, expectedConfigFile: "testdata/providers-empty.tf.json", - },*/ + }, + { + desc: "envConfig set to empty recipe config", + expectedUCPConfiguredProviders: nil, + Err: nil, + envConfig: recipes.Configuration{}, + requiredProviders: nil, + expectedConfigFile: "testdata/providers-empty.tf.json", + }, + { + desc: "envConfig not populated", + expectedUCPConfiguredProviders: nil, + Err: nil, + requiredProviders: nil, + expectedConfigFile: "testdata/providers-empty.tf.json", + }, } for _, tc := range configTests { @@ -549,23 +552,15 @@ func Test_AddProviders(t *testing.T) { ctx := testcontext.New(t) workingDir := t.TempDir() - tfconfig, err := New(ctx, testRecipeName, &envRecipe, nil, &tc.envConfig) + tfconfig, err := New(ctx, testRecipeName, &envRecipe, &resourceRecipe, &tc.envConfig) require.NoError(t, err) - for _, p := range tc.expectedUCPSupportedProviders { - if tc.times > 0 { - mProvider.EXPECT().BuildConfig(ctx, &tc.envConfig).Times(tc.times).Return(p, nil) - } else { - mProvider.EXPECT().BuildConfig(ctx, &tc.envConfig).Return(p, tc.Err).AnyTimes() - } + for _, p := range tc.expectedUCPConfiguredProviders { + mProvider.EXPECT().BuildConfig(ctx, &tc.envConfig).Times(1).Return(p, nil) } if tc.Err != nil { - if tc.times > 0 { - mProvider.EXPECT().BuildConfig(ctx, &tc.envConfig).Times(1).Return(nil, tc.Err) - } else { - mProvider.EXPECT().BuildConfig(ctx, &tc.envConfig).Return(nil, tc.Err).AnyTimes() - } + mProvider.EXPECT().BuildConfig(ctx, &tc.envConfig).Times(1).Return(nil, tc.Err) } - err = tfconfig.AddProviders(ctx, tc.requiredProviders, supportedUCPProviders, &tc.envConfig) + err = tfconfig.AddProviders(ctx, tc.requiredProviders, ucpConfiguredProviders, &tc.envConfig) if tc.Err != nil { require.ErrorContains(t, err, tc.Err.Error()) return diff --git a/pkg/recipes/terraform/config/providers/types.go b/pkg/recipes/terraform/config/providers/types.go index bdef6ab8b01..3b52295f338 100644 --- a/pkg/recipes/terraform/config/providers/types.go +++ b/pkg/recipes/terraform/config/providers/types.go @@ -34,13 +34,37 @@ type Provider interface { BuildConfig(ctx context.Context, envConfig *recipes.Configuration) (map[string]any, error) } -// GetUCPSupportedTerraformProviders returns a map of Terraform provider names with configuration details stored in UCP, to provider config builder. -// These providers represent Terraform providers for which Radius generates custom provider configurations based on credentials stored with UCP. -// For example, the Azure subscription id is added to Azure provider config using Radius Environment's Azure provider scope. -func GetUCPSupportedTerraformProviders(ucpConn sdk.Connection, secretProvider *ucp_provider.SecretProvider) map[string]Provider { +// GetUCPConfiguredTerraformProviders returns a map of Terraform provider names with configuration details stored in UCP, to provider config builder. +// These providers represent Terraform providers for which Radius generates custom provider configurations based on credentials stored with UCP +// and providers configured on the Radius environment. For example, the Azure subscription id is added to Azure provider config using Radius Environment's Azure provider scope. +func GetUCPConfiguredTerraformProviders(ucpConn sdk.Connection, secretProvider *ucp_provider.SecretProvider) map[string]Provider { return map[string]Provider{ AWSProviderName: NewAWSProvider(ucpConn, secretProvider), AzureProviderName: NewAzureProvider(ucpConn, secretProvider), KubernetesProviderName: &kubernetesProvider{}, } } + +// GetRecipeProviderConfigs returns the Terraform provider configurations for Terraform providers +// specified under the RecipeConfig/Terraform/Providers section under environment configuration. +func GetRecipeProviderConfigs(ctx context.Context, envConfig *recipes.Configuration) map[string]any { + providerConfigs := make(map[string]any) + + // If the provider is not configured, or has empty configuration, skip this iteration + if envConfig != nil && envConfig.RecipeConfig.Terraform.Providers != nil { + for provider, config := range envConfig.RecipeConfig.Terraform.Providers { + if len(config) > 0 { + configList := make([]any, 0) + + // Retrieve configuration details from 'AdditionalProperties' property and add to the list. + for _, configDetails := range config { + configList = append(configList, configDetails.AdditionalProperties) + } + + providerConfigs[provider] = configList + } + } + } + + return providerConfigs +} diff --git a/pkg/recipes/terraform/config/testdata/providers-emptyazureconfig.tf.json b/pkg/recipes/terraform/config/testdata/providers-emptyazureconfig.tf.json index 9fe596da35a..efaca8b5f95 100644 --- a/pkg/recipes/terraform/config/testdata/providers-emptyazureconfig.tf.json +++ b/pkg/recipes/terraform/config/testdata/providers-emptyazureconfig.tf.json @@ -9,11 +9,9 @@ } }, "provider": { - "azurerm": [ - { - "features": {} - } - ] + "azurerm": { + "features": {} + } }, "module": { "redis-azure": { diff --git a/pkg/recipes/terraform/config/testdata/providers-overridereqproviders.tf.json b/pkg/recipes/terraform/config/testdata/providers-overridereqproviders.tf.json index 18326eb9833..51ef6252caf 100644 --- a/pkg/recipes/terraform/config/testdata/providers-overridereqproviders.tf.json +++ b/pkg/recipes/terraform/config/testdata/providers-overridereqproviders.tf.json @@ -9,17 +9,15 @@ } }, "provider": { - "aws": [ - { - "region": "test-region" - } - ], + "aws": { + "region": "test-region" + }, "kubernetes": [ { - "config_path": "/home/radius/.kube/configPath1" + "ConfigPath": "/home/radius/.kube/configPath1" }, { - "config_path": "/home/radius/.kube/configPath2" + "ConfigPath": "/home/radius/.kube/configPath2" } ] }, diff --git a/pkg/recipes/terraform/config/testdata/providers-valid.tf.json b/pkg/recipes/terraform/config/testdata/providers-valid.tf.json index df6c7a24980..ff3b36558b1 100644 --- a/pkg/recipes/terraform/config/testdata/providers-valid.tf.json +++ b/pkg/recipes/terraform/config/testdata/providers-valid.tf.json @@ -9,22 +9,16 @@ } }, "provider": { - "aws": [ - { - "region": "test-region" - } - ], - "azurerm": [ - { - "features": {}, - "subscription_id": "test-sub" - } - ], - "kubernetes": [ - { - "config_path": "/home/radius/.kube/config" - } - ] + "aws": { + "region": "test-region" + }, + "azurerm": { + "features": {}, + "subscription_id": "test-sub" + }, + "kubernetes": { + "config_path": "/home/radius/.kube/config" + } }, "module": { "redis-azure": { diff --git a/pkg/recipes/terraform/execute.go b/pkg/recipes/terraform/execute.go index d989907bfec..899be18dc07 100644 --- a/pkg/recipes/terraform/execute.go +++ b/pkg/recipes/terraform/execute.go @@ -211,7 +211,7 @@ func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, opt // Generate Terraform providers configuration for required providers and add it to the Terraform configuration. logger.Info(fmt.Sprintf("Adding provider config for required providers %+v", loadedModule.RequiredProviders)) - if err := tfConfig.AddProviders(ctx, loadedModule.RequiredProviders, providers.GetUCPSupportedTerraformProviders(e.ucpConn, e.secretProvider), + if err := tfConfig.AddProviders(ctx, loadedModule.RequiredProviders, providers.GetUCPConfiguredTerraformProviders(e.ucpConn, e.secretProvider), options.EnvConfig); err != nil { return "", err }