-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(custom-resources): AwsResource - support custom resource type #5248
Conversation
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
if (event.RequestType === 'Delete' && call.physicalResourceIdPath) { | ||
call.parameters[call.physicalResourceIdPath] = physicalResourceId; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand it correctly, you want to be able to use the "current" physical id somewhere in you API call for the delete event?
The physicalResourceIdPath
works the other way around: extracts the physical id from the response to return it to CloudFormation. You didn't update the documentation for this prop, so only you would know about this behavior here.
How about having some sort of tag ({{PHYSICAL_ID}}
?) that could be used in the parameters
prop and that would get replaced in the Lambda before the API call?
Can you maybe detail your use case?
@eladb opinion on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add an example inline here, and will update the PR tomorrow morning.
When I create a DatasetGroup
resource using AwsCustomResource
like this:
new AwsCustomResource(this, 'DatasetGroup', {
onCreate: {
service: 'Personalize',
action: 'createDatasetGroup',
parameters: {
name: 'TestDatasetGroup'
},
physicalResourceIdPath: 'datasetGroupArn'
},
onDelete: {
service: 'Personalize',
action: 'deleteDatasetGroup'
}
})
The resource is created as expected.
But, when I tear down the stack, the call parameters for the deleteDatasetGroup
API call don't contain the field datasetGroupArn
, the only parameter that's needed for the call.
Same issues exists for other resources on Personalize & Forecast.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't the following work here?
const groupName = 'TestDatasetGroup';
new AwsCustomResource(this, 'CreateDatasetGroup', {
onCreate: {
service: 'Personalize',
action: 'createDatasetGroup',
parameters: {
name: groupName
},
physicalResourceId: groupName
},
onDelete: {
service: 'Personalize',
action: 'deleteDatasetGroup',
parameters: { // arn:aws:personalize:<region>:<account>:dataset-group/TestDatasetGroup
datasetGroupArn: cdk.Stack.of(this).formatArn({
service: 'personalize',
resource: 'dataset-group',
resourceName: groupName
})
}
}
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am with @jogold
packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/custom-resources/lib/aws-custom-resource/aws-custom-resource.ts
Outdated
Show resolved
Hide resolved
8da5a11
to
9ec5d60
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert the change related to physical resource ID in DELETE
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
5 similar comments
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
9ec5d60
to
f761a78
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request is now being automatically merged. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request is now being automatically merged. |
Allow specifying a custom resource type for
AwsResource
s.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license