Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parsing nested path ids #3375

Merged
merged 4 commits into from
Jun 24, 2024
Merged

Fix parsing nested path ids #3375

merged 4 commits into from
Jun 24, 2024

Conversation

danielrbradley
Copy link
Member

@danielrbradley danielrbradley commented Jun 21, 2024

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:
    image
    State inputs revert to remove policy.objectId:
                     "policy": { ... },
                     "resourceGroupName": "rgxxxx",
                     "vaultName": "vaultxxxx"
    

Tracing usage of ParseResourceID

The result from this function is used here:

// There may be no old state (i.e., importing a new resource).
// Extract inputs from resource's ID and response body.
pathItems, err := resources.ParseResourceID(id, res.Path)
if err != nil {
return nil, err
}
inputMap := k.converter.ResponseToSdkInputs(res.PutParameters, pathItems, response)
inputs = resource.NewPropertyMapFromMap(inputMap)

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

case param.Location == "path":
name := param.Name
sdkName := name
if param.Value != nil && param.Value.SdkName != "" {
sdkName = param.Value.SdkName
}
result[sdkName] = pathValues[name]

This will match:

{Name: "policy.objectId", Location: "path", Value: &AzureAPIProperty{Type: "string"}},

@danielrbradley danielrbradley self-assigned this Jun 21, 2024
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Copy link

codecov bot commented Jun 21, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Project coverage is 56.88%. Comparing base (9366e2d) to head (674dc7c).

Files Patch % Lines
provider/pkg/resources/parseResourceId.go 81.81% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3375      +/-   ##
==========================================
+ Coverage   56.87%   56.88%   +0.01%     
==========================================
  Files          66       66              
  Lines        8092     8099       +7     
==========================================
+ Hits         4602     4607       +5     
- Misses       3055     3056       +1     
- Partials      435      436       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Nest existing test cases.
Retain the original path parameter names when returning the result object.
@danielrbradley danielrbradley force-pushed the fix-parsing-nested-id branch from e8feca2 to 5a3915c Compare June 24, 2024 12:15
@danielrbradley danielrbradley marked this pull request as ready for review June 24, 2024 12:41
@danielrbradley danielrbradley requested a review from thomas11 June 24, 2024 12:54
@danielrbradley danielrbradley requested a review from thomas11 June 24, 2024 13:33
@danielrbradley danielrbradley enabled auto-merge (squash) June 24, 2024 13:36
@danielrbradley danielrbradley merged commit 9ad4f0e into master Jun 24, 2024
23 checks passed
@danielrbradley danielrbradley deleted the fix-parsing-nested-id branch June 24, 2024 19:19
@scp-mb
Copy link

scp-mb commented Sep 4, 2024

@danielrbradley I am currently trying to import some access policies in a c# stack programmatically, but it seems to think the import will fail due to policy.objectId being missing, despite being specified. I'm assuming a bug from this change.

pulumi import on the cli has the following output for the preview, note the duplicated objectId on both the root and nested under policy.

policy           : {
            objectId   : "xxx"
            permissions: {
                certificates: [
                    [0]: "All"
                ]
                keys        : [
                    [0]: "All"
                ]
                secrets     : [
                    [0]: "All"
                ]
            }
            tenantId   : "xxx"
        }
        policy.objectId  : "xxx"
        resourceGroupName: "xxx"
        vaultName        : "xxx"

Attempting to import via code, with Policy.ObjectId set correctly via AccessPolicyEntryArgs:

warning: inputs to import do not match the existing resource; importing this resource will fail
    = azure-native:keyvault:AccessPolicy: (import)
        [id=/subscriptions/xxx/resourceGroups/xxx/providers/Microsoft.KeyVault/vaults/xxx/accessPolicy/xxx]
        [urn=urn:pulumi:dev::Api::azure-native:keyvault:AccessPolicy::xxx]
        [provider=urn:pulumi:dev::Api::pulumi:providers:azure-native::default_2_59_0::04da6b54-80e4-46f7-96ec-b56ff0331ba9]
      - policy.objectId: "xxx"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Import Key vault access policy : Failed to compile expression
3 participants