Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,13 @@ export type PrepareChangeSetOptions = {
sdkProvider: SdkProvider;
parameters: { [name: string]: string | undefined };
resourcesToImport?: ResourcesToImport;
/**
* Default behavior is to log AWS CloudFormation errors and move on. Set this property to true to instead
* fail on errors received by AWS CloudFormation.
*
* @default false
*/
failOnError?: boolean;
}

export type CreateChangeSetOptions = {
Expand Down Expand Up @@ -240,12 +247,17 @@ async function uploadBodyParameterAndCreateChangeSet(
role: executionRoleArn,
});
} catch (e: any) {
await ioHelper.notify(IO.DEFAULT_TOOLKIT_DEBUG.msg(String(e)));
await ioHelper.notify(IO.DEFAULT_TOOLKIT_INFO.msg(
'Could not create a change set, will base the diff on template differences (run again with -v to see the reason)\n',
));
// This function is currently only used by diff so these messages are diff-specific
if (!options.failOnError) {
await ioHelper.notify(IO.DEFAULT_TOOLKIT_DEBUG.msg(String(e)));
await ioHelper.notify(IO.DEFAULT_TOOLKIT_INFO.msg(
'Could not create a change set, will base the diff on template differences (run again with -v to see the reason)\n',
));

return undefined;
return undefined;
}

throw new ToolkitError('Could not create a change set and failOnError is set. (run again with failOnError off to base the diff on template differences)\n', e);
}
}

Expand Down
17 changes: 8 additions & 9 deletions packages/@aws-cdk/toolkit-lib/lib/actions/diff/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,14 @@ export interface CloudFormationDiffOptions {
}

