fix(cli): hotswap is reporting and running changes that don't happen (#244)

This change is three fold:

- We clean up the private `HotswapOperation` interface (née
`HotswappableChange`) by removing two unused fields and adding in a new
filed in preparation for structured data alongside hotswap messages
- In the hotswap providers, pull gates outside of the apply function.
This prevents hotswaps being reported when nothing would actually be
done. These are local checks and now just run a little bit earlier.
- In `lambda-functions.ts` don't report hotswappable changes for
versions and aliases. These used to be reported with an empty array for
`resourceNames` and a noop `apply` function. This means that literally
nothing happens with these entries, since `apply` doesn't do anything
and the CLI uses `resourceNames` to print anything. Just removing these
as they are reported as part of a change to the function.

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

Author: mrgrain

packages/@aws-cdk/tmp-toolkit-helpers/src/api/io/payloads/hotswap.ts

@@ -0,0 +1,30 @@
+import type { PropertyDifference, Resource } from '@aws-cdk/cloudformation-diff';
+
+/**
+ * Represents a change in a resource
+ */
+export interface ResourceChange {
+  /**
+   * The logical ID of the resource which is being changed
+   */
+  readonly logicalId: string;
+  /**
+   * The value the resource is being updated from
+   */
+  readonly oldValue: Resource;
+  /**
+   * The value the resource is being updated to
+   */
+  readonly newValue: Resource;
+  /**
+   * The changes made to the resource properties
+   */
+  readonly propertyUpdates: Record<string, PropertyDifference<unknown>>;
+}
+
+export interface HotswappableChange {
+  /**
+   * The resource change that is causing the hotswap.
+   */
+  readonly cause: ResourceChange;
+}

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

@@ -13,3 +13,4 @@ export * from './watch';
 export * from './stack-details';
 export * from './diff';
 export * from './logs-monitor';
+export * from './hotswap';

packages/aws-cdk/lib/api/deployments/hotswap-deployments.ts

@@ -3,6 +3,8 @@ import * as cfn_diff from '@aws-cdk/cloudformation-diff';
 import type * as cxapi from '@aws-cdk/cx-api';
 import type { WaiterResult } from '@smithy/util-waiter';
 import * as chalk from 'chalk';
+import type { ResourceChange } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/payloads';
+import type { IoHelper } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
 import type { SDK, SdkProvider } from '../aws-auth';
 import type { CloudFormationStack } from './cloudformation';
 import type { NestedStackTemplates } from './nested-stack-helpers';
