fix: --force flag does not applies to assets (#197)

The `cdk deploy --force` flag is intended to disable all smartness
around saving work. If set, it won't check whether assets already exist
in the cloud, and remove the build and publishing steps from the work
graph.

However, this by itself is not enough to make sure the asset truly gets
published again, because the `publish()` action has its own version of
short-circuiting again.

Rather than remove the short-circuiting behavior from `cdk-assets`, we
add another `{ force }` flag there as well, which gets its value from
the CLI's `--force` flag.

This will make it possible to recover from corrupted assets which were
accidentally published, as fixed in
aws/aws-cdk#33692, by running `rm -rf cdk.out &&
cdk deploy --force`.

Fixes aws/aws-cdk#14474.

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

---------

Signed-off-by: github-actions <[email protected]>
Co-authored-by: github-actions <[email protected]>
Co-authored-by: Kaizen Conroy <[email protected]>
Co-authored-by: Momo Kornher <[email protected]>

Author: 4 people

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

@@ -3,7 +3,7 @@
 // APIs
 export { SdkProvider } from '../../../../aws-cdk/lib/api/aws-auth';
 export { Context, PROJECT_CONTEXT } from '../../../../aws-cdk/lib/api/context';
-export { Deployments, type SuccessfulDeployStackResult } from '../../../../aws-cdk/lib/api/deployments';
+export { Deployments, type SuccessfulDeployStackResult, type DeployStackOptions, type DeployStackResult } from '../../../../aws-cdk/lib/api/deployments';
 export { Settings } from '../../../../aws-cdk/lib/api/settings';
 export { type Tag, tagsForStack } from '../../../../aws-cdk/lib/api/tags';
 export { DEFAULT_TOOLKIT_STACK_NAME } from '../../../../aws-cdk/lib/api/toolkit-info';

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

@@ -317,6 +317,7 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
         stack: assetNode.parentStack,
         roleArn: options.roleArn,
         stackName: assetNode.parentStack.stackName,
+        forcePublish: options.force,
       });
     };
 

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

@@ -1,41 +1,40 @@
-let mockFindCloudWatchLogGroups = jest.fn();
-
 import { RequireApproval, StackParameters } from '../../lib';
+import * as awsCdkApi from '../../lib/api/aws-cdk';
+import type { DeployStackOptions, DeployStackResult } from '../../lib/api/aws-cdk';
 import { Toolkit } from '../../lib/toolkit';
 import { builderFixture, TestIoHost } from '../_helpers';
 import { MockSdk } from '../util/aws-cdk';
 
-const sdk = new MockSdk();
-const ioHost = new TestIoHost();
-const toolkit = new Toolkit({ ioHost });
-const rollbackSpy = jest.spyOn(toolkit as any, '_rollback').mockResolvedValue({});
-
-let mockDeployStack = jest.fn().mockResolvedValue({
-  type: 'did-deploy-stack',
-  stackArn: 'arn:aws:cloudformation:region:account:stack/test-stack',
-  outputs: {},
-  noOp: false,
-});
-
-jest.mock('../../lib/api/aws-cdk', () => {
-  return {
-    ...jest.requireActual('../../lib/api/aws-cdk'),
-    Deployments: jest.fn().mockImplementation(() => ({
-      deployStack: mockDeployStack,
-      resolveEnvironment: jest.fn().mockResolvedValue({}),
-      isSingleAssetPublished: jest.fn().mockResolvedValue(true),
-      readCurrentTemplate: jest.fn().mockResolvedValue({ Resources: {} }),
-    })),
-    findCloudWatchLogGroups: mockFindCloudWatchLogGroups,
-  };
-});
+let ioHost: TestIoHost;
+let toolkit: Toolkit;
+let mockDeployStack: jest.SpyInstance<Promise<DeployStackResult>, [DeployStackOptions]>;
 
 beforeEach(() => {
-  ioHost.notifySpy.mockClear();
-  ioHost.requestSpy.mockClear();
+  jest.restoreAllMocks();
+  ioHost = new TestIoHost();
   ioHost.requireDeployApproval = RequireApproval.NEVER;
-  jest.clearAllMocks();
-  mockFindCloudWatchLogGroups.mockReturnValue({
+
+  toolkit = new Toolkit({ ioHost });
+  const sdk = new MockSdk();
+
+  // Some default implementations
+  mockDeployStack = jest.spyOn(awsCdkApi.Deployments.prototype, 'deployStack').mockResolvedValue({
+    type: 'did-deploy-stack',
+    stackArn: 'arn:aws:cloudformation:region:account:stack/test-stack',
+    outputs: {},
+    noOp: false,
+  });
+  jest.spyOn(awsCdkApi.Deployments.prototype, 'resolveEnvironment').mockResolvedValue({
+    account: '11111111',
+    region: 'aq-south-1',
+    name: 'aws://11111111/aq-south-1',
+  });
+  jest.spyOn(awsCdkApi.Deployments.prototype, 'isSingleAssetPublished').mockResolvedValue(true);
+  jest.spyOn(awsCdkApi.Deployments.prototype, 'readCurrentTemplate').mockResolvedValue({ Resources: {} });
+  jest.spyOn(awsCdkApi.Deployments.prototype, 'buildSingleAsset').mockImplementation();
+  jest.spyOn(awsCdkApi.Deployments.prototype, 'publishSingleAsset').mockImplementation();
+
+  jest.spyOn(awsCdkApi, 'findCloudWatchLogGroups').mockResolvedValue({
     env: { name: 'Z', account: 'X', region: 'Y' },
     sdk,
     logGroupNames: ['/aws/lambda/lambda-function-name'],
@@ -198,6 +197,20 @@ describe('deploy', () => {
 
       successfulDeployment();
     });
+
+    test('force: true option is used for asset publishing', async () => {
+      const publishSingleAsset = jest.spyOn(awsCdkApi.Deployments.prototype, 'publishSingleAsset').mockImplementation();
+
+      const cx = await builderFixture(toolkit, 'stack-with-asset');
+      await toolkit.deploy(cx, {
+        force: true,
+      });
+
+      // THEN
+      expect(publishSingleAsset).toHaveBeenCalledWith(expect.anything(), expect.anything(), expect.objectContaining({
+        forcePublish: true,
+      }));
+    });
   });
 
   describe('deployment results', () => {
@@ -211,22 +224,23 @@ describe('deploy', () => {
     });
 
     test('failpaused-need-rollback-first', async () => {
+      const rollbackSpy = jest.spyOn(toolkit as any, '_rollback').mockResolvedValue({});
+
       // GIVEN
-      mockDeployStack.mockImplementation((params) => {
+      mockDeployStack.mockImplementation(async (params) => {
         if (params.rollback === true) {
           return {
             type: 'did-deploy-stack',
             stackArn: 'arn:aws:cloudformation:region:account:stack/test-stack',
             outputs: {},
             noOp: false,
-          };
+          } satisfies DeployStackResult;
         } else {
           return {
             type: 'failpaused-need-rollback-first',
-            stackArn: 'arn:aws:cloudformation:region:account:stack/test-stack',
-            outputs: {},
-            noOp: false,
-          };
+            reason: 'replacement',
+            status: 'asdf',
+          } satisfies DeployStackResult;
         }
       });
 
@@ -242,21 +256,18 @@ describe('deploy', () => {
 
     test('replacement-requires-rollback', async () => {
       // GIVEN
-      mockDeployStack.mockImplementation((params) => {
+      mockDeployStack.mockImplementation(async (params) => {
         if (params.rollback === true) {
           return {
             type: 'did-deploy-stack',
             stackArn: 'arn:aws:cloudformation:region:account:stack/test-stack',
             outputs: {},
             noOp: false,
-          };
+          } satisfies DeployStackResult;
         } else {
           return {
             type: 'replacement-requires-rollback',
-            stackArn: 'arn:aws:cloudformation:region:account:stack/test-stack',
-            outputs: {},
-            noOp: false,
-          };
+          } satisfies DeployStackResult;
         }
       });
 

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

@@ -272,6 +272,13 @@ interface PublishStackAssetsOptions extends AssetOptions {
    * Stack name this asset is for
    */
   readonly stackName?: string;
+
+  /**
+   * Always publish, even if it already exists
+   *
+   * @default false
+   */
+  readonly forcePublish?: boolean;
 }
 
 export interface DestroyStackOptions {
@@ -645,7 +652,10 @@ export class Deployments {
 
     // No need to validate anymore, we already did that during build
     const publisher = this.cachedPublisher(assetManifest, stackEnv, options.stackName);
-    await publisher.publishEntry(asset, { allowCrossAccount: await this.allowCrossAccountAssetPublishingForEnv(options.stack) });
+    await publisher.publishEntry(asset, {
+      allowCrossAccount: await this.allowCrossAccountAssetPublishingForEnv(options.stack),
+      force: options.forcePublish,
+    });
     if (publisher.hasFailures) {
       throw new ToolkitError(`Failed to publish asset ${asset.displayName(true)}`);
     }

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

@@ -354,6 +354,7 @@ export class CdkToolkit {
         stack: assetNode.parentStack,
         roleArn: options.roleArn,
         stackName: assetNode.parentStack.stackName,
+        forcePublish: options.force,
       });
     };
 

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

@@ -60,6 +60,7 @@ jest.setTimeout(30_000);
 import 'aws-sdk-client-mock';
 import * as os from 'os';
 import * as path from 'path';
+import * as cdkAssets from 'cdk-assets';
 import * as cxschema from '@aws-cdk/cloud-assembly-schema';
 import { Manifest } from '@aws-cdk/cloud-assembly-schema';
 import * as cxapi from '@aws-cdk/cx-api';
@@ -682,6 +683,35 @@ describe('deploy', () => {
       expect(stderrMock).toHaveBeenCalledWith(expect.stringContaining('Publishing Asset Display Name (desto)'));
     });
 
+    test('force flag is passed to asset publishing', async () => {
+      // GIVEN
+      cloudExecutable = new MockCloudExecutable({
+        stacks: [MockStack.MOCK_STACK_WITH_ASSET],
+      });
+      const toolkit = new CdkToolkit({
+        cloudExecutable,
+        configuration: cloudExecutable.configuration,
+        sdkProvider: cloudExecutable.sdkProvider,
+        deployments: new FakeCloudFormation({}),
+      });
+
+      const publishEntry = jest.spyOn(cdkAssets.AssetPublishing.prototype, 'publishEntry');
+
+      // WHEN
+      await toolkit.deploy({
+        selector: { patterns: [MockStack.MOCK_STACK_WITH_ASSET.stackName] },
+        hotswap: HotswapMode.FULL_DEPLOYMENT,
+        force: true,
+      });
+
+      // THEN
+      expect(publishEntry).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
+        force: true,
+      }));
+
+      publishEntry.mockRestore();
+    });
+
     test('with stacks all stacks specified as wildcard', async () => {
       // GIVEN
       const toolkit = defaultToolkitSetup();

packages/cdk-assets/lib/private/asset-handler.ts

@@ -13,6 +13,13 @@ export interface PublishOptions {
    * @default true
    */
   readonly allowCrossAccount?: boolean;
+
+  /**
+   * Always upload, even if the target file already exists
+   *
+   * @default false
+   */
+  readonly force?: boolean;
 }
 
 /**

packages/cdk-assets/lib/private/handlers/files.ts

@@ -112,7 +112,7 @@ export class FileAssetHandler implements IAssetHandler {
         break;
     }
 
-    if (await objectExists(s3, destination.bucketName, destination.objectKey)) {
+    if (!options.force && await objectExists(s3, destination.bucketName, destination.objectKey)) {
       this.host.emitMessage(EventType.FOUND, `Found ${s3Url}`);
       return;
     }

packages/cdk-assets/test/files.test.ts

@@ -123,22 +123,26 @@ test('will only read bucketEncryption once even for multiple assets', async () =
   expect(s3).toHaveReceivedCommandTimes(GetBucketEncryptionCommand, 1);
 });
 
-test('Do nothing if file already exists', async () => {
+test.each([false, true])('file already exists with { force: %p }', async (force) => {
   const s3 = mockClient(S3Client);
 
   const pub = new AssetPublishing(AssetManifest.fromPath(mockfs.path('/simple/cdk.out')), { aws });
   s3.on(ListObjectsV2Command).resolves({ Contents: [{ Key: 'some_key' }] });
 
-  await pub.publish();
-
-  expect(s3).toHaveReceivedCommandWith(ListObjectsV2Command, {
-    Bucket: 'some_bucket',
-    Prefix: 'some_key',
-    MaxKeys: 1,
-  });
+  await pub.publish({ force });
 
   // Upload calls PutObjectCommand under the hood
-  expect(s3).not.toHaveReceivedCommand(PutObjectCommand);
+  if (force) {
+    expect(s3).toHaveReceivedCommand(PutObjectCommand);
+  } else {
+    expect(s3).toHaveReceivedCommandWith(ListObjectsV2Command, {
+      Bucket: 'some_bucket',
+      Prefix: 'some_key',
+      MaxKeys: 1,
+    });
+
+    expect(s3).not.toHaveReceivedCommand(PutObjectCommand);
+  }
 });
 
 test('tiny file does not count as cache hit', async () => {