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

custom-resource: Passthrough physicalResourceId when its not specified in onUpdate at AwsCustomResource #23843

Closed
1 of 2 tasks
konokenj opened this issue Jan 26, 2023 · 3 comments · Fixed by #24194
Closed
1 of 2 tasks
Labels
@aws-cdk/custom-resources Related to AWS CDK Custom Resources effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@konokenj
Copy link
Contributor

Describe the feature

Currently, when using AwsCustomResource construct, users have to specify physicalResourceId in onUpdate.
This limitation is brought by AwsCustomResource copies onUpdate (physicalResourceId is NOT mandatory) to onCreate (physicalResourceId is mandatory) when onCreate is not specified.

AwsCustomResource should be able to omit physicalResourceId in onUpdate with passthrough it from request. This is useful when users want to call an update API without resoure replacing.

Some AWS APIs send empty response in update. This causes problem in below situation:

  • Create API generates unpredictable IDs, and return it in response
    • Users set physicalResourceId: PhysicalResourceId.fromResponse()
  • Update API requires resource ID (not Name), and return empty response
    • Users provide resouce ID by new PhysicalResourceIdReference() in parameters
    • With empty response, users can't use physicalResourceId: PhysicalResourceId.fromResponse() or any ID generation

Use Case

To create / update / delete resources that is not covered by CloudFormation. If API specification matches above conditions, this feature is critical. Like below:

Proposed Solution

Passthrough physicalResourceId from request to response in onUpdate when physicalResourceId is not specified.
To do that, update input check logic for physicalResourceId.

This change is not breaking, because it's not a problem if users can specify physicalResourceId in onUpdate.

onCreate onCreate.physicalResourceId onUpdate onUpdate.physicalResourceId validation error now? validation error after change?
AwsSdkCall specified AwsSdkCall specified no no
AwsSdkCall specified AwsSdkCall undefined yes no <-
AwsSdkCall undefined AwsSdkCall undefined yes yes
AwsSdkCall undefined AwsSdkCall specified yes yes
undefined (copied from onUpdate) undefined AwsSdkCall specified no no
undefined (copied from onUpdate) undefined AwsSdkCall undefined yes yes
AwsSdkCall specified undefined undefined no no
AwsSdkCall undefined undefined undefined yes yes

Other Information

Instead of omit physicalResourceId to passthrough, we can provide a property like passthroughPhysicalResourceId: true. But I think its more intuitive that omit physicalResourceId.

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.61.1

Environment details (OS name and version, etc.)

Node.js 16.13.0, MacOS 12.6.2

@konokenj konokenj added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jan 26, 2023
@github-actions github-actions bot added the @aws-cdk/custom-resources Related to AWS CDK Custom Resources label Jan 26, 2023
@khushail
Copy link
Contributor

Hi @konokenj , thanks for reaching out. It would be great if you could provide some more information on what you are trying to solve and the solution steps as well. Thanks.

@khushail khushail added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 15, 2023
@khushail khushail self-assigned this Feb 15, 2023
konokenj added a commit to konokenj/aws-cdk that referenced this issue Feb 16, 2023
@konokenj
Copy link
Contributor Author

konokenj commented Feb 16, 2023

Hi @khushail, thank you for your ownership.
The handler function is implemented to return from event object if PhysicalResourceId is unspecified, so I simply change Construct's error checking.

physicalResourceId = event.ResourceProperties[event.RequestType]?.physicalResourceId?.id ?? event.PhysicalResourceId;

I created a PR.
#24194

konokenj added a commit to konokenj/aws-cdk that referenced this issue Feb 16, 2023
@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Feb 16, 2023
@khushail khushail removed their assignment Mar 1, 2023
konokenj added a commit to konokenj/aws-cdk that referenced this issue Mar 6, 2023
@mergify mergify bot closed this as completed in #24194 Mar 8, 2023
mergify bot pushed a commit that referenced this issue Mar 8, 2023
…m request when omit it in onUpdate (#24194)

AwsCustomResource is now able to omit `physicalResourceId` in `onUpdate` to copy it from request.

Some `UPDATE` AWS APIs responses with an empty body. When users want to call these APIs using AwsCustomResource, users can't specify physicalResourceId by `PhysicalResourceId.fromResponse()`. Furthermore, when the Create API generates an unpredictable ID and this must be passed to the Update API, this Construct could not be used. For example, following APIs match this situation:

- https://docs.aws.amazon.com/athena/latest/APIReference/API_UpdateNotebook.html
- https://docs.aws.amazon.com/singlesignon/latest/IdentityStoreAPIReference/API_UpdateUser.html

Closes #23843.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Mar 8, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

homakk pushed a commit to homakk/aws-cdk that referenced this issue Mar 28, 2023
…m request when omit it in onUpdate (aws#24194)

AwsCustomResource is now able to omit `physicalResourceId` in `onUpdate` to copy it from request.

Some `UPDATE` AWS APIs responses with an empty body. When users want to call these APIs using AwsCustomResource, users can't specify physicalResourceId by `PhysicalResourceId.fromResponse()`. Furthermore, when the Create API generates an unpredictable ID and this must be passed to the Update API, this Construct could not be used. For example, following APIs match this situation:

- https://docs.aws.amazon.com/athena/latest/APIReference/API_UpdateNotebook.html
- https://docs.aws.amazon.com/singlesignon/latest/IdentityStoreAPIReference/API_UpdateUser.html

Closes aws#23843.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
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 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
2 participants