feat: refactor (dry-run mode only) #342

otaviomacedo · 2025-04-08T13:09:18Z

Add a refactor command. For now, it only works in dry run mode (-dry-run in the command line, or dryRun: true to the toolkit). It computes the mappings based on the difference between the deployed stacks and the cloud assembly stacks, and shows them in a table. Example:

$ cdk refactor --dry-run --unstable=refactor

The following resources were moved or renamed:
┌───────────────────────┬────────────────────────────────────────┬───────────────────────────────────────┐
│ Resource Type         │ Old Construct Path                     │ New Construct Path                    │
├───────────────────────┼────────────────────────────────────────┼───────────────────────────────────────┤
│ AWS::IAM::Role        │ Consumer/Function/ServiceRole/Resource │ Consumer/NewName/ServiceRole/Resource │
├───────────────────────┼────────────────────────────────────────┼───────────────────────────────────────┤
│ AWS::Lambda::Function │ Consumer/Function/Resource             │ Consumer/NewName/Resource             │
└───────────────────────┴────────────────────────────────────────┴───────────────────────────────────────┘

Note that we are launching this feature under unstable mode, which requires the user to acknowledge that by passing the --unstable=refactor flag.

Closes #132, #133, #141, #134 and #135.

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

# Conflicts: # packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts

otaviomacedo · 2025-04-08T13:11:14Z

packages/@aws-cdk/tmp-toolkit-helpers/src/api/io/private/messages.ts

+  // 8. Refactor (8xxx)
+  CDK_TOOLKIT_I8900: make.result<RefactorResult>({
+    code: 'CDK_TOOLKIT_I8900',
+    description: 'Refactor result',
+    interface: 'RefactorResult',
+  }),
+
+  CDK_TOOLKIT_W8010: make.warn({
+    code: 'CDK_TOOLKIT_W8010',
+    description: 'Refactor execution not yet supported',
+  }),
+


Not sure if I got the pattern right, here. Someone please check me on this.

otaviomacedo · 2025-04-08T13:12:23Z

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

@@ -0,0 +1,23 @@
+import { Writable } from 'node:stream';


This was moved from the diff-formatter.ts file.

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

rix0rrr

Surface level stuff, still need to look at the meat & potatoes

rix0rrr · 2025-04-09T13:53:08Z

packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts

