feat(stack): enforce stack value at declaration

Since stack is not a sensitive object and would be stable between stages, we can state it explicitly in the CDK declaration BREAKING CHANGE: This would require existing stacks to declare the stack name in their stacks (we need better terms), but I think the removal of a CFN parameter should be safe
guardian · Feb 12, 2021 · cf0fde3 · cf0fde3
1 parent 6ebe629
commit cf0fde3
Show file tree

Hide file tree

Showing 12 changed files with 47 additions and 161 deletions.
diff --git a/src/constructs/cloudwatch/__snapshots__/lambda-alarms.test.ts.snap b/src/constructs/cloudwatch/__snapshots__/lambda-alarms.test.ts.snap
@@ -3,11 +3,6 @@
 exports[`The GuLambdaErrorPercentageAlarm pattern should create the correct alarm resource with minimal config 1`] = `
 Object {
   "Parameters": Object {
-    "Stack": Object {
-      "Default": "deploy",
-      "Description": "Name of this stack",
-      "Type": "String",
-    },
     "Stage": Object {
       "AllowedValues": Array [
         "CODE",
@@ -49,9 +44,7 @@ Object {
           },
           Object {
             "Key": "Stack",
-            "Value": Object {
-              "Ref": "Stack",
-            },
+            "Value": "test-stack",
           },
           Object {
             "Key": "Stage",
@@ -103,9 +96,7 @@ Object {
           },
           Object {
             "Key": "Stack",
-            "Value": Object {
-              "Ref": "Stack",
-            },
+            "Value": "test-stack",
           },
           Object {
             "Key": "Stage",

diff --git a/src/constructs/core/stack.test.ts b/src/constructs/core/stack.test.ts
@@ -17,30 +17,21 @@ describe("The GuStack construct", () => {
     const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
 
     expect(json.Parameters).toEqual({
-      Stack: {
+      Stage: {
         Type: "String",
-        Description: "Name of this stack",
-        Default: "deploy",
+        Description: "Stage name",
+        AllowedValues: Stages,
+        Default: Stage.CODE,
       },
     });
   });
 
-  it("can accept only one of stack or stage", function () {
-    const stack = simpleGuStackForTesting({ stage: "some-stage" });
-    expect(stack.stage).toEqual("some-stage");
-  });
-
-  it("should have stack and stage parameters", () => {
-    const stack = simpleGuStackForTesting();
+  it("should have a stage parameter", () => {
+    const stack = simpleGuStackForTesting({ stack: "test" });
 
     const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
 
     expect(json.Parameters).toEqual({
-      Stack: {
-        Type: "String",
-        Description: "Name of this stack",
-        Default: "deploy",
-      },
       Stage: {
         Type: "String",
         Description: "Stage name",
@@ -51,7 +42,7 @@ describe("The GuStack construct", () => {
   });
 
   it("should apply the stack, stage and app tags to resources added to it", () => {
-    const stack = new GuStack(new App(), "Test", { app: "MyApp" });
+    const stack = new GuStack(new App(), "Test", { app: "MyApp", stack: "test" });
 
     new Role(stack, "MyRole", {
       assumedBy: new ServicePrincipal("ec2.amazonaws.com"),
@@ -65,9 +56,7 @@ describe("The GuStack construct", () => {
         },
         {
           Key: "Stack",
-          Value: {
-            Ref: "Stack",
-          },
+          Value: "test",
         },
         {
           Key: "Stage",
@@ -81,7 +70,7 @@ describe("The GuStack construct", () => {
   });
 
   it("should return the correct app value when app is set", () => {
-    const stack = new GuStack(new App(), "Test", { app: "MyApp" });
+    const stack = new GuStack(new App(), "Test", { app: "MyApp", stack: "test" });
 
     expect(stack.app).toBe("MyApp");
   });

diff --git a/src/constructs/core/stack.ts b/src/constructs/core/stack.ts
@@ -1,14 +1,13 @@
 import type { App, StackProps } from "@aws-cdk/core";
 import { Stack, Tags } from "@aws-cdk/core";
 import { TrackingTag } from "../../constants/library-info";
-import { GuStackParameter, GuStageParameter } from "./parameters";
+import { GuStageParameter } from "./parameters";
 
 export interface GuStackProps extends StackProps {
   // This limits GuStack to supporting a single app.
   // In the future, support for stacks with multiple apps may be required
   app: string;
-  stack?: string;
-  stage?: string;
+  stack: string;
   migratedFromCloudFormation?: boolean;
 }
 
@@ -40,24 +39,18 @@ export interface GuStackProps extends StackProps {
  * ```
  */
 export class GuStack extends Stack {
-  private readonly _stage: string | GuStageParameter;
-  private readonly _stack: string | GuStackParameter;
+  private readonly _stage: GuStageParameter;
+  private readonly _stack: string;
   private readonly _app: string;
 
   public readonly migratedFromCloudFormation: boolean;
 
   get stage(): string {
-    if (typeof this._stage === "string") {
-      return this._stage;
-    }
     return this._stage.valueAsString;
   }
 
   get stack(): string {
-    if (typeof this._stack === "string") {
-      return this._stack;
-    }
-    return this._stack.valueAsString;
+    return this._stack;
   }
 
   get app(): string {
@@ -86,8 +79,8 @@ export class GuStack extends Stack {
 
     this.migratedFromCloudFormation = !!props.migratedFromCloudFormation;
 
-    this._stage = props.stage ?? new GuStageParameter(this);
-    this._stack = props.stack ?? new GuStackParameter(this);
+    this._stage = new GuStageParameter(this);
+    this._stack = props.stack;
     this._app = props.app;
 
     this.addTag(TrackingTag.Key, TrackingTag.Value);

diff --git a/src/constructs/iam/policies/__snapshots__/ses.test.ts.snap b/src/constructs/iam/policies/__snapshots__/ses.test.ts.snap
@@ -8,11 +8,6 @@ Object {
       "ConstraintDescription": "Must be an @theguardian.com email address",
       "Type": "String",
     },
-    "Stack": Object {
-      "Default": "deploy",
-      "Description": "Name of this stack",
-      "Type": "String",
-    },
     "Stage": Object {
       "AllowedValues": Array [
         "CODE",
@@ -98,9 +93,7 @@ Object {
           },
           Object {
             "Key": "Stack",
-            "Value": Object {
-              "Ref": "Stack",
-            },
+            "Value": "test-stack",
           },
           Object {
             "Key": "Stage",

diff --git a/src/constructs/iam/policies/parameter-store-read.test.ts b/src/constructs/iam/policies/parameter-store-read.test.ts
@@ -6,7 +6,7 @@ import { GuParameterStoreReadPolicy } from "./parameter-store-read";
 
 describe("ParameterStoreReadPolicy", () => {
   it("should constrain the policy to the patch of a stack's identity", () => {
-    const stack = new GuStack(new App(), "my-app", { app: "MyApp" });
+    const stack = new GuStack(new App(), "my-app", { app: "MyApp", stack: "test-stack" });
 
     const policy = new GuParameterStoreReadPolicy(stack);
 
@@ -36,11 +36,7 @@ describe("ParameterStoreReadPolicy", () => {
                   {
                     Ref: "Stage",
                   },
-                  "/",
-                  {
-                    Ref: "Stack",
-                  },
-                  "/MyApp",
+                  "/test-stack/MyApp",
                 ],
               ],
             },

diff --git a/src/constructs/iam/policies/s3-get-object.test.ts b/src/constructs/iam/policies/s3-get-object.test.ts
@@ -57,7 +57,7 @@ describe("The GuGetDistributablePolicy construct", () => {
     const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
 
     const parameterKeys = Object.keys(json.Parameters);
-    const expectedKeys = ["Stage", "Stack", "DistributionBucketName"];
+    const expectedKeys = ["Stage", "DistributionBucketName"];
     expect(parameterKeys).toEqual(expectedKeys);
 
     expect(json.Parameters.DistributionBucketName).toEqual({

diff --git a/src/constructs/iam/roles/__snapshots__/instance-role.test.ts.snap b/src/constructs/iam/roles/__snapshots__/instance-role.test.ts.snap
@@ -8,11 +8,6 @@ Object {
       "Description": "SSM parameter containing the S3 bucket name holding distribution artifacts",
       "Type": "AWS::SSM::Parameter::Value<String>",
     },
-    "Stack": Object {
-      "Default": "deploy",
-      "Description": "Name of this stack",
-      "Type": "String",
-    },
     "Stage": Object {
       "AllowedValues": Array [
         "CODE",
@@ -139,9 +134,7 @@ Object {
           },
           Object {
             "Key": "Stack",
-            "Value": Object {
-              "Ref": "Stack",
-            },
+            "Value": "test-stack",
           },
           Object {
             "Key": "Stage",
@@ -176,11 +169,7 @@ Object {
                     Object {
                       "Ref": "Stage",
                     },
-                    "/",
-                    Object {
-                      "Ref": "Stack",
-                    },
-                    "/testing",
+                    "/test-stack/testing",
                   ],
                 ],
               },
@@ -250,11 +239,6 @@ Object {
       "Description": "SSM parameter containing the Name (not ARN) on the kinesis stream",
       "Type": "AWS::SSM::Parameter::Value<String>",
     },
-    "Stack": Object {
-      "Default": "deploy",
-      "Description": "Name of this stack",
-      "Type": "String",
-    },
     "Stage": Object {
       "AllowedValues": Array [
         "CODE",
@@ -402,9 +386,7 @@ Object {
           },
           Object {
             "Key": "Stack",
-            "Value": Object {
-              "Ref": "Stack",
-            },
+            "Value": "test-stack",
           },
           Object {
             "Key": "Stage",
@@ -439,11 +421,7 @@ Object {
                     Object {
                       "Ref": "Stage",
                     },
-                    "/",
-                    Object {
-                      "Ref": "Stack",
-                    },
-                    "/testing",
+                    "/test-stack/testing",
                   ],
                 ],
               },
@@ -508,11 +486,6 @@ Object {
       "Description": "SSM parameter containing the S3 bucket name holding distribution artifacts",
       "Type": "AWS::SSM::Parameter::Value<String>",
     },
-    "Stack": Object {
-      "Default": "deploy",
-      "Description": "Name of this stack",
-      "Type": "String",
-    },
     "Stage": Object {
       "AllowedValues": Array [
         "CODE",
@@ -618,9 +591,7 @@ Object {
           },
           Object {
             "Key": "Stack",
-            "Value": Object {
-              "Ref": "Stack",
-            },
+            "Value": "test-stack",
           },
           Object {
             "Key": "Stage",
@@ -655,11 +626,7 @@ Object {
                     Object {
                       "Ref": "Stage",
                     },
-                    "/",
-                    Object {
-                      "Ref": "Stack",
-                    },
-                    "/testing",
+                    "/test-stack/testing",
                   ],
                 ],
               },