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

AwsCustomResource with delete only action fails with Invalid PhysicalResourceId #6061

Closed
chrisgit opened this issue Feb 2, 2020 · 6 comments · Fixed by #6363
Closed

AwsCustomResource with delete only action fails with Invalid PhysicalResourceId #6061

chrisgit opened this issue Feb 2, 2020 · 6 comments · Fixed by #6363
Assignees
Labels
@aws-cdk/custom-resources Related to AWS CDK Custom Resources bug This issue is a bug. in-progress This issue is being actively worked on. p1

Comments

@chrisgit
Copy link

chrisgit commented Feb 2, 2020

I have written some lambda backed custom resources using the provider framework but in some cases the task involved is very simple and does not require the overhead of creating a bespoke lambda.

The CDK offers an alternative to the provider framework which is a custom resource that calls an AWS SDK API

Sometimes a single API call can fill the gap in the CloudFormation coverage. In this case you can use the AwsCustomResource construct. This construct creates a custom resource that can be customized to make specific API calls for the CREATE, UPDATE and DELETE events. Additionally, data returned by the API call can be extracted and used in other constructs/resources (creating a real CloudFormation dependency using Fn::GetAtt under the hood).

The physical id of the custom resource can be specified or derived from the data returned by the API call.

The scenarios is that in a development environment I want to remove any CDK generated secrets and KMS keys.

My understanding is that I can use an AwsCustomResource with just the onDelete event specified, however creating the resource produces an Invalid PhysicalResourceId error.

Reproduction Steps

  // Example
  const key = new Key(this, 'kms-key', {
      alias: 'KMS-Test-Delete-Alias',
       description: 'This is a test KMS key using custom resource',
       enabled: true,
   });

  new AwsCustomResource(this, 'kmsDeleteKeyResource', {
             onDelete: {
                service: 'KMS',
                action: 'scheduleKeyDeletion',
                parameters: {
                    KeyId: key.keyArn,
                    PendingWindowInDays: 7
                },
             },
        });

Error Log

5/8 | 14:40:35 | CREATE_COMPLETE      | AWS::KMS::Key         | kms-key (kmskey49FBC3B3)
5/8 | 14:40:37 | CREATE_IN_PROGRESS   | AWS::KMS::Alias       | kms-key/Alias (kmskeyAlias39245779)
5/8 | 14:40:37 | CREATE_IN_PROGRESS   | Custom::AWS           | kmsDeleteKeyResource/Resource/Default (kmsDeleteKeyResource97160D9C)
5/8 Currently in progress: kmskeyAlias39245779, kmsDeleteKeyResource97160D9C
6/8 | 14:41:31 | CREATE_FAILED        | Custom::AWS           | kmsDeleteKeyResource/Resource/Default (kmsDeleteKeyResource97160D9C) Invalid PhysicalResourceId
               new CustomResource (C:\ScratchPad - AWS\cdk\kms-delete-key-api\node_modules\@aws-cdk\aws-cloudformation\lib\custom-resource.ts:163:21)
               \_ new AwsCustomResource (C:\ScratchPad - AWS\cdk\kms-delete-key-api\node_modules\@aws-cdk\custom-resources\lib\aws-custom-resource\aws-custom-resource.ts:209:27)
               \_ new DemoStack (C:\ScratchPad - AWS\cdk\kms-delete-key-api\example\demo-stack.ts:24:9)
               \_ Object.<anonymous> (C:\ScratchPad - AWS\cdk\kms-delete-key-api\example\cdk.ts:6:17)

Environment

  • CLI Version : aws-cli/1.16.185 Python/3.7.3 Windows/10 botocore/1.12.175
  • Framework Version: aws-cdk@1.22.0
  • OS : Windows 10 1803
  • Language : Typescript

Other


This is 🐛 Bug Report

@chrisgit chrisgit added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 2, 2020
@SomayaB SomayaB added the @aws-cdk/custom-resources Related to AWS CDK Custom Resources label Feb 3, 2020
@eladb
Copy link
Contributor

eladb commented Feb 4, 2020

@jogold would you like to take a look?

@eladb eladb added the p1 label Feb 4, 2020
@jogold
Copy link
Contributor

jogold commented Feb 5, 2020

@jogold would you like to take a look?

Yes

@jogold
Copy link
Contributor

jogold commented Feb 6, 2020

@chrisgit in the meantime you can use the following workaround:

const key = new Key(this, 'kms-key', {
  alias: 'KMS-Test-Delete-Alias',
    description: 'This is a test KMS key using custom resource',
    enabled: true,
});

new AwsCustomResource(this, 'kmsDeleteKeyResource', {
  onCreate: { // Dummy call
    service: 'KMS',
    action: 'describeKey',
    parameters: {
      KeyId: key.keyArn
    },
    physicalResourceId: key.keyArn,
  },
  onDelete: {
    service: 'KMS',
    action: 'scheduleKeyDeletion',
    parameters: {
        KeyId: key.keyArn,
        PendingWindowInDays: 7
    },
  },
})

@chrisgit
Copy link
Author

Thank you very much for taking the time to look at this issue.

@jogold Thank you, the work around provided works as expected.

@jogold
Copy link
Contributor

jogold commented Feb 12, 2020

@eladb what do you think is best here?

  • change the API so that we have a new physicalResourceId prop in AwsCustomResourceProps (= default for all type of calls) and keep only physicalResourceIdPath in AwsSdkCall interface that would act as an override for a specific call
  • just handle the case of an AwsCustomResource having only a onDelete by automatically providing some sort of default physical resource id for the Create request

@eladb
Copy link
Contributor

eladb commented Feb 19, 2020

I think that making an action with just onDelete "just work" is probably a better experience.

jogold added a commit to jogold/aws-cdk that referenced this issue Feb 19, 2020
Correctly find the default physical resource id.

Default to logical resource id for a create event with delete only call.

Fixes aws#6061
@SomayaB SomayaB added in-progress This issue is being actively worked on. and removed needs-triage This issue or PR still needs to be triaged. labels Feb 19, 2020
@mergify mergify bot closed this as completed in #6363 Feb 26, 2020
mergify bot added a commit that referenced this issue Feb 26, 2020
#6363)

* fix(custom-resources): AwsCustomResource with delete only action fails

Correctly find the default physical resource id.

Default to logical resource id for a create event with delete only call.

Fixes #6061

* break

Co-Authored-By: Elad Ben-Israel <benisrae@amazon.com>

* integ test

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/custom-resources Related to AWS CDK Custom Resources bug This issue is a bug. in-progress This issue is being actively worked on. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants