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: Support for encrypting the database and bucket with CMK #182

Merged
merged 31 commits into from
Jul 10, 2024
Merged

feat: Support for encrypting the database and bucket with CMK #182

merged 31 commits into from
Jul 10, 2024

Conversation

velotioaastha
Copy link
Contributor

@velotioaastha velotioaastha commented Feb 22, 2024 •
edited
Loading

@velotioaastha velotioaastha self-assigned this Feb 22, 2024
@velotioaastha velotioaastha requested a review from a team February 22, 2024 13:43
@velotioaastha velotioaastha changed the title Added support for encrypting the database and bucket with CMK feat: Support for encrypting the database and bucket with CMK Feb 22, 2024
gls4
gls4 suggested changes Feb 22, 2024
View reviewed changes
Copy link
Contributor

@gls4 gls4 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm going to leave specific comments in the PR.

gls4
gls4 suggested changes Feb 22, 2024
View reviewed changes
Copy link
Contributor

@gls4 gls4 left a comment

Choose a reason for hiding this comment

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

A few changes requested: variable names, consideration of performance insights key, remove boolean which toggles creation of KMS key.

main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
outputs.tf Outdated Show resolved Hide resolved
variables.tf Show resolved Hide resolved
@velotioaastha
Copy link
Contributor Author

Hey @gls4, We have added changes for performance insights kms key and updated the naming convention for default kms key.

Can you also suggest on kms ARN conditions. I've added as a comment there. Will update accordingly.
Please review the PR.

@gls4
Copy link
Contributor

gls4 commented Feb 26, 2024

I need to test this. More to come.

@gls4
Copy link
Contributor

gls4 commented Feb 27, 2024

Hello! Looks like we might've missed changing a variable name here:

 Error: Reference to undeclared local value
│ 
│   on ../../outputs.tf line 51, in output "kms_key_arn":
│   51:   value       = local.kms_key_arn_generic
│ 
│ A local value with the name "kms_key_arn_generic" has not been declared.

@velotioaastha
Copy link
Contributor Author

hey @gls4, all the comments are resolved. And the PR can be reviewed again.

@jsbroks jsbroks linked an issue Apr 30, 2024 that may be closed by this pull request
Support for encrypting the database and bucket with CMK #211
Closed
@velotioaastha velotioaastha merged commit bc7c957 into main Jul 10, 2024
4 checks passed
@velotioaastha velotioaastha deleted the aastha/encryption branch July 10, 2024 12:06
@velotioaastha velotioaastha restored the aastha/encryption branch July 10, 2024 12:06
@velotioaastha velotioaastha deleted the aastha/encryption branch July 10, 2024 12:06
jsbroks pushed a commit that referenced this pull request Jul 10, 2024
@semantic-release-bot
chore(release): version 4.20.0 [skip ci]
3d81781 
## [4.20.0](v4.19.0...v4.20.0) (2024-07-10)

### Features

* Support for encrypting the database and bucket with CMK ([#182](#182)) ([bc7c957](bc7c957))
@jsbroks
Copy link
Member

jsbroks commented Jul 10, 2024

This PR is included in version 4.20.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gls4 gls4 gls4 approved these changes

@adityachoudhari26 adityachoudhari26 adityachoudhari26 approved these changes

@jsbroks jsbroks Awaiting requested review from jsbroks

Assignees

@velotioaastha velotioaastha

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for encrypting the database and bucket with CMK
4 participants
@velotioaastha @gls4 @jsbroks @adityachoudhari26