Skip to content

Commit

Permalink
fix(core): modern deployments fail if bootstrap stack is renamed (aws…
Browse files Browse the repository at this point in the history
…#12594)

One of the goals of the "modern synthesis" was to couple the deployment
only to the naming of the resources in the bootstrap stack, not the
bootstrap stack itself. We *mostly* had that correct, but missed it in
one aspect: because the modern bootstrap stack template is still in
flux, the CLI does a version check against it; however, it currently
performs that version check against the Stack Output of the bootstrap
stack, and in order to do that has to look up the bootstrap stack and
inadvertently became tied to its name again. This was identified by
@redbaron in aws#11952.

In order to be able to do that version check without the CLI needing to
look up the bootstrap stack, the CLI looks up the SSM parameter with the
version number directly.

The following changes enable that:

- Encode the SSM version parameter name into the Cloud Assembly (this
  is the magic that decouples from the bootstrap stack name).
- In order to be able to read the SSM parameter, the Deploy Role needs
  `ssm:GetParameter` permissions to it. Addition of these permissions
  requires a bootstrap stack version bump.
- `ToolkitInfo.lookup()` now always returns an object (even if the
  lookup failed), and that object serves as a cache for the SSM read,
  so that we don't have to re-fetch the parameter for every asset.
- Add an integration test that verifies we can deploy a
  modern-synthesized application without knowing the bootstrap stack
  name.
- Various unit test changes to account for the new API.

There's one little edge case we need to deal with: bootstrap stack
template v5 includes the `ssm:GetParameter` permissions that we need to
check the bootstrap stack version in an out-of-band way...  but if we're
still on bootstrap stack v4 we won't have the permissions yet to read
the version! If we detect that happens (`AccessDeniedException`), we'll
still formulate that into an "upgrade" message that's as accurate as
possible, using the bootstrap stack template's Output version if found,
or a more generic message if not.

This PR supersedes aws#11952. Fixes aws#11420, fixes aws#9053.

BREAKING CHANGE: users of modern synthesis (`DefaultSynthesizer`,
used by CDK Pipelines) must upgrade their bootstrap stacks. Run `cdk bootstrap`.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored and Mohan Rajendran committed Jan 24, 2021
1 parent 37bc501 commit 86c1257
Show file tree
Hide file tree
Showing 26 changed files with 689 additions and 198 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,21 @@ export interface AwsCloudFormationStackProperties {
* @default - No bootstrap stack required
*/
readonly requiresBootstrapStackVersion?: number;

/**
* SSM parameter where the bootstrap stack version number can be found
*
* Only used if `requiresBootstrapStackVersion` is set.
*
* - If this value is not set, the bootstrap stack name must be known at
* deployment time so the stack version can be looked up from the stack
* outputs.
* - If this value is set, the bootstrap stack can have any name because
* we won't need to look it up.
*
* @default - Bootstrap stack version number looked up
*/
readonly bootstrapStackVersionSsmParameter?: string;
}

/**
Expand All @@ -79,6 +94,19 @@ export interface AssetManifestProperties {
* @default - Version 1 (basic modern bootstrap stack)
*/
readonly requiresBootstrapStackVersion?: number;

/**
* SSM parameter where the bootstrap stack version number can be found
*
* - If this value is not set, the bootstrap stack name must be known at
* deployment time so the stack version can be looked up from the stack
* outputs.
* - If this value is set, the bootstrap stack can have any name because
* we won't need to look it up.
*
* @default - Bootstrap stack version number looked up
*/
readonly bootstrapStackVersionSsmParameter?: string;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,10 @@
"requiresBootstrapStackVersion": {
"description": "Version of bootstrap stack required to deploy this stack (Default - No bootstrap stack required)",
"type": "number"
},
"bootstrapStackVersionSsmParameter": {
"description": "SSM parameter where the bootstrap stack version number can be found\n\nOnly used if `requiresBootstrapStackVersion` is set.\n\n- If this value is not set, the bootstrap stack name must be known at\n deployment time so the stack version can be looked up from the stack\n outputs.\n- If this value is set, the bootstrap stack can have any name because\n we won't need to look it up. (Default - Bootstrap stack version number looked up)",
"type": "string"
}
},
"required": [
Expand All @@ -323,6 +327,10 @@
"requiresBootstrapStackVersion": {
"description": "Version of bootstrap stack required to deploy this stack (Default - Version 1 (basic modern bootstrap stack))",
"type": "number"
},
"bootstrapStackVersionSsmParameter": {
"description": "SSM parameter where the bootstrap stack version number can be found\n\n- If this value is not set, the bootstrap stack name must be known at\n deployment time so the stack version can be looked up from the stack\n outputs.\n- If this value is set, the bootstrap stack can have any name because\n we won't need to look it up. (Default - Bootstrap stack version number looked up)",
"type": "string"
}
},
"required": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"8.0.0"}
{"version":"9.0.0"}
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ export class DefaultStackSynthesizer extends StackSynthesizer {
cloudFormationExecutionRoleArn: this._cloudFormationExecutionRoleArn,
stackTemplateAssetObjectUrl: templateManifestUrl,
requiresBootstrapStackVersion: MIN_BOOTSTRAP_STACK_VERSION,
bootstrapStackVersionSsmParameter: `/cdk-bootstrap/${this.qualifier}/version`,
additionalDependencies: [artifactId],
});
}
Expand Down Expand Up @@ -472,6 +473,7 @@ export class DefaultStackSynthesizer extends StackSynthesizer {
properties: {
file: manifestFile,
requiresBootstrapStackVersion: MIN_BOOTSTRAP_STACK_VERSION,
bootstrapStackVersionSsmParameter: `/cdk-bootstrap/${this.qualifier}/version`,
},
});

