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

feat: Add support for creating KMS key for cluster secret encryption #2121

Merged
merged 3 commits into from
Jun 28, 2022
Merged

feat: Add support for creating KMS key for cluster secret encryption #2121

merged 3 commits into from
Jun 28, 2022

Conversation

bryantbiggs
Copy link
Member

@bryantbiggs bryantbiggs commented Jun 26, 2022 •
edited
Loading

Description

  • Add support for creating KMS key for cluster secret encryption

Motivation and Context

  • Adding the cluster IAM role to a KMS key policy creates a cyclical dependency when the key/policy are created externally unless users also create the role externally. Provisioning a cluster with secrets encrypted using KMS requires the KMS key to be created first. Creating the KMS key first with a key policy causes a conflict because the cluster IAM role has not been created yet and cannot be referenced. The only resolution currently is to either create the key, key policy, and role externally to ensure proper ordering and name resolution, or apply secret encryption AFTER the cluster and cluster IAM role have been created (poor user experience). Embedding the key and key policy creation into the module resolves this scenario while allowing users to extend the policy as well

Breaking Changes

  • No

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

@bryantbiggs bryantbiggs marked this pull request as ready for review June 27, 2022 18:41
@bryantbiggs bryantbiggs requested a review from antonbabenko June 27, 2022 18:41
antonbabenko
antonbabenko approved these changes Jun 28, 2022
View reviewed changes
Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm. I was thinking for some time while reading this if we should prefix variables with 'key_' or 'kms_key_' to avoid potential confusion with "private key". Current naming is good regardless :)

@bryantbiggs
Copy link
Member Author

Lgtm. I was thinking for some time while reading this if we should prefix variables with 'key_' or 'kms_key_' to avoid potential confusion with "private key". Current naming is good regardless :)

thats a good point, I've updated to include kms_ to make it more clear

@bryantbiggs bryantbiggs merged commit 75acb09 into terraform-aws-modules:master Jun 28, 2022
@bryantbiggs bryantbiggs deleted the feat/kms-key branch June 28, 2022 14:55
antonbabenko pushed a commit that referenced this pull request Jun 28, 2022
@semantic-release-bot
chore(release): version 18.25.0 [skip ci]
62b776f 
## [18.25.0](v18.24.1...v18.25.0) (2022-06-28)

### Features

* Add support for creating KMS key for cluster secret encryption ([#2121](#2121)) ([75acb09](75acb09))
@antonbabenko
Copy link
Member

This PR is included in version 18.25.0 🎉

@bryantbiggs bryantbiggs mentioned this pull request Jun 28, 2022
Merged
3 tasks
@philicious philicious mentioned this pull request Jul 4, 2022
Closed
spr-mweber3 pushed a commit to spring-media/terraform-aws-eks that referenced this pull request Jul 11, 2022
@bryantbiggs @spr-mweber3
feat: Add support for creating KMS key for cluster secret encryption (t…
cb26b79 
…erraform-aws-modules#2121)

* feat: Add support for creating KMS key for cluster secret encryption

* fix: Update naming scheme to include `kms_`
spr-mweber3 pushed a commit to spring-media/terraform-aws-eks that referenced this pull request Jul 11, 2022
@semantic-release-bot @spr-mweber3
chore(release): version 18.25.0 [skip ci]
618699a 
## [18.25.0](terraform-aws-modules/terraform-aws-eks@v18.24.1...v18.25.0) (2022-06-28)

### Features

* Add support for creating KMS key for cluster secret encryption ([terraform-aws-modules#2121](terraform-aws-modules#2121)) ([75acb09](terraform-aws-modules@75acb09))
baibailiha added a commit to baibailiha/terraform-aws-eks that referenced this pull request Sep 13, 2022
@baibailiha @semantic-release-bot
chore(release): version 18.25.0 [skip ci]
6a67b3f 
## [18.25.0](terraform-aws-modules/terraform-aws-eks@v18.24.1...v18.25.0) (2022-06-28)

### Features

* Add support for creating KMS key for cluster secret encryption ([#2121](terraform-aws-modules/terraform-aws-eks#2121)) ([2635bef](terraform-aws-modules/terraform-aws-eks@2635bef))
@github-actions
Copy link

github-actions bot commented Nov 8, 2022

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 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@antonbabenko antonbabenko antonbabenko approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bryantbiggs @antonbabenko