Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(aws_iam): ManagedPolicy.attachToRole should prevent adding to same role multiple times #28101

Closed
Rouby opened this issue Nov 22, 2023 · 2 comments · Fixed by #28129
Closed

(aws_iam): ManagedPolicy.attachToRole should prevent adding to same role multiple times #28101

Rouby opened this issue Nov 22, 2023 · 2 comments · Fixed by #28129
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@Rouby
Copy link

Rouby commented Nov 22, 2023

Describe the bug

Currently attachToRole checks for object equality.

aws-cdk/packages/aws-cdk-lib/aws-iam/lib/managed-policy.ts

Line 293 in 70f66c7

if (this.roles.find(r => r === role)) { return; }

This does not work if a Role is imported e.g. via aws_iam.Role.fromRoleArn as these are unique objects.

Expected Behavior

attachToRole should skip if a role is already added to the list, so that no Roles: array items are not unique is encounted on deployments.

Current Behavior

attachToRole adds the same role (different objects) twice, which results in a validation error.

Reproduction Steps

  1. import a role
  2. import the same role at another points
  3. create a ManagedPolicy
  4. attach both roles to the policy

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.102.0

Framework Version

No response

Node.js Version

18

OS

Mac

Language

TypeScript

Language Version

No response

Other information

No response
@Rouby Rouby added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 22, 2023
@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Nov 22, 2023
@pahud
Copy link
Contributor

pahud commented Nov 22, 2023

Yes we should improve that. I'm making it a p2 and any pull requests are appreciated.

@pahud pahud added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Nov 22, 2023
@lpizzinidev lpizzinidev mentioned this issue Nov 24, 2023
Merged
@github-actions github-actions bot mentioned this issue Dec 1, 2023
Closed
vinayak-kukreja added a commit to lpizzinidev/aws-cdk that referenced this issue Dec 4, 2023
@vinayak-kukreja
Merge branch 'main' into awsgh-28101
bb68407
mergify bot added a commit to lpizzinidev/aws-cdk that referenced this issue Dec 5, 2023
@mergify
Merge branch 'main' into awsgh-28101
3def470
@mergify mergify bot closed this as completed in #28129 Dec 5, 2023
mergify bot pushed a commit that referenced this issue Dec 5, 2023
@lpizzinidev
fix(iam): attaching a policy is not idempotent with imported resources (
0abd0b5 
#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 #28101.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 5, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

chenjane-dev pushed a commit to chenjane-dev/aws-cdk that referenced this issue Dec 5, 2023
@lpizzinidev @chenjane-dev
fix(iam): attaching a policy is not idempotent with imported resources (
9aa0377 
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*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(iam): attaching a policy is not idempotent with imported resources lpizzinidev/aws-cdk
2 participants
@pahud @Rouby