Repository: cdklabs/cdk-enterprise-iac (language: python)

Issue:
addPermissionsBoundary does not generate unqiue role, policy and instance profile names
### General Issue

addPermissionsBoundary does not generate unqiue role, policy and instance profile names

### The Question

When using the addPermissionsBoundary patch you can end up with duplicates of the same role or policy name. This issues appears in two wasys.

1. If you have two roles that have unique name that when the prefix is is appended you end up with the same role name.
2. If you do not provided a role name but the nesting of your constructs names are large then you can end up with the same role name. 

This is because because the suffix generated comes from the `node.node.path`.

Proposed solution: Generate a unique identifier all roles/polices/instance profiles that are prepended with a prefix

### cdk-enterprise-iac version

latest

### Language

Python

### Other information

_No response_


Discussion / hints:

Fix patch:
diff --git a/src/patches/addPermissionsBoundary.ts b/src/patches/addPermissionsBoundary.ts
index 085da078..c7824480 100644
--- a/src/patches/addPermissionsBoundary.ts
+++ b/src/patches/addPermissionsBoundary.ts
@@ -2,6 +2,7 @@
 Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 SPDX-License-Identifier: Apache-2.0
 */
+import { createHash } from 'crypto';
 import { CfnResource, IAspect, Stack, Token } from 'aws-cdk-lib';
 import {
   CfnInstanceProfile,
@@ -74,11 +75,20 @@ export class AddPermissionBoundary implements IAspect {
       } else {
         return;
       }
-
-      node.addPropertyOverride(
-        cfnProp,
-        `${prefix}${policySuffix.replace(/\s/g, '')}`.substring(0, length - 1)
+      const uniqness_length = 8;
+      let suffix = `${policySuffix.replace(/\s/g, '')}`.substring(
+        0,
+        length - uniqness_length - prefix.length
+      );
+      const hash = createHash('shake256');
+      hash.update(node.node.path);
+      let hash_value = hash.copy().digest('hex');
+      const charUniqness8 = hash_value.substring(
+        hash_value.length - 8,
+        hash_value.length
       );
+      let newSuffix = prefix + suffix + charUniqness8;
+      node.addPropertyOverride(cfnProp, `${newSuffix}`.substring(0, length));
     }
   }
 


Test patch:
diff --git a/test/patches/addPermissionsBoundary.test.ts b/test/patches/addPermissionsBoundary.test.ts
index 76c47b48..7cd307a0 100644
--- a/test/patches/addPermissionsBoundary.test.ts
+++ b/test/patches/addPermissionsBoundary.test.ts
@@ -69,9 +69,47 @@ describe('Permissions Boundary patch', () => {
         })
       );
       const template = Template.fromStack(stack);
