From 3fd969988c599bf15b9b68d718505fcf92045b3a Mon Sep 17 00:00:00 2001
From: Eli Polonsky <epolon@amazon.com>
Date: Sun, 17 Nov 2024 23:45:42 +0200
Subject: [PATCH] fix(cli): `cdk diff` always falls back to template only diff
 (#32165)

Closes https://github.com/aws/aws-cdk/issues/32160

### Reason for this change

<!--What is the bug or use case behind this change?-->

When running `cdk deploy`, it is supposed to (by default) create a
read-only change set and incorporate it within the diff.
However, it currently fails creating the change-set and always falls
back to template only diffs.

### Description of changes

<!--What code changes did you make? Have you made any important design
decisions?-->

There was a wrong invocation of the `makeBodyParameter` parameter after
the [migration to sdk v3](https://github.com/aws/aws-cdk/pull/31702).

### Description of how you validated changes

Added missing integration tests. Unit test for this code are tricky
because they require too many mocks.

### Checklist
- [x] My code adheres to the [CONTRIBUTING
GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and
[DESIGN
GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license*
---
 .../cli-integ/resources/cdk-apps/app/app.js   | 11 +++++
 .../tests/cli-integ-tests/cli.integtest.ts    | 40 +++++++++++++++++++
 .../aws-cdk/lib/api/util/cloudformation.ts    |  1 -
 3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js
index 0f88ddf3bd467..54eb8842eeee8 100755
--- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js
+++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js
@@ -776,6 +776,15 @@ class AppSyncHotswapStack extends cdk.Stack {
   }
 }
 
+class MetadataStack extends cdk.Stack {
+  constructor(parent, id, props) {
+    super(parent, id, props);
+    const handle = new cdk.CfnWaitConditionHandle(this, 'WaitConditionHandle');
+    handle.addMetadata('Key', process.env.INTEG_METADATA_VALUE ?? 'default')
+
+  }  
+}
+
 const app = new cdk.App({
   context: {
     '@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID, // Force all assets to be unique, but consistent in one build
@@ -877,6 +886,8 @@ switch (stackSet) {
     new ExportValueStack(app, `${stackPrefix}-export-value-stack`);
 
     new BundlingStage(app, `${stackPrefix}-bundling-stage`);
+
+    new MetadataStack(app, `${stackPrefix}-metadata`);
     break;
 
   case 'stage-using-context':
diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts
index de888c9062f22..82f18672cfac2 100644
--- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts
+++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts
@@ -1064,6 +1064,46 @@ integTest(
   }),
 );
 
+integTest(
+  'cdk diff doesnt show resource metadata changes',
+  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', fixture.fullStackName('metadata')], {
+      verbose: true,
+      modEnv: {
+        INTEG_METADATA_VALUE: 'custom',
+      },
+    });
+
+    // Assert there are no changes
+    expect(diff).toContain('There were no differences');
+  }),
+);
+
+integTest(
+  'cdk diff shows resource metadata changes with --no-change-set',
+  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 --no-change-set', 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';
diff --git a/packages/aws-cdk/lib/api/util/cloudformation.ts b/packages/aws-cdk/lib/api/util/cloudformation.ts
index 79ac7cb461c1a..dcfe8a62dc6ef 100644
--- a/packages/aws-cdk/lib/api/util/cloudformation.ts
+++ b/packages/aws-cdk/lib/api/util/cloudformation.ts
@@ -400,7 +400,6 @@ async function uploadBodyParameterAndCreateChangeSet(
       env.resolvedEnvironment,
       new AssetManifestBuilder(),
       env.resources,
-      env.sdk,
     );
     const cfn = env.sdk.cloudFormation();
     const exists = (await CloudFormationStack.lookup(cfn, options.stack.stackName, false)).exists;