Skip to content

Commit

Permalink
fix(iam): attaching a policy is not idempotent with imported resources (
Browse files Browse the repository at this point in the history
aws#28129)

Fixes `attachToUser`, `attachToRole`, and `attachToGroup` for `Policy` and `ManagedPolicy` to use ARNs as a discriminant for resource equality to prevent duplicates on imported resources.

Closes aws#28101.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
lpizzinidev authored and chenjane-dev committed Dec 5, 2023
1 parent 995beb1 commit 9aa0377
Show file tree
Hide file tree
Showing 13 changed files with 258 additions and 48 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
"OneManagedPolicy77F9185F": {
"Type": "AWS::IAM::ManagedPolicy",
"Properties": {
"Description": "My Policy",
"ManagedPolicyName": "Default",
"Path": "/some/path/",
"PolicyDocument": {
"Statement": [
{
Expand All @@ -45,9 +48,11 @@
],
"Version": "2012-10-17"
},
"Description": "My Policy",
"ManagedPolicyName": "Default",
"Path": "/some/path/",
"Roles": [
{
"Ref": "Role1ABCC5F0"
}
],
"Users": [
{
"Ref": "MyUserDC45028B"
Expand All @@ -58,6 +63,8 @@
"TwoManagedPolicy7E701864": {
"Type": "AWS::IAM::ManagedPolicy",
"Properties": {
"Description": "",
"Path": "/",
"PolicyDocument": {
"Statement": [
{
Expand All @@ -77,9 +84,7 @@
}
],
"Version": "2012-10-17"
},
"Description": "",
"Path": "/"
}
}
},
"Role1ABCC5F0": {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ const role = new Role(stack, 'Role', { assumedBy: new AccountRootPrincipal() });
role.grantAssumeRole(policy.grantPrincipal);
Grant.addToPrincipal({ actions: ['iam:*'], resourceArns: [role.roleArn], grantee: policy2 });

policy.attachToRole(role);

// Idempotent with imported roles, see https://github.com/aws/aws-cdk/issues/28101
const importedRole = Role.fromRoleArn(stack, 'ImportedRole', role.roleArn);
policy.attachToRole(importedRole);

new IntegTest(app, 'ManagedPolicyInteg', {
testCases: [stack],
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ const policy = new Policy(stack, 'HelloPolicy', { policyName: 'Default' });
policy.addStatements(new PolicyStatement({ actions: ['ec2:*'], resources: ['*'] }));
policy.attachToRole(role);

// Idempotent with imported roles, see https://github.com/aws/aws-cdk/issues/28101
const importedRole = Role.fromRoleArn(stack, 'TestImportedRole', role.roleArn);
policy.attachToRole(importedRole);

// Role with an external ID
new Role(stack, 'TestRole2', {
assumedBy: new AccountRootPrincipal(),
Expand Down
6 changes: 3 additions & 3 deletions packages/aws-cdk-lib/aws-iam/lib/managed-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,23 +282,23 @@ export class ManagedPolicy extends Resource implements IManagedPolicy, IGrantabl
* Attaches this policy to a user.
*/
public attachToUser(user: IUser) {
if (this.users.find(u => u === user)) { return; }
if (this.users.find(u => u.userArn === user.userArn)) { return; }
this.users.push(user);
}

/**
* Attaches this policy to a role.
*/
public attachToRole(role: IRole) {
if (this.roles.find(r => r === role)) { return; }
if (this.roles.find(r => r.roleArn === role.roleArn)) { return; }
this.roles.push(role);
}

/**
* Attaches this policy to a group.
*/
public attachToGroup(group: IGroup) {
if (this.groups.find(g => g === group)) { return; }
if (this.groups.find(g => g.groupArn === group.groupArn)) { return; }
this.groups.push(group);
}

Expand Down
Loading

0 comments on commit 9aa0377

Please sign in to comment.