From 8c20df05c2c2909ca8a718d9db6ec4d9459c08b1 Mon Sep 17 00:00:00 2001 From: Mauricio Teixeira <1847440+badnetmask@users.noreply.github.com> Date: Mon, 3 May 2021 19:21:46 -0400 Subject: [PATCH 1/7] implement adding tags to brand new zones --- plugins/module_utils/route53.py | 42 +++++++++++++++++++++++++++++++++ plugins/modules/route53_zone.py | 42 ++++++++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 plugins/module_utils/route53.py diff --git a/plugins/module_utils/route53.py b/plugins/module_utils/route53.py new file mode 100644 index 00000000000..8830332c0cd --- /dev/null +++ b/plugins/module_utils/route53.py @@ -0,0 +1,42 @@ +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_tag_list +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import compare_aws_tags + + +def manage_tags(module, client, resource_type, resource_spec, resource_id): + tagset = client.list_tags_for_resource( + ResourceType=resource_type, + ResourceId=resource_id, + ) + old_tags = boto3_tag_list_to_ansible_dict(tagset['ResourceTagSet']['Tags']) + new_tags = {} + if resource_spec['tags']: + new_tags = resource_spec['tags'] + tags_to_set, tags_to_delete = compare_aws_tags( + old_tags, new_tags, + purge_tags=resource_spec['purge_tags'], + ) + # boto3 does not provide create/remove functions for tags in Route 53, + # neither it works with empty values as parameters to change_tags_for_resource, + # so we need to call the change function twice + if tags_to_set: + client.change_tags_for_resource( + ResourceType=resource_type, + ResourceId=resource_id, + AddTags=ansible_dict_to_boto3_tag_list(tags_to_set), + ) + if tags_to_delete: + client.change_tags_for_resource( + ResourceType=resource_type, + ResourceId=resource_id, + RemoveTagKeys=tags_to_delete, + ) + if tags_to_set or tags_to_delete: + return True + return False diff --git a/plugins/modules/route53_zone.py b/plugins/modules/route53_zone.py index cdc5538c027..6bc2443f2bd 100644 --- a/plugins/modules/route53_zone.py +++ b/plugins/modules/route53_zone.py @@ -5,7 +5,6 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type - DOCUMENTATION = ''' module: route53_zone short_description: add or delete Route53 zones @@ -47,6 +46,18 @@ - The reusable delegation set ID to be associated with the zone. - Note that you can't associate a reusable delegation set with a private hosted zone. type: str + tags: + description: + - A hash/dictionary of tags to add to the new instance or to add/remove from an existing one. + type: dict + version_added: 2.1.0 + purge_tags: + description: + - Delete any tags not specified in the task that are on the zone. + This means you have to specify all the desired tags on each task affecting a zone. + default: false + type: bool + version_added: 2.1.0 extends_documentation_fragment: - amazon.aws.aws - amazon.aws.ec2 @@ -77,6 +88,21 @@ zone: example.com comment: reusable delegation set example delegation_set_id: A1BCDEF2GHIJKL + +- name: create a public zone with tags + community.aws.route53_zone: + zone: example.com + comment: this is an example + tags: + Owner: Ansible Team + +- name: modify a public zone, removing all previous tags and adding a new one + community.aws.route53_zone: + zone: example.com + comment: this is an example + tags: + Support: Ansible Community + purge_tags: true ''' RETURN = ''' @@ -115,10 +141,15 @@ returned: for public hosted zones, if they have been associated with a reusable delegation set type: str sample: "A1BCDEF2GHIJKL" +tags: + description: tags associated with the zone + returned: when tags are defined + type: dict ''' import time from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule +from ansible_collections.community.aws.plugins.module_utils.route53 import manage_tags try: from botocore.exceptions import BotoCoreError, ClientError @@ -150,6 +181,8 @@ def create(module, client, matching_zones): vpc_region = module.params.get('vpc_region') comment = module.params.get('comment') delegation_set_id = module.params.get('delegation_set_id') + tags = module.params.get('tags') + purge_tags = module.params.get('purge_tags') if not zone_in.endswith('.'): zone_in += "." @@ -164,6 +197,8 @@ def create(module, client, matching_zones): 'name': zone_in, 'delegation_set_id': delegation_set_id, 'zone_id': None, + 'tags': tags, + 'purge_tags': purge_tags, } if private_zone: @@ -287,6 +322,9 @@ def create_or_update_public(module, client, matching_zones, record): record['name'] = zone_details['Name'] record['delegation_set_id'] = zone_delegation_set_details.get('Id', '').replace('/delegationset/', '') + if record['tags'] or record['purge_tags']: + changed = manage_tags(module, client, 'hostedzone', record, zone_details['Id'].replace('/hostedzone/', '')) + return changed, record @@ -394,6 +432,8 @@ def main(): comment=dict(default=''), hosted_zone_id=dict(), delegation_set_id=dict(), + tags=dict(type='dict'), + purge_tags=dict(type='bool', default=False), ) mutually_exclusive = [ From 77236ee3d2af5149f700be914d5e32dc89015952 Mon Sep 17 00:00:00 2001 From: Mauricio Teixeira <1847440+badnetmask@users.noreply.github.com> Date: Tue, 4 May 2021 16:21:21 -0400 Subject: [PATCH 2/7] update route53 integration tests to cover tags --- .../targets/route53/tasks/main.yml | 6 +++ .../targets/route53_zone/tasks/main.yml | 37 ++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/integration/targets/route53/tasks/main.yml b/tests/integration/targets/route53/tasks/main.yml index ae80586dbe8..c5312437f0b 100644 --- a/tests/integration/targets/route53/tasks/main.yml +++ b/tests/integration/targets/route53/tasks/main.yml @@ -29,12 +29,15 @@ route53_zone: zone: '{{ zone_one }}' comment: 'Created in Ansible test {{ resource_prefix }}' + tags: + TestTag: '{{ resource_prefix }}.z1' register: z1 - assert: that: - z1 is success - z1 is changed - "z1.comment == 'Created in Ansible test {{ resource_prefix }}'" + - "z1.tags.TestTag == '{{ resource_prefix }}.z1'" - name: 'Get zone details' route53_info: @@ -53,12 +56,15 @@ vpc_id: '{{ vpc.vpc.id }}' vpc_region: '{{ aws_region }}' comment: Created in Ansible test {{ resource_prefix }} + tags: + TestTag: '{{ resource_prefix }}.z2' register: z2 - assert: that: - z2 is success - z2 is changed - "z2.comment == 'Created in Ansible test {{ resource_prefix }}'" + - "z2.tags.TestTag == '{{ resource_prefix }}.z2'" - name: Get zone details route53_info: diff --git a/tests/integration/targets/route53_zone/tasks/main.yml b/tests/integration/targets/route53_zone/tasks/main.yml index 5fe154a6712..db430df5efc 100644 --- a/tests/integration/targets/route53_zone/tasks/main.yml +++ b/tests/integration/targets/route53_zone/tasks/main.yml @@ -27,6 +27,8 @@ zone: "{{ resource_prefix }}.public" comment: original comment state: present + tags: + TestTag: "{{ resource_prefix }}" register: output - assert: @@ -34,6 +36,7 @@ - output.changed - output.comment == 'original comment' - output.name == '{{ resource_prefix }}.public.' + - output.tags.TestTag == '{{ resource_prefix }}' - not output.private_zone # ============================================================ @@ -42,6 +45,8 @@ zone: "{{ resource_prefix }}.check.public" comment: original comment state: present + tags: + TestTag: "{{ resource_prefix }}" register: output check_mode: yes @@ -50,6 +55,7 @@ - output.changed - output.comment == 'original comment' - output.name == '{{ resource_prefix }}.check.public.' + - output.tags.TestTag == '{{ resource_prefix }}' - not output.private_zone # ============================================================ @@ -58,6 +64,8 @@ zone: "{{ resource_prefix }}.public" comment: original comment state: present + tags: + TestTag: "{{ resource_prefix }}" register: output - assert: @@ -65,6 +73,7 @@ - not output.changed - output.comment == 'original comment' - output.name == '{{ resource_prefix }}.public.' + - output.tags.TestTag == '{{ resource_prefix }}' - not output.private_zone - name: Do an idemptotent update of a public zone (CHECK MODE) @@ -72,6 +81,8 @@ zone: "{{ resource_prefix }}.public" comment: original comment state: present + tags: + TestTag: "{{ resource_prefix }}" register: output check_mode: yes @@ -80,26 +91,47 @@ - not output.changed - output.comment == 'original comment' - output.name == '{{ resource_prefix }}.public.' + - output.tags.TestTag == '{{ resource_prefix }}' - not output.private_zone # ============================================================ - - name: Update comment of a public zone + - name: Modify tags on a public zone + route53_zone: + zone: "{{ resource_prefix }}.public" + comment: original comment + state: present + tags: + AnotherTag: "{{ resource_prefix }}.anothertag" + purge_tags: true + register: output + + - assert: + that: + - output.changed + - "'TestTag' not in output.tags" + - output.tags.AnotherTag == '{{ resource_prefix }}.anothertag' + + # ============================================================ + - name: Update comment and remove tags of a public zone route53_zone: zone: "{{ resource_prefix }}.public" comment: updated comment state: present + purge_tags: true register: output - assert: that: - output.changed - output.result.comment == "updated comment" + - not output.tags - - name: Update comment of a public zone (CHECK MODE) + - name: Update comment and remove tags of a public zone (CHECK MODE) route53_zone: zone: "{{ resource_prefix }}.public" comment: updated comment for check state: present + purge_tags: true register: output check_mode: yes @@ -107,6 +139,7 @@ that: - output.changed - output.result.comment == "updated comment for check" + - not output.tags # ============================================================ - name: Delete public zone (CHECK MODE) From 55fc45fc0e4b6dc50311f88972eb684b59b920f1 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 12 Oct 2021 10:56:09 +0200 Subject: [PATCH 3/7] Make sure we don't update tags in check_mode --- plugins/module_utils/route53.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/plugins/module_utils/route53.py b/plugins/module_utils/route53.py index 8830332c0cd..15b9f63cc3e 100644 --- a/plugins/module_utils/route53.py +++ b/plugins/module_utils/route53.py @@ -22,6 +22,13 @@ def manage_tags(module, client, resource_type, resource_spec, resource_id): old_tags, new_tags, purge_tags=resource_spec['purge_tags'], ) + + if not tags_to_set and not tags_to_delete: + return False + + if module.check_mode: + return True + # boto3 does not provide create/remove functions for tags in Route 53, # neither it works with empty values as parameters to change_tags_for_resource, # so we need to call the change function twice @@ -37,6 +44,4 @@ def manage_tags(module, client, resource_type, resource_spec, resource_id): ResourceId=resource_id, RemoveTagKeys=tags_to_delete, ) - if tags_to_set or tags_to_delete: - return True - return False + return True From 26bebba5074378d88c138d5c954245a091a40e2f Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 12 Oct 2021 13:52:39 +0200 Subject: [PATCH 4/7] Rework route53 tagging logic a little --- plugins/module_utils/route53.py | 72 +++++++++++-------- plugins/modules/route53_zone.py | 14 ++-- .../targets/route53_zone/tasks/main.yml | 2 + 3 files changed, 54 insertions(+), 34 deletions(-) diff --git a/plugins/module_utils/route53.py b/plugins/module_utils/route53.py index 15b9f63cc3e..3d21d8b0802 100644 --- a/plugins/module_utils/route53.py +++ b/plugins/module_utils/route53.py @@ -4,44 +4,58 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_tag_list -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import compare_aws_tags - - -def manage_tags(module, client, resource_type, resource_spec, resource_id): - tagset = client.list_tags_for_resource( - ResourceType=resource_type, - ResourceId=resource_id, - ) - old_tags = boto3_tag_list_to_ansible_dict(tagset['ResourceTagSet']['Tags']) - new_tags = {} - if resource_spec['tags']: - new_tags = resource_spec['tags'] - tags_to_set, tags_to_delete = compare_aws_tags( - old_tags, new_tags, - purge_tags=resource_spec['purge_tags'], - ) - - if not tags_to_set and not tags_to_delete: +try: + import botocore +except ImportError: + pass # caught by AnsibleAWSModule + +from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code +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 manage_tags(module, client, resource_type, resource_id, new_tags, purge_tags): + old_tags = get_tags(module, client, resource_type, resource_id) + tags_to_set, tags_to_delete = compare_aws_tags(old_tags, new_tags, purge_tags=purge_tags) + + change_params = dict() + if tags_to_set: + change_params['AddTags'] = ansible_dict_to_boto3_tag_list(tags_to_set) + if tags_to_delete: + change_params['RemoveTagKeys'] = tags_to_delete + + if not change_params: return False if module.check_mode: return True - # boto3 does not provide create/remove functions for tags in Route 53, - # neither it works with empty values as parameters to change_tags_for_resource, - # so we need to call the change function twice - if tags_to_set: + try: client.change_tags_for_resource( ResourceType=resource_type, ResourceId=resource_id, - AddTags=ansible_dict_to_boto3_tag_list(tags_to_set), + **change_params ) - if tags_to_delete: - client.change_tags_for_resource( + except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + module.fail_json_aws(e, msg='Failed to update tags on {0}'.format(resource_type), + resource_id=resource_id, change_params=change_params) + return True + + +def get_tags(module, client, resource_type, resource_id): + try: + tagset = client.list_tags_for_resource( ResourceType=resource_type, ResourceId=resource_id, - RemoveTagKeys=tags_to_delete, ) - return True + except is_boto3_error_code('NoSuchHealthCheck'): + return {} + except is_boto3_error_code('NoSuchHostedZone'): # pylint: disable=duplicate-except + return {} + except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: # pylint: disable=duplicate-except + module.fail_json_aws(e, msg='Failed to fetch tags on {0}'.format(resource_type), + resource_id=resource_id) + + tags = boto3_tag_list_to_ansible_dict(tagset['ResourceTagSet']['Tags']) + return tags diff --git a/plugins/modules/route53_zone.py b/plugins/modules/route53_zone.py index 6bc2443f2bd..334e6d62718 100644 --- a/plugins/modules/route53_zone.py +++ b/plugins/modules/route53_zone.py @@ -150,6 +150,7 @@ import time from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule from ansible_collections.community.aws.plugins.module_utils.route53 import manage_tags +from ansible_collections.community.aws.plugins.module_utils.route53 import get_tags try: from botocore.exceptions import BotoCoreError, ClientError @@ -197,8 +198,6 @@ def create(module, client, matching_zones): 'name': zone_in, 'delegation_set_id': delegation_set_id, 'zone_id': None, - 'tags': tags, - 'purge_tags': purge_tags, } if private_zone: @@ -206,6 +205,14 @@ def create(module, client, matching_zones): else: changed, result = create_or_update_public(module, client, matching_zones, record) + zone_id = result.get('zone_id') + if zone_id: + if tags is not None: + changed |= manage_tags(module, client, 'hostedzone', zone_id, tags, purge_tags) + result['tags'] = get_tags(module, client, 'hostedzone', zone_id) + else: + result['tags'] = tags + return changed, result @@ -322,9 +329,6 @@ def create_or_update_public(module, client, matching_zones, record): record['name'] = zone_details['Name'] record['delegation_set_id'] = zone_delegation_set_details.get('Id', '').replace('/delegationset/', '') - if record['tags'] or record['purge_tags']: - changed = manage_tags(module, client, 'hostedzone', record, zone_details['Id'].replace('/hostedzone/', '')) - return changed, record diff --git a/tests/integration/targets/route53_zone/tasks/main.yml b/tests/integration/targets/route53_zone/tasks/main.yml index db430df5efc..9b02fd607af 100644 --- a/tests/integration/targets/route53_zone/tasks/main.yml +++ b/tests/integration/targets/route53_zone/tasks/main.yml @@ -118,6 +118,7 @@ comment: updated comment state: present purge_tags: true + tags: {} register: output - assert: @@ -132,6 +133,7 @@ comment: updated comment for check state: present purge_tags: true + tags: {} register: output check_mode: yes From d372b88e7d92992a1909848df9839c20676bad65 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 12 Oct 2021 13:55:08 +0200 Subject: [PATCH 5/7] Add a snake_case tag --- tests/integration/targets/route53_zone/tasks/main.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/integration/targets/route53_zone/tasks/main.yml b/tests/integration/targets/route53_zone/tasks/main.yml index 9b02fd607af..79b87fc47fd 100644 --- a/tests/integration/targets/route53_zone/tasks/main.yml +++ b/tests/integration/targets/route53_zone/tasks/main.yml @@ -66,6 +66,7 @@ state: present tags: TestTag: "{{ resource_prefix }}" + another_tag: "{{ resource_prefix }} again" register: output - assert: @@ -74,6 +75,7 @@ - output.comment == 'original comment' - output.name == '{{ resource_prefix }}.public.' - output.tags.TestTag == '{{ resource_prefix }}' + - output.tags.another_tag == '{{ resource_prefix }} again' - not output.private_zone - name: Do an idemptotent update of a public zone (CHECK MODE) @@ -83,6 +85,7 @@ state: present tags: TestTag: "{{ resource_prefix }}" + another_tag: "{{ resource_prefix }} again" register: output check_mode: yes @@ -92,6 +95,7 @@ - output.comment == 'original comment' - output.name == '{{ resource_prefix }}.public.' - output.tags.TestTag == '{{ resource_prefix }}' + - output.tags.another_tag == '{{ resource_prefix }} again' - not output.private_zone # ============================================================ From 836eb9f07788edef06a886225e2d63d4720a9979 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Thu, 14 Oct 2021 10:55:29 +0200 Subject: [PATCH 6/7] Fix tests --- tests/integration/targets/route53_zone/tasks/main.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/integration/targets/route53_zone/tasks/main.yml b/tests/integration/targets/route53_zone/tasks/main.yml index 79b87fc47fd..9731c4a5c46 100644 --- a/tests/integration/targets/route53_zone/tasks/main.yml +++ b/tests/integration/targets/route53_zone/tasks/main.yml @@ -29,6 +29,7 @@ state: present tags: TestTag: "{{ resource_prefix }}" + another_tag: "{{ resource_prefix }} again" register: output - assert: @@ -37,6 +38,7 @@ - output.comment == 'original comment' - output.name == '{{ resource_prefix }}.public.' - output.tags.TestTag == '{{ resource_prefix }}' + - output.tags.another_tag == '{{ resource_prefix }} again' - not output.private_zone # ============================================================ @@ -47,6 +49,7 @@ state: present tags: TestTag: "{{ resource_prefix }}" + another_tag: "{{ resource_prefix }} again" register: output check_mode: yes @@ -56,6 +59,7 @@ - output.comment == 'original comment' - output.name == '{{ resource_prefix }}.check.public.' - output.tags.TestTag == '{{ resource_prefix }}' + - output.tags.another_tag == '{{ resource_prefix }} again' - not output.private_zone # ============================================================ From b1c2a47fed56381e1987ba5c5076ffb058d36ff7 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Thu, 14 Oct 2021 10:55:41 +0200 Subject: [PATCH 7/7] changelog --- changelogs/fragments/565-route53-tagging.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/565-route53-tagging.yml diff --git a/changelogs/fragments/565-route53-tagging.yml b/changelogs/fragments/565-route53-tagging.yml new file mode 100644 index 00000000000..0a46ad35e52 --- /dev/null +++ b/changelogs/fragments/565-route53-tagging.yml @@ -0,0 +1,2 @@ +minor_changes: +- route53_zone - add support for tagging Route 53 zones (https://github.com/ansible-collections/community.aws/pull/565).