@@ -15,10 +17,10 @@ import { isHotswappableAppSyncChange } from '../hotswap/appsync-mapping-template
 import { isHotswappableCodeBuildProjectChange } from '../hotswap/code-build-projects';
 import type {
   ChangeHotswapResult,
-  HotswappableChange,
+  HotswapOperation,
   NonHotswappableChange,
-  HotswappableChangeCandidate,
-  HotswapPropertyOverrides, ClassifiedResourceChanges,
+  HotswapPropertyOverrides,
+  ClassifiedResourceChanges,
 } from '../hotswap/common';
 import {
   ICON,
@@ -35,15 +37,14 @@ import {
 import { isHotswappableStateMachineChange } from '../hotswap/stepfunctions-state-machines';
 import { Mode } from '../plugin';
 import type { SuccessfulDeployStackResult } from './deployment-result';
-import type { IoHelper } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
 
 // Must use a require() otherwise esbuild complains about calling a namespace
 // eslint-disable-next-line @typescript-eslint/no-require-imports,@typescript-eslint/consistent-type-imports
 const pLimit: typeof import('p-limit') = require('p-limit');
 
 type HotswapDetector = (
   logicalId: string,
-  change: HotswappableChangeCandidate,
+  change: ResourceChange,
   evaluateCfnTemplate: EvaluateCloudFormationTemplate,
   hotswapPropertyOverrides: HotswapPropertyOverrides,
 ) => Promise<ChangeHotswapResult>;
@@ -66,7 +67,7 @@ const RESOURCE_DETECTORS: { [key: string]: HotswapDetector } = {
   'Custom::CDKBucketDeployment': isHotswappableS3BucketDeploymentChange,
   'AWS::IAM::Policy': async (
     logicalId: string,
-    change: HotswappableChangeCandidate,
+    change: ResourceChange,
     evaluateCfnTemplate: EvaluateCloudFormationTemplate,
   ): Promise<ChangeHotswapResult> => {
     // If the policy is for a S3BucketDeploymentChange, we can ignore the change
@@ -154,7 +155,7 @@ async function classifyResourceChanges(
   const resourceDifferences = getStackResourceDifferences(stackChanges);
 
   const promises: Array<() => Promise<ChangeHotswapResult>> = [];
-  const hotswappableResources = new Array<HotswappableChange>();
+  const hotswappableResources = new Array<HotswapOperation>();
   const nonHotswappableResources = new Array<NonHotswappableChange>();
   for (const logicalId of Object.keys(stackChanges.outputs.changes)) {
     nonHotswappableResources.push({
@@ -323,7 +324,8 @@ async function findNestedHotswappableChanges(
     evaluateNestedCfnTemplate,
     sdk,
     nestedStackTemplates[logicalId].nestedStackTemplates,
-    hotswapPropertyOverrides);
+    hotswapPropertyOverrides,
+  );
 }
 
 /** Returns 'true' if a pair of changes is for the same resource. */
@@ -365,7 +367,7 @@ function makeRenameDifference(
 function isCandidateForHotswapping(
   change: cfn_diff.ResourceDifference,
   logicalId: string,
-): HotswappableChange | NonHotswappableChange | HotswappableChangeCandidate {
+): HotswapOperation | NonHotswappableChange | ResourceChange {
   // a resource has been removed OR a resource has been added; we can't short-circuit that change
   if (!change.oldValue) {
     return {
@@ -404,7 +406,7 @@ function isCandidateForHotswapping(
   };
 }
 
-async function applyAllHotswappableChanges(sdk: SDK, ioHelper: IoHelper, hotswappableChanges: HotswappableChange[]): Promise<void[]> {
+async function applyAllHotswappableChanges(sdk: SDK, ioHelper: IoHelper, hotswappableChanges: HotswapOperation[]): Promise<void[]> {
   if (hotswappableChanges.length > 0) {
     await ioHelper.notify(info(`\n${ICON} hotswapping resources:`));
   }
@@ -415,7 +417,7 @@ async function applyAllHotswappableChanges(sdk: SDK, ioHelper: IoHelper, hotswap
   })));
 }
 
-async function applyHotswappableChange(sdk: SDK, ioHelper: IoHelper, hotswapOperation: HotswappableChange): Promise<void> {
+async function applyHotswappableChange(sdk: SDK, ioHelper: IoHelper, hotswapOperation: HotswapOperation): Promise<void> {
   // note the type of service that was successfully hotswapped in the User-Agent
   const customUserAgent = `cdk-hotswap/success-${hotswapOperation.service}`;
   sdk.appendCustomUserAgent(customUserAgent);

packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts

@@ -5,18 +5,18 @@ import type {
 import {
   type ChangeHotswapResult,
   classifyChanges,
-  type HotswappableChangeCandidate,
   lowerCaseFirstCharacter,
   transformObjectKeys,
 } from './common';
+import type { ResourceChange } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/payloads/hotswap';
 import { ToolkitError } from '../../toolkit/error';
 import type { SDK } from '../aws-auth';
 
 import type { EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template';
 
 export async function isHotswappableAppSyncChange(
   logicalId: string,
-  change: HotswappableChangeCandidate,
+  change: ResourceChange,
   evaluateCfnTemplate: EvaluateCloudFormationTemplate,
 ): Promise<ChangeHotswapResult> {
   const isResolver = change.newValue.Type === 'AWS::AppSync::Resolver';
@@ -55,17 +55,20 @@ export async function isHotswappableAppSyncChange(
     } else {
       physicalName = arn;
     }
+
+    // nothing do here
+    if (!physicalName) {
+      return ret;
+    }
+
     ret.push({
+      change: {
+        cause: change,
+      },
       hotswappable: true,
-      resourceType: change.newValue.Type,
-      propsChanged: namesOfHotswappableChanges,
       service: 'appsync',
       resourceNames: [`${change.newValue.Type} '${physicalName}'`],
       apply: async (sdk: SDK) => {
-        if (!physicalName) {
-          return;
-        }
-
         const sdkProperties: { [name: string]: any } = {
           ...change.oldValue.Properties,
           Definition: change.newValue.Properties?.Definition,

packages/aws-cdk/lib/api/hotswap/code-build-projects.ts

@@ -2,16 +2,16 @@ import type { UpdateProjectCommandInput } from '@aws-sdk/client-codebuild';
 import {
   type ChangeHotswapResult,
   classifyChanges,
-  type HotswappableChangeCandidate,
   lowerCaseFirstCharacter,
   transformObjectKeys,
 } from './common';
+import type { ResourceChange } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/payloads/hotswap';
 import type { SDK } from '../aws-auth';
 import type { EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template';
 
 export async function isHotswappableCodeBuildProjectChange(
   logicalId: string,
-  change: HotswappableChangeCandidate,
+  change: ResourceChange,
   evaluateCfnTemplate: EvaluateCloudFormationTemplate,
 ): Promise<ChangeHotswapResult> {
   if (change.newValue.Type !== 'AWS::CodeBuild::Project') {
@@ -30,16 +30,20 @@ export async function isHotswappableCodeBuildProjectChange(
       logicalId,
       change.newValue.Properties?.Name,
     );
+
+    // nothing to do jere
+    if (!projectName) {
+      return ret;
+    }
+
     ret.push({
+      change: {
+        cause: change,
+      },
       hotswappable: true,
-      resourceType: change.newValue.Type,
-      propsChanged: classifiedChanges.namesOfHotswappableProps,
       service: 'codebuild',
       resourceNames: [`CodeBuild Project '${projectName}'`],
       apply: async (sdk: SDK) => {
-        if (!projectName) {
-          return;
-        }
         updateProjectInput.name = projectName;
 
         for (const updatedPropName in change.propertyUpdates) {

packages/aws-cdk/lib/api/hotswap/common.ts

@@ -1,24 +1,35 @@
-import type { PropertyDifference, Resource } from '@aws-cdk/cloudformation-diff';
+import type { PropertyDifference } from '@aws-cdk/cloudformation-diff';
+import type { HotswappableChange, ResourceChange } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/payloads/hotswap';
 import { ToolkitError } from '../../toolkit/error';
 import type { SDK } from '../aws-auth';
 
 export const ICON = '✨';
 
-export interface HotswappableChange {
+export interface HotswapOperation {
+  /**
+   * Marks the operation as hotswappable
+   */
   readonly hotswappable: true;
-  readonly resourceType: string;
-  readonly propsChanged: Array<string>;
+
   /**
    * The name of the service being hotswapped.
    * Used to set a custom User-Agent for SDK calls.
    */
   readonly service: string;
 
+  /**
+   * Description of the change that is applied as part of the operation
+   */
+  readonly change: HotswappableChange;
+
   /**
    * The names of the resources being hotswapped.
    */
   readonly resourceNames: string[];
 
+  /**
+   * Applies the hotswap operation
+   */
   readonly apply: (sdk: SDK) => Promise<void>;
 }
 
@@ -41,10 +52,10 @@ export interface NonHotswappableChange {
   readonly hotswapOnlyVisible?: boolean;
 }
 
-export type ChangeHotswapResult = Array<HotswappableChange | NonHotswappableChange>;
+export type ChangeHotswapResult = Array<HotswapOperation | NonHotswappableChange>;
 
 export interface ClassifiedResourceChanges {
-  hotswappableChanges: HotswappableChange[];
+  hotswappableChanges: HotswapOperation[];
   nonHotswappableChanges: NonHotswappableChange[];
 }
 
@@ -65,38 +76,6 @@ export enum HotswapMode {
   FULL_DEPLOYMENT = 'full-deployment',
 }
 
-/**
- * Represents a change that can be hotswapped.
- */
-export class HotswappableChangeCandidate {
-  /**
-   * The logical ID of the resource which is being changed
-   */
-  public readonly logicalId: string;
-
-  /**
-   * The value the resource is being updated from
-   */
-  public readonly oldValue: Resource;
-
-  /**
-   * The value the resource is being updated to
-   */
-  public readonly newValue: Resource;
-
-  /**
-   * The changes made to the resource properties
-   */
-  public readonly propertyUpdates: PropDiffs;
-
-  public constructor(logicalId: string, oldValue: Resource, newValue: Resource, propertyUpdates: PropDiffs) {
-    this.logicalId = logicalId;
-    this.oldValue = oldValue;
-    this.newValue = newValue;
-    this.propertyUpdates = propertyUpdates;
-  }
-}
-
 type Exclude = { [key: string]: Exclude | true };
 
 /**
@@ -182,11 +161,11 @@ export function lowerCaseFirstCharacter(str: string): string {
   return str.length > 0 ? `${str[0].toLowerCase()}${str.slice(1)}` : str;
 }
 
-export type PropDiffs = Record<string, PropertyDifference<any>>;
+type PropDiffs = Record<string, PropertyDifference<any>>;
 
 export class ClassifiedChanges {
   public constructor(
-    public readonly change: HotswappableChangeCandidate,
+    public readonly change: ResourceChange,
     public readonly hotswappableProps: PropDiffs,
     public readonly nonHotswappableProps: PropDiffs,
   ) {
@@ -212,7 +191,7 @@ export class ClassifiedChanges {
   }
 }
 
-export function classifyChanges(xs: HotswappableChangeCandidate, hotswappablePropNames: string[]): ClassifiedChanges {
+export function classifyChanges(xs: ResourceChange, hotswappablePropNames: string[]): ClassifiedChanges {
   const hotswappableProps: PropDiffs = {};
   const nonHotswappableProps: PropDiffs = {};
 
@@ -229,7 +208,7 @@ export function classifyChanges(xs: HotswappableChangeCandidate, hotswappablePro
 
 export function reportNonHotswappableChange(
   ret: ChangeHotswapResult,
-  change: HotswappableChangeCandidate,
+  change: ResourceChange,
   nonHotswappableProps?: PropDiffs,
   reason?: string,
   hotswapOnlyVisible?: boolean,
@@ -249,7 +228,7 @@ export function reportNonHotswappableChange(
 }
 
 export function reportNonHotswappableResource(
-  change: HotswappableChangeCandidate,
+  change: ResourceChange,
   reason?: string,
 ): ChangeHotswapResult {
   return [