feat(refactor): support for stack filtering

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

@@ -405,6 +405,13 @@ export class TestFixture extends ShellHelper {
     ], options);
   }
 
+  public async cdkRefactor(options: CdkCliOptions = {}) {
+    return this.cdk([
+      'refactor',
+      ...(options.options ?? []),
+    ], options);
+  }
+
   public async cdkDestroy(stackNames: string | string[], options: CdkDestroyCliOptions = {}) {
     stackNames = typeof stackNames === 'string' ? [stackNames] : stackNames;
 

packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/refactoring/cdk.json

@@ -0,0 +1,7 @@
+{
+  "app": "node refactoring.js",
+  "versionReporting": false,
+  "context": {
+    "aws-cdk:enableDiffNoFail": "true"
+  }
+}

packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/refactoring/refactoring.js

@@ -0,0 +1,18 @@
+const cdk = require('aws-cdk-lib');
+const sqs = require('aws-cdk-lib/aws-sqs');
+
+class BasicStack extends cdk.Stack {
+  constructor(parent, id, props) {
+    super(parent, id, props);
+    new sqs.Queue(this, props.queueName);
+  }
+}
+
+const stackPrefix = process.env.STACK_NAME_PREFIX;
+const app = new cdk.App();
+
+new BasicStack(app, `${stackPrefix}-basic`, {
+    queueName: process.env.BASIC_QUEUE_LOGICAL_ID ?? 'BasicQueue',
+});
+
+app.synth();

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cdk-lib-security-related-changes-without-a-cli-are-expected-to-fail-when-approval-is-required.integtest.ts

@@ -13,9 +13,6 @@ integTest(
       neverRequireApproval: false,
     });
 
-    expect(stdErr).toContain(
-      'This deployment will make potentially sensitive changes according to your current security approval level',
-    );
     expect(stdErr).toContain(
       '"--require-approval" is enabled and stack includes security-sensitive updates',
     );

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cdk-refactor-dry-run.integtest.ts