Expand Down
15 changes: 15 additions & 0 deletions packages/@aws-cdk/core/lib/stack-synthesizers/stack-synthesizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,19 @@ export interface SynthesizeStackArtifactOptions {
* @default - No bootstrap stack required
*/
readonly requiresBootstrapStackVersion?: number;

/**
* SSM parameter where the bootstrap stack version number can be found
*
* Only used if `requiresBootstrapStackVersion` is set.
*
* - If this value is not set, the bootstrap stack name must be known at
* deployment time so the stack version can be looked up from the stack
* outputs.
* - If this value is set, the bootstrap stack can have any name because
* we won't need to look it up.
*
* @default - Bootstrap stack version number looked up
*/
readonly bootstrapStackVersionSsmParameter?: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,15 @@ nodeunitShim({
const asm = app.synth();

// THEN - we have an asset manifest with both assets and the stack template in there
const manifest = readAssetManifest(asm);
const manifestArtifact = getAssetManifest(asm);
const manifest = readAssetManifest(manifestArtifact);

test.equals(Object.keys(manifest.files || {}).length, 2);
test.equals(Object.keys(manifest.dockerImages || {}).length, 1);

// THEN - the asset manifest has an SSM parameter entry
expect(manifestArtifact.bootstrapStackVersionSsmParameter).toEqual('/cdk-bootstrap/hnb659fds/version');

// THEN - every artifact has an assumeRoleArn
for (const file of Object.values(manifest.files ?? {})) {
for (const destination of Object.values(file.destinations)) {
Expand Down Expand Up @@ -200,7 +204,7 @@ nodeunitShim({

// THEN
const asm = myapp.synth();
const manifest = readAssetManifest(asm);
const manifest = readAssetManifest(getAssetManifest(asm));

test.deepEqual(manifest.files?.['file-asset-hash']?.destinations?.['current_account-current_region'], {
bucketName: 'file-asset-bucket',
Expand Down Expand Up @@ -247,7 +251,7 @@ nodeunitShim({
const stackArtifact = asm.getStackArtifact('mystack-bucketPrefix');

// THEN - we have an asset manifest with both assets and the stack template in there
const manifest = readAssetManifest(asm);
const manifest = readAssetManifest(getAssetManifest(asm));

// THEN
test.deepEqual(manifest.files?.['file-asset-hash-with-prefix']?.destinations?.['current_account-current_region'], {
Expand Down Expand Up @@ -299,10 +303,13 @@ function isAssetManifest(x: cxapi.CloudArtifact): x is cxapi.AssetManifestArtifa
return x instanceof cxapi.AssetManifestArtifact;
}

function readAssetManifest(asm: cxapi.CloudAssembly): cxschema.AssetManifest {
function getAssetManifest(asm: cxapi.CloudAssembly): cxapi.AssetManifestArtifact {
const manifestArtifact = asm.artifacts.filter(isAssetManifest)[0];
if (!manifestArtifact) { throw new Error('no asset manifest in assembly'); }
return manifestArtifact;
}

function readAssetManifest(manifestArtifact: cxapi.AssetManifestArtifact): cxschema.AssetManifest {
return JSON.parse(fs.readFileSync(manifestArtifact.file, { encoding: 'utf-8' }));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ export class AssetManifestArtifact extends CloudArtifact {
*/
public readonly requiresBootstrapStackVersion: number;

/**
* Name of SSM parameter with bootstrap stack version
*
* @default - Discover SSM parameter by reading stack
*/
public readonly bootstrapStackVersionSsmParameter?: string;

constructor(assembly: CloudAssembly, name: string, artifact: cxschema.ArtifactManifest) {
super(assembly, name, artifact);

Expand All @@ -26,5 +33,6 @@ export class AssetManifestArtifact extends CloudArtifact {
}
this.file = path.resolve(this.assembly.directory, properties.file);
this.requiresBootstrapStackVersion = properties.requiresBootstrapStackVersion ?? 1;
this.bootstrapStackVersionSsmParameter = properties.bootstrapStackVersionSsmParameter;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ export class CloudFormationStackArtifact extends CloudArtifact {
*/
public readonly requiresBootstrapStackVersion?: number;

/**
* Name of SSM parameter with bootstrap stack version
*
* @default - Discover SSM parameter by reading stack
*/
public readonly bootstrapStackVersionSsmParameter?: string;

/**
* Whether termination protection is enabled for this stack.
*/
Expand Down Expand Up @@ -110,6 +117,7 @@ export class CloudFormationStackArtifact extends CloudArtifact {
this.cloudFormationExecutionRoleArn = properties.cloudFormationExecutionRoleArn;
this.stackTemplateAssetObjectUrl = properties.stackTemplateAssetObjectUrl;
this.requiresBootstrapStackVersion = properties.requiresBootstrapStackVersion;
this.bootstrapStackVersionSsmParameter = properties.bootstrapStackVersionSsmParameter;
this.terminationProtection = properties.terminationProtection;

this.stackName = properties.stackName || artifactId;
Expand Down
6 changes: 4 additions & 2 deletions packages/aws-cdk/bin/cdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,10 @@ async function initCommandLine() {
debug('Command line arguments:', argv);

const configuration = new Configuration({
...argv,
_: argv._ as [Command, ...string[]], // TypeScript at its best
commandLineArguments: {
...argv,
_: argv._ as [Command, ...string[]], // TypeScript at its best
},
});
await configuration.load();

Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk/lib/api/aws-auth/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,9 @@ export class SDK implements ISDK {
// do additional things to errors.
return Object.assign(Object.create(response), {
promise() {
return response.promise().catch((e: Error) => {
return response.promise().catch((e: Error & { code?: string }) => {
e = self.makeDetailedException(e);
debug(`Call failed: ${prop}(${JSON.stringify(args[0])}) => ${e.message}`);
debug(`Call failed: ${prop}(${JSON.stringify(args[0])}) => ${e.message} (code=${e.code})`);
return Promise.reject(e); // Re-'throw' the new error
});
},
Expand Down
8 changes: 7 additions & 1 deletion packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,12 @@ Resources:
- sts:GetCallerIdentity
Resource: "*"
Effect: Allow
- Sid: ReadVersion
Effect: Allow
Action:
- ssm:GetParameter
Resource:
- Fn::Sub: "arn:aws:ssm:${AWS::Region}:${AWS::AccountId}:parameter${CdkBootstrapVersion}"
Version: '2012-10-17'
PolicyName: default
RoleName:
Expand Down Expand Up @@ -387,7 +393,7 @@ Resources:
Type: String
Name:
Fn::Sub: '/cdk-bootstrap/${Qualifier}/version'
Value: '4'
Value: '5'
Outputs:
BucketName:
Description: The name of the S3 bucket owned by the CDK toolkit stack
Expand Down
16 changes: 7 additions & 9 deletions packages/aws-cdk/lib/api/bootstrap/deploy-bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,14 @@ import { BOOTSTRAP_VERSION_OUTPUT, BootstrapEnvironmentOptions, BOOTSTRAP_VERSIO
*
* And do something in between the two phases (such as look at the
* current bootstrap stack and doing something intelligent).
*
* This class is different from `ToolkitInfo` in that `ToolkitInfo`
* is purely read-only, and `ToolkitInfo.lookup()` returns `undefined`
* if the stack does not exist. But honestly, these classes could and
* should probably be merged at some point.
*/
export class BootstrapStack {
public static async lookup(sdkProvider: SdkProvider, environment: cxapi.Environment, toolkitStackName?: string) {
toolkitStackName = toolkitStackName ?? DEFAULT_TOOLKIT_STACK_NAME;

const resolvedEnvironment = await sdkProvider.resolveEnvironment(environment);
const sdk = await sdkProvider.forEnvironment(resolvedEnvironment, Mode.ForWriting);

const currentToolkitInfo = await ToolkitInfo.lookup(resolvedEnvironment, sdk, toolkitStackName);

return new BootstrapStack(sdkProvider, sdk, resolvedEnvironment, toolkitStackName, currentToolkitInfo);
Expand All @@ -43,15 +39,15 @@ export class BootstrapStack {
private readonly sdk: ISDK,
private readonly resolvedEnvironment: cxapi.Environment,
private readonly toolkitStackName: string,
private readonly currentToolkitInfo?: ToolkitInfo) {
private readonly currentToolkitInfo: ToolkitInfo) {
}

public get parameters(): Record<string, string> {
return this.currentToolkitInfo?.parameters ?? {};
return this.currentToolkitInfo.found ? this.currentToolkitInfo.bootstrapStack.parameters : {};
}

public get terminationProtection() {
return this.currentToolkitInfo?.stack?.terminationProtection;
return this.currentToolkitInfo.found ? this.currentToolkitInfo.bootstrapStack.terminationProtection : undefined;
}

public async partition(): Promise<string> {
Expand All @@ -68,7 +64,7 @@ export class BootstrapStack {
): Promise<DeployStackResult> {

const newVersion = bootstrapVersionFromTemplate(template);
if (this.currentToolkitInfo && newVersion < this.currentToolkitInfo.version && !options.force) {
if (this.currentToolkitInfo.found && newVersion < this.currentToolkitInfo.version && !options.force) {
throw new Error(`Not downgrading existing bootstrap stack from version '${this.currentToolkitInfo.version}' to version '${newVersion}'. Use --force to force.`);
}

Expand Down Expand Up @@ -99,6 +95,8 @@ export class BootstrapStack {
execute: options.execute,
parameters,
usePreviousParameters: true,
// Obviously we can't need a bootstrap stack to deploy a bootstrap stack
toolkitInfo: ToolkitInfo.bootstraplessDeploymentsOnly(this.sdk),
});
}
}
Expand Down
27 changes: 19 additions & 8 deletions packages/aws-cdk/lib/api/cloudformation-deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,11 @@ export class CloudFormationDeployments {
await this.publishStackAssets(options.stack, toolkitInfo);

// Do a verification of the bootstrap stack version
this.validateBootstrapStackVersion(options.stack.stackName, options.stack.requiresBootstrapStackVersion, toolkitInfo);
await this.validateBootstrapStackVersion(
options.stack.stackName,
options.stack.requiresBootstrapStackVersion,
options.stack.bootstrapStackVersionSsmParameter,
toolkitInfo);

return deployStack({
stack: options.stack,
Expand Down Expand Up @@ -251,12 +255,16 @@ export class CloudFormationDeployments {
/**
* Publish all asset manifests that are referenced by the given stack
*/
private async publishStackAssets(stack: cxapi.CloudFormationStackArtifact, bootstrapStack: ToolkitInfo | undefined) {
private async publishStackAssets(stack: cxapi.CloudFormationStackArtifact, toolkitInfo: ToolkitInfo) {
const stackEnv = await this.sdkProvider.resolveEnvironment(stack.environment);
const assetArtifacts = stack.dependencies.filter(isAssetManifestArtifact);

for (const assetArtifact of assetArtifacts) {
this.validateBootstrapStackVersion(stack.stackName, assetArtifact.requiresBootstrapStackVersion, bootstrapStack);
await this.validateBootstrapStackVersion(
stack.stackName,
assetArtifact.requiresBootstrapStackVersion,
assetArtifact.bootstrapStackVersionSsmParameter,
toolkitInfo);

const manifest = AssetManifest.fromFile(assetArtifact.file);
await publishAssets(manifest, this.sdkProvider, stackEnv);
Expand All @@ -266,19 +274,22 @@ export class CloudFormationDeployments {
/**
* Validate that the bootstrap stack has the right version for this stack
*/
private validateBootstrapStackVersion(
private async validateBootstrapStackVersion(
stackName: string,
requiresBootstrapStackVersion: number | undefined,
bootstrapStack: ToolkitInfo | undefined) {
bootstrapStackVersionSsmParameter: string | undefined,
toolkitInfo: ToolkitInfo) {

if (requiresBootstrapStackVersion === undefined) { return; }

if (!bootstrapStack) {
if (!toolkitInfo.found) {
throw new Error(`${stackName}: publishing assets requires bootstrap stack version '${requiresBootstrapStackVersion}', no bootstrap stack found. Please run 'cdk bootstrap'.`);
}

if (requiresBootstrapStackVersion > bootstrapStack.version) {
throw new Error(`${stackName}: publishing assets requires bootstrap stack version '${requiresBootstrapStackVersion}', found '${bootstrapStack.version}'. Please run 'cdk bootstrap' with a newer CLI version.`);
try {
await toolkitInfo.validateVersion(requiresBootstrapStackVersion, bootstrapStackVersionSsmParameter);
} catch (e) {
throw new Error(`${stackName}: ${e.message}`);
}
}
}
Expand Down
8 changes: 3 additions & 5 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,8 @@ export interface DeployStackOptions {

/**
* Information about the bootstrap stack found in the target environment
*
* @default - Assume there is no bootstrap stack
*/
toolkitInfo?: ToolkitInfo;
toolkitInfo: ToolkitInfo;

/**
* Role to pass to CloudFormation to execute the change set
Expand Down Expand Up @@ -313,7 +311,7 @@ async function makeBodyParameter(
stack: cxapi.CloudFormationStackArtifact,
resolvedEnvironment: cxapi.Environment,
assetManifest: AssetManifestBuilder,
toolkitInfo?: ToolkitInfo): Promise<TemplateBodyParameter> {
toolkitInfo: ToolkitInfo): Promise<TemplateBodyParameter> {

// If the template has already been uploaded to S3, just use it from there.
if (stack.stackTemplateAssetObjectUrl) {
Expand All @@ -327,7 +325,7 @@ async function makeBodyParameter(
return { TemplateBody: templateJson };
}

if (!toolkitInfo) {
if (!toolkitInfo.found) {
error(
`The template for stack "${stack.displayName}" is ${Math.round(templateJson.length / 1024)}KiB. ` +
`Templates larger than ${LARGE_TEMPLATE_SIZE_KB}KiB must be uploaded to S3.\n` +
Expand Down
Loading

0 comments on commit 86c1257

Please sign in to comment.