Skip to content

Commit

Permalink
fix(iam): Modify addManagedPolicy to compare ARN instead of instance …
Browse files Browse the repository at this point in the history
…reference (aws#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*
  • Loading branch information
d0819 authored Jun 8, 2023
1 parent 6d32b65 commit 5cc2b0b
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-iam/lib/role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
33 changes: 32 additions & 1 deletion packages/aws-cdk-lib/aws-iam/test/role.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand All @@ -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;

Expand Down

0 comments on commit 5cc2b0b

Please sign in to comment.