refactor(GuLogShippingPolicy): make GuLogShippingPolicy a singleton (#…

…337)

The policy `GuLogShippingPolicy` is now a [singleton](https://en.wikipedia.org/wiki/Singleton_pattern) as we only ever want one per stack. If there are multiple roles defined in this stack that require log shipping permissions, a single policy will be defined and attached to the roles.

This DRYness helps keep within the file size limits of cloudformation too.

Also included in this change is a new `GuLoggingStreamNameParameter`, also written as a singleton for the same reasons.

Usage hasn't really changed as this policy will never really be used by consumer stacks as they'l be using `GuInstanceRole` instead, which adds this policy by default.

BREAKING CHANGE:
* The API for `GuLogShippingPolicy` has changed. It is now a singleton and direct instantiation is not possible.

src/constructs/core/parameters/log-shipping.ts

@@ -0,0 +1,27 @@
+import type { GuStack } from "../stack";
+import { GuStringParameter } from "./base";
+
+export class GuLoggingStreamNameParameter extends GuStringParameter {
+  private static instance: GuStringParameter | undefined;
+
+  // eslint-disable-next-line custom-rules/valid-constructors -- TODO be better
+  private constructor(scope: GuStack) {
+    super(scope, "LoggingStreamName", {
+      description: "SSM parameter containing the Name (not ARN) on the kinesis stream",
+      default: "/account/services/logging.stream.name",
+      fromSSM: true,
+    });
+  }
+
+  public static getInstance(stack: GuStack): GuLoggingStreamNameParameter {
+    // Resources can only live in the same App so return a new `GuSSMRunCommandPolicy` where necessary.
+    // See https://github.com/aws/aws-cdk/blob/0ea4b19afd639541e5f1d7c1783032ee480c307e/packages/%40aws-cdk/core/lib/private/refs.ts#L47-L50
+    const isSameStack = this.instance?.node.root === stack.node.root;
+
+    if (!this.instance || !isSameStack) {
+      this.instance = new GuLoggingStreamNameParameter(stack);
+    }
+
+    return this.instance;
+  }
+}

src/constructs/iam/policies/__snapshots__/log-shipping.test.ts.snap

@@ -0,0 +1,163 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`The GuLogShippingPolicy singleton class will only be defined once in a stack, even when attached to multiple roles 1`] = `
+Object {
+  "Parameters": Object {
+    "LoggingStreamName": Object {
+      "Default": "/account/services/logging.stream.name",
+      "Description": "SSM parameter containing the Name (not ARN) on the kinesis stream",
+      "Type": "AWS::SSM::Parameter::Value<String>",
+    },
+    "Stage": Object {
+      "AllowedValues": Array [
+        "CODE",
+        "PROD",
+      ],
+      "Default": "CODE",
+      "Description": "Stage name",
+      "Type": "String",
+    },
+  },
+  "Resources": Object {
+    "GuLogShippingPolicy981BFE5A": Object {
+      "Properties": Object {
+        "PolicyDocument": Object {
+          "Statement": Array [
+            Object {
+              "Action": Array [
+                "kinesis:Describe*",
+                "kinesis:Put*",
+              ],
+              "Effect": "Allow",
+              "Resource": Object {
+                "Fn::Join": Array [
+                  "",
+                  Array [
+                    "arn:aws:kinesis:",
+                    Object {
+                      "Ref": "AWS::Region",
+                    },
+                    ":",
+                    Object {
+                      "Ref": "AWS::AccountId",
+                    },
+                    ":stream/",
+                    Object {
+                      "Ref": "LoggingStreamName",
+                    },
+                  ],
+                ],
+              },
+            },
+          ],
+          "Version": "2012-10-17",
+        },
+        "PolicyName": "GuLogShippingPolicy981BFE5A",
+        "Roles": Array [
+          Object {
+            "Ref": "MyFirstRoleAF66ED75",
+          },
+          Object {
+            "Ref": "MySecondRoleB9F57405",
+          },
+        ],
+      },
+      "Type": "AWS::IAM::Policy",
+    },
+    "MyFirstRoleAF66ED75": Object {
+      "Properties": Object {
+        "AssumeRolePolicyDocument": Object {
+          "Statement": Array [
+            Object {
+              "Action": "sts:AssumeRole",
+              "Effect": "Allow",
+              "Principal": Object {
+                "Service": Object {
+                  "Fn::Join": Array [
+                    "",
+                    Array [
+                      "ec2.",
+                      Object {
+                        "Ref": "AWS::URLSuffix",
+                      },
+                    ],
+                  ],
+                },
+              },
+            },
+          ],
+          "Version": "2012-10-17",
+        },
+        "Tags": Array [
+          Object {
+            "Key": "App",
+            "Value": "testing",
+          },
+          Object {
+            "Key": "gu:cdk:version",
+            "Value": "TEST",
+          },
+          Object {
+            "Key": "Stack",
+            "Value": "test-stack",
+          },
+          Object {
+            "Key": "Stage",
+            "Value": Object {
+              "Ref": "Stage",
+            },
+          },
+        ],
+      },
+      "Type": "AWS::IAM::Role",
+    },
+    "MySecondRoleB9F57405": Object {
+      "Properties": Object {
+        "AssumeRolePolicyDocument": Object {
+          "Statement": Array [
+            Object {
+              "Action": "sts:AssumeRole",
+              "Effect": "Allow",
+              "Principal": Object {
+                "Service": Object {
+                  "Fn::Join": Array [
+                    "",
+                    Array [
+                      "ec2.",
+                      Object {
+                        "Ref": "AWS::URLSuffix",
+                      },
+                    ],
+                  ],
+                },
+              },
+            },
+          ],
+          "Version": "2012-10-17",
+        },
+        "Tags": Array [
+          Object {
+            "Key": "App",
+            "Value": "testing",
+          },
+          Object {
+            "Key": "gu:cdk:version",
+            "Value": "TEST",
+          },
+          Object {
+            "Key": "Stack",
+            "Value": "test-stack",
+          },
+          Object {
+            "Key": "Stage",
+            "Value": Object {
+              "Ref": "Stage",
+            },
+          },
+        ],
+      },
+      "Type": "AWS::IAM::Role",
+    },
+  },
+}
+`;

