feat(toolkit-lib): fallBackToTemplate option for diffs (#354)

The primary motivation for this PR is
https://github.com/aws/aws-cdk/issues/28753 and the ensuing PR
aws/aws-cdk#32830.

When implementing diff in the programmatic toolkit library we included
the diff method options, and this PR adds the `fallBackToTemplate`
property to the `ChangeSet` mode. The idea is that people can specify
`fallBackToTemplate=false` to explicitly fail if we cannot successfully
create the changeset. The current behavior is to fallback to the
template-only method.


---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license

Author: kaizencc

packages/@aws-cdk/tmp-toolkit-helpers/src/api/deployments/cfn-api.ts

@@ -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 = {
@@ -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);
   }
 }
 

packages/@aws-cdk/toolkit-lib/lib/actions/diff/index.ts

@@ -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

packages/@aws-cdk/toolkit-lib/lib/actions/diff/private/helpers.ts

@@ -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;
@@ -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,
     });
   }
 
@@ -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 {
@@ -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;
@@ -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;
   }
 }

packages/@aws-cdk/toolkit-lib/test/actions/diff.test.ts

@@ -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';
@@ -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({
@@ -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');