feat(cli): allow disabling parallel asset publishing (#22579)

Once again, any change to anything anywhere broke someone. In this particular case, parallel asset publishing (#19367, and in particular building) broke a use case where someone uses a tool to build assets that is not concurrency-safe. So -- now we need to make that configurable.

Command line:

```
cdk deploy --no-asset-parallelism
```

cdk.json:

```
{
  "assetParallelism": false
}
```

Environment variables:

```
export CDK_ASSET_PARALLELISM=false
```


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

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

Author: rix0rrr

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

@@ -2,7 +2,7 @@ import * as cxapi from '@aws-cdk/cx-api';
 import { AssetManifest } from 'cdk-assets';
 import { Tag } from '../cdk-toolkit';
 import { debug, warning } from '../logging';
-import { buildAssets, publishAssets } from '../util/asset-publishing';
+import { buildAssets, publishAssets, BuildAssetsOptions, PublishAssetsOptions } from '../util/asset-publishing';
 import { Mode } from './aws-auth/credentials';
 import { ISDK } from './aws-auth/sdk';
 import { SdkProvider } from './aws-auth/sdk-provider';
@@ -253,6 +253,13 @@ export interface DeployStackOptions {
    * @default true To remain backward compatible.
    */
   readonly buildAssets?: boolean;
+
+  /**
+   * Whether to build/publish assets in parallel
+   *
+   * @default true To remain backward compatible.
+   */
+  readonly assetParallelism?: boolean;
 }
 
 export interface BuildStackAssetsOptions {
@@ -274,6 +281,11 @@ export interface BuildStackAssetsOptions {
    * @default - Current role
    */
   readonly roleArn?: string;
+
+  /**
+   * Options to pass on to `buildAsests()` function
+   */
+  readonly buildOptions?: BuildAssetsOptions;
 }
 
 interface PublishStackAssetsOptions {
@@ -283,6 +295,11 @@ interface PublishStackAssetsOptions {
    * @default true To remain backward compatible.
    */
   readonly buildAssets?: boolean;
+
+  /**
+   * Options to pass on to `publishAsests()` function
+   */
+  readonly publishOptions?: Omit<PublishAssetsOptions, 'buildAssets'>;
 }
 
 export interface DestroyStackOptions {
@@ -401,6 +418,9 @@ export class CloudFormationDeployments {
     if (options.resourcesToImport === undefined) {
       await this.publishStackAssets(options.stack, toolkitInfo, {
         buildAssets: options.buildAssets ?? true,
+        publishOptions: {
+          parallel: options.assetParallelism,
+        },
       });
     }
 
@@ -434,6 +454,7 @@ export class CloudFormationDeployments {
       extraUserAgent: options.extraUserAgent,
       resourcesToImport: options.resourcesToImport,
       overrideTemplate: options.overrideTemplate,
+      assetParallelism: options.assetParallelism,
     });
   }
 
@@ -529,7 +550,7 @@ export class CloudFormationDeployments {
         toolkitInfo);
 
       const manifest = AssetManifest.fromFile(assetArtifact.file);
-      await buildAssets(manifest, this.sdkProvider, stackEnv);
+      await buildAssets(manifest, this.sdkProvider, stackEnv, options.buildOptions);
     }
   }
 
@@ -549,6 +570,7 @@ export class CloudFormationDeployments {
 
       const manifest = AssetManifest.fromFile(assetArtifact.file);
       await publishAssets(manifest, this.sdkProvider, stackEnv, {
+        ...options.publishOptions,
         buildAssets: options.buildAssets ?? true,
       });
     }

packages/aws-cdk/lib/api/deploy-stack.ts

@@ -198,6 +198,13 @@ export interface DeployStackOptions {
    * @default - Use the stored template
    */
   readonly overrideTemplate?: any;
+
+  /**
+   * Whether to build/publish assets in parallel
+   *
+   * @default true To remain backward compatible.
+   */
+  readonly assetParallelism?: boolean;
 }
 
 export type DeploymentMethod =
@@ -287,7 +294,9 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
     options.toolkitInfo,
     options.sdk,
     options.overrideTemplate);
-  await publishAssets(legacyAssets.toManifest(stackArtifact.assembly.directory), options.sdkProvider, stackEnv);
+  await publishAssets(legacyAssets.toManifest(stackArtifact.assembly.directory), options.sdkProvider, stackEnv, {
+    parallel: options.assetParallelism,
+  });
 
   if (options.hotswap) {
     // attempt to short-circuit the deployment if possible

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

@@ -258,6 +258,7 @@ export class CdkToolkit {
           hotswap: options.hotswap,
           extraUserAgent: options.extraUserAgent,
           buildAssets: false,
+          assetParallelism: options.assetParallelism,
         });
 
         const message = result.noOp
@@ -764,7 +765,7 @@ export class CdkToolkit {
     }
   }
 
-  private async buildAllAssetsForSingleStack(stack: cxapi.CloudFormationStackArtifact, options: Pick<DeployOptions, 'roleArn' | 'toolkitStackName'>): Promise<void> {
+  private async buildAllAssetsForSingleStack(stack: cxapi.CloudFormationStackArtifact, options: Pick<DeployOptions, 'roleArn' | 'toolkitStackName' | 'assetParallelism'>): Promise<void> {
     // Check whether the stack has an asset manifest before trying to build and publish.
     if (!stack.dependencies.some(cxapi.AssetManifestArtifact.isAssetManifestArtifact)) {
       return;
@@ -775,6 +776,9 @@ export class CdkToolkit {
       stack,
       roleArn: options.roleArn,
       toolkitStackName: options.toolkitStackName,
+      buildOptions: {
+        parallel: options.assetParallelism,
+      },
     });
     print('\n%s: assets built\n', chalk.bold(stack.displayName));
   }
@@ -1029,6 +1033,15 @@ export interface DeployOptions extends CfnDeployOptions, WatchOptions {
    * @default 1
    */
   readonly concurrency?: number;
+
+  /**
+   * Build/publish assets for a single stack in parallel
+   *
+   * Independent of whether stacks are being done in parallel or no.
+   *
+   * @default true
+   */
+  readonly assetParallelism?: boolean;
 }
 
 export interface ImportOptions extends CfnDeployOptions {

packages/aws-cdk/lib/cli.ts

@@ -155,7 +155,8 @@ async function parseCommandLineArguments() {
           "'true' by default, use --no-logs to turn off. " +
           "Only in effect if specified alongside the '--watch' option",
       })
-      .option('concurrency', { type: 'number', desc: 'Maximum number of simultaneous deployments (dependency permitting) to execute.', default: 1, requiresArg: true }),
+      .option('concurrency', { type: 'number', desc: 'Maximum number of simultaneous deployments (dependency permitting) to execute.', default: 1, requiresArg: true })
+      .option('asset-parallelism', { type: 'boolean', desc: 'Whether to build/publish assets in parallel' }),
     )
     .command('import [STACK]', 'Import existing resource(s) into the given STACK', (yargs: Argv) => yargs
       .option('execute', { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true })
@@ -514,6 +515,7 @@ async function initCommandLine() {
           watch: args.watch,
           traceLogs: args.logs,
           concurrency: args.concurrency,
+          assetParallelism: args.assetParallelism,
         });
 
       case 'import':

packages/aws-cdk/lib/settings.ts

@@ -289,6 +289,7 @@ export class Settings {
       lookups: argv.lookups,
       rollback: argv.rollback,
       notices: argv.notices,
+      assetParallelism: argv['asset-parallelism'],
     });
   }
 

packages/aws-cdk/lib/util/asset-publishing.ts

@@ -18,6 +18,13 @@ export interface PublishAssetsOptions {
    * @default true To remain backward compatible.
    */
   readonly buildAssets?: boolean;
+
+  /**
+   * Whether to build/publish assets in parallel
+   *
+   * @default true To remain backward compatible.
+   */
+  readonly parallel?: boolean;
 }
 
 /**
@@ -44,7 +51,7 @@ export async function publishAssets(
     aws: new PublishingAws(sdk, targetEnv),
     progressListener: new PublishingProgressListener(options.quiet ?? false),
     throwOnError: false,
-    publishInParallel: true,
+    publishInParallel: options.parallel ?? true,
     buildAssets: options.buildAssets ?? true,
     publishAssets: true,
   });
@@ -59,6 +66,13 @@ export interface BuildAssetsOptions {
    * Print progress at 'debug' level
    */
   readonly quiet?: boolean;
