fix(cli): cdk diff always falls back to template only diff
#32165
Conversation
iliapolo
temporarily deployed
to
test-pipeline
GitHub Actions
Inactive
| @@ -1064,6 +1064,46 @@ integTest( | |||
| }), | |||
| ); | |||
|
|
|||
| integTest( | |||
| 'cdk diff doesnt show resource metadata changes', | |||
There was a problem hiding this comment.
I'm not entirely sure why, but removing resource metadata changes is the expected behavior when performing diffs with change-sets (see #28336), so this is a solid way to make sure the diff actually creates the change set and uses it.
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.
aws-cdk-automation
added
the
pr/needs-cli-test-run
| }); | ||
|
|
||
| // Assert there are no changes | ||
| expect(diff).toContain('There were no differences'); |
There was a problem hiding this comment.
Not ideal but better than asserting on a debug statement. I don't see us changing this specific output string any time soon.
| @@ -400,7 +400,6 @@ async function uploadBodyParameterAndCreateChangeSet( | |||
| env.resolvedEnvironment, | |||
| new AssetManifestBuilder(), | |||
| env.resources, | |||
| env.sdk, | |||
There was a problem hiding this comment.
It doesn't need the SDK anymore because:
aws-cdk/packages/aws-cdk/lib/api/util/template-body-parameter.ts
Lines 126 to 132 in 740db43
Not removing it from the invocation resulted in it being passed as the overrideTemplate argument to the function (which is of type any), which later on is being passed to YAML for serialization. So...thats no good.
|
➡️ PR build request submitted to A maintainer must now check the pipeline and add the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #32165 +/- ##
=======================================
Coverage 77.19% 77.19%
=======================================
Files 105 105
Lines 7164 7164
Branches 1311 1311
=======================================
Hits 5530 5530
Misses 1454 1454
Partials 180 180
Flags with carried forward coverage won't be shown. Click here to find out more.
|
mrgrain
added
the
pr-linter/exempt-integ-test
|
Build passed in CodeBuild. Not clear why status not reported back. |
mrgrain
added
pr-linter/cli-integ-tested
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Closes #32160
Reason for this change
When running
cdk deploy, it is supposed to (by default) create a read-only change set and incorporate it within the diff.However, it currently fails creating the change-set and always falls back to template only diffs.
Description of changes
There was a wrong invocation of the
makeBodyParameterparameter after the migration to sdk v3.Description of how you validated changes
Added missing integration tests. Unit test for this code are tricky because they require too many mocks.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license