custom-resources: PR:29648 caused Custom resources to be replaced #30121
Comments
p3tek
added
bug
github-actions
bot
added
the
@aws-cdk/custom-resources
khushail
added
investigating
khushail
added
response-requested
|
Hey @p3tek, thanks for raising this. From what I'm seeing the problem isn't necessarily the new event property being passed. What I mean is that any change we make to the underlying handler code (new property or not) will also trigger an update since it will force a change in the asset hash. This would lock us into a situation where we are unable to make any code changes because they would all force an update. Are you able to just duplicate whatever SDK call you're making for |
github-actions
bot
removed
the
response-requested
|
Basically the CDK version upgrade causes new parameter to be added -> CR onUpdate is executed -> random problem if the CR Update method is not tested/used a lot/never. |
|
@khushail is it known and accepted issue that CDK upgrade can trigger all custom resources update execution? |
|
Thanks for the response and apologies for delay in getting back to you I actually resolved it on my end by doing exactly as you recommended (added an onUpdate) and duplicated the onCreate Idealy though when a new attribute is added it shouldn't be forced into the template when it's not specified (for custom resources at least where some custom code is going to get executed) |
|
@colifran Triggering all CR's is a big thing. The pipeline can be stuck for hours, onUpdate etc. method are not normally tested -> suddenly those are tested/used. What you think? |
|
@heikkis I was under the impression that any change to the handler code would have triggered an update, but I've learned that is not the case. That said I agree. I think this should be fixed so that the default is that no parameter is passed. Will work on a PR for this. |
…ce event properties by default (#30418) Closes #30121, #29949 ### Reason for this change PR #29648 introduced a new resource property `logApiResponseData`. This resource property is `true` by default which forces an update for `AwsCustomResource`. For users without `onUpdate` configured an empty data object is returned if no SDK call is configured. This can cause an attribute error if the user is depending on a data from a specific SDK call. ### Description of changes Made `logApiResponseData` undefined by default which will not trigger `onUpdate`. To maintain backwards compatibility with the original PR introducing `logApiResponseData` as true by default, I've also introduced a feature flag that will allow users to keep the current behavior so they aren't now forced into another `onUpdate` event. ### Description of how you validated changes Updated unit tests where `logApiResponseData` was added as a resource property. Added new unit test to verify that `logApiResponseData` could be added to the event. Updated unit tests that test `_render()` to ensure that the default case will result in an empty object. Updated integ tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
…ce event properties by default (aws#30418) Closes aws#30121, aws#29949 ### Reason for this change PR aws#29648 introduced a new resource property `logApiResponseData`. This resource property is `true` by default which forces an update for `AwsCustomResource`. For users without `onUpdate` configured an empty data object is returned if no SDK call is configured. This can cause an attribute error if the user is depending on a data from a specific SDK call. ### Description of changes Made `logApiResponseData` undefined by default which will not trigger `onUpdate`. To maintain backwards compatibility with the original PR introducing `logApiResponseData` as true by default, I've also introduced a feature flag that will allow users to keep the current behavior so they aren't now forced into another `onUpdate` event. ### Description of how you validated changes Updated unit tests where `logApiResponseData` was added as a resource property. Added new unit test to verify that `logApiResponseData` could be added to the event. Updated unit tests that test `_render()` to ensure that the default case will result in an empty object. Updated integ tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ce event properties by default (aws#30418) Closes aws#30121, aws#29949 ### Reason for this change PR aws#29648 introduced a new resource property `logApiResponseData`. This resource property is `true` by default which forces an update for `AwsCustomResource`. For users without `onUpdate` configured an empty data object is returned if no SDK call is configured. This can cause an attribute error if the user is depending on a data from a specific SDK call. ### Description of changes Made `logApiResponseData` undefined by default which will not trigger `onUpdate`. To maintain backwards compatibility with the original PR introducing `logApiResponseData` as true by default, I've also introduced a feature flag that will allow users to keep the current behavior so they aren't now forced into another `onUpdate` event. ### Description of how you validated changes Updated unit tests where `logApiResponseData` was added as a resource property. Added new unit test to verify that `logApiResponseData` could be added to the event. Updated unit tests that test `_render()` to ensure that the default case will result in an empty object. Updated integ tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one. |
Describe the bug
the following PR - #29648 added an additional "logApiResponseData":true to the custom resource properties.
This triggered a replacement of existing custom resources which is not a good outcome for "onCreate" resources that are only meant to be executed once
Expected Behavior
Not have existing custom-resources replaced with this update.
Current Behavior
"logApiResponseData":true was not present prior to this PR and the custom resource was not triggered on stack update
Reproduction Steps
will trigger after an update of aws-cdk-lib
Possible Solution
allow this value to be suppressed so it doesn't trigger a resource replacement ?
Additional Information/Context
No response
CDK CLI Version
2.138.0
Framework Version
2.138.0
Node.js Version
18.16.1
OS
linux
Language
TypeScript
Language Version
No response
Other information
No response
The text was updated successfully, but these errors were encountered: