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

kms: support for key rotation period #29927

Closed
1 of 2 tasks
badmintoncryer opened this issue Apr 22, 2024 · 7 comments · Fixed by #29928, rwlxxvii/containers#124 or rwlxxvii/containers#140 · May be fixed by NOUIY/aws-solutions-constructs#99 or NOUIY/aws-solutions-constructs#101
Closed
1 of 2 tasks
Labels
@aws-cdk/aws-kms Related to AWS Key Management effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@badmintoncryer
Copy link
Contributor

Describe the feature

Configuring automatic key rotation period.

new kms.Key(this, 'MyKey', {
  enableKeyRotation: true,
  rotationPeriod: Duration.days(180), // Add
});

Use Case

Cloudformation supports for configuring period of automatic key rotation but CDK does not.

Proposed Solution

No response

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.127.0

Environment details (OS name and version, etc.)

irrelevant

@badmintoncryer badmintoncryer added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Apr 22, 2024
@github-actions github-actions bot added the @aws-cdk/aws-kms Related to AWS Key Management label Apr 22, 2024
@badmintoncryer badmintoncryer changed the title kms: key rotation period kms: support for key rotation period Apr 22, 2024
@khushail
Copy link
Contributor

khushail commented Apr 22, 2024

@badmintoncryer , there is already a somewhat similar feature request in the past for enabling key rotation. See this -#22125. As mentioned in AWS Docs and suggested here in comments as well

AWS KMS charges a monthly fee for first and second rotation of key material maintained for your KMS key.

Please feel free to reach out if this one is different.

@khushail khushail added wontfix We have determined that we will not resolve the issue. duplicate This issue is a duplicate. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. wontfix We have determined that we will not resolve the issue. labels Apr 22, 2024
@badmintoncryer
Copy link
Contributor Author

badmintoncryer commented Apr 23, 2024

Thank you, @khushail, for the information. Issue #22125 discusses enabling key rotation feature that has already been implemented.

However, this issue pertains to configuring the rotation period, so I believe there is a distinction between the two.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 23, 2024
@khushail
Copy link
Contributor

khushail commented Apr 23, 2024

Yes that's true , you are talking about rotation period. I might be wrong
but what I think the reason behind not keeping the rotation period was the same as well. By default this is false which means if user wants, and is aware of the billing, they can set it to true using the template or directly.

@pahud , wdyt?

@badmintoncryer
Copy link
Contributor Author

badmintoncryer commented Apr 23, 2024

@khushail
I also agree that key rotation should be disabled by default, and I understand that enabling it could incur charges.

This feature request is intended for users who do not mind the additional costs and want to perform key rotations. Currently, the only setting available is the default 365 days.

Therefore, even if key rotation is disabled by default, I believe there is still merit in implementing this feature.

Additionally, my understanding is that the discussion in issue #22125 revolves around the keys that are automatically generated at bootstrap. Indeed, it would be hard to accept if this setting incurs additional charges.

This time, we are considering an option for keys freely created by users. Wouldn't this be a different situation from the previous issue?

What are your thoughts about this?

@khushail
Copy link
Contributor

@khushail I also agree that key rotation should be disabled by default, and I understand that enabling it could incur charges.

This feature request is intended for users who do not mind the additional costs and want to perform key rotations. Currently, the only setting available is the default 365 days.

Therefore, even if key rotation is disabled by default, I believe there is still merit in implementing this feature.

Additionally, my understanding is that the discussion in issue #22125 revolves around the keys that are automatically generated at bootstrap. Indeed, it would be hard to accept if this setting incurs additional charges.

This time, we are considering an option for keys freely created by users. Wouldn't this be a different situation from the previous issue?

What are your thoughts about this?

Hmmm, I understand your point of view in enabling this feature. Marking this as appropriate.

@khushail khushail added p2 effort/small Small work item – less than a day of effort and removed duplicate This issue is a duplicate. labels Apr 23, 2024
@mergify mergify bot closed this as completed in #29928 Apr 24, 2024
mergify bot pushed a commit that referenced this issue Apr 24, 2024
### Issue # (if applicable)

Closes #29927.

### Reason for this change

Cloudformation [supports for configuring period of automatic key rotation](https://docs.aws.amazon.com/ja_jp/AWSCloudFormation/latest/UserGuide/aws-resource-kms-key.html#cfn-kms-key-rotationperiodindays) but CDK does not.

### Description of changes

Added `rotationPeriod` to `KeyProps`.

### Description of how you validated changes

I've added both unit and integ tests.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

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

⚠️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.

@aws-cdk-automation
Copy link
Collaborator

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.