From 2ae5cc15572c67417c7878b8b02c9c5a2bf88891 Mon Sep 17 00:00:00 2001 From: Ryan Whalen Date: Tue, 20 Dec 2022 12:52:54 -0500 Subject: [PATCH 01/11] Add encryption_config option --- plugins/modules/ecs_ecr.py | 39 ++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/plugins/modules/ecs_ecr.py b/plugins/modules/ecs_ecr.py index d472af2756c..df01bb90b82 100644 --- a/plugins/modules/ecs_ecr.py +++ b/plugins/modules/ecs_ecr.py @@ -85,6 +85,23 @@ default: false type: bool version_added: 1.3.0 + encryption_configuration: + description: + - The encryption configuration for the repository. + required: false + suboptions: + encryptionType: + description: + - The encryption type to use. + choices: [AES256, KMS] + default: 'AES256' + type: str + kmsKey: + description: + - If I(encryption_type=KMS), specify the KMS key to use for encryption. + - The alias, key ID, or full ARN of the KMS key can be specified. + type: str + type: dict author: - David M. Lee (@leedm777) extends_documentation_fragment: @@ -161,6 +178,13 @@ community.aws.ecs_ecr: name: needs-no-lifecycle-policy purge_lifecycle_policy: true + +- name: set-encryption-configuration + community.aws.ecs_ecr: + name: uses-custom-kms-key + encryption_configuration: + encryptionType: KMS + kmsKey: custom-kms-key-alias ''' RETURN = ''' @@ -248,17 +272,21 @@ def get_repository_policy(self, registry_id, name): except is_boto3_error_code(['RepositoryNotFoundException', 'RepositoryPolicyNotFoundException']): return None - def create_repository(self, registry_id, name, image_tag_mutability): + def create_repository(self, registry_id, name, image_tag_mutability, encryption_configuration): if registry_id: default_registry_id = self.sts.get_caller_identity().get('Account') if registry_id != default_registry_id: raise Exception('Cannot create repository in registry {0}.' 'Would be created in {1} instead.'.format(registry_id, default_registry_id)) + if encryption_configuration is None: + encryption_configuration = dict(encryptionType='AES256') + if not self.check_mode: repo = self.ecr.create_repository( repositoryName=name, - imageTagMutability=image_tag_mutability).get('repository') + imageTagMutability=image_tag_mutability, + encryptionConfiguration=encryption_configuration).get('repository') self.changed = True return repo else: @@ -411,6 +439,7 @@ def run(ecr, params): lifecycle_policy_text = params['lifecycle_policy'] purge_lifecycle_policy = params['purge_lifecycle_policy'] scan_on_push = params['scan_on_push'] + encryption_configuration = params['encryption_configuration'] # Parse policies, if they are given try: @@ -437,7 +466,8 @@ def run(ecr, params): result['created'] = False if not repo: - repo = ecr.create_repository(registry_id, name, image_tag_mutability) + repo = ecr.create_repository( + registry_id, name, image_tag_mutability, encryption_configuration) result['changed'] = True result['created'] = True else: @@ -550,7 +580,8 @@ def main(): purge_policy=dict(required=False, type='bool'), lifecycle_policy=dict(required=False, type='json'), purge_lifecycle_policy=dict(required=False, type='bool'), - scan_on_push=(dict(required=False, type='bool', default=False)) + scan_on_push=(dict(required=False, type='bool', default=False)), + encryption_configuration=(dict(required=False, type='dict')) ) mutually_exclusive = [ ['policy', 'purge_policy'], From 95a9be9777691a1fbcc804b812daf1547fe0af2b Mon Sep 17 00:00:00 2001 From: Ryan Whalen Date: Tue, 20 Dec 2022 13:20:09 -0500 Subject: [PATCH 02/11] Add changelog fragment --- changelogs/fragments/1623-ecs_ecr-add-kms-config.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/1623-ecs_ecr-add-kms-config.yml diff --git a/changelogs/fragments/1623-ecs_ecr-add-kms-config.yml b/changelogs/fragments/1623-ecs_ecr-add-kms-config.yml new file mode 100644 index 00000000000..b0e194bc32d --- /dev/null +++ b/changelogs/fragments/1623-ecs_ecr-add-kms-config.yml @@ -0,0 +1,2 @@ +minor_changes: +- ecs_ecr - add `encryption_configuration` option (https://github.com/ansible-collections/community.aws/pull/1623). From c0b0d379978269cbd59aa876552645a665a3b2d4 Mon Sep 17 00:00:00 2001 From: Ryan Whalen Date: Wed, 21 Dec 2022 11:27:12 -0500 Subject: [PATCH 03/11] Add version_added info --- plugins/modules/ecs_ecr.py | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/modules/ecs_ecr.py b/plugins/modules/ecs_ecr.py index df01bb90b82..80157cf6579 100644 --- a/plugins/modules/ecs_ecr.py +++ b/plugins/modules/ecs_ecr.py @@ -102,6 +102,7 @@ - The alias, key ID, or full ARN of the KMS key can be specified. type: str type: dict + version_added: 5.2.0 author: - David M. Lee (@leedm777) extends_documentation_fragment: From 67aab4d5eb15644e9f93f9b1e32172e59924c39e Mon Sep 17 00:00:00 2001 From: Ryan Whalen Date: Wed, 21 Dec 2022 11:33:26 -0500 Subject: [PATCH 04/11] Add integration tests --- .../targets/ecs_ecr/tasks/main.yml | 52 ++++++++++++++ .../targets/ecs_ecr/templates/kms_policy.j2 | 72 +++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 tests/integration/targets/ecs_ecr/templates/kms_policy.j2 diff --git a/tests/integration/targets/ecs_ecr/tasks/main.yml b/tests/integration/targets/ecs_ecr/tasks/main.yml index 73fcc3530f1..83e26b31fa3 100644 --- a/tests/integration/targets/ecs_ecr/tasks/main.yml +++ b/tests/integration/targets/ecs_ecr/tasks/main.yml @@ -10,6 +10,24 @@ - set_fact: ecr_name: '{{ resource_prefix }}-ecr' + - name: get ARN of calling user + aws_caller_info: + register: aws_caller_info + + - name: create KMS key for testing + aws_kms: + alias: "{{ resource_prefix }}-kms" + description: a key used for testing ECR + state: present + enabled: yes + key_spec: SYMMETRIC_DEFAULT + key_usage: ENCRYPT_DECRYPT + policy: "{{ lookup('template', 'kms_policy.j2') }}" + tags: + Name: "{{ resource_prefix }}-kms" + AnsibleTest: AnsibleTestVpc + register: kms_test_key + - name: When creating with check mode ecs_ecr: name: '{{ ecr_name }}' @@ -54,6 +72,11 @@ that: - result.repository.imageTagMutability == "MUTABLE" + - name: it should use AES256 encryption by default + assert: + that: + - result.repository.encryptionConfiguration.encryptionType == "AES256" + - name: When pulling an existing repository that has no existing policy ecs_ecr: name: '{{ ecr_name }}' @@ -538,9 +561,38 @@ - result is changed - not result.repository.imageScanningConfiguration.scanOnPush + - name: delete repository + ecs_ecr: + name: '{{ ecr_name }}' + state: absent + + - name: When creating a repo using KMS encryption + ecs_ecr: + name: '{{ ecr_name }}' + encryption_configuration: + encryptionType: KMS + kmsKey: '{{ kms_test_key.key_arn }}' + register: result + + - name: it should create the repo and use KMS encryption + assert: + that: + - result is changed + - result.repository.encryptionConfiguration.encryptionType == "KMS" + + - name: it should use the provided KMS key + assert: + that: + - result.repository.encryptionConfiguration.kmsKey == '{{ kms_test_key.key_arn }}' + always: - name: Delete lingering ECR repository ecs_ecr: name: '{{ ecr_name }}' state: absent + + - name: Delete KMS key + aws_kms: + key_id: '{{ kms_test_key.key_arn }}' + state: absent diff --git a/tests/integration/targets/ecs_ecr/templates/kms_policy.j2 b/tests/integration/targets/ecs_ecr/templates/kms_policy.j2 new file mode 100644 index 00000000000..17108e5b367 --- /dev/null +++ b/tests/integration/targets/ecs_ecr/templates/kms_policy.j2 @@ -0,0 +1,72 @@ +{ + "Id": "key-ansible-test-policy-123", + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "Allow access for root user", + "Effect": "Allow", + "Principal": { + "AWS": "arn:aws:iam::{{ aws_caller_info.account }}:root" + }, + "Action": "kms:*", + "Resource": "*" + }, + { + "Sid": "Allow access for calling user", + "Effect": "Allow", + "Principal": { + "AWS": "{{ aws_caller_info.arn }}" + }, + "Action": [ + "kms:Create*", + "kms:Describe*", + "kms:Enable*", + "kms:List*", + "kms:Put*", + "kms:Update*", + "kms:Revoke*", + "kms:Disable*", + "kms:Get*", + "kms:Delete*", + "kms:TagResource", + "kms:UntagResource", + "kms:ScheduleKeyDeletion", + "kms:CancelKeyDeletion" + ], + "Resource": "*" + }, + { + "Sid": "Allow use of the key", + "Effect": "Allow", + "Principal": { + "AWS": "{{ aws_caller_info.arn }}" + }, + "Action": [ + "kms:Encrypt", + "kms:Decrypt", + "kms:ReEncrypt*", + "kms:GenerateDataKey*", + "kms:DescribeKey" + ], + "Resource": "*" + }, + { + "Sid": "Allow attachment of persistent resources", + "Effect": "Allow", + "Principal": { + "AWS": "{{ aws_caller_info.arn }}" + }, + "Action": [ + "kms:CreateGrant", + "kms:ListGrants", + "kms:RevokeGrant" + ], + "Resource": "*", + "Condition": { + "Bool": { + "kms:GrantIsForAWSResource": "true" + } + } + } + ] +} From 3c01595516db4e3117ae0e7e7afdc0fc9841fb5b Mon Sep 17 00:00:00 2001 From: Ryan Whalen Date: Thu, 22 Dec 2022 10:34:45 -0500 Subject: [PATCH 05/11] Use snake case instead of camel case --- plugins/modules/ecs_ecr.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/plugins/modules/ecs_ecr.py b/plugins/modules/ecs_ecr.py index 80157cf6579..009f2a72552 100644 --- a/plugins/modules/ecs_ecr.py +++ b/plugins/modules/ecs_ecr.py @@ -90,13 +90,13 @@ - The encryption configuration for the repository. required: false suboptions: - encryptionType: + encryption_type: description: - The encryption type to use. choices: [AES256, KMS] default: 'AES256' type: str - kmsKey: + kms_key: description: - If I(encryption_type=KMS), specify the KMS key to use for encryption. - The alias, key ID, or full ARN of the KMS key can be specified. @@ -184,8 +184,8 @@ community.aws.ecs_ecr: name: uses-custom-kms-key encryption_configuration: - encryptionType: KMS - kmsKey: custom-kms-key-alias + encryption_type: KMS + kms_key: custom-kms-key-alias ''' RETURN = ''' @@ -226,6 +226,7 @@ except ImportError: pass # Handled by AnsibleAWSModule +from ansible.module_utils.common.dict_transformations import snake_dict_to_camel_dict from ansible.module_utils.six import string_types from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule @@ -440,7 +441,7 @@ def run(ecr, params): lifecycle_policy_text = params['lifecycle_policy'] purge_lifecycle_policy = params['purge_lifecycle_policy'] scan_on_push = params['scan_on_push'] - encryption_configuration = params['encryption_configuration'] + encryption_configuration = snake_dict_to_camel_dict(params['encryption_configuration']) # Parse policies, if they are given try: From a812103fe391ef7ce6d24ddc3b29b8f686fb4602 Mon Sep 17 00:00:00 2001 From: Ryan Whalen Date: Thu, 22 Dec 2022 10:36:02 -0500 Subject: [PATCH 06/11] Update argument spec --- plugins/modules/ecs_ecr.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/plugins/modules/ecs_ecr.py b/plugins/modules/ecs_ecr.py index 009f2a72552..d2b67f6a35f 100644 --- a/plugins/modules/ecs_ecr.py +++ b/plugins/modules/ecs_ecr.py @@ -583,7 +583,17 @@ def main(): lifecycle_policy=dict(required=False, type='json'), purge_lifecycle_policy=dict(required=False, type='bool'), scan_on_push=(dict(required=False, type='bool', default=False)), - encryption_configuration=(dict(required=False, type='dict')) + encryption_configuration=dict( + required=False, + type='dict', + options=dict( + encryption_type=dict(required=True, type='str', choices=['AES256', 'KMS']), + kms_key=dict(required=False, type='str'), + ), + required_if=[ + ['encryption_type', 'KMS', ['kms_key']], + ], + ), ) mutually_exclusive = [ ['policy', 'purge_policy'], From 7ff6ad353817787f44c8abec14f840e4c2d127ba Mon Sep 17 00:00:00 2001 From: Ryan Whalen Date: Thu, 22 Dec 2022 10:37:50 -0500 Subject: [PATCH 07/11] Fail when attempting to modify --- plugins/modules/ecs_ecr.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/modules/ecs_ecr.py b/plugins/modules/ecs_ecr.py index d2b67f6a35f..fa3c7c09db3 100644 --- a/plugins/modules/ecs_ecr.py +++ b/plugins/modules/ecs_ecr.py @@ -473,6 +473,11 @@ def run(ecr, params): result['changed'] = True result['created'] = True else: + if encryption_configuration is not None: + if repo.get('encryptionConfiguration') != encryption_configuration: + result['msg'] = 'Cannot modify repository encryption type' + return False, result + repo = ecr.put_image_tag_mutability(registry_id, name, image_tag_mutability) result['repository'] = repo From ebbaae2ee74058b40e00f0ee2f89cdf7137d027a Mon Sep 17 00:00:00 2001 From: Ryan Whalen Date: Thu, 22 Dec 2022 10:38:53 -0500 Subject: [PATCH 08/11] Update integration tests --- .../integration/targets/ecs_ecr/tasks/main.yml | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/integration/targets/ecs_ecr/tasks/main.yml b/tests/integration/targets/ecs_ecr/tasks/main.yml index 83e26b31fa3..7d2c2d4a0c2 100644 --- a/tests/integration/targets/ecs_ecr/tasks/main.yml +++ b/tests/integration/targets/ecs_ecr/tasks/main.yml @@ -561,6 +561,20 @@ - result is changed - not result.repository.imageScanningConfiguration.scanOnPush + - name: When modifying the encryption setting of an existing repository + ecs_ecr: + name: '{{ ecr_name }}' + encryption_configuration: + encryption_type: KMS + kms_key: '{{ kms_test_key.key_arn }}' + register: result + ignore_errors: true + + - name: it should fail + assert: + that: + - result is failed + - name: delete repository ecs_ecr: name: '{{ ecr_name }}' @@ -570,8 +584,8 @@ ecs_ecr: name: '{{ ecr_name }}' encryption_configuration: - encryptionType: KMS - kmsKey: '{{ kms_test_key.key_arn }}' + encryption_type: KMS + kms_key: '{{ kms_test_key.key_arn }}' register: result - name: it should create the repo and use KMS encryption From 77e1059520d2d41584abf2e64c6c6fbe2fde5969 Mon Sep 17 00:00:00 2001 From: Ryan Whalen Date: Thu, 22 Dec 2022 11:25:32 -0500 Subject: [PATCH 09/11] Update arg spec and documentation block --- plugins/modules/ecs_ecr.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/modules/ecs_ecr.py b/plugins/modules/ecs_ecr.py index fa3c7c09db3..02ec6fbecaf 100644 --- a/plugins/modules/ecs_ecr.py +++ b/plugins/modules/ecs_ecr.py @@ -96,6 +96,7 @@ choices: [AES256, KMS] default: 'AES256' type: str + required: true kms_key: description: - If I(encryption_type=KMS), specify the KMS key to use for encryption. @@ -592,8 +593,8 @@ def main(): required=False, type='dict', options=dict( - encryption_type=dict(required=True, type='str', choices=['AES256', 'KMS']), - kms_key=dict(required=False, type='str'), + encryption_type=dict(required=True, type='str', default='AES256', choices=['AES256', 'KMS']), + kms_key=dict(required=False, type='str', no_log=False), ), required_if=[ ['encryption_type', 'KMS', ['kms_key']], From c36bfd7807aae804cad937f27caa4bb6a4a3571a Mon Sep 17 00:00:00 2001 From: Ryan Whalen Date: Thu, 22 Dec 2022 11:47:45 -0500 Subject: [PATCH 10/11] Fix validation test Arguments with a default should not be marked as required --- plugins/modules/ecs_ecr.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/modules/ecs_ecr.py b/plugins/modules/ecs_ecr.py index 02ec6fbecaf..294d0ae2ad9 100644 --- a/plugins/modules/ecs_ecr.py +++ b/plugins/modules/ecs_ecr.py @@ -96,7 +96,6 @@ choices: [AES256, KMS] default: 'AES256' type: str - required: true kms_key: description: - If I(encryption_type=KMS), specify the KMS key to use for encryption. @@ -593,7 +592,7 @@ def main(): required=False, type='dict', options=dict( - encryption_type=dict(required=True, type='str', default='AES256', choices=['AES256', 'KMS']), + encryption_type=dict(required=False, type='str', default='AES256', choices=['AES256', 'KMS']), kms_key=dict(required=False, type='str', no_log=False), ), required_if=[ From 28df6d6f02b401f1da63a2800a37eb71cd9a27d8 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Wed, 18 Jan 2023 16:09:01 +0100 Subject: [PATCH 11/11] Use ECR in the name of the KMS CMK - makes it easier to trace dangling resources --- tests/integration/targets/ecs_ecr/tasks/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/targets/ecs_ecr/tasks/main.yml b/tests/integration/targets/ecs_ecr/tasks/main.yml index 7d2c2d4a0c2..e0ce4f3f664 100644 --- a/tests/integration/targets/ecs_ecr/tasks/main.yml +++ b/tests/integration/targets/ecs_ecr/tasks/main.yml @@ -16,7 +16,7 @@ - name: create KMS key for testing aws_kms: - alias: "{{ resource_prefix }}-kms" + alias: "{{ resource_prefix }}-ecr" description: a key used for testing ECR state: present enabled: yes @@ -24,7 +24,7 @@ key_usage: ENCRYPT_DECRYPT policy: "{{ lookup('template', 'kms_policy.j2') }}" tags: - Name: "{{ resource_prefix }}-kms" + Name: "{{ resource_prefix }}-ecr" AnsibleTest: AnsibleTestVpc register: kms_test_key