Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): diff allow specifying changeSet via cdk.json #29375

Closed
wants to merge 9 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,27 @@ integTest('enableDiffNoFail', withDefaultFixture(async (fixture) => {
type DiffParameters = { fail?: boolean; enableDiffNoFail: boolean };
}));

integTest('cdk diff should not show verbose messages by default', withDefaultFixture(async (fixture) => {
const diff = await fixture.cdk(['diff', fixture.fullStackName('test-1')], { verbose: false });
expect(diff).not.toContain('Hold on while we create a read-only change set');
}));

integTest('cdk diff should only show verbose messages in --verbose mode', withDefaultFixture(async (fixture) => {
const diff = await fixture.cdk(['diff', fixture.fullStackName('test-1')], { verbose: true });
expect(diff).toContain('Hold on while we create a read-only change set');
}));

integTest('cdk diff should allow configuring whether to use changesets via cdk.json', withDefaultFixture(async (fixture) => {
const cdkJson = JSON.parse(await fs.readFile(path.join(fixture.integTestDir, 'cdk.json'), 'utf8'));
cdkJson.diff = {
changeSet: false,
};
await fs.writeFile(path.join(fixture.integTestDir, 'cdk.json'), JSON.stringify(cdkJson));

const diff = await fixture.cdk(['diff', fixture.fullStackName('test-1')], { verbose: true });
expect(diff).not.toContain('Hold on while we create a read-only change set');
}));

integTest('cdk diff --fail on multiple stacks exits with error if any of the stacks contains a diff', withDefaultFixture(async (fixture) => {
// GIVEN
const diff1 = await fixture.cdk(['diff', fixture.fullStackName('test-1')]);
Expand Down
10 changes: 10 additions & 0 deletions packages/aws-cdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,16 @@ The `change-set` flag will make `diff` create a change set and extract resource
The `--no-change-set` mode will consider any change to a property that requires replacement to be a resource replacement,
even if the change is purely cosmetic (like replacing a resource reference with a hardcoded arn).

The `change-set` option can be set in the `cdk.json` file.

```json
{
"diff": {
"changeSet": false
}
}
```

### `cdk deploy`

Deploys a stack of your CDK app to its environment. During the deployment, the toolkit will output progress
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp
const exists = (await CloudFormationStack.lookup(cfn, options.stack.stackName, false)).exists;

const executionRoleArn = preparedSdk.cloudFormationRoleArn;
options.stream.write('Hold on while we create a read-only change set to get a diff with accurate replacement information (use --no-change-set to use a less accurate but faster template-only diff)\n');
debug('Hold on while we create a read-only change set to get a diff with accurate replacement information (use --no-change-set to use a less accurate but faster template-only diff)');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 - thank you! This message appears too often now.


return await createChangeSet({
cfn,
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
stream: args.ci ? process.stdout : undefined,
compareAgainstProcessedTemplate: args.processed,
quiet: args.quiet,
changeSet: args['change-set'],
changeSet: configuration.settings.get(['diff', 'changeSet']) ?? args['change-set'],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this needs to be swapped so that the cdk.json setting can be overridden via the cli argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
changeSet: configuration.settings.get(['diff', 'changeSet']) ?? args['change-set'],
changeSet: args['change-set'] ?? configuration.settings.get(['diff', 'changeSet'],

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right about this order of operations. Only question is if there is a difference between no flag being provided and the default.

If the flag is always given a value regardless of being set, we may have to put the context first (but I will ask some others on the team about this).

});

case 'bootstrap':
Expand Down
Loading