Skip to content

Commit fc4668d

Browse files
authored
fix(cli): asset prebuild breaks some custom bootstrap scenarios (#22930)
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. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 741b5e5 commit fc4668d

File tree

5 files changed

+76
-12
lines changed

5 files changed

+76
-12
lines changed

packages/aws-cdk/jest.config.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,7 @@ module.exports = {
77
branches: 45,
88
},
99
},
10+
11+
// We have many tests here that commonly time out
12+
testTimeout: 30_000,
1013
};

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

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,24 @@ export interface CdkToolkitProps {
6969
sdkProvider: SdkProvider;
7070
}
7171

72+
/**
73+
* When to build assets
74+
*/
75+
export enum AssetBuildTime {
76+
/**
77+
* Build all assets before deploying the first stack
78+
*
79+
* This is intended for expensive Docker image builds; so that if the Docker image build
80+
* fails, no stacks are unnecessarily deployed (with the attendant wait time).
81+
*/
82+
ALL_BEFORE_DEPLOY,
83+
84+
/**
85+
* Build assets just-in-time, before publishing
86+
*/
87+
JUST_IN_TIME,
88+
};
89+
7290
/**
7391
* Toolkit logic
7492
*
@@ -167,17 +185,22 @@ export class CdkToolkit {
167185
}
168186

169187
const stacks = stackCollection.stackArtifacts;
188+
const assetBuildTime = options.assetBuildTime ?? AssetBuildTime.ALL_BEFORE_DEPLOY;
189+
170190

171191
const stackOutputs: { [key: string]: any } = { };
172192
const outputsFile = options.outputsFile;
173193

174-
try {
175-
await buildAllStackAssets(stackCollection.stackArtifacts, {
176-
buildStackAssets: (a) => this.buildAllAssetsForSingleStack(a, options),
177-
});
178-
} catch (e) {
179-
error('\n ❌ Building assets failed: %s', e);
180-
throw e;
194+
if (assetBuildTime === AssetBuildTime.ALL_BEFORE_DEPLOY) {
195+
// Prebuild all assets
196+
try {
197+
await buildAllStackAssets(stackCollection.stackArtifacts, {
198+
buildStackAssets: (a) => this.buildAllAssetsForSingleStack(a, options),
199+
});
200+
} catch (e) {
201+
error('\n ❌ Building assets failed: %s', e);
202+
throw e;
203+
}
181204
}
182205

183206
const deployStack = async (stack: cxapi.CloudFormationStackArtifact) => {
@@ -257,7 +280,7 @@ export class CdkToolkit {
257280
rollback: options.rollback,
258281
hotswap: options.hotswap,
259282
extraUserAgent: options.extraUserAgent,
260-
buildAssets: false,
283+
buildAssets: assetBuildTime !== AssetBuildTime.ALL_BEFORE_DEPLOY,
261284
assetParallelism: options.assetParallelism,
262285
});
263286

@@ -1042,6 +1065,15 @@ export interface DeployOptions extends CfnDeployOptions, WatchOptions {
10421065
* @default true
10431066
*/
10441067
readonly assetParallelism?: boolean;
1068+
1069+
/**
1070+
* When to build assets
1071+
*
1072+
* The default is the Docker-friendly default.
1073+
*
1074+
* @default AssetBuildTime.ALL_BEFORE_DEPLOY
1075+
*/
1076+
readonly assetBuildTime?: AssetBuildTime;
10451077
}
10461078

