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.

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

feat: Add additional IAM policy to allow cluster role to use KMS key provided for cluster encryption #1915

Conversation

bryantbiggs
Copy link
Member

@bryantbiggs bryantbiggs commented Mar 2, 2022

Description

  • Add additional IAM policy to allow cluster role to use KMS key provided for cluster encryption
  • Fix bug in EKS managed node group example where the IPv6 permissions need to be created on new cluster creation despite the use of IRSA for VPC CNI addon

Motivation and Context

Breaking Changes

  • No

How Has This Been Tested?

  • I have tested and validated these changes using one or more of the provided examples/* projects
    • complete example
    • eks_managed_node_group example
    • irsa_autoscale_refresh example

@mariadb-nickvenenga
Copy link

Why does eks_addons depend on node groups? https://github.com/terraform-aws-modules/terraform-aws-eks/pull/1915/files#diff-dc46acf24afd63ef8c556b77c126ccc6e578bc87e3aa09a931f33d9bf2532fbbR255

It seems like, when using VPC CNI, the addon should be created/setup before node pools are created. Not sure about the other add-ons but seems to make more sense for node pools to depend on addons than the other way around (although I didn't dig in deep enough to see why the dependencies are setup like that in the first place)

@antonbabenko antonbabenko merged commit 7644952 into terraform-aws-modules:master Mar 2, 2022
antonbabenko pushed a commit that referenced this pull request Mar 2, 2022
## [18.8.0](v18.7.3...v18.8.0) (2022-03-02)

### Features

* Add additional IAM policy to allow cluster role to use KMS key provided for cluster encryption ([#1915](#1915)) ([7644952](7644952))
@antonbabenko
Copy link
Member

This PR is included in version 18.8.0 🎉

@bryantbiggs
Copy link
Member Author

Why does eks_addons depend on node groups? #1915 (files)

It seems like, when using VPC CNI, the addon should be created/setup before node pools are created. Not sure about the other add-ons but seems to make more sense for node pools to depend on addons than the other way around (although I didn't dig in deep enough to see why the dependencies are setup like that in the first place)

@mariadb-nickvenenga - see here #1840 (comment)

@bryantbiggs bryantbiggs deleted the feat/secrets-kms-permissions branch March 2, 2022 17:51
@youwalther65
Copy link

@antonbabenko @bryantbiggs I have just upgrade a working Terraform eks cluster to 18.8.0 and get this:

│ Error: Invalid index

│ on .terraform/modules/eks/main.tf line 226, in resource "aws_iam_role_policy_attachment" "cluster_encryption":
│ 226: role = aws_iam_role.this[0].name
│ ├────────────────
│ │ aws_iam_role.this is empty tuple

│ The given key does not identify an element in this collection value: the collection has no elements.

Any thoughts? Thanks!

@bryantbiggs
Copy link
Member Author

@youwalther65 do you have a reproduction of your configs that we can take a look at?

@bryantbiggs
Copy link
Member Author

ah, I think I know actually - you are providing your own IAM role for the cluster, you're not using the IAM role created by the module - is that correct @youwalther65 ?

@youwalther65
Copy link

youwalther65 commented Mar 2, 2022

Yes, exactly @bryantbiggs

@darrenfurr
Copy link

@bryantbiggs - same issue here...

  create_iam_role                 = false
  iam_role_arn                    = data.aws_iam_role.eks_admin.arn
  iam_role_name                   = data.aws_iam_role.eks_admin.name

@darrenfurr
Copy link

@bryantbiggs - I reverted to 18.7.3 & the plan runs successfully

@bryantbiggs
Copy link
Member Author

@youwalther65 / @darrenfurr please let me know if v18.8.1 resolves the issue for you if you get a chance, thank you 🙏🏽

@darrenfurr
Copy link

@bryantbiggs - 18.8.1 worked successfully. Thanks for the quick fix.

@youwalther65
Copy link

Same here @bryantbiggs - 18.8.1 worked successfully. Thanks for the quick fix..

baibailiha added a commit to baibailiha/terraform-aws-eks that referenced this pull request Sep 13, 2022
## [18.8.0](terraform-aws-modules/terraform-aws-eks@v18.7.3...v18.8.0) (2022-03-02)

### Features

* Add additional IAM policy to allow cluster role to use KMS key provided for cluster encryption ([#1915](terraform-aws-modules/terraform-aws-eks#1915)) ([b666996](terraform-aws-modules/terraform-aws-eks@b666996))
@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 Nov 10, 2022
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.

IAM Role missing kms:ListGrants permission eks_managed_node_groups with irsa enabled fails
5 participants