From 16c227df1f83df4224700c8adf9f3235324bea20 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 16 Nov 2022 11:05:53 +0100 Subject: [PATCH] fix(cli): asset prebuild breaks some custom bootstrap scenarios If people have managed to adjust the bootstrapping process to have bootstrap resources be created as part of normal stack deploys, the eager asset prebuild breaks the very first deploy (since it requires bootstrap resources to already be present). Allow disabling the prebuild by passing `--no-asset-prebuild`. Fixes #21965. --- packages/aws-cdk/jest.config.js | 3 ++ packages/aws-cdk/lib/cdk-toolkit.ts | 50 +++++++++++++++++++---- packages/aws-cdk/lib/cli.ts | 6 ++- packages/aws-cdk/lib/settings.ts | 1 + packages/aws-cdk/test/cdk-toolkit.test.ts | 28 ++++++++++++- 5 files changed, 76 insertions(+), 12 deletions(-) diff --git a/packages/aws-cdk/jest.config.js b/packages/aws-cdk/jest.config.js index 292f68b459252..ae3fb7ad49a8d 100644 --- a/packages/aws-cdk/jest.config.js +++ b/packages/aws-cdk/jest.config.js @@ -7,4 +7,7 @@ module.exports = { branches: 45, }, }, + + // We have many tests here that commonly time out + testTimeout: 30_000, }; diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 9304758e28030..320cb82d45128 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -69,6 +69,24 @@ export interface CdkToolkitProps { sdkProvider: SdkProvider; } +/** + * When to build assets + */ +export enum AssetBuildTime { + /** + * Build all assets before deploying the first stack + * + * This is intended for expensive Docker image builds; so that if the Docker image build + * fails, no stacks are unnecessarily deployed (with the attendant wait time). + */ + ALL_BEFORE_DEPLOY, + + /** + * Build assets just-in-time, before publishing + */ + JUST_IN_TIME, +}; + /** * Toolkit logic * @@ -167,17 +185,22 @@ export class CdkToolkit { } const stacks = stackCollection.stackArtifacts; + const assetBuildTime = options.assetBuildTime ?? AssetBuildTime.ALL_BEFORE_DEPLOY; + const stackOutputs: { [key: string]: any } = { }; const outputsFile = options.outputsFile; - try { - await buildAllStackAssets(stackCollection.stackArtifacts, { - buildStackAssets: (a) => this.buildAllAssetsForSingleStack(a, options), - }); - } catch (e) { - error('\n ❌ Building assets failed: %s', e); - throw e; + if (assetBuildTime === AssetBuildTime.ALL_BEFORE_DEPLOY) { + // Prebuild all assets + try { + await buildAllStackAssets(stackCollection.stackArtifacts, { + buildStackAssets: (a) => this.buildAllAssetsForSingleStack(a, options), + }); + } catch (e) { + error('\n ❌ Building assets failed: %s', e); + throw e; + } } const deployStack = async (stack: cxapi.CloudFormationStackArtifact) => { @@ -257,7 +280,7 @@ export class CdkToolkit { rollback: options.rollback, hotswap: options.hotswap, extraUserAgent: options.extraUserAgent, - buildAssets: false, + buildAssets: assetBuildTime !== AssetBuildTime.ALL_BEFORE_DEPLOY, assetParallelism: options.assetParallelism, }); @@ -1042,6 +1065,15 @@ export interface DeployOptions extends CfnDeployOptions, WatchOptions { * @default true */ readonly assetParallelism?: boolean; + + /** + * When to build assets + * + * The default is the Docker-friendly default. + * + * @default AssetBuildTime.ALL_BEFORE_DEPLOY + */ + readonly assetBuildTime?: AssetBuildTime; } export interface ImportOptions extends CfnDeployOptions { @@ -1136,4 +1168,4 @@ function roundPercentage(num: number): number { */ function millisecondsToSeconds(num: number): number { return num / 1000; -} +} \ No newline at end of file diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 516e6d30f20dd..4192c795689cb 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -13,7 +13,7 @@ import { execProgram } from '../lib/api/cxapp/exec'; import { PluginHost } from '../lib/api/plugin'; import { ToolkitInfo } from '../lib/api/toolkit-info'; import { StackActivityProgress } from '../lib/api/util/cloudformation/stack-activity-monitor'; -import { CdkToolkit } from '../lib/cdk-toolkit'; +import { CdkToolkit, AssetBuildTime } from '../lib/cdk-toolkit'; import { realHandler as context } from '../lib/commands/context'; import { realHandler as docs } from '../lib/commands/docs'; import { realHandler as doctor } from '../lib/commands/doctor'; @@ -157,7 +157,8 @@ async function parseCommandLineArguments() { "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('asset-parallelism', { type: 'boolean', desc: 'Whether to build/publish assets in parallel' }), + .option('asset-parallelism', { type: 'boolean', desc: 'Whether to build/publish assets in parallel' }) + .option('asset-prebuild', { type: 'boolean', desc: 'Whether to build all assets before deploying the first stack (useful for failing Docker builds)', default: true }), ) .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 }) @@ -521,6 +522,7 @@ async function initCommandLine() { traceLogs: args.logs, concurrency: args.concurrency, assetParallelism: configuration.settings.get(['assetParallelism']), + assetBuildTime: configuration.settings.get(['assetPrebuild']) ? AssetBuildTime.ALL_BEFORE_DEPLOY : AssetBuildTime.JUST_IN_TIME, }); case 'import': diff --git a/packages/aws-cdk/lib/settings.ts b/packages/aws-cdk/lib/settings.ts index b4e3a2f4ebf7d..e6505d829c1af 100644 --- a/packages/aws-cdk/lib/settings.ts +++ b/packages/aws-cdk/lib/settings.ts @@ -290,6 +290,7 @@ export class Settings { rollback: argv.rollback, notices: argv.notices, assetParallelism: argv['asset-parallelism'], + assetPrebuild: argv['asset-prebuild'], }); } diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index 205eb8b28910a..d2a8000464281 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cdk-toolkit.test.ts @@ -62,7 +62,7 @@ import { Bootstrapper } from '../lib/api/bootstrap'; import { CloudFormationDeployments, DeployStackOptions, DestroyStackOptions } from '../lib/api/cloudformation-deployments'; import { DeployStackResult } from '../lib/api/deploy-stack'; import { Template } from '../lib/api/util/cloudformation'; -import { CdkToolkit, Tag } from '../lib/cdk-toolkit'; +import { CdkToolkit, Tag, AssetBuildTime } from '../lib/cdk-toolkit'; import { RequireApproval } from '../lib/diff'; import { flatten } from '../lib/util'; import { instanceMockFrom, MockCloudExecutable, TestStackArtifact, withMocked } from './util'; @@ -589,6 +589,32 @@ describe('deploy', () => { })); }); }); + + test('can disable asset prebuild', 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'] }, + assetBuildTime: AssetBuildTime.JUST_IN_TIME, + }); + + expect(mockBuildStackAssets).not.toHaveBeenCalled(); + }); + }); }); });