From 5cc2b0ba03d1f57f6d33dfb1ad838107874a074d Mon Sep 17 00:00:00 2001 From: d0819 <54255835+d0819@users.noreply.github.com> Date: Fri, 9 Jun 2023 02:02:30 +0900 Subject: [PATCH] fix(iam): Modify addManagedPolicy to compare ARN instead of instance reference (#25529) ## Issue When creating a role, the following warning message appeared: ``` Policy large: 11 exceeds 10 managed policies attached to a Role, this requires a quota increase ``` This was caused by the same managed policy being added multiple times. Although there was only one managed policy in the created template, it appears that the managedPolicies field of the Role class has multiple instances of the same managed policy added to it. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/aws-cdk-lib/aws-iam/lib/role.ts | 2 +- .../aws-cdk-lib/aws-iam/test/role.test.ts | 33 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk-lib/aws-iam/lib/role.ts b/packages/aws-cdk-lib/aws-iam/lib/role.ts index 0e7d451341eda..b2e38505cc967 100644 --- a/packages/aws-cdk-lib/aws-iam/lib/role.ts +++ b/packages/aws-cdk-lib/aws-iam/lib/role.ts @@ -549,7 +549,7 @@ export class Role extends Resource implements IRole { if (this._precreatedRole) { return this._precreatedRole.addManagedPolicy(policy); } else { - if (this.managedPolicies.find(mp => mp === policy)) { return; } + if (this.managedPolicies.some(mp => mp.managedPolicyArn === policy.managedPolicyArn)) { return; } this.managedPolicies.push(policy); } } diff --git a/packages/aws-cdk-lib/aws-iam/test/role.test.ts b/packages/aws-cdk-lib/aws-iam/test/role.test.ts index 82791ec8e0af3..5d239254d5bbc 100644 --- a/packages/aws-cdk-lib/aws-iam/test/role.test.ts +++ b/packages/aws-cdk-lib/aws-iam/test/role.test.ts @@ -1166,7 +1166,18 @@ test('managed policy ARNs are deduplicated', () => { ManagedPolicy.fromAwsManagedPolicyName('SuperDeveloper'), ], }); - role.addManagedPolicy(ManagedPolicy.fromAwsManagedPolicyName('SuperDeveloper')); + role.addToPrincipalPolicy( + new PolicyStatement({ + actions: ['s3:*'], + resources: ['*'], + }), + ); + + for (let i = 0; i < 20; i++) { + role.addManagedPolicy(ManagedPolicy.fromAwsManagedPolicyName('SuperDeveloper')); + } + + Annotations.fromStack(stack).hasNoWarning('/my-stack/MyRole', Match.stringLikeRegexp('.*')); Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', { ManagedPolicyArns: [ @@ -1184,6 +1195,26 @@ test('managed policy ARNs are deduplicated', () => { }); }); +test('too many managed policies warning', () => { + const app = new App(); + const stack = new Stack(app, 'my-stack'); + const role = new Role(stack, 'MyRole', { + assumedBy: new ServicePrincipal('sns.amazonaws.com'), + }); + role.addToPrincipalPolicy( + new PolicyStatement({ + actions: ['s3:*'], + resources: ['*'], + }), + ); + + for (let i = 0; i < 20; i++) { + role.addManagedPolicy(ManagedPolicy.fromAwsManagedPolicyName(`SuperDeveloper${i}`)); + } + + Annotations.fromStack(stack).hasWarning('/my-stack/MyRole', Match.stringLikeRegexp('.*')); +}); + describe('role with too large inline policy', () => { const N = 100;