From 284f26303caeebe9a85dfcdd8b8ce9e69ffb4087 Mon Sep 17 00:00:00 2001 From: Clint Byrum Date: Thu, 19 Dec 2019 10:41:41 -0800 Subject: [PATCH] Add support for ECR Lifecycle Policies to ecs_ecr (#48997) * Fix copy/pasta for ecs_ecr test names * Add support for lifecycle policies to ecs_ecr New feature for ecs_ecr to support [ECR Lifecycle Policies][]. Fixes #32003 [ECR Lifecycle Policies]: https://docs.aws.amazon.com/AmazonECR/latest/userguide/LifecyclePolicies.html * Improve error message for ecs_ecr parsing errors Replaces the exception and stack trace with a description of what's actually going wrong from a user perspective. * Rename delete policy to purge policy Marks the `delete_policy` parameter as deprecated, to be removed in Ansible 2.6. * Add version_added to purge_policy * Remove changing results based on verbosity What I really want is --diff support, and changing results based on verbosity is abnormal. * Ensure repository name is lowercase * Fix deprecation cycle to 4 releases * Use a YAML anchor for credentials * Remove filters from assertions * Add minimal permissions needed * Updating version_added and deprecation cycle The original PR sat while a few releases happened. * Bumping version added and deprecation version We missed the 2.8 release. * Removing bare except: This is not allowed and is generally bad practice. * Fix lint errors * update ansible release metadata * Use the new alias deprecation scheme This was added in the time the PR has been in development, so rework things to use it. * Add test coverage This makes sure that lifecycle_policy is produced when passed in. *Also a minor suggestion for simplification from PR. * Restore changes from 62871 lost in rebase * Add changelog * Remove version_added for new purge_policy option Per sanity test fail. --- .../48997-ecs_ecr-livecycle-policy.yml | 3 + .../testing_policies/container-policy.json | 3 + lib/ansible/modules/cloud/amazon/ecs_ecr.py | 171 ++++++++-- .../targets/ecs_ecr/defaults/main.yml | 12 + .../targets/ecs_ecr/tasks/main.yml | 314 ++++++++++++------ 5 files changed, 386 insertions(+), 117 deletions(-) create mode 100644 changelogs/fragments/48997-ecs_ecr-livecycle-policy.yml diff --git a/changelogs/fragments/48997-ecs_ecr-livecycle-policy.yml b/changelogs/fragments/48997-ecs_ecr-livecycle-policy.yml new file mode 100644 index 0000000000..3b31ed2ac1 --- /dev/null +++ b/changelogs/fragments/48997-ecs_ecr-livecycle-policy.yml @@ -0,0 +1,3 @@ +minor_changes: + - ecr_ecs - add support for lifecycle policies + - ecr_ecs - delete_policy has been renamed to purge_policy (with an alias) and will be removed in 2.14 diff --git a/hacking/aws_config/testing_policies/container-policy.json b/hacking/aws_config/testing_policies/container-policy.json index 1a6641f36b..41ef600d61 100644 --- a/hacking/aws_config/testing_policies/container-policy.json +++ b/hacking/aws_config/testing_policies/container-policy.json @@ -14,6 +14,9 @@ "Sid": "SpecifiedCodeRepositories", "Effect": "Allow", "Action": [ + "ecr:GetLifecyclePolicy", + "ecr:PutLifecyclePolicy", + "ecr:DeleteLifecyclePolicy", "ecr:GetRepositoryPolicy", "ecr:SetRepositoryPolicy", "ecr:DeleteRepository", diff --git a/lib/ansible/modules/cloud/amazon/ecs_ecr.py b/lib/ansible/modules/cloud/amazon/ecs_ecr.py index 10bc3020f6..4ebe74fc4c 100644 --- a/lib/ansible/modules/cloud/amazon/ecs_ecr.py +++ b/lib/ansible/modules/cloud/amazon/ecs_ecr.py @@ -44,12 +44,14 @@ options: required: false default: false type: bool - delete_policy: + purge_policy: description: - If yes, remove the policy from the repository. + - Alias C(delete_policy) has been deprecated and will be removed in Ansible 2.14 required: false default: false type: bool + aliases: [ delete_policy ] image_tag_mutability: description: - Configure whether repository should be mutable (ie. an already existing tag can be overwritten) or not. @@ -58,6 +60,19 @@ options: default: 'mutable' version_added: '2.10' type: str + lifecycle_policy: + description: + - JSON or dict that represents the new lifecycle policy + required: false + version_added: '2.10' + type: json + purge_lifecycle_policy: + description: + - if yes, remove the lifecycle policy from the repository + required: false + default: false + version_added: '2.10' + type: bool state: description: - Create or destroy the repository. @@ -107,13 +122,32 @@ EXAMPLES = ''' - name: delete-policy ecs_ecr: name: needs-no-policy - delete_policy: yes + purge_policy: yes - name: create immutable ecr-repo ecs_ecr: name: super/cool image_tag_mutability: immutable +- name: set-lifecycle-policy + ecs_ecr: + name: needs-lifecycle-policy + lifecycle_policy: + rules: + - rulePriority: 1 + description: new policy + selection: + tagStatus: untagged + countType: sinceImagePushed + countUnit: days + countNumber: 365 + action: + type: expire + +- name: purge-lifecycle-policy + ecs_ecr: + name: needs-no-lifecycle-policy + purge_lifecycle_policy: true ''' RETURN = ''' @@ -151,7 +185,8 @@ except ImportError: from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.ec2 import (HAS_BOTO3, boto3_conn, boto_exception, ec2_argument_spec, - get_aws_connection_info, compare_policies) + get_aws_connection_info, compare_policies, + sort_json_policy_dict) from ansible.module_utils.six import string_types @@ -286,6 +321,49 @@ class EcsEcr: repo['imageTagMutability'] = new_mutability_configuration return repo + def get_lifecycle_policy(self, registry_id, name): + try: + res = self.ecr.get_lifecycle_policy( + repositoryName=name, **build_kwargs(registry_id)) + text = res.get('lifecyclePolicyText') + return text and json.loads(text) + except ClientError as err: + code = err.response['Error'].get('Code', 'Unknown') + if code == 'LifecyclePolicyNotFoundException': + return None + raise + + def put_lifecycle_policy(self, registry_id, name, policy_text): + if not self.check_mode: + policy = self.ecr.put_lifecycle_policy( + repositoryName=name, + lifecyclePolicyText=policy_text, + **build_kwargs(registry_id)) + self.changed = True + return policy + else: + self.skipped = True + if self.get_repository(registry_id, name) is None: + printable = name + if registry_id: + printable = '{0}:{1}'.format(registry_id, name) + raise Exception( + 'could not find repository {0}'.format(printable)) + return + + def purge_lifecycle_policy(self, registry_id, name): + if not self.check_mode: + policy = self.ecr.delete_lifecycle_policy( + repositoryName=name, **build_kwargs(registry_id)) + self.changed = True + return policy + else: + policy = self.get_lifecycle_policy(registry_id, name) + if policy: + self.skipped = True + return policy + return None + def sort_lists_of_strings(policy): for statement_index in range(0, len(policy.get('Statement', []))): @@ -296,20 +374,35 @@ def sort_lists_of_strings(policy): return policy -def run(ecr, params, verbosity): +def run(ecr, params): # type: (EcsEcr, dict, int) -> Tuple[bool, dict] result = {} try: name = params['name'] state = params['state'] policy_text = params['policy'] - delete_policy = params['delete_policy'] + purge_policy = params['purge_policy'] registry_id = params['registry_id'] force_set_policy = params['force_set_policy'] image_tag_mutability = params['image_tag_mutability'].upper() + lifecycle_policy_text = params['lifecycle_policy'] + purge_lifecycle_policy = params['purge_lifecycle_policy'] - # If a policy was given, parse it - policy = policy_text and json.loads(policy_text) + # Parse policies, if they are given + try: + policy = policy_text and json.loads(policy_text) + except ValueError: + result['policy'] = policy_text + result['msg'] = 'Could not parse policy' + return False, result + + try: + lifecycle_policy = \ + lifecycle_policy_text and json.loads(lifecycle_policy_text) + except ValueError: + result['lifecycle_policy'] = lifecycle_policy_text + result['msg'] = 'Could not parse lifecycle_policy' + return False, result result['state'] = state result['created'] = False @@ -327,14 +420,42 @@ def run(ecr, params, verbosity): repo = ecr.put_image_tag_mutability(registry_id, name, image_tag_mutability) result['repository'] = repo - if delete_policy: + if purge_lifecycle_policy: + original_lifecycle_policy = \ + ecr.get_lifecycle_policy(registry_id, name) + + result['lifecycle_policy'] = None + + if original_lifecycle_policy: + ecr.purge_lifecycle_policy(registry_id, name) + result['changed'] = True + + elif lifecycle_policy_text is not None: + try: + lifecycle_policy = sort_json_policy_dict(lifecycle_policy) + result['lifecycle_policy'] = lifecycle_policy + + original_lifecycle_policy = ecr.get_lifecycle_policy( + registry_id, name) + + if original_lifecycle_policy: + original_lifecycle_policy = sort_json_policy_dict( + original_lifecycle_policy) + + if original_lifecycle_policy != lifecycle_policy: + ecr.put_lifecycle_policy(registry_id, name, + lifecycle_policy_text) + result['changed'] = True + except Exception: + # Some failure w/ the policy. It's helpful to know what the + # policy is. + result['lifecycle_policy'] = lifecycle_policy_text + raise + + if purge_policy: original_policy = ecr.get_repository_policy(registry_id, name) - if verbosity >= 2: - result['policy'] = None - - if verbosity >= 3: - result['original_policy'] = original_policy + result['policy'] = None if original_policy: ecr.delete_repository_policy(registry_id, name) @@ -345,17 +466,13 @@ def run(ecr, params, verbosity): # Sort any lists containing only string types policy = sort_lists_of_strings(policy) - if verbosity >= 2: - result['policy'] = policy + result['policy'] = policy + original_policy = ecr.get_repository_policy( registry_id, name) - if original_policy: original_policy = sort_lists_of_strings(original_policy) - if verbosity >= 3: - result['original_policy'] = original_policy - if compare_policies(original_policy, policy): ecr.set_repository_policy( registry_id, name, policy_text, force_set_policy) @@ -398,20 +515,28 @@ def main(): default='present'), force_set_policy=dict(required=False, type='bool', default=False), policy=dict(required=False, type='json'), - delete_policy=dict(required=False, type='bool'), image_tag_mutability=dict(required=False, choices=['mutable', 'immutable'], - default='mutable'))) + default='mutable'), + purge_policy=dict(required=False, type='bool', aliases=['delete_policy']), + lifecycle_policy=dict(required=False, type='json'), + purge_lifecycle_policy=dict(required=False, type='bool'))) module = AnsibleModule(argument_spec=argument_spec, supports_check_mode=True, mutually_exclusive=[ - ['policy', 'delete_policy']]) + ['policy', 'purge_policy'], + ['lifecycle_policy', 'purge_lifecycle_policy']]) + if module.params.get('delete_policy'): + module.deprecate( + 'The alias "delete_policy" has been deprecated and will be removed, ' + 'use "purge_policy" instead', + version='2.14') if not HAS_BOTO3: module.fail_json(msg='boto3 required for this module') ecr = EcsEcr(module) - passed, result = run(ecr, module.params, module._verbosity) + passed, result = run(ecr, module.params) if passed: module.exit_json(**result) diff --git a/test/integration/targets/ecs_ecr/defaults/main.yml b/test/integration/targets/ecs_ecr/defaults/main.yml index a244f90c31..4a9127942f 100644 --- a/test/integration/targets/ecs_ecr/defaults/main.yml +++ b/test/integration/targets/ecs_ecr/defaults/main.yml @@ -8,3 +8,15 @@ policy: - ecr:GetDownloadUrlForLayer - ecr:BatchGetImage - ecr:BatchCheckLayerAvailability + +lifecycle_policy: + rules: + - rulePriority: 1 + description: new policy + selection: + tagStatus: untagged + countType: sinceImagePushed + countUnit: days + countNumber: 365 + action: + type: expire diff --git a/test/integration/targets/ecs_ecr/tasks/main.yml b/test/integration/targets/ecs_ecr/tasks/main.yml index 7a97dce2ad..362cd8175d 100644 --- a/test/integration/targets/ecs_ecr/tasks/main.yml +++ b/test/integration/targets/ecs_ecr/tasks/main.yml @@ -4,13 +4,19 @@ - block: + - name: set connection information for all tasks + set_fact: + aws_connection_info: &aws_connection_info + aws_access_key: "{{ aws_access_key }}" + aws_secret_key: "{{ aws_secret_key }}" + security_token: "{{ security_token }}" + region: "{{ aws_region }}" + no_log: yes + - name: When creating with check mode ecs_ecr: name: '{{ ecr_name }}' - region: '{{ ec2_region }}' - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result check_mode: yes @@ -26,10 +32,7 @@ ecs_ecr: registry_id: 999999999999 name: '{{ ecr_name }}' - region: '{{ ec2_region }}' - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result ignore_errors: true @@ -43,10 +46,7 @@ - name: When creating a repository ecs_ecr: name: '{{ ecr_name }}' - region: '{{ ec2_region }}' - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result - name: it should change and create @@ -64,10 +64,7 @@ - name: When creating a repository that already exists in check mode ecs_ecr: name: '{{ ecr_name }}' - region: '{{ ec2_region }}' - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result check_mode: yes @@ -81,10 +78,7 @@ - name: When creating a repository that already exists ecs_ecr: name: '{{ ecr_name }}' - region: '{{ ec2_region }}' - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result - name: it should not change @@ -95,12 +89,9 @@ - name: When in check mode, and deleting a policy that does not exist ecs_ecr: - region: '{{ ec2_region }}' name: '{{ ecr_name }}' - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' - delete_policy: yes + purge_policy: yes + <<: *aws_connection_info register: result check_mode: yes @@ -113,12 +104,9 @@ - name: When in check mode, setting policy on a repository that has no policy ecs_ecr: - region: '{{ ec2_region }}' name: '{{ ecr_name }}' policy: '{{ policy }}' - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result check_mode: yes @@ -132,12 +120,9 @@ - name: When setting policy on a repository that has no policy ecs_ecr: - region: '{{ ec2_region }}' name: '{{ ecr_name }}' policy: '{{ policy }}' - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result - name: it should change and not create @@ -149,31 +134,43 @@ - name: When in check mode, and deleting a policy that exists ecs_ecr: - region: '{{ ec2_region }}' name: '{{ ecr_name }}' delete_policy: yes - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result check_mode: yes - - name: it should skip, change but not create + - name: it should skip, change but not create, have deprecations assert: that: - result is skipped - result is changed - not result.created + - result.deprecations - - name: When deleting a policy that exists + - name: When in check mode, and purging a policy that exists ecs_ecr: - region: '{{ ec2_region }}' name: '{{ ecr_name }}' - delete_policy: yes - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + purge_policy: yes + <<: *aws_connection_info + register: result + check_mode: yes + + - name: it should skip, change but not create, no deprecations + assert: + that: + - result is skipped + - result is changed + - not result.created + - result.deprecations is not defined + + + - name: When purging a policy that exists + ecs_ecr: + name: '{{ ecr_name }}' + purge_policy: yes + <<: *aws_connection_info register: result - name: it should change and not create @@ -185,12 +182,9 @@ - name: When setting a policy as a string ecs_ecr: - region: '{{ ec2_region }}' name: '{{ ecr_name }}' policy: '{{ policy | to_json }}' - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result - name: it should change and not create @@ -202,12 +196,9 @@ - name: When setting a policy to its current value ecs_ecr: - region: '{{ ec2_region }}' name: '{{ ecr_name }}' policy: '{{ policy }}' - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result - name: it should not change @@ -215,14 +206,10 @@ that: - result is not changed - - name: When omitting policy on a repository that has a policy ecs_ecr: - region: '{{ ec2_region }}' name: '{{ ecr_name }}' - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result - name: it should not change @@ -230,16 +217,12 @@ that: - result is not changed - - - name: When specifying both policy and delete_policy + - name: When specifying both policy and purge_policy ecs_ecr: - region: '{{ ec2_region }}' name: '{{ ecr_name }}' policy: '{{ policy }}' - delete_policy: yes - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + purge_policy: yes + <<: *aws_connection_info register: result ignore_errors: true @@ -251,12 +234,9 @@ - name: When specifying invalid JSON for policy ecs_ecr: - region: '{{ ec2_region }}' name: '{{ ecr_name }}' - policy_text: "Ceci n'est pas une JSON" - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + policy: "Ceci n'est pas une JSON" + <<: *aws_connection_info register: result ignore_errors: true @@ -266,14 +246,26 @@ - result is failed - - name: When in check mode, deleting a policy that exists + - name: When in check mode, and purging a lifecycle policy that does not exists ecs_ecr: name: '{{ ecr_name }}' - region: '{{ ec2_region }}' - state: absent - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + purge_lifecycle_policy: yes + <<: *aws_connection_info + register: result + check_mode: yes + + - name: it should not skip and not change + assert: + that: + - not result is skipped + - not result is changed + + + - name: When in check mode, setting lifecyle policy on a repository that has no policy + ecs_ecr: + name: '{{ ecr_name }}' + lifecycle_policy: '{{ lifecycle_policy }}' + <<: *aws_connection_info register: result check_mode: yes @@ -285,14 +277,157 @@ - not result.created - - name: When deleting a policy that exists + - name: When setting lifecycle policy on a repository that has no policy + ecs_ecr: + name: '{{ ecr_name }}' + lifecycle_policy: '{{ lifecycle_policy }}' + <<: *aws_connection_info + register: result + + - name: it should change and not create + assert: + that: + - result is changed + - not result.created + - result.lifecycle_policy is defined + - result.lifecycle_policy.rules|length == 1 + + + - name: When in check mode, and purging a lifecyle policy that exists + ecs_ecr: + name: '{{ ecr_name }}' + purge_lifecycle_policy: yes + <<: *aws_connection_info + register: result + check_mode: yes + + - name: it should skip, change but not create + assert: + that: + - result is skipped + - result is changed + - not result.created + + + - name: When purging a lifecycle policy that exists + ecs_ecr: + name: '{{ ecr_name }}' + purge_lifecycle_policy: yes + <<: *aws_connection_info + register: result + + - name: it should change and not create + assert: + that: + - result is changed + - not result.created + + + - name: When setting a lifecyle policy as a string + ecs_ecr: + name: '{{ ecr_name }}' + lifecycle_policy: '{{ lifecycle_policy | to_json }}' + <<: *aws_connection_info + register: result + + - name: it should change and not create + assert: + that: + - result is changed + - not result.created + + + - name: When setting a lifecycle policy to its current value + ecs_ecr: + name: '{{ ecr_name }}' + lifecycle_policy: '{{ lifecycle_policy }}' + <<: *aws_connection_info + register: result + + - name: it should not change + assert: + that: + - not result is changed + + + - name: When omitting lifecycle policy on a repository that has a policy + ecs_ecr: + name: '{{ ecr_name }}' + <<: *aws_connection_info + register: result + + - name: it should not change + assert: + that: + - not result is changed + + + - name: When specifying both lifecycle_policy and purge_lifecycle_policy + ecs_ecr: + name: '{{ ecr_name }}' + lifecycle_policy: '{{ lifecycle_policy }}' + purge_lifecycle_policy: yes + <<: *aws_connection_info + register: result + ignore_errors: true + + - name: it should fail + assert: + that: + - result is failed + + + - name: When specifying invalid JSON for lifecycle policy + ecs_ecr: + name: '{{ ecr_name }}' + lifecycle_policy: "Ceci n'est pas une JSON" + <<: *aws_connection_info + register: result + ignore_errors: true + + - name: it should fail + assert: + that: + - result is failed + + + - name: When specifying an invalid document for lifecycle policy + ecs_ecr: + name: '{{ ecr_name }}' + lifecycle_policy: + rules: + - invalid: "Ceci n'est pas une rule" + <<: *aws_connection_info + register: result + ignore_errors: true + + - name: it should fail + assert: + that: + - result is failed + + + - name: When in check mode, deleting a repository that exists ecs_ecr: name: '{{ ecr_name }}' - region: '{{ ec2_region }}' state: absent - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info + register: result + check_mode: yes + + - name: it should skip, change and not create + assert: + that: + - result is skipped + - result is changed + - not result.created + + + - name: When deleting a repository that exists + ecs_ecr: + name: '{{ ecr_name }}' + state: absent + <<: *aws_connection_info register: result - name: it should change @@ -301,14 +436,11 @@ - result is changed - - name: When in check mode, deleting a policy that does not exist + - name: When in check mode, deleting a repository that does not exist ecs_ecr: name: '{{ ecr_name }}' - region: '{{ ec2_region }}' state: absent - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result check_mode: yes @@ -319,14 +451,11 @@ - result is not changed - - name: When deleting a policy that does not exist + - name: When deleting a repository that does not exist ecs_ecr: name: '{{ ecr_name }}' - region: '{{ ec2_region }}' state: absent - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info register: result - name: it should not change @@ -410,8 +539,5 @@ - name: Delete lingering ECR repository ecs_ecr: name: '{{ ecr_name }}' - region: '{{ ec2_region }}' state: absent - ec2_access_key: '{{ec2_access_key}}' - ec2_secret_key: '{{ec2_secret_key}}' - security_token: '{{security_token}}' + <<: *aws_connection_info