@@ -0,0 +1,53 @@
+import { integTest, withSpecificFixture } from '../../lib';
+
+integTest(
+  'detects refactoring changes and prints the result',
+  withSpecificFixture('refactoring', async (fixture) => {
+    // First, deploy a stack
+    await fixture.cdkDeploy('basic', {
+      modEnv: {
+        BASIC_QUEUE_LOGICAL_ID: 'OldName',
+      },
+    });
+
+    // Then see if the refactoring tool detects the change
+    const stdErr = await fixture.cdkRefactor({
+      options: ['--dry-run', '--unstable=refactor'],
+      allowErrExit: true,
+      // Making sure the synthesized stack has the new name
+      // so that a refactor is detected
+      modEnv: {
+        BASIC_QUEUE_LOGICAL_ID: 'NewName',
+      },
+    });
+
+    expect(stdErr).toContain('The following resources were moved or renamed:');
+    expect(removeColor(stdErr)).toMatch(/│ AWS::SQS::Queue │ .*\/OldName\/Resource │ .*\/NewName\/Resource │/);
+  }),
+);
+
+integTest(
+  'no refactoring changes detected',
+  withSpecificFixture('refactoring', async (fixture) => {
+    const modEnv = {
+      BASIC_QUEUE_LOGICAL_ID: 'OldName',
+    };
+
+    // First, deploy a stack
+    await fixture.cdkDeploy('basic', { modEnv });
+
+    // Then see if the refactoring tool detects the change
+    const stdErr = await fixture.cdkRefactor({
+      options: ['--dry-run', '--unstable=refactor'],
+      allowErrExit: true,
+      modEnv,
+    });
+
+    expect(stdErr).toContain('Nothing to refactor');
+  }),
+);
+
+function removeColor(str: string): string {
+  return str.replace(/\x1B[[(?);]{0,2}(;?\d)*./g, '');
+}
+

packages/@aws-cdk/tmp-toolkit-helpers/src/api/diff/diff-formatter.ts

@@ -9,21 +9,26 @@ import {
 } from '@aws-cdk/cloudformation-diff';
 import type * as cxapi from '@aws-cdk/cx-api';
 import * as chalk from 'chalk';
+import { PermissionChangeType } from '../../payloads';
 import type { NestedStackTemplates } from '../cloudformation';
 import type { IoHelper } from '../io/private';
 import { IoDefaultMessages } from '../io/private';
-import { RequireApproval } from '../require-approval';
 import { StringWriteStream } from '../streams';
-import { ToolkitError } from '../toolkit-error';
 
 /**
  * Output of formatSecurityDiff
  */
 interface FormatSecurityDiffOutput {
   /**
-   * Complete formatted security diff, if it is prompt-worthy
+   * Complete formatted security diff
    */
-  readonly formattedDiff?: string;
+  readonly formattedDiff: string;
+
+  /**
+   * The type of permission changes in the security diff.
+   * The IoHost will use this to decide whether or not to print.
+   */
+  readonly permissionChangeType: PermissionChangeType;
 }
 
 /**
@@ -57,16 +62,6 @@ interface DiffFormatterProps {
   readonly templateInfo: TemplateInfo;
 }
 
-/**
- * Properties specific to formatting the security diff
- */
-interface FormatSecurityDiffOptions {
-  /**
-   * The approval level of the security diff
-   */
-  readonly requireApproval: RequireApproval;
-}
-
 /**
  * PRoperties specific to formatting the stack diff
  */
@@ -169,6 +164,42 @@ export class DiffFormatter {
     return this._diffs;
   }
 
+  /**
+   * Get or creates the diff of a stack.
+   * If it creates the diff, it stores the result in a map for
+   * easier retreval later.
+   */
+  private diff(stackName?: string, oldTemplate?: any) {
+    const realStackName = stackName ?? this.stackName;
+
+    if (!this._diffs[realStackName]) {
+      this._diffs[realStackName] = fullDiff(
+        oldTemplate ?? this.oldTemplate,
+        this.newTemplate.template,
+        this.changeSet,
+        this.isImport,
+      );
+    }
+    return this._diffs[realStackName];
+  }
+
+  /**
+   * Return whether the diff has security-impacting changes that need confirmation.
+   *
+   * If no stackName is given, then the root stack name is used.
+   */
+  private permissionType(stackName?: string): PermissionChangeType {
+    const diff = this.diff(stackName);
+
+    if (diff.permissionsBroadened) {
+      return PermissionChangeType.BROADENING;
+    } else if (diff.permissionsAnyChanges) {
+      return PermissionChangeType.NON_BROADENING;
+    } else {
+      return PermissionChangeType.NONE;
+    }
+  }
+
   /**
    * Format the stack diff
    */
@@ -191,8 +222,7 @@ export class DiffFormatter {
     nestedStackTemplates: { [nestedStackLogicalId: string]: NestedStackTemplates } | undefined,
     options: ReusableStackDiffOptions,
   ) {
-    let diff = fullDiff(oldTemplate, this.newTemplate.template, this.changeSet, this.isImport);
-    this._diffs[stackName] = diff;
+    let diff = this.diff(stackName, oldTemplate);
 
     // The stack diff is formatted via `Formatter`, which takes in a stream
     // and sends its output directly to that stream. To faciliate use of the
@@ -277,51 +307,27 @@ export class DiffFormatter {
   /**
    * Format the security diff
    */
-  public formatSecurityDiff(options: FormatSecurityDiffOptions): FormatSecurityDiffOutput {
-    const ioDefaultHelper = new IoDefaultMessages(this.ioHelper);
+  public formatSecurityDiff(): FormatSecurityDiffOutput {
+    const diff = this.diff();
 
-    const diff = fullDiff(this.oldTemplate, this.newTemplate.template, this.changeSet);
-    this._diffs[this.stackName] = diff;
-
-    if (diffRequiresApproval(diff, options.requireApproval)) {
-      // The security diff is formatted via `Formatter`, which takes in a stream
-      // and sends its output directly to that stream. To faciliate use of the
-      // global CliIoHost, we create our own stream to capture the output of
-      // `Formatter` and return the output as a string for the consumer of
-      // `formatSecurityDiff` to decide what to do with it.
-      const stream = new StringWriteStream();
-
-      stream.write(format(`Stack ${chalk.bold(this.stackName)}\n`));
-
-      // eslint-disable-next-line max-len
-      ioDefaultHelper.warning(`This deployment will make potentially sensitive changes according to your current security approval level (--require-approval ${options.requireApproval}).`);
-      ioDefaultHelper.warning('Please confirm you intend to make the following modifications:\n');
-      try {
-        // formatSecurityChanges updates the stream with the formatted security diff
-        formatSecurityChanges(stream, diff, buildLogicalToPathMap(this.newTemplate));
-      } finally {
-        stream.end();
-      }
-      // store the stream containing a formatted stack diff
-      const formattedDiff = stream.toString();
-      return { formattedDiff };
-    }
-    return {};
-  }
-}
+    // The security diff is formatted via `Formatter`, which takes in a stream
+    // and sends its output directly to that stream. To faciliate use of the
+    // global CliIoHost, we create our own stream to capture the output of
+    // `Formatter` and return the output as a string for the consumer of
+    // `formatSecurityDiff` to decide what to do with it.
+    const stream = new StringWriteStream();
 
-/**
- * Return whether the diff has security-impacting changes that need confirmation
- *
- * TODO: Filter the security impact determination based off of an enum that allows
- * us to pick minimum "severities" to alert on.
- */
-function diffRequiresApproval(diff: TemplateDiff, requireApproval: RequireApproval) {
-  switch (requireApproval) {
-    case RequireApproval.NEVER: return false;
-    case RequireApproval.ANY_CHANGE: return diff.permissionsAnyChanges;
-    case RequireApproval.BROADENING: return diff.permissionsBroadened;
-    default: throw new ToolkitError(`Unrecognized approval level: ${requireApproval}`);
+    stream.write(format(`Stack ${chalk.bold(this.stackName)}\n`));
+
+    try {
+      // formatSecurityChanges updates the stream with the formatted security diff
+      formatSecurityChanges(stream, diff, buildLogicalToPathMap(this.newTemplate));
+    } finally {
+      stream.end();
+    }
+    // store the stream containing a formatted stack diff
+    const formattedDiff = stream.toString();
+    return { formattedDiff, permissionChangeType: this.permissionType() };
   }
 }
 

packages/@aws-cdk/tmp-toolkit-helpers/src/api/refactoring/index.ts

@@ -39,7 +39,7 @@ export class AmbiguityError extends Error {
  * merely the stack name.
  */
 export class ResourceLocation {
-  constructor(readonly stack: CloudFormationStack, readonly logicalResourceId: string) {
+  constructor(public readonly stack: CloudFormationStack, public readonly logicalResourceId: string) {
   }
 
   public toPath(): string {
@@ -118,10 +118,20 @@ export function ambiguousMovements(movements: ResourceMovement[]) {
  * Converts a list of unambiguous resource movements into a list of resource mappings.
  *
  */
-export function resourceMappings(movements: ResourceMovement[]): ResourceMapping[] {
+export function resourceMappings(movements: ResourceMovement[], stacks?: CloudFormationStack[]): ResourceMapping[] {
+  const predicate = stacks == null
+    ? () => true
+    : (m: ResourceMapping) => {
+      // Any movement that involves one of the selected stacks (either moving from or to)
+      // is considered a candidate for refactoring.
+      const stackNames = [m.source.stack.stackName, m.destination.stack.stackName];
+      return stacks.some((stack) => stackNames.includes(stack.stackName));
+    };
+
   return movements
     .filter(([pre, post]) => pre.length === 1 && post.length === 1 && !pre[0].equalTo(post[0]))
-    .map(([pre, post]) => new ResourceMapping(pre[0], post[0]));
+    .map(([pre, post]) => new ResourceMapping(pre[0], post[0]))
+    .filter(predicate);
 }
 
 function removeUnmovedResources(