fix(diff): properties from ChangeSet diff were ignored #30268
Conversation
…s from the template with what's in the changeset
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
bergjaak
changed the title
github-actions
bot
added
bug
| // If fetchAll is true, traverse all pages from the change set description. | ||
| while (fetchAll && response.NextToken != null) { | ||
| const nextPage = await cfn.describeChangeSet({ | ||
| StackName: stackName, | ||
| ChangeSetName: response.ChangeSetId ?? changeSetName, | ||
| NextToken: response.NextToken, | ||
| IncludePropertyValues: true, |
There was a problem hiding this comment.
I need to handle this for regions where this parameter isn't supported
| @@ -191,21 +191,31 @@ export class CloudFormationStack { | |||
| * | |||
| * @returns CloudFormation information about the ChangeSet | |||
| */ | |||
| async function describeChangeSet( | |||
| export async function describeChangeSet( | |||
There was a problem hiding this comment.
this is where we get the changeset for the diff
|
The pull request linter fails with the following errors: PRs must pass status checks before we can provide a meaningful review. If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing |
|
➡️ PR build request submitted to A maintainer must now check the pipeline and add the |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
…eset to templateDiff (#30332) ### Reason for this change I am making this change as part of #30268, but implementing the bug fix in a satisfactory way is becoming much, much, much more difficult than I thought it would. As it's now possible to view the changed values before and after a changeset is applied by using the DescribeChangeSets api with IncludePropertyValues, but the API is difficult to use because of not being supported in all regions, not including StatusReason, and being unable to paginate. So, I want to make that fix in a separate PR, once this refactor change is done. ### Description of changes * A ton of unit tests and moved changeset diff logic into a dedicated class and file. ### Description of how you validated changes * Many unit tests, integration tests, and manual 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*
Issue # (if applicable)
Closes #29731
Reason for this change
This is a second attempt at https://github.com/aws/aws-cdk/pull/30093/files, after that PR had to be reverted due to causing issues. (That pr made it possible for ResourceTypes to be undefined, which caused
includesto be invoked on undefined.)Description of changes
includesto be called onundefinedin the previous PR attempting this change -- I changed the resourceType getter to state that it can return undefined, so that callers easily know to handle undefined.include-property-valuesto our describeChangeSet API call so that the changeset description includes the before and after values of the changed properties, which is a pretty new CloudFormation feature https://aws.amazon.com/about-aws/whats-new/2024/04/aws-cloudformation-changesets-enhanced-change-visibility-deployments/cdk diff --security-only).Description of how you validated changes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license