aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.41k stars 3.8k forks source link

custom_resources: onEvent response disagreement on if PhysicalResourceId is required #29304

Open mtfurlan opened 5 months ago

mtfurlan commented 5 months ago

Describe the issue

When implementing a CustomResource, the onEvent handler must return some json. There is disagreement about if PhysicalResourceId is required.

This repo says it's not required.

The CloudFormation guide says that it is required.

The typescript definitions say it is required, but looks like it was trying to make it only required if it's Update or Delete and then they did all event types.

I can make PRs once I know what it should be. Though I don't know where the CloudFormation guide source is.

Links

pahud commented 5 months ago

The allocated/assigned physical ID of the resource. If omitted for Create events, the event's RequestId will be used. For Update, the current physical ID will be used. If a different value is returned, CloudFormation will follow with a subsequent Delete for the previous ID (resource replacement). For Delete, it will always return the current physical resource ID, and if the user returns a different one, an error will occur.

I believe it could be omitted in the custom resource provider framework because the framework would generate one for you in some cases.

https://github.com/aws/aws-cdk/blob/a7de7feb6a14658ec25f4cfda434d5e1d69157d2/packages/aws-cdk-lib/custom-resources/lib/provider-framework/runtime/framework.ts#L160

mtfurlan commented 5 months ago

Cool, so it's optional in all cases according to the actual code.

How do you feel about the wording of

The allocated/assigned physical ID of the resource. Default value for Create events is the event's RequestId. Default value for Update and Delete events is the current physical ID. If a different value is returned for an Update, CloudFormation will follow with a subsequent Delete for the previous ID (resource replacement). If a different value for Delete is returned, an error will occur.

Where the source of CloudFormation guide?

Actually, any idea if any of the other fields are required? The tests seem to just return PhysicalResourceId, Data, and ArbitraryField, and an aws example provider lambda only returns PhysicalResourceId for Update/Delete.