-      template.hasResourceProperties('AWS::IAM::Role', {
-        RoleName: `${rolePrefix}${roleName}`.substring(0, 64 - 1),
+      let roles = template.findResources('AWS::IAM::Role');
+      const names: string[] = [];
+      let i = 0;
+      for (const templateRole of Object.keys(roles)) {
+        const role = roles[templateRole].Properties.RoleName as string;
+        names[i] = role;
+        i++;
+      }
+      expect(names.length).toBe(1);
+      expect(names[0].startsWith(rolePrefix)).toBe(true);
+      expect(names[0].length).toBe(64);
+    });
+    test('Role Prefix is prepended and is unique', () => {
+      let firstRoleName =
+        'my-awesome-role-that-has-quite-a-long-name-seriously-it-is-so-long-it-exceeds-64-characters-1';
+      let secondRoleName =
+        'my-awesome-role-that-has-quite-a-long-name-seriously-it-is-so-long-it-exceeds-64-characters-2';
+      const rolePrefix = 'MY_PREFIX';
+      new Role(stack, 'testFirstRole', {
+        assumedBy: new ServicePrincipal('ec2.amazonaws.com'),
+        roleName: firstRoleName,
+      });
+      new Role(stack, 'testSecondRole', {
+        assumedBy: new ServicePrincipal('ec2.amazonaws.com'),
+        roleName: secondRoleName,
       });
+      Aspects.of(stack).add(
+        new AddPermissionBoundary({
+          permissionsBoundaryPolicyName: pbName,
+          rolePrefix,
+        })
+      );
+      const template = Template.fromStack(stack);
+      let roles = template.findResources('AWS::IAM::Role');
+      const names: { [index: string]: boolean } = {};
+      for (const roleName of Object.keys(roles)) {
+        const role = roles[roleName].Properties.RoleName as string;
+        expect(role.startsWith(rolePrefix)).toBe(true);
+        if (names[role]) throw new Error('Duplicate role name found');
+        names[role] = true;
+      }
     });
     test('Roles without RoleName are handled', () => {
       new Bucket(stack, 'TestBucket', {
@@ -116,9 +154,16 @@ describe('Permissions Boundary patch', () => {
         })
       );
       const template = Template.fromStack(stack);
-      template.hasResourceProperties('AWS::IAM::Role', {
-        RoleName: `Bananas_Default-${idWithSpaces.replace(/\s/g, '')}-Resource`,
-      });
+      let roles = template.findResources('AWS::IAM::Role');
+      const names: string[] = [];
+      let i = 0;
+      for (const templateRole of Object.keys(roles)) {
+        const role = roles[templateRole].Properties.RoleName as string;
+        names[i] = role;
+        i++;
+      }
+      expect(names[0].indexOf(' ')).toBe(-1);
+      expect(names[0].startsWith('Bananas_')).toBe(true);
     });
     test('Role name is not altered if it already complies with naming convention', () => {
       const rolePrefix = 'Cust_';
@@ -161,12 +206,18 @@ describe('Permissions Boundary patch', () => {
         })
       );
       const template = Template.fromStack(stack);
-      template.hasResourceProperties('AWS::IAM::ManagedPolicy', {
-        ManagedPolicyName: `${policyPrefix}${managedPolicyName}`.substring(
-          0,
-          128 - 1
-        ),
-      });
+      let polices = template.findResources('AWS::IAM::ManagedPolicy');
+      const names: string[] = [];
+      let i = 0;
+      for (const templatePolicy of Object.keys(polices)) {
+        const tmpPolicy = polices[templatePolicy].Properties
+          .ManagedPolicyName as string;
+        names[i] = tmpPolicy;
+        i++;
+      }
+      expect(names.length).toBe(1);
+      expect(names[0].startsWith(policyPrefix)).toBe(true);
+      expect(names[0].length).toBe(128);
     });
     test('Managed Policy name is not altered if it already complies with naming convention', () => {
       const managedPolicyName = `${policyPrefix}MyPolicy`;
@@ -214,9 +265,18 @@ describe('Permissions Boundary patch', () => {
         })
       );
       const template = Template.fromStack(stack);
-      template.hasResourceProperties('AWS::IAM::Policy', {
-        PolicyName: `${policyPrefix}${policyName}`.substring(0, 128 - 1),
-      });
+      let polices = template.findResources('AWS::IAM::Policy');
+      const names: string[] = [];
+      let i = 0;
+      for (const templatePolicy of Object.keys(polices)) {
+        const tmpPolicy = polices[templatePolicy].Properties
+          .PolicyName as string;
+        names[i] = tmpPolicy;
+        i++;
+      }
+      expect(names.length).toBe(1);
+      expect(names[0].startsWith(policyPrefix)).toBe(true);
+      expect(names[0].length).toBe(128);
     });
 
     test('Instance Profile prefix is added and does not exceed limit', () => {
@@ -237,13 +297,18 @@ describe('Permissions Boundary patch', () => {
         })
       );
       const template = Template.fromStack(stack);
-      template.hasResourceProperties('AWS::IAM::InstanceProfile', {
-        InstanceProfileName:
-          `${instanceProfilePrefix}${instanceProfileName}`.substring(
-            0,
-            128 - 1
-          ),
-      });
+      let profiles = template.findResources('AWS::IAM::InstanceProfile');
+      const names: string[] = [];
+      let i = 0;
+      for (const templateInstanceProfile of Object.keys(profiles)) {
+        const profile = profiles[templateInstanceProfile].Properties
+          .InstanceProfileName as string;
+        names[i] = profile;
+        i++;
+      }
+      expect(names.length).toBe(1);
+      expect(names[0].startsWith(instanceProfilePrefix)).toBe(true);
+      expect(names[0].length).toBe(128);
     });
     test('Instance Profile works fine when no prefix needed', () => {
       const role = new Role(stack, 'TestRole', {
diff --git a/test/patches/convertInlinePoliciesToManaged.test.ts b/test/patches/convertInlinePoliciesToManaged.test.ts
index d4ee47c1..2e6abc73 100644
--- a/test/patches/convertInlinePoliciesToManaged.test.ts
+++ b/test/patches/convertInlinePoliciesToManaged.test.ts
@@ -60,9 +60,22 @@ describe('Updating Resource Types', () => {
     );
     Aspects.of(stack).add(new ConvertInlinePoliciesToManaged());
     const template = Template.fromStack(stack);
-    template.hasResourceProperties('AWS::IAM::ManagedPolicy', {
-      ManagedPolicyName: `${policyPrefix}${policyName}`.substring(0, 128 - 1),
-    });
+    let polices = template.findResources('AWS::IAM::ManagedPolicy');
+    console.log(`polices: ${polices}`);
+    const names: string[] = [];
+    let i = 0;
+    for (const templatePolicy of Object.keys(polices)) {
+      const tmpPolicy = polices[templatePolicy].Properties
+        .ManagedPolicyName as string;
+      names[i] = tmpPolicy;
+      i++;
+    }
+    expect(names.length).toBe(1);
+    expect(names[0].startsWith(policyPrefix)).toBe(true);
+    const uniqness_length = 8;
+    expect(names[0].length).toBe(
+      policyPrefix.length + policyName.length + uniqness_length
+    );
   });
 
   test('ManagedPolicy works with resource extractor', () => {