fix(core): modern deployments fail if bootstrap stack is renamed

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 #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. BREAKING CHANGES: users of modern synthesis (`DefaultSynthesizer`, used by CDK Pipelines) must upgrade their bootstrap stacks. Run `cdk bootstrap`.
aws · Jan 19, 2021 · 3a135a3 · 3a135a3
1 parent f4abd23
commit 3a135a3
Show file tree

Hide file tree

Showing 23 changed files with 640 additions and 180 deletions.
diff --git a/packages/@aws-cdk/cloud-assembly-schema/lib/cloud-assembly/artifact-schema.ts b/packages/@aws-cdk/cloud-assembly-schema/lib/cloud-assembly/artifact-schema.ts
@@ -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;
 }
 
 /**
@@ -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;
 }
 
 /**

diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.schema.json
@@ -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": [
@@ -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": [

diff --git a/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json b/packages/@aws-cdk/cloud-assembly-schema/schema/cloud-assembly.version.json
@@ -1 +1 @@
-{"version":"8.0.0"}
+{"version":"9.0.0"}
diff --git a/packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts b/packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts
@@ -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],
     });
   }
@@ -472,6 +473,7 @@ export class DefaultStackSynthesizer extends StackSynthesizer {
       properties: {
         file: manifestFile,
         requiresBootstrapStackVersion: MIN_BOOTSTRAP_STACK_VERSION,
+        bootstrapStackVersionSsmParameter: `/cdk-bootstrap/${this.qualifier}/version`,
       },
     });
 

diff --git a/packages/@aws-cdk/core/lib/stack-synthesizers/stack-synthesizer.ts b/packages/@aws-cdk/core/lib/stack-synthesizers/stack-synthesizer.ts
@@ -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;
 }
diff --git a/packages/@aws-cdk/core/test/stack-synthesis/new-style-synthesis.test.ts b/packages/@aws-cdk/core/test/stack-synthesis/new-style-synthesis.test.ts
@@ -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)) {
@@ -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',
@@ -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'], {
@@ -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' }));
 }
 

diff --git a/packages/@aws-cdk/cx-api/lib/artifacts/asset-manifest-artifact.ts b/packages/@aws-cdk/cx-api/lib/artifacts/asset-manifest-artifact.ts
@@ -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);
 
@@ -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;
   }
 }
diff --git a/packages/@aws-cdk/cx-api/lib/artifacts/cloudformation-artifact.ts b/packages/@aws-cdk/cx-api/lib/artifacts/cloudformation-artifact.ts
@@ -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.
    */
@@ -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;

diff --git a/packages/aws-cdk/lib/api/aws-auth/sdk.ts b/packages/aws-cdk/lib/api/aws-auth/sdk.ts
@@ -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
               });
             },

diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml b/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml
@@ -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:
@@ -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

diff --git a/packages/aws-cdk/lib/api/bootstrap/deploy-bootstrap.ts b/packages/aws-cdk/lib/api/bootstrap/deploy-bootstrap.ts
@@ -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);
@@ -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> {
@@ -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.`);
     }
 
@@ -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),
     });
   }
 }

diff --git a/packages/aws-cdk/lib/api/cloudformation-deployments.ts b/packages/aws-cdk/lib/api/cloudformation-deployments.ts
@@ -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,
@@ -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, bootstrapStack: 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,
+        bootstrapStack);
 
       const manifest = AssetManifest.fromFile(assetArtifact.file);
       await publishAssets(manifest, this.sdkProvider, stackEnv);
@@ -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,
+    bootstrapStack: ToolkitInfo) {
 
     if (requiresBootstrapStackVersion === undefined) { return; }
 
     if (!bootstrapStack) {
       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 bootstrapStack.validateVersion(requiresBootstrapStackVersion, bootstrapStackVersionSsmParameter);
+    } catch (e) {
+      throw new Error(`${stackName}: ${e.message}`);
     }
   }
 }

diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts
@@ -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
@@ -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) {
@@ -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` +