Skip to content

Commit

Permalink
support slash in name and label property of `azurerm_app_configur…
Browse files Browse the repository at this point in the history
…ation_feature` (hashicorp#19470)

fix hashicorp#19437
  • Loading branch information
ziyeqf authored and favoretti committed Jan 12, 2023
1 parent 0b605fd commit d9f441e
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,20 @@ func TestAccAppConfigurationFeature_basicNoLabel(t *testing.T) {
})
}

func TestAccAppConfigurationFeature_basicWithSlash(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_app_configuration_feature", "test")
r := AppConfigurationFeatureResource{}
data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.basicWithSlash(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func TestAccAppConfigurationFeature_basicNoFilters(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_app_configuration_feature", "test")
r := AppConfigurationFeatureResource{}
Expand Down Expand Up @@ -258,6 +272,47 @@ resource "azurerm_app_configuration_feature" "test" {
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger)
}

func (t AppConfigurationFeatureResource) basicWithSlash(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}
resource "azurerm_resource_group" "test" {
name = "acctestRG-appconfig-%d"
location = "%s"
}
data "azurerm_client_config" "test" {
}
resource "azurerm_role_assignment" "test" {
scope = azurerm_resource_group.test.id
role_definition_name = "App Configuration Data Owner"
principal_id = data.azurerm_client_config.test.object_id
}
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"
depends_on = [
azurerm_role_assignment.test,
]
}
resource "azurerm_app_configuration_feature" "test" {
configuration_store_id = azurerm_app_configuration.test.id
description = "test description"
name = "acctest/ackey/%d"
label = "acctest/label"
enabled = true
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger)
}

func (t AppConfigurationFeatureResource) requiresImport(data acceptance.TestData) string {
return fmt.Sprintf(`
%s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ func (k KeyResource) Create() sdk.ResourceFunc {

appCfgKeyResourceID := parse.AppConfigurationKeyId{
ConfigurationStoreId: model.ConfigurationStoreId,
Key: url.QueryEscape(model.Key),
Label: url.QueryEscape(model.Label),
Key: model.Key,
Label: model.Label,
}

kv, err := client.GetKeyValue(ctx, model.Key, model.Label, "", "", "", []string{})
Expand Down Expand Up @@ -213,26 +213,16 @@ func (k KeyResource) Read() sdk.ResourceFunc {
return err
}

decodedKey, err := url.QueryUnescape(resourceID.Key)
if err != nil {
return fmt.Errorf("while decoding key of resource ID: %+v", err)
}

decodedLabel, err := url.QueryUnescape(resourceID.Label)
if err != nil {
return fmt.Errorf("while decoding label of resource ID: %+v", err)
}

kv, err := client.GetKeyValue(ctx, decodedKey, decodedLabel, "", "", "", []string{})
kv, err := client.GetKeyValue(ctx, resourceID.Key, 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", 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)
return fmt.Errorf("while checking for key's %q existence: %+v", resourceID.Label, err)
}

model := KeyResourceModel{
Expand Down
85 changes: 85 additions & 0 deletions internal/services/appconfiguration/parse/app_configuration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package parse

import (
"fmt"
"net/url"
"strings"

"github.com/hashicorp/terraform-provider-azurerm/helpers/azure"
)

// 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, err := url.QueryUnescape(components[current+1])
if err != nil {
return nil, fmt.Errorf("parsing value of id components %q: %+v", value, err)
}

// 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
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package parse

import (
"fmt"
"net/url"
"regexp"
"strings"

"github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids"
"github.com/hashicorp/terraform-provider-azurerm/helpers/azure"
)

var _ resourceids.Id = AppConfigurationFeatureId{}
Expand All @@ -30,7 +31,7 @@ func (id AppConfigurationFeatureId) String() string {
}

func FeatureId(input string) (*AppConfigurationFeatureId, error) {
resourceID, err := azure.ParseAzureResourceID(input)
resourceID, err := parseAzureResourceID(handleSlashInIdForFeature(input))
if err != nil {
return nil, fmt.Errorf("while parsing resource ID: %+v", err)
}
Expand All @@ -54,3 +55,18 @@ func FeatureId(input string) (*AppConfigurationFeatureId, error) {

return &appcfgID, nil
}

// a workaround to support "/" in id
func handleSlashInIdForFeature(input string) string {
oldNames := regexp.MustCompile(`AppConfigurationFeature\/(.+)\/Label`).FindStringSubmatch(input)
if len(oldNames) == 2 {
input = strings.Replace(input, oldNames[1], url.QueryEscape(oldNames[1]), 1)
}

oldNames = regexp.MustCompile(`AppConfigurationFeature\/.+\/Label\/(.+)`).FindStringSubmatch(input)
if len(oldNames) == 2 {
input = strings.Replace(input, oldNames[1], url.QueryEscape(oldNames[1]), 1)
}

return input
}
80 changes: 12 additions & 68 deletions internal/services/appconfiguration/parse/app_configuration_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package parse

import (
"fmt"
"net/url"
"regexp"
"strings"

"github.com/hashicorp/go-azure-helpers/resourcemanager/resourceids"
"github.com/hashicorp/terraform-provider-azurerm/helpers/azure"
)

var _ resourceids.Id = AppConfigurationKeyId{}
Expand All @@ -30,7 +31,7 @@ func (id AppConfigurationKeyId) String() string {
}

func KeyId(input string) (*AppConfigurationKeyId, error) {
resourceID, err := parseAzureResourceID(input)
resourceID, err := parseAzureResourceID(handleSlashInIdForKey(input))
if err != nil {
return nil, fmt.Errorf("while parsing resource ID: %+v", err)
}
Expand All @@ -55,75 +56,18 @@ 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)
// a workaround to support "/" in id
func handleSlashInIdForKey(input string) string {
oldNames := regexp.MustCompile(`AppConfigurationKey\/(.+)\/Label`).FindStringSubmatch(input)
if len(oldNames) == 2 {
input = strings.Replace(input, oldNames[1], url.QueryEscape(oldNames[1]), 1)
}

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
}
oldNames = regexp.MustCompile(`AppConfigurationKey\/.+\/Label\/(.+)`).FindStringSubmatch(input)
if len(oldNames) == 2 {
input = strings.Replace(input, oldNames[1], url.QueryEscape(oldNames[1]), 1)
}

// 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 input

return idObj, nil
}

0 comments on commit d9f441e

Please sign in to comment.