+
+  /**
+   * Build assets in parallel
+   *
+   * @default true
+   */
+  readonly parallel?: boolean;
 }
 
 /**
@@ -85,7 +99,7 @@ export async function buildAssets(
     aws: new PublishingAws(sdk, targetEnv),
     progressListener: new PublishingProgressListener(options.quiet ?? false),
     throwOnError: false,
-    publishInParallel: true,
+    publishInParallel: options.parallel ?? true,
     buildAssets: true,
     publishAssets: false,
   });

packages/aws-cdk/test/api/cloudformation-deployments.test.ts

@@ -904,7 +904,7 @@ test('building assets', async () => {
     name: 'aws://account/region',
     region: 'region',
   });
-  expect(buildAssets).toBeCalledWith(expectedAssetManifest, sdkProvider, expectedEnvironment);
+  expect(buildAssets).toBeCalledWith(expectedAssetManifest, sdkProvider, expectedEnvironment, undefined);
 });
 
 function pushStackResourceSummaries(stackName: string, ...items: CloudFormation.StackResourceSummary[]) {

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

@@ -52,8 +52,11 @@ jest.mock('../lib/logging', () => ({
   ...jest.requireActual('../lib/logging'),
   data: mockData,
 }));
+jest.setTimeout(30_000);
 
+import * as path from 'path';
 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';
 import { Bootstrapper } from '../lib/api/bootstrap';
 import { CloudFormationDeployments, DeployStackOptions, DestroyStackOptions } from '../lib/api/cloudformation-deployments';
@@ -62,7 +65,8 @@ import { Template } from '../lib/api/util/cloudformation';
 import { CdkToolkit, Tag } from '../lib/cdk-toolkit';
 import { RequireApproval } from '../lib/diff';
 import { flatten } from '../lib/util';
-import { instanceMockFrom, MockCloudExecutable, TestStackArtifact } from './util';
+import { instanceMockFrom, MockCloudExecutable, TestStackArtifact, withMocked } from './util';
+import { MockSdkProvider } from './util/mock-sdk';
 
 let cloudExecutable: MockCloudExecutable;
 let bootstrapper: jest.Mocked<Bootstrapper>;
@@ -555,6 +559,36 @@ describe('deploy', () => {
       expect(cloudExecutable.hasApp).toEqual(false);
       expect(mockSynthesize).not.toHaveBeenCalled();
     });
+
+    test('can disable asset parallelism', async () => {
+      // GIVEN
+      cloudExecutable = new MockCloudExecutable({
+        stacks: [MockStack.MOCK_STACK_WITH_ASSET],
+      });
+      const fakeCloudFormation = new FakeCloudFormation({});
+
+      const toolkit = new CdkToolkit({
+        cloudExecutable,
+        configuration: cloudExecutable.configuration,
+        sdkProvider: cloudExecutable.sdkProvider,
+        cloudFormation: fakeCloudFormation,
+      });
+
+      // WHEN
+      // Not the best test but following this through to the asset publishing library fails
+      await withMocked(fakeCloudFormation, 'buildStackAssets', async (mockBuildStackAssets) => {
+        await toolkit.deploy({
+          selector: { patterns: ['Test-Stack-Asset'] },
+          assetParallelism: false,
+        });
+
+        expect(mockBuildStackAssets).toHaveBeenCalledWith(expect.objectContaining({
+          buildOptions: expect.objectContaining({
+            parallel: false,
+          }),
+        }));
+      });
+    });
   });
 });
 
@@ -911,6 +945,23 @@ class MockStack {
     },
     displayName: 'Test-Stack-A/witherrors',
   }
+  public static readonly MOCK_STACK_WITH_ASSET: TestStackArtifact = {
+    stackName: 'Test-Stack-Asset',
+    template: { Resources: { TemplateName: 'Test-Stack-Asset' } },
+    env: 'aws://123456789012/bermuda-triangle-1',
+    assetManifest: {
+      version: Manifest.version(),
+      files: {
+        xyz: {
+          source: {
+            path: path.resolve(__dirname, '..', 'LICENSE'),
+          },
+          destinations: {
+          },
+        },
+      },
+    },
+  }
 }
 
 class FakeCloudFormation extends CloudFormationDeployments {
@@ -921,7 +972,7 @@ class FakeCloudFormation extends CloudFormationDeployments {
     expectedTags: { [stackName: string]: { [key: string]: string } } = {},
     expectedNotificationArns?: string[],
   ) {
-    super({ sdkProvider: undefined as any });
+    super({ sdkProvider: new MockSdkProvider() });
 
     for (const [stackName, tags] of Object.entries(expectedTags)) {
       this.expectedTags[stackName] =
@@ -934,9 +985,17 @@ class FakeCloudFormation extends CloudFormationDeployments {
   }
 
   public deployStack(options: DeployStackOptions): Promise<DeployStackResult> {
-    expect([MockStack.MOCK_STACK_A.stackName, MockStack.MOCK_STACK_B.stackName, MockStack.MOCK_STACK_C.stackName])
-      .toContain(options.stack.stackName);
-    expect(options.tags).toEqual(this.expectedTags[options.stack.stackName]);
+    expect([
+      MockStack.MOCK_STACK_A.stackName,
+      MockStack.MOCK_STACK_B.stackName,
+      MockStack.MOCK_STACK_C.stackName,
+      MockStack.MOCK_STACK_WITH_ASSET.stackName,
+    ]).toContain(options.stack.stackName);
+
+    if (this.expectedTags[options.stack.stackName]) {
+      expect(options.tags).toEqual(this.expectedTags[options.stack.stackName]);
+    }
+
     expect(options.notificationArns).toEqual(this.expectedNotificationArns);
     return Promise.resolve({
       stackArn: `arn:aws:cloudformation:::stack/${options.stack.stackName}/MockedOut`,
@@ -959,6 +1018,8 @@ class FakeCloudFormation extends CloudFormationDeployments {
         return Promise.resolve({});
       case MockStack.MOCK_STACK_C.stackName:
         return Promise.resolve({});
+      case MockStack.MOCK_STACK_WITH_ASSET.stackName:
+        return Promise.resolve({});
       default:
         return Promise.reject(`Not an expected mock stack: ${stack.stackName}`);
     }