From a1932c7b1787256a7a4031423121bd2405366620 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sun, 5 Jun 2022 16:13:12 +0200 Subject: [PATCH] cloudtrail - add support for purge_tags --- .../fragments/1219-cloudtrail-taging.yml | 4 + plugins/modules/cloudtrail.py | 115 ++++---- .../targets/cloudtrail/defaults/main.yml | 2 +- .../targets/cloudtrail/tasks/main.yml | 159 +---------- .../targets/cloudtrail/tasks/tagging.yml | 252 ++++++++++++++++++ 5 files changed, 313 insertions(+), 219 deletions(-) create mode 100644 changelogs/fragments/1219-cloudtrail-taging.yml create mode 100644 tests/integration/targets/cloudtrail/tasks/tagging.yml diff --git a/changelogs/fragments/1219-cloudtrail-taging.yml b/changelogs/fragments/1219-cloudtrail-taging.yml new file mode 100644 index 00000000000..f39b01369cb --- /dev/null +++ b/changelogs/fragments/1219-cloudtrail-taging.yml @@ -0,0 +1,4 @@ +minor_changes: +- cloudtrail - the default value for ``tags`` has been updated, to remove all tags the ``tags`` parameter must be explicitly set to the empty dict ``{}`` (https://github.com/ansible-collections/community.aws/pull/1219). +- cloudtrail - ``resource_tags`` has been added as an alias for the ``tags`` parameter (https://github.com/ansible-collections/community.aws/pull/1219). +- cloudtrail - updated to pass tags as part of the create API call rather than tagging the trail after creation (https://github.com/ansible-collections/community.aws/pull/1219). diff --git a/plugins/modules/cloudtrail.py b/plugins/modules/cloudtrail.py index d30466710eb..df95d5bfb7b 100644 --- a/plugins/modules/cloudtrail.py +++ b/plugins/modules/cloudtrail.py @@ -14,9 +14,9 @@ description: - Creates, deletes, or updates CloudTrail configuration. Ensures logging is also enabled. author: - - Ansible Core Team - - Ted Timmons (@tedder) - - Daniel Shepherd (@shepdelacreme) + - Ansible Core Team + - Ted Timmons (@tedder) + - Daniel Shepherd (@shepdelacreme) options: state: description: @@ -88,16 +88,13 @@ - The value can be an alias name prefixed by "alias/", a fully specified ARN to an alias, a fully specified ARN to a key, or a globally unique identifier. - See U(https://docs.aws.amazon.com/awscloudtrail/latest/userguide/encrypting-cloudtrail-log-files-with-aws-kms.html). type: str - tags: - description: - - A hash/dictionary of tags to be applied to the CloudTrail resource. - - Remove completely or specify an empty dictionary to remove all tags. - default: {} - type: dict +notes: + - The I(purge_tags) option was added in release 4.0.0 extends_documentation_fragment: -- amazon.aws.aws -- amazon.aws.ec2 + - amazon.aws.aws + - amazon.aws.ec2 + - amazon.aws.tags ''' @@ -251,11 +248,12 @@ except ImportError: pass # Handled by AnsibleAWSModule +from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict + from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import (camel_dict_to_snake_dict, - ansible_dict_to_boto3_tag_list, - boto3_tag_list_to_ansible_dict, - ) +from ansible_collections.amazon.aws.plugins.module_utils.tagging import ansible_dict_to_boto3_tag_list +from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict +from ansible_collections.amazon.aws.plugins.module_utils.tagging import compare_aws_tags def get_kms_key_aliases(module, client, keyId): @@ -293,7 +291,7 @@ def create_trail(module, client, ct_params): return resp -def tag_trail(module, client, tags, trail_arn, curr_tags=None, dry_run=False): +def tag_trail(module, client, tags, trail_arn, curr_tags=None, purge_tags=True): """ Creates, updates, removes tags on a CloudTrail resource @@ -304,45 +302,35 @@ def tag_trail(module, client, tags, trail_arn, curr_tags=None, dry_run=False): curr_tags : Dict of the current tags on resource, if any dry_run : true/false to determine if changes will be made if needed """ - adds = [] - removes = [] - updates = [] - changed = False - - if curr_tags is None: - # No current tags so just convert all to a tag list - adds = ansible_dict_to_boto3_tag_list(tags) - else: - curr_keys = set(curr_tags.keys()) - new_keys = set(tags.keys()) - add_keys = new_keys - curr_keys - remove_keys = curr_keys - new_keys - update_keys = dict() - for k in curr_keys.intersection(new_keys): - if curr_tags[k] != tags[k]: - update_keys.update({k: tags[k]}) - - adds = get_tag_list(add_keys, tags) - removes = get_tag_list(remove_keys, curr_tags) - updates = get_tag_list(update_keys, tags) - - if removes or updates: - changed = True - if not dry_run: - try: - client.remove_tags(ResourceId=trail_arn, TagsList=removes + updates) - except (BotoCoreError, ClientError) as err: - module.fail_json_aws(err, msg="Failed to remove tags from Trail") - if updates or adds: - changed = True - if not dry_run: - try: - client.add_tags(ResourceId=trail_arn, TagsList=updates + adds) - except (BotoCoreError, ClientError) as err: - module.fail_json_aws(err, msg="Failed to add tags to Trail") + if tags is None: + return False + + curr_tags = curr_tags or {} - return changed + tags_to_add, tags_to_remove = compare_aws_tags(curr_tags, tags, purge_tags=purge_tags) + if not tags_to_add and not tags_to_remove: + return False + + if module.check_mode: + return True + + if tags_to_remove: + remove = {k: curr_tags[k] for k in tags_to_remove} + tags_to_remove = ansible_dict_to_boto3_tag_list(remove) + try: + client.remove_tags(ResourceId=trail_arn, TagsList=tags_to_remove) + except (BotoCoreError, ClientError) as err: + module.fail_json_aws(err, msg="Failed to remove tags from Trail") + + if tags_to_add: + tags_to_add = ansible_dict_to_boto3_tag_list(tags_to_add) + try: + client.add_tags(ResourceId=trail_arn, TagsList=tags_to_add) + except (BotoCoreError, ClientError) as err: + module.fail_json_aws(err, msg="Failed to add tags to Trail") + + return True def get_tag_list(keys, tags): @@ -461,7 +449,8 @@ def main(): cloudwatch_logs_role_arn=dict(), cloudwatch_logs_log_group_arn=dict(), kms_key_id=dict(), - tags=dict(default={}, type='dict'), + tags=dict(type='dict', aliases=['resource_tags']), + purge_tags=dict(default=True, type='bool') ) required_if = [('state', 'present', ['s3_bucket_name']), ('state', 'enabled', ['s3_bucket_name'])] @@ -475,6 +464,7 @@ def main(): elif module.params['state'] in ('absent', 'disabled'): state = 'absent' tags = module.params['tags'] + purge_tags = module.params['purge_tags'] enable_logging = module.params['enable_logging'] ct_params = dict( Name=module.params['name'], @@ -586,13 +576,16 @@ def main(): set_logging(module, client, name=ct_params['Name'], action='stop') # Check if we need to update tags on resource - tag_dry_run = False - if module.check_mode: - tag_dry_run = True - tags_changed = tag_trail(module, client, tags=tags, trail_arn=trail['TrailARN'], curr_tags=trail['tags'], dry_run=tag_dry_run) + tags_changed = tag_trail(module, client, tags=tags, trail_arn=trail['TrailARN'], curr_tags=trail['tags'], + purge_tags=purge_tags) if tags_changed: + updated_tags = dict() + if not purge_tags: + updated_tags = trail['tags'] + updated_tags.update(tags) results['changed'] = True - trail['tags'] = tags + trail['tags'] = updated_tags + # Populate trail facts in output results['trail'] = camel_dict_to_snake_dict(trail, ignore_list=['tags']) @@ -601,10 +594,10 @@ def main(): results['changed'] = True results['exists'] = True if not module.check_mode: + if tags: + ct_params['TagList'] = ansible_dict_to_boto3_tag_list(tags) # If we aren't in check_mode then actually create it created_trail = create_trail(module, client, ct_params) - # Apply tags - tag_trail(module, client, tags=tags, trail_arn=created_trail['TrailARN']) # Get the trail status try: status_resp = client.get_trail_status(Name=created_trail['Name']) diff --git a/tests/integration/targets/cloudtrail/defaults/main.yml b/tests/integration/targets/cloudtrail/defaults/main.yml index 93cea45da99..2174b31aefa 100644 --- a/tests/integration/targets/cloudtrail/defaults/main.yml +++ b/tests/integration/targets/cloudtrail/defaults/main.yml @@ -2,7 +2,7 @@ cloudtrail_name: '{{ resource_prefix }}-cloudtrail' s3_bucket_name: '{{ resource_prefix }}-cloudtrail-bucket' kms_alias: '{{ resource_prefix }}-cloudtrail' sns_topic: '{{ resource_prefix }}-cloudtrail-notifications' -cloudtrail_prefix: 'test-prefix' +cloudtrail_prefix: 'ansible-test-prefix' cloudwatch_log_group: '{{ resource_prefix }}-cloudtrail' cloudwatch_role: '{{ resource_prefix }}-cloudtrail' cloudwatch_no_kms_role: '{{ resource_prefix }}-cloudtrail2' diff --git a/tests/integration/targets/cloudtrail/tasks/main.yml b/tests/integration/targets/cloudtrail/tasks/main.yml index 32688315a72..bbfa948bfd8 100644 --- a/tests/integration/targets/cloudtrail/tasks/main.yml +++ b/tests/integration/targets/cloudtrail/tasks/main.yml @@ -32,7 +32,7 @@ security_token: '{{ security_token | default(omit) }}' region: '{{ aws_region }}' # Add this as a default because we (almost) always need it - cloudtrail: + community.aws.cloudtrail: s3_bucket_name: '{{ s3_bucket_name }}' collections: - amazon.aws @@ -361,162 +361,7 @@ # ============================================================ - - name: 'Add Tag (CHECK MODE)' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - tags: - tag1: Value1 - register: output - check_mode: yes - - assert: - that: - - output is changed - - - name: 'Add Tag' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - tags: - tag1: Value1 - register: output - - assert: - that: - - output is changed - - output.trail.name == cloudtrail_name - - output.trail.tags | length == 1 - - '("tag1" in output.trail.tags) and (output.trail.tags["tag1"] == "Value1")' - - - name: 'Add Tag (no change)' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - tags: - tag1: Value1 - register: output - - assert: - that: - - output is not changed - - output.trail.name == cloudtrail_name - - output.trail.tags | length == 1 - - '("tag1" in output.trail.tags) and (output.trail.tags["tag1"] == "Value1")' - - - name: 'Change tags (CHECK MODE)' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - tags: - tag2: Value2 - register: output - check_mode: yes - - assert: - that: - - output is changed - - - name: 'Change tags' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - tags: - tag2: Value2 - register: output - - assert: - that: - - output is changed - - output.trail.name == cloudtrail_name - - output.trail.tags | length == 1 - - '("tag2" in output.trail.tags) and (output.trail.tags["tag2"] == "Value2")' - - - name: 'Change tags (no change)' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - tags: - tag2: Value2 - register: output - - assert: - that: - - output is not changed - - output.trail.name == cloudtrail_name - - output.trail.tags | length == 1 - - '("tag2" in output.trail.tags) and (output.trail.tags["tag2"] == "Value2")' - - - name: 'Change tags (CHECK MODE)' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - tags: - tag2: Value2 - Tag3: Value3 - register: output - check_mode: yes - - assert: - that: - - output is changed - - - name: 'Change tags' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - tags: - tag2: Value2 - Tag3: Value3 - register: output - - assert: - that: - - output is changed - - output.trail.name == cloudtrail_name - - output.trail.tags | length == 2 - - '("tag2" in output.trail.tags) and (output.trail.tags["tag2"] == "Value2")' - - '("Tag3" in output.trail.tags) and (output.trail.tags["Tag3"] == "Value3")' - - - name: 'Change tags (no change)' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - tags: - tag2: Value2 - Tag3: Value3 - register: output - - assert: - that: - - output is not changed - - output.trail.name == cloudtrail_name - - output.trail.tags | length == 2 - - '("tag2" in output.trail.tags) and (output.trail.tags["tag2"] == "Value2")' - - '("Tag3" in output.trail.tags) and (output.trail.tags["Tag3"] == "Value3")' - - - name: 'Remove tags (CHECK MODE)' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - register: output - check_mode: yes - - assert: - that: - - output is changed - - - name: 'Remove tags' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - register: output - - assert: - that: - - output is changed - - output.trail.name == cloudtrail_name - - output.trail.tags | length == 0 - - - name: 'Remove tags (no change)' - cloudtrail: - state: present - name: '{{ cloudtrail_name }}' - register: output - - assert: - that: - - output is not changed - - output.trail.name == cloudtrail_name - - output.trail.tags | length == 0 + - include_tasks: 'tagging.yml' # ============================================================ diff --git a/tests/integration/targets/cloudtrail/tasks/tagging.yml b/tests/integration/targets/cloudtrail/tasks/tagging.yml new file mode 100644 index 00000000000..082e831ea55 --- /dev/null +++ b/tests/integration/targets/cloudtrail/tasks/tagging.yml @@ -0,0 +1,252 @@ +- name: Tests relating to tagging cloudtrails + vars: + first_tags: + 'Key with Spaces': Value with spaces + CamelCaseKey: CamelCaseValue + pascalCaseKey: pascalCaseValue + snake_case_key: snake_case_value + second_tags: + 'New Key with Spaces': Value with spaces + NewCamelCaseKey: CamelCaseValue + newPascalCaseKey: pascalCaseValue + new_snake_case_key: snake_case_value + third_tags: + 'Key with Spaces': Value with spaces + CamelCaseKey: CamelCaseValue + pascalCaseKey: pascalCaseValue + snake_case_key: snake_case_value + 'New Key with Spaces': Updated Value with spaces + final_tags: + 'Key with Spaces': Value with spaces + CamelCaseKey: CamelCaseValue + pascalCaseKey: pascalCaseValue + snake_case_key: snake_case_value + 'New Key with Spaces': Updated Value with spaces + NewCamelCaseKey: CamelCaseValue + newPascalCaseKey: pascalCaseValue + new_snake_case_key: snake_case_value + # Mandatory settings + module_defaults: + community.aws.cloudtrail: + name: '{{ cloudtrail_name }}' + s3_bucket_name: '{{ s3_bucket_name }}' + state: present +# community.aws.cloudtrail_info: +# name: '{{ cloudtrail_name }}' + block: + + ### + + - name: test adding tags to cloudtrail (check mode) + cloudtrail: + tags: '{{ first_tags }}' + purge_tags: True + register: update_result + check_mode: yes + - name: assert that update succeeded + assert: + that: + - update_result is changed + + - name: test adding tags to cloudtrail + cloudtrail: + tags: '{{ first_tags }}' + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is changed + - update_result.trail.tags == first_tags + + - name: test adding tags to cloudtrail - idempotency (check mode) + cloudtrail: + tags: '{{ first_tags }}' + purge_tags: True + register: update_result + check_mode: yes + - name: assert that update succeeded + assert: + that: + - update_result is not changed + + - name: test adding tags to cloudtrail - idempotency + cloudtrail: + tags: '{{ first_tags }}' + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is not changed + - update_result.trail.tags == first_tags + + ### + + - name: test updating tags with purge on cloudtrail (check mode) + cloudtrail: + tags: '{{ second_tags }}' + purge_tags: True + register: update_result + check_mode: yes + - name: assert that update succeeded + assert: + that: + - update_result is changed + + - name: test updating tags with purge on cloudtrail + cloudtrail: + tags: '{{ second_tags }}' + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is changed + - update_result.trail.tags == second_tags + + - name: test updating tags with purge on cloudtrail - idempotency (check mode) + cloudtrail: + tags: '{{ second_tags }}' + purge_tags: True + register: update_result + check_mode: yes + - name: assert that update succeeded + assert: + that: + - update_result is not changed + + - name: test updating tags with purge on cloudtrail - idempotency + cloudtrail: + tags: '{{ second_tags }}' + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is not changed + - update_result.trail.tags == second_tags + + ### + + - name: test updating tags without purge on cloudtrail (check mode) + cloudtrail: + tags: '{{ third_tags }}' + purge_tags: False + register: update_result + check_mode: yes + - name: assert that update succeeded + assert: + that: + - update_result is changed + + - name: test updating tags without purge on cloudtrail + cloudtrail: + tags: '{{ third_tags }}' + purge_tags: False + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is changed + - update_result.trail.tags == final_tags + + - name: test updating tags without purge on cloudtrail - idempotency (check mode) + cloudtrail: + tags: '{{ third_tags }}' + purge_tags: False + register: update_result + check_mode: yes + - name: assert that update succeeded + assert: + that: + - update_result is not changed + + - name: test updating tags without purge on cloudtrail - idempotency + cloudtrail: + tags: '{{ third_tags }}' + purge_tags: False + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is not changed + - update_result.trail.tags == final_tags + +# ### +# +# - name: test that cloudtrail_info returns the tags +# cloudtrail_info: +# register: tag_info +# - name: assert tags present +# assert: +# that: +# - tag_info.trail.tags == final_tags +# +# ### + + - name: test no tags param cloudtrail (check mode) + cloudtrail: {} + register: update_result + check_mode: yes + - name: assert no change + assert: + that: + - update_result is not changed + - update_result.trail.tags == final_tags + + + - name: test no tags param cloudtrail + cloudtrail: {} + register: update_result + - name: assert no change + assert: + that: + - update_result is not changed + - update_result.trail.tags == final_tags + + ### + + - name: test removing tags from cloudtrail (check mode) + cloudtrail: + tags: {} + purge_tags: True + register: update_result + check_mode: yes + - name: assert that update succeeded + assert: + that: + - update_result is changed + + - name: test removing tags from cloudtrail + cloudtrail: + tags: {} + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is changed + - update_result.trail.tags == {} + + - name: test removing tags from cloudtrail - idempotency (check mode) + cloudtrail: + tags: {} + purge_tags: True + register: update_result + check_mode: yes + - name: assert that update succeeded + assert: + that: + - update_result is not changed + + - name: test removing tags from cloudtrail - idempotency + cloudtrail: + tags: {} + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is not changed + - update_result.trail.tags == {}