Skip to content

Commit

Permalink
fix(cli): cdk diff always falls back to template only diff (#32165)
Browse files Browse the repository at this point in the history
Closes #32160

### Reason for this change

<!--What is the bug or use case behind 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

<!--What code changes did you make? Have you made any important design
decisions?-->

There was a wrong invocation of the `makeBodyParameter` parameter after
the [migration to sdk v3](#31702).

### Description of how you validated changes

Added missing integration tests. Unit test for this code are tricky
because they require too many mocks.

### 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*
  • Loading branch information
iliapolo authored Nov 17, 2024
1 parent 740db43 commit 089e9d8
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 1 deletion.
11 changes: 11 additions & 0 deletions packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,15 @@ class AppSyncHotswapStack extends cdk.Stack {
}
}

class MetadataStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);
const handle = new cdk.CfnWaitConditionHandle(this, 'WaitConditionHandle');
handle.addMetadata('Key', process.env.INTEG_METADATA_VALUE ?? 'default')

}
}

const app = new cdk.App({
context: {
'@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID, // Force all assets to be unique, but consistent in one build
Expand Down Expand Up @@ -877,6 +886,8 @@ switch (stackSet) {
new ExportValueStack(app, `${stackPrefix}-export-value-stack`);

new BundlingStage(app, `${stackPrefix}-bundling-stage`);

new MetadataStack(app, `${stackPrefix}-metadata`);
break;

case 'stage-using-context':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,46 @@ integTest(
}),
);

integTest(
'cdk diff doesnt show resource metadata changes',
withDefaultFixture(async (fixture) => {

// GIVEN - small initial stack with default resource metadata
await fixture.cdkDeploy('metadata');

// WHEN - changing resource metadata value
const diff = await fixture.cdk(['diff', fixture.fullStackName('metadata')], {
verbose: true,
modEnv: {
INTEG_METADATA_VALUE: 'custom',
},
});

// Assert there are no changes
expect(diff).toContain('There were no differences');
}),
);

integTest(
'cdk diff shows resource metadata changes with --no-change-set',
withDefaultFixture(async (fixture) => {

// GIVEN - small initial stack with default resource metadata
await fixture.cdkDeploy('metadata');

// WHEN - changing resource metadata value
const diff = await fixture.cdk(['diff --no-change-set', fixture.fullStackName('metadata')], {
verbose: true,
modEnv: {
INTEG_METADATA_VALUE: 'custom',
},
});

// Assert there are changes
expect(diff).not.toContain('There were no differences');
}),
);

integTest('cdk diff with large changeset and custom toolkit stack name and qualifier does not fail', withoutBootstrap(async (fixture) => {
// Bootstrapping with custom toolkit stack name and qualifier
const qualifier = 'abc1111';
Expand Down
1 change: 0 additions & 1 deletion packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,6 @@ async function uploadBodyParameterAndCreateChangeSet(
env.resolvedEnvironment,
new AssetManifestBuilder(),
env.resources,
env.sdk,
);
const cfn = env.sdk.cloudFormation();
const exists = (await CloudFormationStack.lookup(cfn, options.stack.stackName, false)).exists;
Expand Down

0 comments on commit 089e9d8

Please sign in to comment.