From 53e3bb2b81d2ab56585d005b96757c2cf3c75247 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 7 Jun 2022 09:34:41 +0200 Subject: [PATCH] cloudtrail - add support for purge_tags (#1219) cloudtrail - add support for purge_tags SUMMARY Move to tagging docs fragment Update tagging code so that "tags" must be explicitly passed to remove tags add purge_tags parameter add resource_tags as an alias for tags Update tagging code so that tags are set as part of the create call rather than tagging after creation ISSUE TYPE Feature Pull Request COMPONENT NAME cloudtrail ADDITIONAL INFORMATION Note: tests are currently not run in CI. Reviewed-by: Joseph Torcasso --- .../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 == {}