src/constructs/iam/policies/log-shipping.test.ts

@@ -1,15 +1,23 @@
 import "@aws-cdk/assert/jest";
+import { SynthUtils } from "@aws-cdk/assert";
+import type { SynthedStack } from "../../../../test/utils";
 import { attachPolicyToTestRole, simpleGuStackForTesting } from "../../../../test/utils";
 import { GuLogShippingPolicy } from "./log-shipping";
 
-describe("The GuLogShippingPolicy class", () => {
-  it("sets default props", () => {
+describe("The GuLogShippingPolicy singleton class", () => {
+  it("creates a policy restricted to a kinesis stream defined in a parameter", () => {
     const stack = simpleGuStackForTesting();
 
-    const logShippingPolicy = new GuLogShippingPolicy(stack);
-
+    const logShippingPolicy = GuLogShippingPolicy.getInstance(stack);
     attachPolicyToTestRole(stack, logShippingPolicy);
 
+    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
+    expect(json.Parameters.LoggingStreamName).toEqual({
+      Type: "AWS::SSM::Parameter::Value<String>",
+      Default: "/account/services/logging.stream.name",
+      Description: "SSM parameter containing the Name (not ARN) on the kinesis stream",
+    });
+
     expect(stack).toHaveResource("AWS::IAM::Policy", {
       PolicyName: "GuLogShippingPolicy981BFE5A",
       PolicyDocument: {
@@ -43,45 +51,32 @@ describe("The GuLogShippingPolicy class", () => {
     });
   });
 
-  it("merges defaults and passed in props", () => {
+  it("will only be defined once in a stack, even when attached to multiple roles", () => {
     const stack = simpleGuStackForTesting();
 
-    const logShippingPolicy = new GuLogShippingPolicy(stack, "LogShippingPolicy", {
-      policyName: "test",
-    });
+    const logShippingPolicy = GuLogShippingPolicy.getInstance(stack);
+    attachPolicyToTestRole(stack, logShippingPolicy, "MyFirstRole");
+    attachPolicyToTestRole(stack, logShippingPolicy, "MySecondRole");
 
-    attachPolicyToTestRole(stack, logShippingPolicy);
+    expect(stack).toCountResources("AWS::IAM::Policy", 1);
+    expect(stack).toCountResources("AWS::IAM::Role", 2);
+    expect(SynthUtils.toCloudFormation(stack)).toMatchSnapshot();
+  });
 
-    expect(stack).toHaveResource("AWS::IAM::Policy", {
-      PolicyName: "test",
-      PolicyDocument: {
-        Version: "2012-10-17",
-        Statement: [
-          {
-            Action: ["kinesis:Describe*", "kinesis:Put*"],
-            Effect: "Allow",
-            Resource: {
-              "Fn::Join": [
-                "",
-                [
-                  "arn:aws:kinesis:",
-                  {
-                    Ref: "AWS::Region",
-                  },
-                  ":",
-                  {
-                    Ref: "AWS::AccountId",
-                  },
-                  ":stream/",
-                  {
-                    Ref: "LoggingStreamName",
-                  },
-                ],
-              ],
-            },
-          },
-        ],
-      },
-    });
+  it("works across multiple stacks", () => {
+    const stack1 = simpleGuStackForTesting();
+    const stack2 = simpleGuStackForTesting();
+
+    const logShippingPolicy1 = GuLogShippingPolicy.getInstance(stack1);
+    const logShippingPolicy2 = GuLogShippingPolicy.getInstance(stack2);
+
+    attachPolicyToTestRole(stack1, logShippingPolicy1);
+    attachPolicyToTestRole(stack2, logShippingPolicy2);
+
+    expect(stack1).toCountResources("AWS::IAM::Policy", 1);
+    expect(stack1).toCountResources("AWS::IAM::Role", 1);
+
+    expect(stack2).toCountResources("AWS::IAM::Policy", 1);
+    expect(stack2).toCountResources("AWS::IAM::Role", 1);
   });
 });

src/constructs/iam/policies/log-shipping.ts

@@ -1,25 +1,31 @@
-import { Effect, PolicyStatement } from "@aws-cdk/aws-iam";
 import type { GuStack } from "../../core";
-import { GuStringParameter } from "../../core";
-import type { GuPolicyProps } from "./base-policy";
-import { GuPolicy } from "./base-policy";
+import { GuLoggingStreamNameParameter } from "../../core/parameters/log-shipping";
+import { GuAllowPolicy } from "./base-policy";
 
-export class GuLogShippingPolicy extends GuPolicy {
-  constructor(scope: GuStack, id: string = "GuLogShippingPolicy", props?: GuPolicyProps) {
-    super(scope, id, { ...props });
+export class GuLogShippingPolicy extends GuAllowPolicy {
+  private static instance: GuLogShippingPolicy | undefined;
 
-    const loggingStreamNameParam = new GuStringParameter(scope, "LoggingStreamName", {
-      description: "SSM parameter containing the Name (not ARN) on the kinesis stream",
-      default: "/account/services/logging.stream.name",
-      fromSSM: true,
+  // eslint-disable-next-line custom-rules/valid-constructors -- TODO be better
+  private constructor(scope: GuStack) {
+    super(scope, "GuLogShippingPolicy", {
+      actions: ["kinesis:Describe*", "kinesis:Put*"],
+      resources: [
+        `arn:aws:kinesis:${scope.region}:${scope.account}:stream/${
+          GuLoggingStreamNameParameter.getInstance(scope).valueAsString
+        }`,
+      ],
     });
+  }
+
+  public static getInstance(stack: GuStack): GuLogShippingPolicy {
+    // Resources can only live in the same App so return a new `GuSSMRunCommandPolicy` where necessary.
+    // See https://github.com/aws/aws-cdk/blob/0ea4b19afd639541e5f1d7c1783032ee480c307e/packages/%40aws-cdk/core/lib/private/refs.ts#L47-L50
+    const isSameStack = this.instance?.node.root === stack.node.root;
+
+    if (!this.instance || !isSameStack) {
+      this.instance = new GuLogShippingPolicy(stack);
+    }
 
-    this.addStatements(
-      new PolicyStatement({
-        effect: Effect.ALLOW,
-        actions: ["kinesis:Describe*", "kinesis:Put*"],
-        resources: [`arn:aws:kinesis:${scope.region}:${scope.account}:stream/${loggingStreamNameParam.valueAsString}`],
-      })
-    );
+    return this.instance;
   }
 }

src/constructs/iam/roles/instance-role.ts

@@ -32,7 +32,7 @@ export class GuInstanceRole extends GuRole {
       new GuGetDistributablePolicy(scope, props),
       new GuDescribeEC2Policy(scope),
       new GuParameterStoreReadPolicy(scope, props),
-      ...(props.withoutLogShipping ? [] : [new GuLogShippingPolicy(scope)]),
+      ...(props.withoutLogShipping ? [] : [GuLogShippingPolicy.getInstance(scope)]),
       ...(props.additionalPolicies ? props.additionalPolicies : []),
     ];
 

test/utils/attach-policy-to-test-role.ts

@@ -4,10 +4,10 @@ import type { Stack } from "@aws-cdk/core";
 import { Tags } from "@aws-cdk/core";
 
 // IAM Policies need to be attached to a role, group or user to be created in a stack
-export const attachPolicyToTestRole = (stack: Stack, policy: Policy, app: string = "testing"): void => {
-  const role = new Role(stack, "TestRole", {
+export const attachPolicyToTestRole = (stack: Stack, policy: Policy, id: string = "TestRole"): void => {
+  const role = new Role(stack, id, {
     assumedBy: new ServicePrincipal("ec2.amazonaws.com"),
   });
-  Tags.of(role).add("App", app);
+  Tags.of(role).add("App", "testing");
   policy.attachToRole(role);
 };