-
Notifications
You must be signed in to change notification settings - Fork 13
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: ecr - enable scan on push and KMS encryption #161
feat: ecr - enable scan on push and KMS encryption #161
Conversation
Signed-off-by: Anton Kukushkin <kukushkin.anton@gmail.com>
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@@ -34,6 +42,11 @@ def __init__( | |||
repository_name=repository_name, | |||
image_tag_mutability=IMAGE_MUTABILITY[image_tag_mutability], | |||
removal_policy=RemovalPolicy.DESTROY if removal_policy in ["DESTROY"] else RemovalPolicy.RETAIN, | |||
image_scan_on_push=image_scan_on_push, | |||
encryption=ENCRYPTION[encryption], | |||
encryption_key=kms.Key.from_key_arn(self, "Key", key_arn=kms_key_arn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of asking for 2 input parameters encryption
and kms-key-arn
, can we just ask for 1 user input parameter kms-key-arn
:
if kms-key-arn
input param is declared, then encryption
should be set to KMS
else set to AES:
encryption="KMS" if kms_key_arn else "AES256",
encryption_key=kms.Key.from_key_arn(self, "Key", key_arn=kms_key_arn) if kms_key_arn else None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If no custom key arn is passed, AWS-managed key is used thus the introduction of encryption
param... I guess one option that will make the choices more clear is to change encryption
to accept one of KMS_MANAGED
, KMS_CUSTOM
, AES256
- does that work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense! Thanks for clarifying.
Signed-off-by: Anton Kukushkin <kukushkin.anton@gmail.com>
3280919
@malachi-constant @srinivasreddych another round of approvals, please. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Issue #, if available:
Description of changes:
added image scan on push and KMS encryption to ECR module
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.