Skip to content

Commit 6f3e838

Browse files
authored
fix(cli): deployment stops on AccessDenied looking up bootstrap stack (#26925)
The CLI always looks up the default bootstrap stack, for backwards compatibility reasons: in case the attributes introduced by the V2 `DefaultStackSynthesizer` that tell it what SSM parameter to use and what bucket to write assets to are not present, it needs to fall back to the default bootstrap stack found in CloudFormation. The code happily survives a `StackNotFound` error, but is not prepared to deal with an `AccessDenied` error, that a customer in #26588 had configured their AWS account for. The essence of the fix here is to catch all errors when looking up the toolkit stack, because they only become relevant if any of the properties of the toolkit stack are ever accessed. The customer also made the point that the lookup didn't even need to happen in the first place, because all information was already there. This is fair, and the organization of the code in this area has been a thorn in my side for a while now. There is some code that doesn't need to be on `ToolkitInfo` (which is the ancient name for the Bootstrap Stack), but is there for legacy reasons. This PR introduces a refactor, where we introduce a new class `EnvironmentResources`, that manages interacting with the bootstrap resources in a particular environment. We can now pass `EnvironmentResources` everywhere we used to pass `ToolkitInfo`, and the actual lookup of the Bootstrap Stack is only triggered if the need arises (which hopefully should be never). Closes #26588. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent e52acd8 commit 6f3e838

16 files changed

+460
-343
lines changed

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { BOOTSTRAP_VERSION_OUTPUT, BootstrapEnvironmentOptions, BOOTSTRAP_VERSIO
77
import * as logging from '../../logging';
88
import { Mode, SdkProvider, ISDK } from '../aws-auth';
99
import { deployStack, DeployStackResult } from '../deploy-stack';
10+
import { NoBootstrapStackEnvironmentResources } from '../environment-resources';
1011
import { DEFAULT_TOOLKIT_STACK_NAME, ToolkitInfo } from '../toolkit-info';
1112

1213
/**
@@ -121,7 +122,7 @@ export class BootstrapStack {
121122
parameters,
122123
usePreviousParameters: options.usePreviousParameters ?? true,
123124
// Obviously we can't need a bootstrap stack to deploy a bootstrap stack
124-
toolkitInfo: ToolkitInfo.bootstraplessDeploymentsOnly(this.sdk),
125+
envResources: new NoBootstrapStackEnvironmentResources(this.resolvedEnvironment, this.sdk),
125126
});
126127
}
127128
}

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

+8-7
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ import * as chalk from 'chalk';
44
import * as fs from 'fs-extra';
55
import * as uuid from 'uuid';
66
import { ISDK, SdkProvider } from './aws-auth';
7+
import { EnvironmentResources } from './environment-resources';
78
import { CfnEvaluationException } from './evaluate-cloudformation-template';
89
import { HotswapMode, ICON } from './hotswap/common';
910
import { tryHotswapDeployment } from './hotswap-deployments';
10-
import { ToolkitInfo } from './toolkit-info';
1111
import {
1212
changeSetHasNoChanges, CloudFormationStack, TemplateParameters, waitForChangeSet,
1313
waitForStackDeploy, waitForStackDelete, ParameterValues, ParameterChanges, ResourcesToImport,
@@ -71,7 +71,7 @@ export interface DeployStackOptions {
7171
/**
7272
* Information about the bootstrap stack found in the target environment
7373
*/
74-
readonly toolkitInfo: ToolkitInfo;
74+
readonly envResources: EnvironmentResources;
7575

7676
/**
7777
* Role to pass to CloudFormation to execute the change set
@@ -262,7 +262,7 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
262262
// an ad-hoc asset manifest, while passing their locations via template
263263
// parameters.
264264
const legacyAssets = new AssetManifestBuilder();
265-
const assetParams = await addMetadataAssetsToManifest(stackArtifact, legacyAssets, options.toolkitInfo, options.reuseAssets);
265+
const assetParams = await addMetadataAssetsToManifest(stackArtifact, legacyAssets, options.envResources, options.reuseAssets);
266266

267267
const finalParameterValues = { ...options.parameters, ...assetParams };
268268

@@ -291,7 +291,7 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
291291
stackArtifact,
292292
options.resolvedEnvironment,
293293
legacyAssets,
294-
options.toolkitInfo,
294+
options.envResources,
295295
options.sdk,
296296
options.overrideTemplate);
297297
await publishAssets(legacyAssets.toManifest(stackArtifact.assembly.directory), options.sdkProvider, stackEnv, {
@@ -565,7 +565,7 @@ async function makeBodyParameter(
565565
stack: cxapi.CloudFormationStackArtifact,
566566
resolvedEnvironment: cxapi.Environment,
567567
assetManifest: AssetManifestBuilder,
568-
toolkitInfo: ToolkitInfo,
568+
resources: EnvironmentResources,
569569
sdk: ISDK,
570570
overrideTemplate?: any,
571571
): Promise<TemplateBodyParameter> {
@@ -582,6 +582,7 @@ async function makeBodyParameter(
582582
return { TemplateBody: templateJson };
583583
}
584584

585+
const toolkitInfo = await resources.lookupToolkit();
585586
if (!toolkitInfo.found) {
586587
error(
587588
`The template for stack "${stack.displayName}" is ${Math.round(templateJson.length / 1024)}KiB. ` +
@@ -623,7 +624,7 @@ async function makeBodyParameter(
623624
export async function makeBodyParameterAndUpload(
624625
stack: cxapi.CloudFormationStackArtifact,
625626
resolvedEnvironment: cxapi.Environment,
626-
toolkitInfo: ToolkitInfo,
627+
resources: EnvironmentResources,
627628
sdkProvider: SdkProvider,
628629
sdk: ISDK,
629630
overrideTemplate?: any): Promise<TemplateBodyParameter> {
@@ -635,7 +636,7 @@ export async function makeBodyParameterAndUpload(
635636
});
636637

637638
const builder = new AssetManifestBuilder();
638-
const bodyparam = await makeBodyParameter(forceUploadStack, resolvedEnvironment, builder, toolkitInfo, sdk, overrideTemplate);
639+
const bodyparam = await makeBodyParameter(forceUploadStack, resolvedEnvironment, builder, resources, sdk, overrideTemplate);
639640
const manifest = builder.toManifest(stack.assembly.directory);
640641
await publishAssets(manifest, sdkProvider, resolvedEnvironment, { quiet: true });
641642
return bodyparam;

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

+41-41
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import { Mode } from './aws-auth/credentials';
55
import { ISDK } from './aws-auth/sdk';
66
import { CredentialsOptions, SdkForEnvironment, SdkProvider } from './aws-auth/sdk-provider';
77
import { deployStack, DeployStackResult, destroyStack, makeBodyParameterAndUpload, DeploymentMethod } from './deploy-stack';
8+
import { EnvironmentResources, EnvironmentResourcesRegistry } from './environment-resources';
89
import { HotswapMode } from './hotswap/common';
910
import { loadCurrentTemplateWithNestedStacks, loadCurrentTemplate, flattenNestedStackNames, TemplateWithNestedStackCount } from './nested-stack-helpers';
10-
import { ToolkitInfo } from './toolkit-info';
1111
import { CloudFormationStack, Template, ResourcesToImport, ResourceIdentifierSummaries } from './util/cloudformation';
1212
import { StackActivityProgress } from './util/cloudformation/stack-activity-monitor';
1313
import { replaceEnvPlaceholders } from './util/placeholders';
@@ -38,6 +38,11 @@ export interface PreparedSdkWithLookupRoleForEnvironment {
3838
* the default credentials (not the assume role credentials)
3939
*/
4040
readonly didAssumeRole: boolean;
41+
42+
/**
43+
* An object for accessing the bootstrap resources in this environment
44+
*/
45+
readonly envResources: EnvironmentResources;
4146
}
4247

4348
export interface DeployStackOptions {
@@ -256,6 +261,7 @@ export interface StackExistsOptions {
256261

257262
export interface DeploymentsProps {
258263
sdkProvider: SdkProvider;
264+
readonly toolkitStackName?: string;
259265
readonly quiet?: boolean;
260266
}
261267

@@ -280,6 +286,11 @@ export interface PreparedSdkForEnvironment {
280286
* @default - no execution role is used
281287
*/
282288
readonly cloudFormationRoleArn?: string;
289+
290+
/**
291+
* Access class for environmental resources to help the deployment
292+
*/
293+
readonly envResources: EnvironmentResources;
283294
}
284295

285296
/**
@@ -289,12 +300,13 @@ export interface PreparedSdkForEnvironment {
289300
*/
290301
export class Deployments {
291302
private readonly sdkProvider: SdkProvider;
292-
private readonly toolkitInfoCache = new Map<string, ToolkitInfo>();
293303
private readonly sdkCache = new Map<string, SdkForEnvironment>();
294304
private readonly publisherCache = new Map<AssetManifest, cdk_assets.AssetPublishing>();
305+
private readonly environmentResources: EnvironmentResourcesRegistry;
295306

296307
constructor(private readonly props: DeploymentsProps) {
297308
this.sdkProvider = props.sdkProvider;
309+
this.environmentResources = new EnvironmentResourcesRegistry(props.toolkitStackName);
298310
}
299311

300312
public async readCurrentTemplateWithNestedStacks(
@@ -317,21 +329,18 @@ export class Deployments {
317329

318330
public async resourceIdentifierSummaries(
319331
stackArtifact: cxapi.CloudFormationStackArtifact,
320-
toolkitStackName?: string,
321332
): Promise<ResourceIdentifierSummaries> {
322333
debug(`Retrieving template summary for stack ${stackArtifact.displayName}.`);
323334
// Currently, needs to use `deploy-role` since it may need to read templates in the staging
324335
// bucket which have been encrypted with a KMS key (and lookup-role may not read encrypted things)
325-
const { stackSdk, resolvedEnvironment } = await this.prepareSdkFor(stackArtifact, undefined, Mode.ForReading);
336+
const { stackSdk, resolvedEnvironment, envResources } = await this.prepareSdkFor(stackArtifact, undefined, Mode.ForReading);
326337
const cfn = stackSdk.cloudFormation();
327338

328-
const toolkitInfo = await this.lookupToolkit(resolvedEnvironment, stackSdk, toolkitStackName);
329-
330339
// Upload the template, if necessary, before passing it to CFN
331340
const cfnParam = await makeBodyParameterAndUpload(
332341
stackArtifact,
333342
resolvedEnvironment,
334-
toolkitInfo,
343+
envResources,
335344
this.sdkProvider,
336345
stackSdk);
337346

@@ -355,16 +364,19 @@ export class Deployments {
355364
};
356365
}
357366

358-
const { stackSdk, resolvedEnvironment, cloudFormationRoleArn } = await this.prepareSdkFor(options.stack, options.roleArn, Mode.ForWriting);
359-
360-
const toolkitInfo = await this.lookupToolkit(resolvedEnvironment, stackSdk, options.toolkitStackName);
367+
const {
368+
stackSdk,
369+
resolvedEnvironment,
370+
cloudFormationRoleArn,
371+
envResources,
372+
} = await this.prepareSdkFor(options.stack, options.roleArn, Mode.ForWriting);
361373

362374
// Do a verification of the bootstrap stack version
363375
await this.validateBootstrapStackVersion(
364376
options.stack.stackName,
365377
options.stack.requiresBootstrapStackVersion,
366378
options.stack.bootstrapStackVersionSsmParameter,
367-
toolkitInfo);
379+
envResources);
368380

369381
return deployStack({
370382
stack: options.stack,
@@ -376,7 +388,7 @@ export class Deployments {
376388
sdkProvider: this.sdkProvider,
377389
roleArn: cloudFormationRoleArn,
378390
reuseAssets: options.reuseAssets,
379-
toolkitInfo,
391+
envResources,
380392
tags: options.tags,
381393
deploymentMethod,
382394
force: options.force,
@@ -420,6 +432,7 @@ export class Deployments {
420432
return {
421433
resolvedEnvironment: result.resolvedEnvironment,
422434
stackSdk: result.sdk,
435+
envResources: result.envResources,
423436
};
424437
}
425438
} catch { }
@@ -464,6 +477,7 @@ export class Deployments {
464477
stackSdk: stackSdk.sdk,
465478
resolvedEnvironment,
466479
cloudFormationRoleArn: arns.cloudFormationRoleArn,
480+
envResources: this.environmentResources.for(resolvedEnvironment, stackSdk.sdk),
467481
};
468482
}
469483

@@ -504,9 +518,11 @@ export class Deployments {
504518
assumeRoleExternalId: stack.lookupRole?.assumeRoleExternalId,
505519
});
506520

521+
const envResources = this.environmentResources.for(resolvedEnvironment, stackSdk.sdk);
522+
507523
// if we succeed in assuming the lookup role, make sure we have the correct bootstrap stack version
508524
if (stackSdk.didAssumeRole && stack.lookupRole?.bootstrapStackVersionSsmParameter && stack.lookupRole.requiresBootstrapStackVersion) {
509-
const version = await ToolkitInfo.versionFromSsmParameter(stackSdk.sdk, stack.lookupRole.bootstrapStackVersionSsmParameter);
525+
const version = await envResources.versionFromSsmParameter(stack.lookupRole.bootstrapStackVersionSsmParameter);
510526
if (version < stack.lookupRole.requiresBootstrapStackVersion) {
511527
throw new Error(`Bootstrap stack version '${stack.lookupRole.requiresBootstrapStackVersion}' is required, found version '${version}'.`);
512528
}
@@ -515,7 +531,7 @@ export class Deployments {
515531
} else if (!stackSdk.didAssumeRole && stack.lookupRole?.requiresBootstrapStackVersion) {
516532
warning(upgradeMessage);
517533
}
518-
return { ...stackSdk, resolvedEnvironment };
534+
return { ...stackSdk, resolvedEnvironment, envResources };
519535
} catch (e: any) {
520536
debug(e);
521537
// only print out the warnings if the lookupRole exists AND there is a required
@@ -528,33 +544,18 @@ export class Deployments {
528544
}
529545
}
530546

531-
/**
532-
* Look up the toolkit for a given environment, using a given SDK
533-
*/
534-
public async lookupToolkit(resolvedEnvironment: cxapi.Environment, sdk: ISDK, toolkitStackName?: string) {
535-
const key = `${resolvedEnvironment.account}:${resolvedEnvironment.region}:${toolkitStackName}`;
536-
const existing = this.toolkitInfoCache.get(key);
537-
if (existing) {
538-
return existing;
539-
}
540-
const ret = await ToolkitInfo.lookup(resolvedEnvironment, sdk, toolkitStackName);
541-
this.toolkitInfoCache.set(key, ret);
542-
return ret;
543-
}
544-
545547
private async prepareAndValidateAssets(asset: cxapi.AssetManifestArtifact, options: AssetOptions) {
546-
const { stackSdk, resolvedEnvironment } = await this.prepareSdkFor(options.stack, options.roleArn, Mode.ForWriting);
547-
const toolkitInfo = await this.lookupToolkit(resolvedEnvironment, stackSdk, options.toolkitStackName);
548-
const stackEnv = await this.sdkProvider.resolveEnvironment(options.stack.environment);
548+
const { envResources } = await this.prepareSdkFor(options.stack, options.roleArn, Mode.ForWriting);
549+
549550
await this.validateBootstrapStackVersion(
550551
options.stack.stackName,
551552
asset.requiresBootstrapStackVersion,
552553
asset.bootstrapStackVersionSsmParameter,
553-
toolkitInfo);
554+
envResources);
554555

555556
const manifest = AssetManifest.fromFile(asset.file);
556557

557-
return { manifest, stackEnv };
558+
return { manifest, stackEnv: envResources.environment };
558559
}
559560

560561
/**
@@ -582,16 +583,15 @@ export class Deployments {
582583
*/
583584
// eslint-disable-next-line max-len
584585
public async buildSingleAsset(assetArtifact: cxapi.AssetManifestArtifact, assetManifest: AssetManifest, asset: IManifestEntry, options: BuildStackAssetsOptions) {
585-
const { stackSdk, resolvedEnvironment: stackEnv } = await this.prepareSdkFor(options.stack, options.roleArn, Mode.ForWriting);
586-
const toolkitInfo = await this.lookupToolkit(stackEnv, stackSdk, options.toolkitStackName);
586+
const { resolvedEnvironment, envResources } = await this.prepareSdkFor(options.stack, options.roleArn, Mode.ForWriting);
587587

588588
await this.validateBootstrapStackVersion(
589589
options.stack.stackName,
590590
assetArtifact.requiresBootstrapStackVersion,
591591
assetArtifact.bootstrapStackVersionSsmParameter,
592-
toolkitInfo);
592+
envResources);
593593

594-
const publisher = this.cachedPublisher(assetManifest, stackEnv, options.stackName);
594+
const publisher = this.cachedPublisher(assetManifest, resolvedEnvironment, options.stackName);
595595
await publisher.buildEntry(asset);
596596
if (publisher.hasFailures) {
597597
throw new Error(`Failed to build asset ${asset.id}`);
@@ -624,17 +624,17 @@ export class Deployments {
624624

625625
/**
626626
* Validate that the bootstrap stack has the right version for this stack
627+
*
628+
* Call into envResources.validateVersion, but prepend the stack name in case of failure.
627629
*/
628630
private async validateBootstrapStackVersion(
629631
stackName: string,
630632
requiresBootstrapStackVersion: number | undefined,
631633
bootstrapStackVersionSsmParameter: string | undefined,
632-
toolkitInfo: ToolkitInfo) {
633-
634-
if (requiresBootstrapStackVersion === undefined) { return; }
634+
envResources: EnvironmentResources) {
635635

636636
try {
637-
await toolkitInfo.validateVersion(requiresBootstrapStackVersion, bootstrapStackVersionSsmParameter);
637+
await envResources.validateVersion(requiresBootstrapStackVersion, bootstrapStackVersionSsmParameter);
638638
} catch (e: any) {
639639
throw new Error(`${stackName}: ${e.message}`);
640640
}

0 commit comments

Comments
 (0)