Skip to content

Conversation

@fschykowski
Copy link

Description

Allows modifications to the assume role policy of the eks cluster role and node role. Simply translates a condition object to a condition block, so different combinations for test,variable and values can be passed.

Motivation and Context

The recommended action by AWS to prevent confused deputy attacks is to add sourceArn, sourceAccount or sourceOrg as conditions as condition to the role assumption. https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html

This has already been provided for sourceArn Conditon for the fargate module: a070d7b

I raised a similar PR to VPC module as well which got approved: terraform-aws-modules/terraform-aws-vpc#1138

Breaking Changes

Should not break anything, as the default variable is empty and will simply not render the dynamic block.

How Has This Been Tested?

  • I have executed pre-commit run -a on my pull request
  • I have validated locally that the config is rendered correctly from terraform plan for both adding sourceAccount as condition and not adding conditions.

@bryantbiggs
Copy link
Member

thank you, but we will not be accepting this change at this time. if the service docs mention additional configuration that can be set on the trust policy for confused deputy or other guidance, then we will typically support that here. Right now I don't see that in the EKS docs

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants