From 38cd303bcb13749e812bd917e8d84bb4c5a09a1c Mon Sep 17 00:00:00 2001
From: Akash Askoolum <akash1810@gmail.com>
Date: Mon, 12 Apr 2021 20:02:15 +0100
Subject: [PATCH] fix: Update `GuPolicy` with revised logicalId logic

In #364 we placed the logic of overriding a construct's logicalId into a single place.

In this change, we're updating the `GuPolicy` construct to adopt the new logic. As of #418 it's as simple as using the GuStatefulMigratableConstruct mixin!
---
 .../iam/policies/base-policy.test.ts          | 54 +++++++++++++++++++
 src/constructs/iam/policies/base-policy.ts    | 15 ++----
 2 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/src/constructs/iam/policies/base-policy.test.ts b/src/constructs/iam/policies/base-policy.test.ts
index fef7ed2f5..54d511cc9 100644
--- a/src/constructs/iam/policies/base-policy.test.ts
+++ b/src/constructs/iam/policies/base-policy.test.ts
@@ -51,6 +51,33 @@ describe("GuAllowPolicy", () => {
       },
     });
   });
+
+  test("auto-generates the logicalId by default", () => {
+    const stack = simpleGuStackForTesting();
+    attachPolicyToTestRole(
+      stack,
+      new GuAllowPolicy(stack, "AllowS3GetObject", {
+        actions: ["s3:GetObject"],
+        resources: ["*"],
+      })
+    );
+
+    expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::IAM::Policy", /^AllowS3GetObject.+/);
+  });
+
+  test("overrides the logicalId when existingLogicalId is set in a migrating stack", () => {
+    const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true });
+    attachPolicyToTestRole(
+      stack,
+      new GuAllowPolicy(stack, "AllowS3GetObject", {
+        actions: ["s3:GetObject"],
+        resources: ["*"],
+        existingLogicalId: "MyAwesomeAllowPolicy",
+      })
+    );
+
+    expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::IAM::Policy", "MyAwesomeAllowPolicy");
+  });
 });
 
 describe("GuDenyPolicy", () => {
@@ -101,4 +128,31 @@ describe("GuDenyPolicy", () => {
       },
     });
   });
+
+  test("auto-generates the logicalId by default", () => {
+    const stack = simpleGuStackForTesting();
+    attachPolicyToTestRole(
+      stack,
+      new GuDenyPolicy(stack, "DenyS3GetObject", {
+        actions: ["s3:GetObject"],
+        resources: ["*"],
+      })
+    );
+
+    expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::IAM::Policy", /^DenyS3GetObject.+/);
+  });
+
+  test("overrides the logicalId when existingLogicalId is set in a migrating stack", () => {
+    const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true });
+    attachPolicyToTestRole(
+      stack,
+      new GuDenyPolicy(stack, "DenyS3GetObject", {
+        actions: ["s3:GetObject"],
+        resources: ["*"],
+        existingLogicalId: "MyAwesomeDenyPolicy",
+      })
+    );
+
+    expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::IAM::Policy", "MyAwesomeDenyPolicy");
+  });
 });
diff --git a/src/constructs/iam/policies/base-policy.ts b/src/constructs/iam/policies/base-policy.ts
index e345f8956..8189884e8 100644
--- a/src/constructs/iam/policies/base-policy.ts
+++ b/src/constructs/iam/policies/base-policy.ts
@@ -1,21 +1,16 @@
-import type { CfnPolicy, PolicyProps } from "@aws-cdk/aws-iam";
+import type { PolicyProps } from "@aws-cdk/aws-iam";
 import { Effect, Policy, PolicyStatement } from "@aws-cdk/aws-iam";
+import { GuMigratableConstruct } from "../../../utils/mixin";
 import type { GuStack } from "../../core";
+import type { GuMigratingResource } from "../../core/migrating";
 
-export interface GuPolicyProps extends PolicyProps {
-  overrideId?: boolean;
-}
+export interface GuPolicyProps extends PolicyProps, GuMigratingResource {}
 
 export type GuNoStatementsPolicyProps = Omit<GuPolicyProps, "statements">;
 
-export abstract class GuPolicy extends Policy {
+export abstract class GuPolicy extends GuMigratableConstruct(Policy) {
   protected constructor(scope: GuStack, id: string, props: GuPolicyProps) {
     super(scope, id, props);
-
-    if (props.overrideId) {
-      const child = this.node.defaultChild as CfnPolicy;
-      child.overrideLogicalId(id);
-    }
   }
 }