+  /**
+   * Refactor Action. Moves resources from one location (stack + logical ID) to another.
+   */
+  public async refactor(cx: ICloudAssemblySource, options: RefactorOptions = {}): Promise<void> {


Refactor should probably return some useful structure as well, shouldn't it?

Possibly. I'm confused as to what should get notified to the IoHost, and what should be returned.

rix0rrr · 2025-04-09T13:54:02Z

packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts

+
+    const strategy = options.stacks?.strategy ?? StackSelectionStrategy.ALL_STACKS;
+    if (strategy !== StackSelectionStrategy.ALL_STACKS) {
+      await ioHelper.notify(IO.CDK_TOOLKIT_W8010.msg(


Shouldn't this just be an error?

If you want to convert user input into a warning and proceeding anyway, then do the conversion in the CLI?

rix0rrr · 2025-04-09T13:56:16Z

packages/aws-cdk/lib/cli/cdk-toolkit.ts

+
+    const stacks = await this.selectStacksForList([]);
+    try {
+      const mappings = await detectRefactorMappings(stacks.stackArtifacts, this.props.sdkProvider);


Hm. I thought the CLI would use the toolkit code here, but I see we haven't gotten that far yet.

From what I understand, the idea is that both the CLI and the toolkit library share common code, but the CLI doesn't call the toolkit library. Not yet, at least.

rix0rrr

I have some comments that I'll leave for you to decide what to do with, but I don't want to stand in the way of this, especially since you're going to iterate on it over time anyway.

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

rix0rrr · 2025-04-10T12:39:17Z

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

+  // 3. Topological sort
+  const inDegree = Object.keys(graph).reduce((acc, k) => {


I'm pretty sure already have a couple of flavors of toposort lying around the code base already you should be able to reuse 😉

rix0rrr · 2025-04-10T12:41:37Z

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

+  }
+
+  // 2. Detect dependencies by searching for Ref/Fn::GetAtt
+  const findDependencies = (value: any): string[] => {


Don't forget that dependencies can also come from DependsOn at the top level, which contains logical IDs.

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

rix0rrr

One more thing: in calculating the digest, should we normalize property order and dependency order?

A reordering of the properties in the bag will affect the hash right now, but it probably shouldn't.

otaviomacedo added 5 commits April 8, 2025 12:05

WIP

dd9ce15

WIP

2fcce95

Merge branch 'main' into otaviom/refactor-dry-run-2

bdf143c

# Conflicts: # packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts

Comments

3bc01b3

Comments

c9569f9

otaviomacedo had a problem deploying to integ-approval April 8, 2025 13:09 — with GitHub Actions Failure

github-actions bot added the p2 label Apr 8, 2025

aws-cdk-automation enabled auto-merge April 8, 2025 13:09

aws-cdk-automation requested a review from a team April 8, 2025 13:10

otaviomacedo commented Apr 8, 2025

View reviewed changes

otaviomacedo mentioned this pull request Apr 8, 2025

feat: add tests for refactor dry-run aws/aws-cdk-cli-testing#77

Closed

Set up MockSdkProvider

2cd48ac

otaviomacedo had a problem deploying to integ-approval April 8, 2025 13:26 — with GitHub Actions Failure

Change account to an allowed pattern

03a9e5e

otaviomacedo temporarily deployed to integ-approval April 8, 2025 13:44 — with GitHub Actions Inactive

otaviomacedo temporarily deployed to run-tests April 8, 2025 14:06 — with GitHub Actions Inactive

Merge branch 'main' into otaviom/refactor-dry-run-2

47a8a3b

otaviomacedo had a problem deploying to integ-approval April 9, 2025 09:13 — with GitHub Actions Failure

rix0rrr reviewed Apr 9, 2025

View reviewed changes

Merge branch 'main' into otaviom/refactor-dry-run-2

bdc099d

otaviomacedo had a problem deploying to integ-approval April 10, 2025 09:23 — with GitHub Actions Failure

otaviomacedo had a problem deploying to integ-approval April 10, 2025 12:26 — with GitHub Actions Failure

rix0rrr disabled auto-merge April 10, 2025 12:47

rix0rrr approved these changes Apr 10, 2025

View reviewed changes

rix0rrr reviewed Apr 10, 2025

View reviewed changes

Resource without properties

17c3cee

otaviomacedo had a problem deploying to integ-approval April 10, 2025 13:14 — with GitHub Actions Failure

Addressed some comments

69b5b02

otaviomacedo had a problem deploying to integ-approval April 10, 2025 14:46 — with GitHub Actions Failure

Pagination

7a4b3f8

otaviomacedo had a problem deploying to integ-approval April 10, 2025 15:31 — with GitHub Actions Failure

Let the caller decide whether to throw error on ambiguity

b105e10

otaviomacedo had a problem deploying to integ-approval April 11, 2025 11:06 — with GitHub Actions Failure

Take DependsOn into account

0ef9678

otaviomacedo had a problem deploying to integ-approval April 11, 2025 12:45 — with GitHub Actions Failure

Merge branch 'main' into otaviom/refactor-dry-run-2

74bbf8c

otaviomacedo temporarily deployed to integ-approval April 11, 2025 13:14 — with GitHub Actions Inactive

otaviomacedo temporarily deployed to run-tests April 11, 2025 13:38 — with GitHub Actions Inactive

otaviomacedo added this pull request to the merge queue Apr 11, 2025

Merged via the queue into main with commit 24b23e4 Apr 11, 2025
19 of 20 checks passed

otaviomacedo deleted the otaviom/refactor-dry-run-2 branch April 11, 2025 14:14

		// 3. Topological sort
		const inDegree = Object.keys(graph).reduce((acc, k) => {

feat: refactor (dry-run mode only) #342

feat: refactor (dry-run mode only) #342

Uh oh!

Conversation

otaviomacedo commented Apr 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rix0rrr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rix0rrr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rix0rrr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!