Support configuring storage module for service principal access#997
Conversation
…port - Add explicit KMS key policy with root account admin access - Add optional service principal access via service_principals_with_access variable - Add modules/storage/kms_key_arn output - Add checkov skips for CKV_AWS_109, CKV_AWS_111, CKV_AWS_356
doshitan
left a comment
doshitan
left a comment
There was a problem hiding this comment.
Mostly just need to tighten up comments and documentation, but biggest question I think is do we need the kms_key_arn output at the moment? If not we should drop it.
I feel like the title could be more better phrased as well, maybe more like "Support configuring storage module for service principal access"
- Remove kms_key_arn output (not needed) - Improve variable description with usage context - Update checkov skip comments for clarity - Add context re: root account policy is AWS default
doshitan
left a comment
doshitan
left a comment
There was a problem hiding this comment.
A couple things from previous review still unadressed:
- Calling out the first statement of the key policy is the same as the default AWS generated policy
- Updating the title to mention this change is related to the storage module
|
Just connecting some work together for future record, related to #515 as well. |
laurencegoolsby
changed the title
- Add comment explicitly stating the root account statement matches AWS's default key policy for programmatically generated KMS keys.
doshitan
left a comment
doshitan
left a comment
There was a problem hiding this comment.
The PR description is inaccurate (there is no longer a kms_key_arn output), so update that before merging.
Done! |
laurencegoolsby
deleted the
lgoolsby/update-storage-module-kms-settings
branch
2 participants
Ticket
Related to
Changes
Context for reviewers
A subset of AWS services (e.g., Bedrock Data Automation) need KMS key permissions to access S3 buckets.
Added the
service_principals_with_accessvariable to the storage module to grant specific AWS services access to S3 objects via KMS.Testing
Tested in navapbc/platform-test#254