From 9cf05eefdb9393db00391c31327ed9af60f0fbcb Mon Sep 17 00:00:00 2001 From: aristosvo <8375124+aristosvo@users.noreply.github.com> Date: Mon, 25 Oct 2021 22:57:50 +0200 Subject: [PATCH] `azurerm_app_configuration_key`: Support for slashes in `key` (#13859) Fixes #13852 --- .../app_configuration_key_resource.go | 27 ++++-- .../app_configuration_key_resource_test.go | 42 ++++++++++ .../parse/app_configuration_key.go | 83 +++++++++++++++++-- 3 files changed, 139 insertions(+), 13 deletions(-) diff --git a/internal/services/appconfiguration/app_configuration_key_resource.go b/internal/services/appconfiguration/app_configuration_key_resource.go index 4c2217519e00..9dc974ca3e52 100644 --- a/internal/services/appconfiguration/app_configuration_key_resource.go +++ b/internal/services/appconfiguration/app_configuration_key_resource.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "net/url" "time" "github.com/Azure/go-autorest/autorest" @@ -128,7 +129,7 @@ func (k KeyResource) Create() sdk.ResourceFunc { appCfgKeyResourceID := parse.AppConfigurationKeyId{ ConfigurationStoreId: model.ConfigurationStoreId, - Key: model.Key, + Key: url.QueryEscape(model.Key), Label: model.Label, } @@ -206,16 +207,21 @@ func (k KeyResource) Read() sdk.ResourceFunc { return err } - kv, err := client.GetKeyValue(ctx, resourceID.Key, resourceID.Label, "", "", "", []string{}) + decodedKey, err := url.QueryUnescape(resourceID.Key) + if err != nil { + return fmt.Errorf("while decoding key of resource ID: %+v", err) + } + + kv, err := client.GetKeyValue(ctx, decodedKey, resourceID.Label, "", "", "", []string{}) if err != nil { if v, ok := err.(autorest.DetailedError); ok { if utils.ResponseWasNotFound(autorest.Response{Response: v.Response}) { return metadata.MarkAsGone(resourceID) } } else { - return fmt.Errorf("while checking for key's %q existence: %+v", resourceID.Key, err) + return fmt.Errorf("while checking for key's %q existence: %+v", decodedKey, err) } - return fmt.Errorf("while checking for key's %q existence: %+v", resourceID.Key, err) + return fmt.Errorf("while checking for key's %q existence: %+v", decodedKey, err) } model := KeyResourceModel{ @@ -325,13 +331,18 @@ func (k KeyResource) Delete() sdk.ResourceFunc { return err } - if _, err = client.DeleteLock(ctx, resourceID.Key, resourceID.Label, "", ""); err != nil { - return fmt.Errorf("while unlocking key/label pair %s/%s: %+v", resourceID.Key, resourceID.Label, err) + decodedKey, err := url.QueryUnescape(resourceID.Key) + if err != nil { + return fmt.Errorf("while decoding key of resource ID: %+v", err) + } + + if _, err = client.DeleteLock(ctx, decodedKey, resourceID.Label, "", ""); err != nil { + return fmt.Errorf("while unlocking key/label pair %s/%s: %+v", decodedKey, resourceID.Label, err) } - _, err = client.DeleteKeyValue(ctx, resourceID.Key, resourceID.Label, "") + _, err = client.DeleteKeyValue(ctx, decodedKey, resourceID.Label, "") if err != nil { - return fmt.Errorf("while removing key %q from App Configuration Store %q: %+v", resourceID.Key, resourceID.ConfigurationStoreId, err) + return fmt.Errorf("while removing key %q from App Configuration Store %q: %+v", decodedKey, resourceID.ConfigurationStoreId, err) } return nil diff --git a/internal/services/appconfiguration/app_configuration_key_resource_test.go b/internal/services/appconfiguration/app_configuration_key_resource_test.go index 0436ed0c190f..37644b25814d 100644 --- a/internal/services/appconfiguration/app_configuration_key_resource_test.go +++ b/internal/services/appconfiguration/app_configuration_key_resource_test.go @@ -96,6 +96,20 @@ func TestAccAppConfigurationKey_KVToVault(t *testing.T) { }) } +func TestAccAppConfigurationKey_slash(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_app_configuration_key", "test") + r := AppConfigurationKeyResource{} + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.slash(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + func TestAccAppConfigurationKey_requiresImport(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_app_configuration_key", "test") r := AppConfigurationKeyResource{} @@ -176,6 +190,34 @@ resource "azurerm_app_configuration_key" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger) } +func (t AppConfigurationKeyResource) slash(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-appconfig-%d" + location = "%s" +} + +resource "azurerm_app_configuration" "test" { + name = "testacc-appconf%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + sku = "standard" +} + +resource "azurerm_app_configuration_key" "test" { + configuration_store_id = azurerm_app_configuration.test.id + key = "/acctest/-ackey/-%d" + content_type = "test" + label = "acctest-ackeylabel-%d" + value = "a test" +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger) +} + func (t AppConfigurationKeyResource) basicNoLabel(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { diff --git a/internal/services/appconfiguration/parse/app_configuration_key.go b/internal/services/appconfiguration/parse/app_configuration_key.go index 5643472161d8..5df2d6788388 100644 --- a/internal/services/appconfiguration/parse/app_configuration_key.go +++ b/internal/services/appconfiguration/parse/app_configuration_key.go @@ -18,7 +18,7 @@ func (k AppConfigurationKeyId) ID() string { } func KeyId(input string) (*AppConfigurationKeyId, error) { - resourceID, err := azure.ParseAzureResourceID(input) + resourceID, err := parseAzureResourceID(input) if err != nil { return nil, fmt.Errorf("while parsing resource ID: %+v", err) } @@ -31,10 +31,10 @@ func KeyId(input string) (*AppConfigurationKeyId, error) { Label: label, } - // Golang's URL parser will translate %00 to \000 (NUL). This will only happen if we're dealing with an empty - // label, so we set the label to the expected value (empty string) and trim the input string, so we can properly - // extract the configuration store ID out of it. - if label == "\000" { + // Label will have a "%00" placeholder if we're dealing with an empty label, + // so we set the label to the expected value (empty string) and trim the input + // string, so we can properly extract the configuration store ID out of it. + if label == "%00" { appcfgID.Label = "" input = strings.TrimSuffix(input, "%00") } @@ -42,3 +42,76 @@ func KeyId(input string) (*AppConfigurationKeyId, error) { return &appcfgID, nil } + +// specific parser to prevent decoding of the ID +func parseAzureResourceID(id string) (*azure.ResourceID, error) { + id = strings.TrimPrefix(id, "/") + id = strings.TrimSuffix(id, "/") + + components := strings.Split(id, "/") + + // We should have an even number of key-value pairs. + if len(components)%2 != 0 { + return nil, fmt.Errorf("the number of path segments is not divisible by 2 in %q", id) + } + + var subscriptionID string + var provider string + + // Put the constituent key-value pairs into a map + componentMap := make(map[string]string, len(components)/2) + for current := 0; current < len(components); current += 2 { + key := components[current] + value := components[current+1] + + // Check key/value for empty strings. + if key == "" || value == "" { + return nil, fmt.Errorf("Key/Value cannot be empty strings. Key: '%s', Value: '%s'", key, value) + } + + switch { + case key == "subscriptions" && subscriptionID == "": + // Catch the subscriptionID before it can be overwritten by another "subscriptions" + // value in the ID which is the case for the Service Bus subscription resource + subscriptionID = value + case key == "providers" && provider == "": + // Catch the provider before it can be overwritten by another "providers" + // value in the ID which can be the case for the Role Assignment resource + provider = value + default: + componentMap[key] = value + } + } + + // Build up a TargetResourceID from the map + idObj := &azure.ResourceID{} + idObj.Path = componentMap + + if subscriptionID != "" { + idObj.SubscriptionID = subscriptionID + } else { + return nil, fmt.Errorf("no subscription ID found in: %q", id) + } + + if resourceGroup, ok := componentMap["resourceGroups"]; ok { + idObj.ResourceGroup = resourceGroup + delete(componentMap, "resourceGroups") + } else if resourceGroup, ok := componentMap["resourcegroups"]; ok { + // Some Azure APIs are weird and provide things in lower case... + // However it's not clear whether the casing of other elements in the URI + // matter, so we explicitly look for that case here. + idObj.ResourceGroup = resourceGroup + delete(componentMap, "resourcegroups") + } + + if provider != "" { + idObj.Provider = provider + } + + if secondaryProvider := componentMap["providers"]; secondaryProvider != "" { + idObj.SecondaryProvider = secondaryProvider + delete(componentMap, "providers") + } + + return idObj, nil +}