From 386b3bd1b7b7bff02d3b3e47677a307f24442560 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Fri, 15 Mar 2024 18:41:39 +0100 Subject: [PATCH] `appconfiguration`: updating to validate the `DomainSuffix` from the Environment rather than assuming `*.azconfig.*` This commit also adds tests covering the state migrations --- .../app_configuration_feature_resource.go | 14 +++-- .../app_configuration_key_resource.go | 11 ++-- .../appconfiguration/client/helpers.go | 12 ++--- .../migration/feature_resource_v0_to_v1.go | 5 +- .../feature_resource_v0_to_v1_test.go | 49 +++++++++++++---- .../migration/key_resource_v0_to_v1.go | 4 +- .../migration/key_resource_v1_to_v2.go | 5 +- .../migration/key_resource_v1_to_v2_test.go | 52 +++++++++++++++---- 8 files changed, 113 insertions(+), 39 deletions(-) diff --git a/internal/services/appconfiguration/app_configuration_feature_resource.go b/internal/services/appconfiguration/app_configuration_feature_resource.go index 786197e6b6150..86b6b673622bc 100644 --- a/internal/services/appconfiguration/app_configuration_feature_resource.go +++ b/internal/services/appconfiguration/app_configuration_feature_resource.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/hashicorp/go-azure-helpers/lang/response" "log" "strings" "time" @@ -334,8 +335,13 @@ func (k FeatureResource) Read() sdk.ResourceFunc { return fmt.Errorf("while parsing resource ID: %+v", err) } + domainSuffix, ok := metadata.Client.Account.Environment.AppConfiguration.DomainSuffix() + if !ok { + return fmt.Errorf("could not determine AppConfiguration domain suffix for environment %q", metadata.Client.Account.Environment.Name) + } + subscriptionId := commonids.NewSubscriptionID(metadata.Client.Account.SubscriptionId) - configurationStoreIdRaw, err := metadata.Client.AppConfiguration.ConfigurationStoreIDFromEndpoint(ctx, subscriptionId, nestedItemId.ConfigurationStoreEndpoint) + configurationStoreIdRaw, err := metadata.Client.AppConfiguration.ConfigurationStoreIDFromEndpoint(ctx, subscriptionId, nestedItemId.ConfigurationStoreEndpoint, *domainSuffix) if err != nil { return fmt.Errorf("while retrieving the Resource ID of Configuration Store at Endpoint: %q: %s", nestedItemId.ConfigurationStoreEndpoint, err) } @@ -350,11 +356,11 @@ func (k FeatureResource) Read() sdk.ResourceFunc { return err } - ok, err := metadata.Client.AppConfiguration.Exists(ctx, *configurationStoreId) + exists, err := metadata.Client.AppConfiguration.Exists(ctx, *configurationStoreId) if err != nil { return fmt.Errorf("while checking Configuration Store %q for feature %q existence: %v", *configurationStoreId, *nestedItemId, err) } - if !ok { + if !exists { log.Printf("[DEBUG] Configuration Store %q for feature %q was not found - removing from state", *configurationStoreId, *nestedItemId) return metadata.MarkAsGone(nestedItemId) } @@ -367,7 +373,7 @@ func (k FeatureResource) Read() sdk.ResourceFunc { kv, err := client.GetKeyValue(ctx, nestedItemId.Key, nestedItemId.Label, "", "", "", []appconfiguration.KeyValueFields{}) if err != nil { if v, ok := err.(autorest.DetailedError); ok { - if utils.ResponseWasNotFound(autorest.Response{Response: v.Response}) { + if response.WasNotFound(v.Response) { return metadata.MarkAsGone(nestedItemId) } } diff --git a/internal/services/appconfiguration/app_configuration_key_resource.go b/internal/services/appconfiguration/app_configuration_key_resource.go index 43c0b033bbe29..9d1aa87279673 100644 --- a/internal/services/appconfiguration/app_configuration_key_resource.go +++ b/internal/services/appconfiguration/app_configuration_key_resource.go @@ -251,8 +251,13 @@ func (k KeyResource) Read() sdk.ResourceFunc { return fmt.Errorf("while parsing resource ID: %+v", err) } + domainSuffix, ok := metadata.Client.Account.Environment.AppConfiguration.DomainSuffix() + if !ok { + return fmt.Errorf("could not determine AppConfiguration domain suffix for environment %q", metadata.Client.Account.Environment.Name) + } + subscriptionId := commonids.NewSubscriptionID(metadata.Client.Account.SubscriptionId) - configurationStoreIdRaw, err := metadata.Client.AppConfiguration.ConfigurationStoreIDFromEndpoint(ctx, subscriptionId, nestedItemId.ConfigurationStoreEndpoint) + configurationStoreIdRaw, err := metadata.Client.AppConfiguration.ConfigurationStoreIDFromEndpoint(ctx, subscriptionId, nestedItemId.ConfigurationStoreEndpoint, *domainSuffix) if err != nil { return fmt.Errorf("while retrieving the Resource ID of Configuration Store at Endpoint: %q: %s", nestedItemId.ConfigurationStoreEndpoint, err) } @@ -267,11 +272,11 @@ func (k KeyResource) Read() sdk.ResourceFunc { return err } - ok, err := metadata.Client.AppConfiguration.Exists(ctx, *configurationStoreId) + exists, err := metadata.Client.AppConfiguration.Exists(ctx, *configurationStoreId) if err != nil { return fmt.Errorf("while checking Configuration Store %q for feature %q existence: %v", *configurationStoreId, *nestedItemId, err) } - if !ok { + if !exists { log.Printf("[DEBUG] Configuration Store %q for feature %q was not found - removing from state", *configurationStoreId, *nestedItemId) return metadata.MarkAsGone(nestedItemId) } diff --git a/internal/services/appconfiguration/client/helpers.go b/internal/services/appconfiguration/client/helpers.go index f05402b5046ce..4125056415cac 100644 --- a/internal/services/appconfiguration/client/helpers.go +++ b/internal/services/appconfiguration/client/helpers.go @@ -36,8 +36,8 @@ func (c *Client) AddToCache(configurationStoreId configurationstores.Configurati keysmith.Unlock() } -func (c *Client) ConfigurationStoreIDFromEndpoint(ctx context.Context, subscriptionId commonids.SubscriptionId, configurationStoreEndpoint string) (*string, error) { - configurationStoreName, err := c.parseNameFromEndpoint(configurationStoreEndpoint) +func (c *Client) ConfigurationStoreIDFromEndpoint(ctx context.Context, subscriptionId commonids.SubscriptionId, configurationStoreEndpoint, domainSuffix string) (*string, error) { + configurationStoreName, err := c.parseNameFromEndpoint(configurationStoreEndpoint, domainSuffix) if err != nil { return nil, err } @@ -160,7 +160,7 @@ func (c *Client) cacheKeyForConfigurationStore(name string) string { return strings.ToLower(name) } -func (c *Client) parseNameFromEndpoint(input string) (*string, error) { +func (c *Client) parseNameFromEndpoint(input, domainSuffix string) (*string, error) { uri, err := url.ParseRequestURI(input) if err != nil { return nil, err @@ -169,10 +169,10 @@ func (c *Client) parseNameFromEndpoint(input string) (*string, error) { // https://the-appconfiguration.azconfig.io // https://the-appconfiguration.azconfig.azure.cn // https://the-appconfiguration.azconfig.azure.us + if !strings.HasPrefix(uri.Host, domainSuffix) { + return nil, fmt.Errorf("expected a URI in the format `https://somename.%s` but got %q", domainSuffix, uri.Host) + } segments := strings.Split(uri.Host, ".") - if len(segments) < 3 || segments[1] != "azconfig" { - return nil, fmt.Errorf("expected a URI in the format `https://the-appconfiguration.azconfig.**` but got %q", uri.Host) - } return &segments[0], nil } diff --git a/internal/services/appconfiguration/migration/feature_resource_v0_to_v1.go b/internal/services/appconfiguration/migration/feature_resource_v0_to_v1.go index f0b6dc7dafcd0..f299c041d5917 100644 --- a/internal/services/appconfiguration/migration/feature_resource_v0_to_v1.go +++ b/internal/services/appconfiguration/migration/feature_resource_v0_to_v1.go @@ -9,7 +9,6 @@ import ( "log" "strings" - "github.com/hashicorp/go-azure-sdk/sdk/environments" "github.com/hashicorp/go-azure-sdk/resource-manager/appconfiguration/2023-03-01/configurationstores" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" "github.com/hashicorp/terraform-provider-azurerm/internal/services/appconfiguration/parse" @@ -53,10 +52,10 @@ func (FeatureResourceV0ToV1) UpgradeFunc() pluginsdk.StateUpgraderFunc { domainSuffix, ok := meta.(*clients.Client).Account.Environment.AppConfiguration.DomainSuffix() if !ok { - return fmt.Errorf("App Configuration is not supported in this Environment") + return rawState, fmt.Errorf("App Configuration is not supported in this Environment") } - configurationStoreEndpoint := fmt.Sprintf("https://%s.%s", configurationStoreId.ConfigurationStoreName, domainSuffix) + configurationStoreEndpoint := fmt.Sprintf("https://%s.%s", configurationStoreId.ConfigurationStoreName, *domainSuffix) featureKey := fmt.Sprintf("%s/%s", FeatureKeyPrefix, parsedOldId.Name) nestedItemId, err := parse.NewNestedItemID(configurationStoreEndpoint, featureKey, parsedOldId.Label) if err != nil { diff --git a/internal/services/appconfiguration/migration/feature_resource_v0_to_v1_test.go b/internal/services/appconfiguration/migration/feature_resource_v0_to_v1_test.go index a715e16786678..468b8a58a9676 100644 --- a/internal/services/appconfiguration/migration/feature_resource_v0_to_v1_test.go +++ b/internal/services/appconfiguration/migration/feature_resource_v0_to_v1_test.go @@ -7,54 +7,85 @@ import ( "context" "testing" + "github.com/hashicorp/go-azure-sdk/sdk/environments" + "github.com/hashicorp/terraform-provider-azurerm/internal/clients" "github.com/hashicorp/terraform-provider-azurerm/utils" ) func TestFeatureResourceV0ToV1(t *testing.T) { testData := []struct { - name string - input map[string]interface{} - expected *string + name string + input map[string]interface{} + expected *string + appConfigurationEnvironment environments.Api }{ { name: "old id (normal)", input: map[string]interface{}{ "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationFeature/keyName/Label/labelName", }, - expected: utils.String("https://appConf1.azconfig.io/kv/.appconfig.featureflag%2FkeyName?label=labelName"), + expected: utils.String("https://appConf1.azconfig.io/kv/.appconfig.featureflag%2FkeyName?label=labelName"), + appConfigurationEnvironment: environments.AzurePublic().AppConfiguration, }, { name: "old id (complicated)", input: map[string]interface{}{ "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationFeature/key:name/test/Label/test:label/name", }, - expected: utils.String("https://appConf1.azconfig.io/kv/.appconfig.featureflag%2Fkey:name%2Ftest?label=test%3Alabel%2Fname"), + expected: utils.String("https://appConf1.azconfig.io/kv/.appconfig.featureflag%2Fkey:name%2Ftest?label=test%3Alabel%2Fname"), + appConfigurationEnvironment: environments.AzurePublic().AppConfiguration, }, { name: "old id (no label)", input: map[string]interface{}{ "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationFeature/keyName/Label/%00", }, - expected: utils.String("https://appConf1.azconfig.io/kv/.appconfig.featureflag%2FkeyName?label="), + expected: utils.String("https://appConf1.azconfig.io/kv/.appconfig.featureflag%2FkeyName?label="), + appConfigurationEnvironment: environments.AzurePublic().AppConfiguration, }, { name: "old id (\000 label)", input: map[string]interface{}{ "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationFeature/keyName/Label/\000", }, - expected: utils.String("https://appConf1.azconfig.io/kv/.appconfig.featureflag%2FkeyName?label="), + expected: utils.String("https://appConf1.azconfig.io/kv/.appconfig.featureflag%2FkeyName?label="), + appConfigurationEnvironment: environments.AzurePublic().AppConfiguration, }, { name: "old id (empty label)", input: map[string]interface{}{ "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationFeature/keyName/Label/", }, - expected: utils.String("https://appConf1.azconfig.io/kv/.appconfig.featureflag%2FkeyName?label="), + expected: utils.String("https://appConf1.azconfig.io/kv/.appconfig.featureflag%2FkeyName?label="), + appConfigurationEnvironment: environments.AzurePublic().AppConfiguration, + }, + { + name: "old id (empty label - china)", + input: map[string]interface{}{ + "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationFeature/keyName/Label/", + }, + expected: utils.String("https://appConf1.azconfig.azure.cn/kv/.appconfig.featureflag%2FkeyName?label="), + appConfigurationEnvironment: environments.AzureChina().AppConfiguration, + }, + { + name: "old id (empty label - usgov)", + input: map[string]interface{}{ + "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationFeature/keyName/Label/", + }, + expected: utils.String("https://appConf1.azconfig.azure.us/kv/.appconfig.featureflag%2FkeyName?label="), + appConfigurationEnvironment: environments.AzureUSGovernment().AppConfiguration, }, } for _, test := range testData { t.Logf("Testing %q...", test.name) - result, err := FeatureResourceV0ToV1{}.UpgradeFunc()(context.TODO(), test.input, nil) + client := &clients.Client{ + Account: &clients.ResourceManagerAccount{ + Environment: environments.Environment{ + AppConfiguration: test.appConfigurationEnvironment, + }, + }, + } + result, err := FeatureResourceV0ToV1{}.UpgradeFunc()(context.TODO(), test.input, client) if err != nil && test.expected == nil { continue } else { diff --git a/internal/services/appconfiguration/migration/key_resource_v0_to_v1.go b/internal/services/appconfiguration/migration/key_resource_v0_to_v1.go index debd23af2b4e8..774ccf2dc01eb 100644 --- a/internal/services/appconfiguration/migration/key_resource_v0_to_v1.go +++ b/internal/services/appconfiguration/migration/key_resource_v0_to_v1.go @@ -82,7 +82,9 @@ func KeyResourceSchemaForV0AndV1() map[string]*pluginsdk.Schema { Type: pluginsdk.TypeBool, }, "tags": { - Elem: &pluginsdk.Schema{Type: pluginsdk.TypeString}, + Elem: &pluginsdk.Schema{ + Type: pluginsdk.TypeString, + }, Optional: true, Type: pluginsdk.TypeMap, }, diff --git a/internal/services/appconfiguration/migration/key_resource_v1_to_v2.go b/internal/services/appconfiguration/migration/key_resource_v1_to_v2.go index 34d42c7381888..7a6fc4bd036cd 100644 --- a/internal/services/appconfiguration/migration/key_resource_v1_to_v2.go +++ b/internal/services/appconfiguration/migration/key_resource_v1_to_v2.go @@ -9,7 +9,6 @@ import ( "log" "strings" - "github.com/hashicorp/go-azure-sdk/sdk/environments" "github.com/hashicorp/go-azure-sdk/resource-manager/appconfiguration/2023-03-01/configurationstores" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" "github.com/hashicorp/terraform-provider-azurerm/internal/services/appconfiguration/parse" @@ -55,10 +54,10 @@ func (KeyResourceV1ToV2) UpgradeFunc() pluginsdk.StateUpgraderFunc { domainSuffix, ok := meta.(*clients.Client).Account.Environment.AppConfiguration.DomainSuffix() if !ok { - return fmt.Errorf("App Configuration is not supported in this Environment") + return rawState, fmt.Errorf("App Configuration is not supported in this Environment") } - configurationStoreEndpoint := fmt.Sprintf("https://%s.%s", configurationStoreId.ConfigurationStoreName, domainSuffix) + configurationStoreEndpoint := fmt.Sprintf("https://%s.%s", configurationStoreId.ConfigurationStoreName, *domainSuffix) nestedItemId, err := parse.NewNestedItemID(configurationStoreEndpoint, parsedOldId.Key, parsedOldId.Label) if err != nil { return rawState, err diff --git a/internal/services/appconfiguration/migration/key_resource_v1_to_v2_test.go b/internal/services/appconfiguration/migration/key_resource_v1_to_v2_test.go index 2882132ac9955..ad405f336b01c 100644 --- a/internal/services/appconfiguration/migration/key_resource_v1_to_v2_test.go +++ b/internal/services/appconfiguration/migration/key_resource_v1_to_v2_test.go @@ -7,61 +7,93 @@ import ( "context" "testing" + "github.com/hashicorp/go-azure-sdk/sdk/environments" + "github.com/hashicorp/terraform-provider-azurerm/internal/clients" "github.com/hashicorp/terraform-provider-azurerm/utils" ) func TestKeyResourceV1ToV2(t *testing.T) { testData := []struct { - name string - input map[string]interface{} - expected *string + name string + input map[string]interface{} + expected *string + appConfigurationEnvironment environments.Api }{ { name: "old id (normal)", input: map[string]interface{}{ "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationKey/keyName/Label/labelName", }, - expected: utils.String("https://appConf1.azconfig.io/kv/keyName?label=labelName"), + expected: utils.String("https://appConf1.azconfig.io/kv/keyName?label=labelName"), + appConfigurationEnvironment: environments.AzurePublic().AppConfiguration, }, { name: "old id (complicated)", input: map[string]interface{}{ "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationKey/key:name/test/Label/test:label/name", }, - expected: utils.String("https://appConf1.azconfig.io/kv/key:name%2Ftest?label=test%3Alabel%2Fname"), + expected: utils.String("https://appConf1.azconfig.io/kv/key:name%2Ftest?label=test%3Alabel%2Fname"), + appConfigurationEnvironment: environments.AzurePublic().AppConfiguration, }, { name: "old id (no label)", input: map[string]interface{}{ "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationKey/keyName/Label/%00", }, - expected: utils.String("https://appConf1.azconfig.io/kv/keyName?label="), + expected: utils.String("https://appConf1.azconfig.io/kv/keyName?label="), + appConfigurationEnvironment: environments.AzurePublic().AppConfiguration, }, { name: "old id (\000 label)", input: map[string]interface{}{ "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationKey/keyName/Label/\000", }, - expected: utils.String("https://appConf1.azconfig.io/kv/keyName?label="), + expected: utils.String("https://appConf1.azconfig.io/kv/keyName?label="), + appConfigurationEnvironment: environments.AzurePublic().AppConfiguration, }, { name: "old id (empty label)", input: map[string]interface{}{ "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationKey/keyName/Label/", }, - expected: utils.String("https://appConf1.azconfig.io/kv/keyName?label="), + expected: utils.String("https://appConf1.azconfig.io/kv/keyName?label="), + appConfigurationEnvironment: environments.AzurePublic().AppConfiguration, }, { name: "old id (fix bug with no-label)", input: map[string]interface{}{ "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationKey/keyName/Label/\000/AppConfigurationKey/keyName/Label/", }, - expected: utils.String("https://appConf1.azconfig.io/kv/keyName?label="), + expected: utils.String("https://appConf1.azconfig.io/kv/keyName?label="), + appConfigurationEnvironment: environments.AzurePublic().AppConfiguration, + }, + { + name: "old id (fix bug with no-label - china)", + input: map[string]interface{}{ + "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationKey/keyName/Label/\000/AppConfigurationKey/keyName/Label/", + }, + expected: utils.String("https://appConf1.azconfig.azure.cn/kv/keyName?label="), + appConfigurationEnvironment: environments.AzureChina().AppConfiguration, + }, + { + name: "old id (fix bug with no-label - usgov)", + input: map[string]interface{}{ + "id": "/subscriptions/12345678-1234-5678-1234-123456789012/resourceGroups/resourceGroup1/providers/Microsoft.AppConfiguration/configurationStores/appConf1/AppConfigurationKey/keyName/Label/\000/AppConfigurationKey/keyName/Label/", + }, + expected: utils.String("https://appConf1.azconfig.azure.us/kv/keyName?label="), + appConfigurationEnvironment: environments.AzureUSGovernment().AppConfiguration, }, } for _, test := range testData { t.Logf("Testing %q...", test.name) - result, err := KeyResourceV1ToV2{}.UpgradeFunc()(context.TODO(), test.input, nil) + client := &clients.Client{ + Account: &clients.ResourceManagerAccount{ + Environment: environments.Environment{ + AppConfiguration: test.appConfigurationEnvironment, + }, + }, + } + result, err := KeyResourceV1ToV2{}.UpgradeFunc()(context.TODO(), test.input, client) if err != nil && test.expected == nil { continue } else {