10471079
export interface ImportOptions extends CfnDeployOptions {
@@ -1136,4 +1168,4 @@ function roundPercentage(num: number): number {
11361168
*/
11371169
function millisecondsToSeconds(num: number): number {
11381170
return num / 1000;
1139-
}
1171+
}

packages/aws-cdk/lib/cli.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { execProgram } from '../lib/api/cxapp/exec';
1313
import { PluginHost } from '../lib/api/plugin';
1414
import { ToolkitInfo } from '../lib/api/toolkit-info';
1515
import { StackActivityProgress } from '../lib/api/util/cloudformation/stack-activity-monitor';
16-
import { CdkToolkit } from '../lib/cdk-toolkit';
16+
import { CdkToolkit, AssetBuildTime } from '../lib/cdk-toolkit';
1717
import { realHandler as context } from '../lib/commands/context';
1818
import { realHandler as docs } from '../lib/commands/docs';
1919
import { realHandler as doctor } from '../lib/commands/doctor';
@@ -157,7 +157,8 @@ async function parseCommandLineArguments() {
157157
"Only in effect if specified alongside the '--watch' option",
158158
})
159159
.option('concurrency', { type: 'number', desc: 'Maximum number of simultaneous deployments (dependency permitting) to execute.', default: 1, requiresArg: true })
160-
.option('asset-parallelism', { type: 'boolean', desc: 'Whether to build/publish assets in parallel' }),
160+
.option('asset-parallelism', { type: 'boolean', desc: 'Whether to build/publish assets in parallel' })
161+
.option('asset-prebuild', { type: 'boolean', desc: 'Whether to build all assets before deploying the first stack (useful for failing Docker builds)', default: true }),
161162
)
162163
.command('import [STACK]', 'Import existing resource(s) into the given STACK', (yargs: Argv) => yargs
163164
.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() {
521522
traceLogs: args.logs,
522523
concurrency: args.concurrency,
523524
assetParallelism: configuration.settings.get(['assetParallelism']),
525+
assetBuildTime: configuration.settings.get(['assetPrebuild']) ? AssetBuildTime.ALL_BEFORE_DEPLOY : AssetBuildTime.JUST_IN_TIME,
524526
});
525527

526528
case 'import':

packages/aws-cdk/lib/settings.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ export class Settings {
290290
rollback: argv.rollback,
291291
notices: argv.notices,
292292
assetParallelism: argv['asset-parallelism'],
293+
assetPrebuild: argv['asset-prebuild'],
293294
});
294295
}
295296

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ import { Bootstrapper } from '../lib/api/bootstrap';
6262
import { CloudFormationDeployments, DeployStackOptions, DestroyStackOptions } from '../lib/api/cloudformation-deployments';
6363
import { DeployStackResult } from '../lib/api/deploy-stack';
6464
import { Template } from '../lib/api/util/cloudformation';
65-
import { CdkToolkit, Tag } from '../lib/cdk-toolkit';
65+
import { CdkToolkit, Tag, AssetBuildTime } from '../lib/cdk-toolkit';
6666
import { RequireApproval } from '../lib/diff';
6767
import { flatten } from '../lib/util';
6868
import { instanceMockFrom, MockCloudExecutable, TestStackArtifact, withMocked } from './util';
@@ -589,6 +589,32 @@ describe('deploy', () => {
589589
}));
590590
});
591591
});
592+
593+
test('can disable asset prebuild', async () => {
594+
// GIVEN
595+
cloudExecutable = new MockCloudExecutable({
596+
stacks: [MockStack.MOCK_STACK_WITH_ASSET],
597+
});
598+
const fakeCloudFormation = new FakeCloudFormation({});
599+
600+
const toolkit = new CdkToolkit({
601+
cloudExecutable,
602+
configuration: cloudExecutable.configuration,
603+
sdkProvider: cloudExecutable.sdkProvider,
604+
cloudFormation: fakeCloudFormation,
605+
});
606+
607+
// WHEN
608+
// Not the best test but following this through to the asset publishing library fails
609+
await withMocked(fakeCloudFormation, 'buildStackAssets', async (mockBuildStackAssets) => {
610+
await toolkit.deploy({
611+
selector: { patterns: ['Test-Stack-Asset'] },
612+
assetBuildTime: AssetBuildTime.JUST_IN_TIME,
613+
});
614+
615+
expect(mockBuildStackAssets).not.toHaveBeenCalled();
616+
});
617+
});
592618
});
593619
});
594620

0 commit comments

Comments
 (0)