Skip to content

Commit

Permalink
feat: replace --no-fallback option with --mode switch
Browse files Browse the repository at this point in the history
  • Loading branch information
msessa committed Jan 14, 2025
1 parent 95be843 commit 7a8dc32
Show file tree
Hide file tree
Showing 12 changed files with 125 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const sns = require('aws-cdk-lib/aws-sns');
* # This will surface an error to the user about the missing macro
* ```
*/
class DiffNoFallbackTestStack extends cdk.Stack {
class FailingChangesetStack extends cdk.Stack {
constructor(scope, id, props) {
super(scope, id, props);

Expand All @@ -33,6 +33,6 @@ class DiffNoFallbackTestStack extends cdk.Stack {
const stackPrefix = process.env.STACK_NAME_PREFIX;
const app = new cdk.App();

new DiffNoFallbackTestStack(app, `${stackPrefix}-test-diff-no-fallback`);
new FailingChangesetStack(app, `${stackPrefix}-test-failing-changeset`);

app.synth();
Original file line number Diff line number Diff line change
Expand Up @@ -1300,6 +1300,26 @@ integTest(
}),
);

integTest(
'cdk diff shows resource metadata changes with --mode=local',
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 --mode=local', 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 Expand Up @@ -2933,29 +2953,29 @@ function actions(requests: CompletedRequest[]): string[] {
}

integTest(
'cdk diff noFallback',
withSpecificFixture('diff-no-fallback-app', async (fixture) => {
'cdk diff returns change-set creation errors when using --mode=change-set',
withSpecificFixture('failing-changeset-app', async (fixture) => {
// Should succeed
await fixture.cdkDeploy('test-diff-no-fallback', {
await fixture.cdkDeploy('test-failing-changeset', {
verbose: false,
});
try {
// Should surface the missing transform error from cloudformation in the output
const diffOutput = await fixture.cdk(['diff', '--no-fallback', fixture.fullStackName('test-diff-no-fallback')], {
const diffOutput = await fixture.cdk(['diff', '--mode=change-set', fixture.fullStackName('test-failing-changeset')], {
modEnv: { DIFF_PHASE: 'on' },
allowErrExit: true,
captureStderr: true,
});
expect(diffOutput).toContain('No transform named');

// Should throw
await expect(fixture.cdk(['diff', '--no-fallback', fixture.fullStackName('test-diff-no-fallback')], {
await expect(fixture.cdk(['diff', '--mode=change-set', fixture.fullStackName('test-failing-changeset')], {
modEnv: { DIFF_PHASE: 'on' },
captureStderr: true,
})).rejects.toThrow();

// Should fallback to template-only diff as normal
const diffOutputWithFallback = await fixture.cdk(['diff', fixture.fullStackName('test-diff-no-fallback')], {
const diffOutputWithFallback = await fixture.cdk(['diff', fixture.fullStackName('test-failing-changeset')], {
modEnv: { DIFF_PHASE: 'on' },
captureStderr: true,
allowErrExit: true,
Expand All @@ -2964,7 +2984,7 @@ integTest(
expect(diffOutputWithFallback).toContain('will base the diff on template differences');

} finally {
await fixture.cdkDestroy('test-diff-no-fallback', { verbose: false });
await fixture.cdkDestroy('test-failing-changeset', { verbose: false });
}
}),
);
8 changes: 4 additions & 4 deletions packages/aws-cdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,11 @@ $ cdk diff --quiet --app='node bin/main.js' MyStackName

Note that the CDK::Metadata resource and the `CheckBootstrapVersion` Rule are excluded from `cdk diff` by default. You can force `cdk diff` to display them by passing the `--strict` flag.

The `change-set` flag will make `diff` create a change set and extract resource replacement data from it. This is a bit slower, but will provide no false positives for resource replacement.
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 `mode` option selects the approach that will be used to analyze the differences:

If the `change-set` flag is enabled and an error is encountered during changeset creation, CDK will automatically fall back to template-only diff. This behaviour can be disabled by passing the `--no-fallback` flag.
- When set to `auto`: CDK will first attempt to create a ChangeSet, should that not be possible due to the stack not yet being created, or any error encountered during the creation of the StackSet, it will automatically fallback to `local` mode.
- When set to `change-set`: CDK will create a change set and extract resource replacement data from it. This is a bit slower, but will provide no false positives for resource replacement. Errors in creating a change-set will result in a non-zero exit code. Note that when using this mode against a stack that has not been created will still result in a fallback to `local` mode. Also note that this mode will always return an error when used against stacks that contain nested stacks.
- When set to `local`: CDK will compare the local template with the template currently applied to the stack. Note that this 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)

### `cdk deploy`

Expand Down
5 changes: 1 addition & 4 deletions packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,7 @@ export async function createDiffChangeSet(
// This causes CreateChangeSet to fail with `Template Error: Fn::Equals cannot be partially collapsed`.
for (const resource of Object.values(options.stack.template.Resources ?? {})) {
if ((resource as any).Type === 'AWS::CloudFormation::Stack') {
// eslint-disable-next-line no-console
debug('This stack contains one or more nested stacks, falling back to template-only diff...');

return undefined;
throw new Error('Cannot create change-set diff when using nested stacks');
}
}

Expand Down
13 changes: 8 additions & 5 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ export class CdkToolkit {

let changeSet = undefined;

if (options.changeSet) {
if (options.changeSet || options.mode === 'auto' || options.mode === 'change-set') {
let stackExists = false;
try {
stackExists = await this.props.deployments.stackExists({
Expand Down Expand Up @@ -224,7 +224,7 @@ export class CdkToolkit {
stream,
});
} catch (e: any) {
if (options.fallback ?? true) {
if (options.mode === 'auto') {
debug(e);
stream.write(
'Could not create a change set, will base the diff on template differences (run again with -v to see the reason)\n',
Expand Down Expand Up @@ -1427,16 +1427,19 @@ export interface DiffOptions {
/**
* Whether or not to create, analyze, and subsequently delete a changeset
*
* @deprecated - use `mode` option instead
* @default true
*/
changeSet?: boolean;

/**
* If the changeset option is turned on, whether to enable falling back to template-only diff in case creating the changeset results in an error
* Auto mode will first attempt to use change-set mode, and if any error should occur it will fallback to local mode.
* Change-set mode will use a change-set to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role.
* Local mode compares the current local template with template applied on the stack
*
* @default true
* @default 'auto'
*/
fallback?: boolean;
mode?: 'auto' | 'change-set' | 'local';
}

interface CfnDeployOptions {
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 @@ -239,7 +239,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
compareAgainstProcessedTemplate: args.processed,
quiet: args.quiet,
changeSet: args['change-set'],
fallback: args.fallback,
mode: args.mode,
toolkitStackName: toolkitStackName,
});

Expand Down
14 changes: 12 additions & 2 deletions packages/aws-cdk/lib/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,18 @@ export async function makeConfig(): Promise<CliConfig> {
'fail': { type: 'boolean', desc: 'Fail with exit code 1 in case of diff' },
'processed': { type: 'boolean', desc: 'Whether to compare against the template with Transforms already processed', default: false },
'quiet': { type: 'boolean', alias: 'q', desc: 'Do not print stack name and default message when there is no diff to stdout', default: false },
'change-set': { type: 'boolean', alias: 'changeset', desc: 'Whether to create a changeset to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role.', default: true },
'fallback': { type: 'boolean', desc: 'If the changeset option is turned on, whether to enable falling back to template-only diff in case creating the changeset results in an error.', default: true },
'change-set': { type: 'boolean', alias: 'changeset', desc: 'Whether to create a changeset to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role', conflicts: 'mode', deprecated: 'Use mode=auto or mode=local instead' },
'mode': {
type: 'string',
choices: ['auto', 'change-set', 'local'],
default: 'auto',
conflicts: 'change-set',
requiresArg: true,
desc: 'How to perform the the diff operation. ' +
'Auto mode will first attempt to use change-set mode, and if any error should occur it will fallback to local mode. ' +
'Change-set mode will use a change-set to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role. Unhandled errors in change-set creation will return a non-zero exit code ' +
'Local mode compares the current local template with template applied on the stack',
},
},
},
metadata: {
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk/lib/convert-to-user-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ export function convertYargsToUserInput(args: any): UserInput {
processed: args.processed,
quiet: args.quiet,
changeSet: args.changeSet,
fallback: args.fallback,
mode: args.mode,
STACKS: args.STACKS,
};
break;
Expand Down Expand Up @@ -397,7 +397,7 @@ export function convertConfigToUserInput(config: any): UserInput {
processed: config.diff?.processed,
quiet: config.diff?.quiet,
changeSet: config.diff?.changeSet,
fallback: config.diff?.fallback,
mode: config.diff?.mode,
};
const metadataOptions = {};
const acknowledgeOptions = {};
Expand Down
17 changes: 11 additions & 6 deletions packages/aws-cdk/lib/parse-command-line-arguments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -706,15 +706,20 @@ export function parseCommandLineArguments(args: Array<string>): any {
desc: 'Do not print stack name and default message when there is no diff to stdout',
})
.option('change-set', {
default: true,
default: undefined,
type: 'boolean',
alias: 'changeset',
desc: 'Whether to create a changeset to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role.',
desc: 'Whether to create a changeset to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role',
conflicts: 'mode',
deprecated: 'Use mode=auto or mode=local instead',
})
.option('fallback', {
default: true,
type: 'boolean',
desc: 'If the changeset option is turned on, whether to enable falling back to template-only diff in case creating the changeset results in an error.',
.option('mode', {
default: 'auto',
type: 'string',
choices: ['auto', 'change-set', 'local'],
conflicts: 'change-set',
requiresArg: true,
desc: 'How to perform the the diff operation. Auto mode will first attempt to use change-set mode, and if any error should occur it will fallback to local mode. Change-set mode will use a change-set to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role. Unhandled errors in change-set creation will return a non-zero exit code Local mode compares the current local template with template applied on the stack',
}),
)
.command('metadata [STACK]', 'Returns all metadata associated with this stack')
Expand Down
11 changes: 6 additions & 5 deletions packages/aws-cdk/lib/user-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1090,20 +1090,21 @@ export interface DiffOptions {
readonly quiet?: boolean;

/**
* Whether to create a changeset to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role.
* Whether to create a changeset to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role
*
* aliases: changeset
*
* @default - true
* @deprecated Use mode=auto or mode=local instead
* @default - undefined
*/
readonly changeSet?: boolean;

/**
* If the changeset option is turned on, whether to enable falling back to template-only diff in case creating the changeset results in an error.
* How to perform the the diff operation. Auto mode will first attempt to use change-set mode, and if any error should occur it will fallback to local mode. Change-set mode will use a change-set to analyze resource replacements. In this mode, diff will use the deploy role instead of the lookup role. Unhandled errors in change-set creation will return a non-zero exit code Local mode compares the current local template with template applied on the stack
*
* @default - true
* @default - "auto"
*/
readonly fallback?: boolean;
readonly mode?: string;

/**
* Positional argument for diff
Expand Down
Loading

0 comments on commit 7a8dc32

Please sign in to comment.