From d6ce7e60aadc270d0c2093996c778239a113f537 Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Fri, 21 Jun 2024 09:36:51 +0100 Subject: [PATCH 1/3] Add new failing test case - Nest existing test cases. --- .../pkg/resources/parseResourceId_test.go | 90 +++++++++++-------- 1 file changed, 53 insertions(+), 37 deletions(-) diff --git a/provider/pkg/resources/parseResourceId_test.go b/provider/pkg/resources/parseResourceId_test.go index 7abbdd54ecbe..9bf0240aecd0 100644 --- a/provider/pkg/resources/parseResourceId_test.go +++ b/provider/pkg/resources/parseResourceId_test.go @@ -11,43 +11,59 @@ type resourceIDCase struct { path string } -func TestParseInvalidResourceID(t *testing.T) { - cases := []resourceIDCase{ - // ID shorter than Path - {"/resourceGroup/myrg", "/resourceGroup/{resourceGroup}/subResource"}, - // ID longer than Path - {"/resourceGroup/myrg/cdn/mycdn", "/resourceGroup/{resourceGroup}/cdn"}, - // Segment names don't match - {"/resourceGroup/myrg/foo/mycdn", "/resourceGroup/{resourceGroup}/bar/{cdn}"}, - } - for _, testCase := range cases { - _, err := ParseResourceID(testCase.id, testCase.path) - assert.Error(t, err) - } -} +func TestParseResourceID(t *testing.T) { + t.Run("invalid", func(t *testing.T) { + cases := []resourceIDCase{ + // ID shorter than Path + {"/resourceGroup/myrg", "/resourceGroup/{resourceGroup}/subResource"}, + // ID longer than Path + {"/resourceGroup/myrg/cdn/mycdn", "/resourceGroup/{resourceGroup}/cdn"}, + // Segment names don't match + {"/resourceGroup/myrg/foo/mycdn", "/resourceGroup/{resourceGroup}/bar/{cdn}"}, + } + for _, testCase := range cases { + _, err := ParseResourceID(testCase.id, testCase.path) + assert.Error(t, err) + } + }) -func TestParseFullResourceID(t *testing.T) { - id := "/subscriptions/0282681f-7a9e-123b-40b2-96babd57a8a1/resourcegroups/pulumi-name/providers/Microsoft.Network/networkInterfaces/pulumi-nic/ipConfigurations/ipconfig1" - path := "/subscriptions/{subscriptionID}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/networkInterfaces/{networkInterfaceName}/ipConfigurations/{ipConfigurationName}" - actual, err := ParseResourceID(id, path) - assert.NoError(t, err) - expected := map[string]string{ - "subscriptionID": "0282681f-7a9e-123b-40b2-96babd57a8a1", - "resourceGroupName": "pulumi-name", - "networkInterfaceName": "pulumi-nic", - "ipConfigurationName": "ipconfig1", - } - assert.Equal(t, expected, actual) -} + t.Run("full", func(t *testing.T) { + id := "/subscriptions/0282681f-7a9e-123b-40b2-96babd57a8a1/resourcegroups/pulumi-name/providers/Microsoft.Network/networkInterfaces/pulumi-nic/ipConfigurations/ipconfig1" + path := "/subscriptions/{subscriptionID}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/networkInterfaces/{networkInterfaceName}/ipConfigurations/{ipConfigurationName}" + actual, err := ParseResourceID(id, path) + assert.NoError(t, err) + expected := map[string]string{ + "subscriptionID": "0282681f-7a9e-123b-40b2-96babd57a8a1", + "resourceGroupName": "pulumi-name", + "networkInterfaceName": "pulumi-nic", + "ipConfigurationName": "ipconfig1", + } + assert.Equal(t, expected, actual) + }) + + t.Run("scoped", func(t *testing.T) { + id := "/subscriptions/1200b1c8-3c58-42db-b33a-304a75913333/resourceGroups/devops-dev/providers/Microsoft.Authorization/roleAssignments/2a88abc7-f599-0eba-a21f-a1817e597115" + path := "/{scope}/providers/Microsoft.Authorization/roleAssignments/{roleAssignmentName}" + actual, err := ParseResourceID(id, path) + assert.NoError(t, err) + expected := map[string]string{ + "scope": "subscriptions/1200b1c8-3c58-42db-b33a-304a75913333/resourceGroups/devops-dev", + "roleAssignmentName": "2a88abc7-f599-0eba-a21f-a1817e597115", + } + assert.Equal(t, expected, actual) + }) -func TestParseScopedResourceID(t *testing.T) { - id := "/subscriptions/1200b1c8-3c58-42db-b33a-304a75913333/resourceGroups/devops-dev/providers/Microsoft.Authorization/roleAssignments/2a88abc7-f599-0eba-a21f-a1817e597115" - path := "/{scope}/providers/Microsoft.Authorization/roleAssignments/{roleAssignmentName}" - actual, err := ParseResourceID(id, path) - assert.NoError(t, err) - expected := map[string]string{ - "scope": "subscriptions/1200b1c8-3c58-42db-b33a-304a75913333/resourceGroups/devops-dev", - "roleAssignmentName": "2a88abc7-f599-0eba-a21f-a1817e597115", - } - assert.Equal(t, expected, actual) + t.Run("nested identifier", func(t *testing.T) { + id := "/subscriptions/1200b1c8-3c58-42db-b33a-304a75913333/resourceGroups/resource-group/providers/Microsoft.KeyVault/vaults/vault-name/accessPolicy/policy-object-id" + path := "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.KeyVault/vaults/{vaultName}/accessPolicy/{policy.objectId}" + actual, err := ParseResourceID(id, path) + assert.NoError(t, err) + expected := map[string]string{ + "subscriptionId": "1200b1c8-3c58-42db-b33a-304a75913333", + "resourceGroupName": "resource-group", + "vaultName": "vault-name", + "policy.objectId": "policy-object-id", + } + assert.Equal(t, expected, actual) + }) } From 5a3915c0450ee8bb9e02c039b72e106887ae53ee Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Fri, 21 Jun 2024 10:05:48 +0100 Subject: [PATCH 2/3] Fix parsing ids with odd path parts Retain the original path parameter names when returning the result object. --- provider/pkg/resources/parseResourceId.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/provider/pkg/resources/parseResourceId.go b/provider/pkg/resources/parseResourceId.go index 40ab3abadb58..108461512139 100644 --- a/provider/pkg/resources/parseResourceId.go +++ b/provider/pkg/resources/parseResourceId.go @@ -14,10 +14,15 @@ import ( func ParseResourceID(id, path string) (map[string]string, error) { pathParts := strings.Split(path, "/") regexParts := make([]string, len(pathParts)) + // Track the names of the regex matches so we can map them back to the original names. + regexNames := make(map[string]string, len(pathParts)) + regexMatchGroupNameReplacer := regexp.MustCompile(`[^a-zA-Z0-9]`) for i, s := range pathParts { if strings.HasPrefix(s, "{") && strings.HasSuffix(s, "}") { name := s[1 : len(s)-1] - regexParts[i] = fmt.Sprintf("(?P<%s>.*?)", name) + regexName := regexMatchGroupNameReplacer.ReplaceAllString(name, "") + regexNames[regexName] = name + regexParts[i] = fmt.Sprintf("(?P<%s>.*?)", regexName) } else { regexParts[i] = pathParts[i] } @@ -35,9 +40,10 @@ func ParseResourceID(id, path string) (map[string]string, error) { } result := map[string]string{} - for i, name := range pattern.SubexpNames() { - if i > 0 && name != "" { - result[name] = match[i] + for i, regexpGroupName := range pattern.SubexpNames() { + if i > 0 && regexpGroupName != "" { + originalName := regexNames[regexpGroupName] + result[originalName] = match[i] } } return result, nil From e1553f3a1c35587306a96c94ac00e4638b1483f4 Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Mon, 24 Jun 2024 14:07:04 +0100 Subject: [PATCH 3/3] Fail on conflicting regex names --- provider/pkg/resources/parseResourceId.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/provider/pkg/resources/parseResourceId.go b/provider/pkg/resources/parseResourceId.go index 108461512139..870ea326eb23 100644 --- a/provider/pkg/resources/parseResourceId.go +++ b/provider/pkg/resources/parseResourceId.go @@ -21,6 +21,9 @@ func ParseResourceID(id, path string) (map[string]string, error) { if strings.HasPrefix(s, "{") && strings.HasSuffix(s, "}") { name := s[1 : len(s)-1] regexName := regexMatchGroupNameReplacer.ReplaceAllString(name, "") + if clashingPathName, ok := regexNames[regexName]; ok { + return nil, errors.Errorf("clashing path names: '%s' and '%s' while parsing resource ID for path '%s", clashingPathName, name, path) + } regexNames[regexName] = name regexParts[i] = fmt.Sprintf("(?P<%s>.*?)", regexName) } else {