diff --git a/src/rpdk/core/data/managed-upload-infrastructure.yaml b/src/rpdk/core/data/managed-upload-infrastructure.yaml index c1a7f71f..560810fc 100644 --- a/src/rpdk/core/data/managed-upload-infrastructure.yaml +++ b/src/rpdk/core/data/managed-upload-infrastructure.yaml @@ -3,6 +3,21 @@ Description: > This CloudFormation template provisions all the infrastructure that is required to upload artifacts to CloudFormation's managed experience. +Parameters: + EnableKMSKeyForS3: + Type: String + Default: "True" + AllowedValues: + - "True" + - "False" + Description: > + Whether to enable a KMS key for S3 bucket encryption. + ConstraintDescription: > + Must be either "True" or "False". + +Conditions: + UseKMSKeyForS3: !Equals [!Ref EnableKMSKeyForS3, "True"] + Resources: ArtifactBucket: Type: AWS::S3::Bucket @@ -10,11 +25,15 @@ Resources: UpdateReplacePolicy: Delete Properties: AccessControl: BucketOwnerFullControl - BucketEncryption: - ServerSideEncryptionConfiguration: - - ServerSideEncryptionByDefault: - SSEAlgorithm: aws:kms - KMSMasterKeyID: !Ref EncryptionKey + BucketEncryption: !If + - UseKMSKeyForS3 + - ServerSideEncryptionConfiguration: + - ServerSideEncryptionByDefault: + SSEAlgorithm: aws:kms + KMSMasterKeyID: !Ref EncryptionKey + - ServerSideEncryptionConfiguration: + - ServerSideEncryptionByDefault: + SSEAlgorithm: AES256 LifecycleConfiguration: Rules: - Id: MultipartUploadLifecycleRule @@ -110,6 +129,7 @@ Resources: Type: AWS::KMS::Key DeletionPolicy: Retain UpdateReplacePolicy: Retain + Condition: UseKMSKeyForS3 Properties: Description: KMS key used to encrypt the resource type artifacts EnableKeyRotation: true diff --git a/src/rpdk/core/package.py b/src/rpdk/core/package.py index b82f7d69..98a06067 100644 --- a/src/rpdk/core/package.py +++ b/src/rpdk/core/package.py @@ -20,6 +20,7 @@ def package(_args): use_role=False, set_default=False, profile_name=False, + use_kms_key=True, ) diff --git a/src/rpdk/core/project.py b/src/rpdk/core/project.py index 10c48a40..bb65cbab 100644 --- a/src/rpdk/core/project.py +++ b/src/rpdk/core/project.py @@ -710,6 +710,7 @@ def submit( use_role, set_default, profile_name, + use_kms_key, ): # pylint: disable=too-many-arguments context_mgr = self._create_context_manager(dry_run) @@ -753,6 +754,7 @@ def submit( use_role, set_default, profile_name, + use_kms_key, ) def _add_overrides_file_to_zip(self, zip_file): @@ -1180,13 +1182,14 @@ def _upload( use_role, set_default, profile_name, + use_kms_key, ): # pylint: disable=too-many-arguments, too-many-locals LOG.debug("Packaging complete, uploading...") session = create_sdk_session(region_name, profile_name) LOG.debug("Uploading to region '%s'", session.region_name) cfn_client = session.client("cloudformation", endpoint_url=endpoint_url) s3_client = session.client("s3") - uploader = Uploader(cfn_client, s3_client) + uploader = Uploader(cfn_client, s3_client, use_kms_key) if use_role and not role_arn and "handlers" in self.schema: LOG.debug("Creating execution role for provider to use") diff --git a/src/rpdk/core/submit.py b/src/rpdk/core/submit.py index 8d6acefd..2604b55c 100644 --- a/src/rpdk/core/submit.py +++ b/src/rpdk/core/submit.py @@ -3,6 +3,7 @@ Projects can be created via the 'init' sub command. """ import logging +import os from .project import Project @@ -16,6 +17,9 @@ def submit(args): if args.use_docker or args.no_docker: project.settings["use_docker"] = args.use_docker project.settings["no_docker"] = args.no_docker + use_kms_key = True + if args.no_kms_key or os.getenv("CFN_CLI_NO_KMS_KEY"): + use_kms_key = False project.submit( args.dry_run, args.endpoint_url, @@ -24,6 +28,7 @@ def submit(args): args.use_role, args.set_default, args.profile, + str(use_kms_key), ) @@ -43,6 +48,15 @@ def setup_subparser(subparsers, parents): help="If registration is successful, set submitted version to the default.", ) parser.add_argument("--profile", help="AWS profile to use.") + parser.add_argument( + "--no-kms-key", + action="store_true", + help=( + "Use the default Server Side Encryption algorithm for the S3 Bucket." + "Does not create a KMS key or removes the KMS key from the management of the stack if it has already been created." + "Alternatively, the environment variable CFN_CLI_NO_KMS_KEY can be set to any truthy value." + ), + ) role_group = parser.add_mutually_exclusive_group() role_group.add_argument( "--role-arn", diff --git a/src/rpdk/core/upload.py b/src/rpdk/core/upload.py index 546a0cde..35826a6a 100644 --- a/src/rpdk/core/upload.py +++ b/src/rpdk/core/upload.py @@ -15,11 +15,12 @@ class Uploader: - def __init__(self, cfn_client, s3_client): + def __init__(self, cfn_client, s3_client, use_kms_key: str): self.cfn_client = cfn_client self.s3_client = s3_client self.bucket_name = "" self.log_delivery_role_arn = "" + self.use_kms_key = use_kms_key @staticmethod def _get_template(): @@ -85,6 +86,15 @@ def _get_stack_output(self, stack_id, output_key): def _create_or_update_stack(self, template, stack_name): args = {"StackName": stack_name, "TemplateBody": template} + if stack_name == INFRA_STACK_NAME: + args |= { + "Parameters": [ + { + "ParameterKey": "EnableKMSKeyForS3", + "ParameterValue": self.use_kms_key, + } + ] + } # attempt to create stack. if the stack already exists, try to update it LOG.info("Creating %s", stack_name) try: diff --git a/tests/test_package.py b/tests/test_package.py index c75c151c..27b09977 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -19,4 +19,5 @@ def test_package_command_valid_schema(): use_role=False, set_default=False, profile_name=False, + use_kms_key=True, ) diff --git a/tests/test_project.py b/tests/test_project.py index 80d87510..f2ea057f 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -1439,7 +1439,8 @@ def test_submit_dry_run(project, is_type_configuration_available): role_arn=None, use_role=True, set_default=False, - profile_name=PROFILE + profile_name=PROFILE, + use_kms_key=True, ) # fmt: on @@ -1540,7 +1541,8 @@ def test_submit_dry_run_modules(project): role_arn=None, use_role=True, set_default=False, - profile_name=PROFILE + profile_name=PROFILE, + use_kms_key=True, ) # fmt: on @@ -1606,7 +1608,8 @@ def test_submit_dry_run_hooks(project): role_arn=None, use_role=True, set_default=False, - profile_name=PROFILE + profile_name=PROFILE, + use_kms_key=True, ) # fmt: on @@ -1732,7 +1735,8 @@ def test_submit_dry_run_hooks_with_target_info(project, session): role_arn=None, use_role=True, set_default=False, - profile_name=PROFILE + profile_name=PROFILE, + use_kms_key=True, ) # fmt: on @@ -1811,7 +1815,8 @@ def test_submit_live_run(project): role_arn=None, use_role=True, set_default=True, - profile_name=PROFILE + profile_name=PROFILE, + use_kms_key=True, ) # fmt: on @@ -1830,6 +1835,7 @@ def test_submit_live_run(project): use_role=True, set_default=True, profile_name=PROFILE, + use_kms_key=True, ) assert temp_file._was_closed @@ -1865,7 +1871,8 @@ def test_submit_live_run_for_module(project): role_arn=None, use_role=True, set_default=True, - profile_name=PROFILE + profile_name=PROFILE, + use_kms_key=True, ) # fmt: on @@ -1908,7 +1915,8 @@ def test_submit_live_run_for_hooks(project): role_arn=None, use_role=True, set_default=True, - profile_name=PROFILE + profile_name=PROFILE, + use_kms_key=True, ) # fmt: on @@ -1927,6 +1935,7 @@ def test_submit_live_run_for_hooks(project): use_role=True, set_default=True, profile_name=PROFILE, + use_kms_key=True, ) assert temp_file._was_closed @@ -1964,6 +1973,7 @@ def test__upload_good_path_create_role_and_set_default(project): use_role=True, set_default=True, profile_name=None, + use_kms_key=True, ) mock_sdk.assert_called_once_with(region_name=None, profile_name=None) @@ -2018,6 +2028,7 @@ def test__upload_good_path_create_role_and_set_default_hook(project): use_role=True, set_default=True, profile_name=None, + use_kms_key=True, ) mock_sdk.assert_called_once_with(region_name=None, profile_name=None) @@ -2075,6 +2086,7 @@ def test__upload_good_path_skip_role_creation( use_role=use_role, set_default=True, profile_name=None, + use_kms_key=True, ) mock_sdk.assert_called_once_with(region_name=None, profile_name=None) @@ -2130,6 +2142,7 @@ def test__upload_good_path_skip_role_creation_hook( use_role=use_role, set_default=True, profile_name=None, + use_kms_key=True, ) mock_sdk.assert_called_once_with(region_name=None, profile_name=None) @@ -2181,6 +2194,7 @@ def test__upload_clienterror(project): use_role=False, set_default=True, profile_name=None, + use_kms_key=True, ) mock_sdk.assert_called_once_with(region_name=None, profile_name=None) @@ -2229,6 +2243,7 @@ def test__upload_clienterror_module(project): use_role=False, set_default=True, profile_name=None, + use_kms_key=True, ) mock_sdk.assert_called_once_with(region_name=None, profile_name=None) @@ -2277,6 +2292,7 @@ def test__upload_clienterror_hook(project): use_role=False, set_default=True, profile_name=None, + use_kms_key=True, ) mock_sdk.assert_called_once_with(region_name=None, profile_name=None) diff --git a/tests/test_upload.py b/tests/test_upload.py index 19ed8c84..89da6d57 100644 --- a/tests/test_upload.py +++ b/tests/test_upload.py @@ -66,7 +66,7 @@ class AlreadyExistsException(Exception): @pytest.fixture def uploader(): - uploader = Uploader(Mock(), Mock()) + uploader = Uploader(Mock(), Mock(), True) uploader.cfn_client.exceptions.AlreadyExistsException = AlreadyExistsException return uploader @@ -206,6 +206,12 @@ def test__create_or_update_stack_stack_doesnt_exist(uploader): StackName=INFRA_STACK_NAME, TemplateBody=CONTENTS_UTF8, EnableTerminationProtection=True, + Parameters=[ + { + "ParameterKey": "EnableKMSKeyForS3", + "ParameterValue": True, + } + ], ) mock_wait.assert_called_once_with(STACK_ID, "stack_create_complete", ANY) @@ -231,11 +237,23 @@ class AlreadyExistsException(Exception): StackName=INFRA_STACK_NAME, TemplateBody=CONTENTS_UTF8, EnableTerminationProtection=True, + Parameters=[ + { + "ParameterKey": "EnableKMSKeyForS3", + "ParameterValue": True, + } + ], ) uploader.cfn_client.update_stack.assert_called_once_with( Capabilities=["CAPABILITY_IAM"], StackName=INFRA_STACK_NAME, TemplateBody=CONTENTS_UTF8, + Parameters=[ + { + "ParameterKey": "EnableKMSKeyForS3", + "ParameterValue": True, + } + ], ) mock_wait.assert_not_called() @@ -255,11 +273,23 @@ def test__create_or_update_stack_stack_exists_and_needs_changes(uploader): StackName=INFRA_STACK_NAME, TemplateBody=CONTENTS_UTF8, EnableTerminationProtection=True, + Parameters=[ + { + "ParameterKey": "EnableKMSKeyForS3", + "ParameterValue": True, + } + ], ) uploader.cfn_client.update_stack.assert_called_once_with( Capabilities=["CAPABILITY_IAM"], StackName=INFRA_STACK_NAME, TemplateBody=CONTENTS_UTF8, + Parameters=[ + { + "ParameterKey": "EnableKMSKeyForS3", + "ParameterValue": True, + } + ], ) mock_wait.assert_called_once_with(STACK_ID, "stack_update_complete", ANY) @@ -279,6 +309,12 @@ def test__create_or_update_stack_create_unknown_failure(uploader): StackName=INFRA_STACK_NAME, TemplateBody=CONTENTS_UTF8, EnableTerminationProtection=True, + Parameters=[ + { + "ParameterKey": "EnableKMSKeyForS3", + "ParameterValue": True, + } + ], ) assert uploader.get_log_delivery_role_arn() == "" @@ -301,11 +337,23 @@ def test__create_or_update_stack_update_unknown_failure(uploader): StackName=INFRA_STACK_NAME, TemplateBody=CONTENTS_UTF8, EnableTerminationProtection=True, + Parameters=[ + { + "ParameterKey": "EnableKMSKeyForS3", + "ParameterValue": True, + } + ], ) uploader.cfn_client.update_stack.assert_called_once_with( Capabilities=["CAPABILITY_IAM"], StackName=INFRA_STACK_NAME, TemplateBody=CONTENTS_UTF8, + Parameters=[ + { + "ParameterKey": "EnableKMSKeyForS3", + "ParameterValue": True, + } + ], ) mock_wait.assert_not_called()