export interface ChangeSetDiffOptions extends CloudFormationDiffOptions {
// @TODO: add this as a feature
// /**
// * Enable falling back to template-based diff in case creating the changeset is not possible or results in an error.
// *
// * Should be used for stacks containing nested stacks or when change set permissions aren't available.
// *
// * @default true
// */
// readonly fallbackToTemplate?: boolean;
/**
* Enable falling back to template-based diff in case creating the changeset is not possible or results in an error.
*
* Should be used for stacks containing nested stacks or when change set permissions aren't available.
*
* @default true
*/
readonly fallbackToTemplate?: boolean;

/**
* Additional parameters for CloudFormation when creating a diff change set
Expand Down
26 changes: 23 additions & 3 deletions packages/@aws-cdk/toolkit-lib/lib/actions/diff/private/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ async function cfnDiff(
deployments: Deployments,
options: DiffOptions,
sdkProvider: SdkProvider,
changeSet: boolean,
includeChangeSet: boolean,
): Promise<TemplateInfo[]> {
const templateInfos = [];
const methodOptions = (options.method?.options ?? {}) as ChangeSetDiffOptions;
Expand All @@ -78,13 +78,23 @@ async function cfnDiff(
removeNonImportResources(stack);
}

const changeSet = includeChangeSet ? await changeSetDiff(
ioHelper,
deployments,
stack,
sdkProvider,
resourcesToImport,
methodOptions.parameters,
methodOptions.fallbackToTemplate,
) : undefined;

templateInfos.push({
oldTemplate: currentTemplate,
newTemplate: stack,
stackName: stack.stackName,
isImport: !!resourcesToImport,
nestedStacks,
changeSet: changeSet ? await changeSetDiff(ioHelper, deployments, stack, sdkProvider, resourcesToImport, methodOptions.parameters) : undefined,
changeSet,
});
}

Expand All @@ -98,6 +108,7 @@ async function changeSetDiff(
sdkProvider: SdkProvider,
resourcesToImport?: ResourcesToImport,
parameters: { [name: string]: string | undefined } = {},
fallBackToTemplate: boolean = true,
): Promise<any | undefined> {
let stackExists = false;
try {
Expand All @@ -107,6 +118,10 @@ async function changeSetDiff(
tryLookupRole: true,
});
} catch (e: any) {
if (!fallBackToTemplate) {
throw new ToolkitError(`describeStacks call failed with ${e} for ${stack.stackName}, set fallBackToTemplate to true or use DiffMethod.templateOnly to base the diff on template differences.`);
}

await ioHelper.notify(IO.DEFAULT_TOOLKIT_DEBUG.msg(`Checking if the stack ${stack.stackName} exists before creating the changeset has failed, will base the diff on template differences.\n`));
await ioHelper.notify(IO.DEFAULT_TOOLKIT_DEBUG.msg(formatErrorMessage(e)));
stackExists = false;
Expand All @@ -121,9 +136,14 @@ async function changeSetDiff(
sdkProvider,
parameters: parameters,
resourcesToImport,
failOnError: !fallBackToTemplate,
});
} else {
await ioHelper.notify(IO.DEFAULT_TOOLKIT_DEBUG.msg(`the stack '${stack.stackName}' has not been deployed to CloudFormation or describeStacks call failed, skipping changeset creation.`));
if (!fallBackToTemplate) {
throw new ToolkitError(`the stack '${stack.stackName}' has not been deployed to CloudFormation, set fallBackToTemplate to true or use DiffMethod.templateOnly to base the diff on template differences.`);
}

await ioHelper.notify(IO.DEFAULT_TOOLKIT_DEBUG.msg(`the stack '${stack.stackName}' has not been deployed to CloudFormation, skipping changeset creation.`));
return;
}
}
Expand Down
45 changes: 42 additions & 3 deletions packages/@aws-cdk/toolkit-lib/test/actions/diff.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as path from 'path';
import { format } from 'util';
import * as chalk from 'chalk';
import { DiffMethod } from '../../lib/actions/diff';
import * as apis from '../../lib/api/shared-private';
Expand All @@ -18,7 +17,6 @@ beforeEach(() => {
ioHost.requireDeployApproval = RequireApproval.NEVER;

toolkit = new Toolkit({ ioHost });
const sdk = new MockSdk();

// Some default implementations
jest.spyOn(apis.Deployments.prototype, 'readCurrentTemplateWithNestedStacks').mockResolvedValue({
Expand Down Expand Up @@ -177,7 +175,48 @@ describe('diff', () => {
}));
});

describe('templatePath', () => {
describe('DiffMethod.ChangeSet', () => {
test('ChangeSet diff method falls back to template only if changeset not found', async () => {
// WHEN
ioHost.level = 'debug';
const cx = await builderFixture(toolkit, 'stack-with-bucket');
await toolkit.diff(cx, {
stacks: { strategy: StackSelectionStrategy.ALL_STACKS },
method: DiffMethod.ChangeSet(),
});

// THEN
expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
action: 'diff',
level: 'info',
code: 'CDK_TOOLKIT_I0000',
message: expect.stringContaining('Could not create a change set, will base the diff on template differences'),
}));
});

test('ChangeSet diff method throws if changeSet fails and fallBackToTemplate = false', async () => {
// WHEN
const cx = await builderFixture(toolkit, 'stack-with-bucket');
await expect(async () => toolkit.diff(cx, {
stacks: { strategy: StackSelectionStrategy.ALL_STACKS },
method: DiffMethod.ChangeSet({ fallbackToTemplate: false }),
})).rejects.toThrow(/Could not create a change set and failOnError is set/);
});

test('ChangeSet diff method throws if stack not found and fallBackToTemplate = false', async () => {
// GIVEN
jest.spyOn(apis.Deployments.prototype, 'stackExists').mockResolvedValue(false);

// WHEN
const cx = await builderFixture(toolkit, 'stack-with-bucket');
await expect(async () => toolkit.diff(cx, {
stacks: { strategy: StackSelectionStrategy.ALL_STACKS },
method: DiffMethod.ChangeSet({ fallbackToTemplate: false }),
})).rejects.toThrow(/the stack 'Stack1' has not been deployed to CloudFormation/);
});
});

describe('DiffMethod.LocalFile', () => {
test('fails with multiple stacks', async () => {
// WHEN + THEN
const cx = await builderFixture(toolkit, 'two-empty-stacks');
Expand Down
Loading