Skip to content

Commit

Permalink
Fix parsing nested path ids (#3375)
Browse files Browse the repository at this point in the history 
Fixes #3368

- Sanitise regex group name to only allow alpha-numeric characters.
- Map back to original name.
- Tidy test cases with nesting.

This will currently result in an additional input being generated during
the import. This then appears during the next up as a property deletion,
though it actually has no effect.

## Demonstration

Using the `examples/keyvault-accesspolicies/` example:

1. Perform `up` gives the state (inputs and outputs):
   ```
                     "policy": {
                        "objectId": "xxxx",
                        "permissions": {
                            "keys": [
                                "get",
                                "create",
                                "delete",
                                "list"
                            ],
                            "secrets": [
                                "get",
                                "list",
                                "set",
                                "delete"
                            ]
                        },
                        "tenantId": "xxxx"
                    },
                    "resourceGroupName": "rgxxxx",
                    "vaultName": "vaultxxxx"
   ```
2. Delete the resource from the state: `pulumi state delete
urn:pulumi:dev::scratch::azure-native:keyvault:AccessPolicy::ap1`
3. Import the resource: `pulumi import resource ap1
"/subscriptions/xxxx/resourceGroups/rgxxxx/providers/Microsoft.KeyVault/vaults/vaultxxxx/accessPolicy/xxxx"`
4. Check the state with `pulumi stack export`, the inputs now include
`object.Id`:
   ```
                    "policy": { ... },
                    "policy.objectId": "xxxx",
                    "resourceGroupName": "rgxxxx",
                    "vaultName": "vaultxxxx"
   ```
5. Perform another up (`pulumi up`).
   Preview shows removal of policy => ObjectId: 
<img width="529" alt="image"
src="https://github.com/pulumi/pulumi-azure-native/assets/331676/77a8605a-518d-43fb-929e-86ed6195aaf8">
   State inputs revert to remove `policy.objectId`: 
   ```
                    "policy": { ... },
                    "resourceGroupName": "rgxxxx",
                    "vaultName": "vaultxxxx"
   ```

## Tracing usage of ParseResourceID

The result from this function is used here:


https://github.com/pulumi/pulumi-azure-native/blob/e8feca2c41d0e5d904eee9cc13eeea774d51ec1d/provider/pkg/provider/provider.go#L1080-L1087

The `pathItems` are passed into `ResponseToSdkInputs` where they are
used if they match one of the parameters in the metadata:


https://github.com/pulumi/pulumi-azure-native/blob/e8feca2c41d0e5d904eee9cc13eeea774d51ec1d/provider/pkg/convert/responseToSdkInputs.go#L18-L24

This will match:

https://github.com/pulumi/pulumi-azure-native/blob/e8feca2c41d0e5d904eee9cc13eeea774d51ec1d/provider/pkg/resources/customresources/custom_keyvault_accesspolicy.go#L67
  • Loading branch information
@danielrbradley
danielrbradley authored Jun 24, 2024
1 parent 9366e2d commit 9ad4f0e
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 41 deletions.
17 changes: 13 additions & 4 deletions provider/pkg/resources/parseResourceId.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,18 @@ 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, "")
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 {
regexParts[i] = pathParts[i]
}
Expand All @@ -35,9 +43,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
Expand Down
90 changes: 53 additions & 37 deletions provider/pkg/resources/parseResourceId_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}

0 comments on commit 9ad4f0e

Please sign in to comment.