From 3cfa776243c02f89a1621eab7c935af834b04f90 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 10 Sep 2024 15:44:54 -0700 Subject: [PATCH 01/12] add support of updating reverse dns record for eip --- plugins/modules/ec2_eip.py | 73 ++++++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 10 deletions(-) diff --git a/plugins/modules/ec2_eip.py b/plugins/modules/ec2_eip.py index 10a0ef463de..62d77f591e7 100644 --- a/plugins/modules/ec2_eip.py +++ b/plugins/modules/ec2_eip.py @@ -76,6 +76,24 @@ - Allocates the new Elastic IP from the provided public IPv4 pool (BYOIP) only applies to newly allocated Elastic IPs, isn't validated when O(reuse_existing_ip_allowed=true). type: str + update_reverse_dns_record: + description: Whether to update the reverse DNS record of ec2 Elastic IP Address (eip) + required: false + type: bool + allocation_id: + description: The allocation ID of ec2 EIP. + required: false + type: str + domain_name: + description: The domain name to modify for the IP address. + required: false + type: str + dry_run: + description: + - Checks whether you have the required permissions for the action, without actually making the request, and provides an error response. + - If you have the required permissions, the error response is DryRunOperation. Otherwise, it is UnauthorizedOperation. + required: false + type: bool extends_documentation_fragment: - amazon.aws.common.modules - amazon.aws.region.modules @@ -201,6 +219,12 @@ tag_name: reserved_for tag_value: "{{ inventory_hostname }}" public_ipv4_pool: ipv4pool-ec2-0588c9b75a25d1a02 + +- name: Modify reverse DNS record of EIP + amazon.aws.ec2_eip: + update_reverse_dns: true + allocation_id: eipalloc-00a61ec1234567890 + domain_name: example.com """ RETURN = r""" @@ -226,6 +250,7 @@ from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AnsibleEC2Error from ansible_collections.amazon.aws.plugins.module_utils.ec2 import allocate_address as allocate_ip_address from ansible_collections.amazon.aws.plugins.module_utils.ec2 import associate_address +from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_addresses from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_instances from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_network_interfaces @@ -466,9 +491,30 @@ def ensure_present( return result +def update_reverse_dns_record_of_eip(client, module: AnsibleAWSModule): + changed = False + allocation_id = module.params.get("allocation_id") + domain_name = module.params.get("domain_name") + dry_run = module.params.get("dry_run") + + try: + update_reverse_dns_record_result = client.modify_address_attribute(AllocationId=allocation_id, DomainName=domain_name, DryRun=dry_run) + changed = True + except AnsibleEC2Error as e: + module.fail_json_aws_error(e) + + if "ResponseMetadata" in update_reverse_dns_record_result: + del update_reverse_dns_record_result["ResponseMetadata"] + + return {"changed": changed, "update_reverse_dns_record_result": camel_dict_to_snake_dict(update_reverse_dns_record_result)} + + def main(): argument_spec = dict( + allocation_id=dict(required=False, type="str"), device_id=dict(required=False), + domain_name=dict(required=False, type="str"), + dry_run=dict(default=False, type="bool"), public_ip=dict(required=False, aliases=["ip"]), state=dict(required=False, default="present", choices=["present", "absent"]), in_vpc=dict(required=False, type="bool", default=False), @@ -480,6 +526,7 @@ def main(): purge_tags=dict(required=False, type="bool", default=True), tag_name=dict(), tag_value=dict(), + update_reverse_dns_record=dict(required=False, type="bool"), public_ipv4_pool=dict(), ) @@ -490,6 +537,9 @@ def main(): "private_ip_address": ["device_id"], "tag_value": ["tag_name"], }, + required_if=[ + ('update_reverse_dns_record', True, ('allocation_id', 'domain_name')), + ], ) ec2 = module.client("ec2") @@ -500,16 +550,19 @@ def main(): is_instance = check_is_instance(module) - try: - # Find existing address - address = find_address(ec2, public_ip, device_id, is_instance) - if state == "present": - result = ensure_present(ec2, module, address, is_instance) - else: - result = ensure_absent(ec2, module, address, is_instance) - - except AnsibleEC2Error as e: - module.fail_json_aws_error(e) + if module.params.get("update_reverse_dns_record") == True: + result = update_reverse_dns_record_of_eip(ec2, module) + else: + try: + # Find existing address + address = find_address(ec2, public_ip, device_id, is_instance) + if state == "present": + result = ensure_present(ec2, module, address, is_instance) + else: + result = ensure_absent(ec2, module, address, is_instance) + + except AnsibleEC2Error as e: + module.fail_json_aws_error(e) module.exit_json(**result) From 7711f1fdb267f4dd9a6809259484565fca5a4a60 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 10 Sep 2024 16:03:13 -0700 Subject: [PATCH 02/12] add return block and changelog fragment --- ...d-support-to-update-reverse-dns-record.yml | 3 + plugins/modules/ec2_eip.py | 77 +++++++++++++++---- 2 files changed, 64 insertions(+), 16 deletions(-) create mode 100644 changelogs/fragments/2292-ec2_eip-add-support-to-update-reverse-dns-record.yml diff --git a/changelogs/fragments/2292-ec2_eip-add-support-to-update-reverse-dns-record.yml b/changelogs/fragments/2292-ec2_eip-add-support-to-update-reverse-dns-record.yml new file mode 100644 index 00000000000..01961e06ecb --- /dev/null +++ b/changelogs/fragments/2292-ec2_eip-add-support-to-update-reverse-dns-record.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - ec2_eip - Add support to update reverse DNS record of an EIP (https://github.com/ansible-collections/amazon.aws/pull/2292). diff --git a/plugins/modules/ec2_eip.py b/plugins/modules/ec2_eip.py index 62d77f591e7..e6ed7cb7efc 100644 --- a/plugins/modules/ec2_eip.py +++ b/plugins/modules/ec2_eip.py @@ -238,6 +238,46 @@ returned: on success type: str sample: 52.88.159.209 +update_reverse_dns_record_result: + description: Information about result of update reverse dns record operation. + returned: When O(update_reverse_dns_record=true). + type: dict + contains: + address: + description: Information about the Elastic IP address. + returned: always + type: dict + contains: + allocation_id: + description: The allocation ID. + returned: always + type: str + sample: "eipalloc-00a11aa111aaa1a11" + ptr_record: + description: The pointer (PTR) record for the IP address. + returned: always + type: str + sample: "ec2-11-22-33-44.us-east-2.compute.amazonaws.com." + ptr_record_update: + description: The updated PTR record for the IP address. + returned: always + type: dict + contains: + status: + description: The status of the PTR record update. + returned: always + type: str + sample: "PENDING" + value: + description: The value for the PTR record update. + returned: always + type: str + sample: "example.com" + public_ip: + description: The public IP address. + returned: always + type: str + sample: "11.22.33.44" """ from typing import Any @@ -491,14 +531,16 @@ def ensure_present( return result -def update_reverse_dns_record_of_eip(client, module: AnsibleAWSModule): +def update_reverse_dns_record_of_eip(client, module: AnsibleAWSModule): changed = False allocation_id = module.params.get("allocation_id") domain_name = module.params.get("domain_name") dry_run = module.params.get("dry_run") try: - update_reverse_dns_record_result = client.modify_address_attribute(AllocationId=allocation_id, DomainName=domain_name, DryRun=dry_run) + update_reverse_dns_record_result = client.modify_address_attribute( + AllocationId=allocation_id, DomainName=domain_name, DryRun=dry_run + ) changed = True except AnsibleEC2Error as e: module.fail_json_aws_error(e) @@ -506,7 +548,10 @@ def update_reverse_dns_record_of_eip(client, module: AnsibleAWSModule): if "ResponseMetadata" in update_reverse_dns_record_result: del update_reverse_dns_record_result["ResponseMetadata"] - return {"changed": changed, "update_reverse_dns_record_result": camel_dict_to_snake_dict(update_reverse_dns_record_result)} + return { + "changed": changed, + "update_reverse_dns_record_result": camel_dict_to_snake_dict(update_reverse_dns_record_result), + } def main(): @@ -514,7 +559,7 @@ def main(): allocation_id=dict(required=False, type="str"), device_id=dict(required=False), domain_name=dict(required=False, type="str"), - dry_run=dict(default=False, type="bool"), + dry_run=dict(required=False, type="bool"), public_ip=dict(required=False, aliases=["ip"]), state=dict(required=False, default="present", choices=["present", "absent"]), in_vpc=dict(required=False, type="bool", default=False), @@ -538,7 +583,7 @@ def main(): "tag_value": ["tag_name"], }, required_if=[ - ('update_reverse_dns_record', True, ('allocation_id', 'domain_name')), + ("update_reverse_dns_record", True, ("allocation_id", "domain_name")), ], ) @@ -550,19 +595,19 @@ def main(): is_instance = check_is_instance(module) - if module.params.get("update_reverse_dns_record") == True: + if module.params.get("update_reverse_dns_record") is True: result = update_reverse_dns_record_of_eip(ec2, module) else: - try: - # Find existing address - address = find_address(ec2, public_ip, device_id, is_instance) - if state == "present": - result = ensure_present(ec2, module, address, is_instance) - else: - result = ensure_absent(ec2, module, address, is_instance) - - except AnsibleEC2Error as e: - module.fail_json_aws_error(e) + try: + # Find existing address + address = find_address(ec2, public_ip, device_id, is_instance) + if state == "present": + result = ensure_present(ec2, module, address, is_instance) + else: + result = ensure_absent(ec2, module, address, is_instance) + + except AnsibleEC2Error as e: + module.fail_json_aws_error(e) module.exit_json(**result) From 6e565d2456a77c671072c5e82a8989b1e2173b81 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 10 Sep 2024 16:18:27 -0700 Subject: [PATCH 03/12] minor fix --- plugins/modules/ec2_eip.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/ec2_eip.py b/plugins/modules/ec2_eip.py index e6ed7cb7efc..f2e908dc5eb 100644 --- a/plugins/modules/ec2_eip.py +++ b/plugins/modules/ec2_eip.py @@ -222,7 +222,7 @@ - name: Modify reverse DNS record of EIP amazon.aws.ec2_eip: - update_reverse_dns: true + update_reverse_dns_record: true allocation_id: eipalloc-00a61ec1234567890 domain_name: example.com """ From 9ed640de0920a41fdd4a7bc49dbe455f0663fe8b Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Wed, 11 Sep 2024 09:30:36 -0700 Subject: [PATCH 04/12] modified based on feedback --- plugins/modules/ec2_eip.py | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/plugins/modules/ec2_eip.py b/plugins/modules/ec2_eip.py index f2e908dc5eb..f33dab65536 100644 --- a/plugins/modules/ec2_eip.py +++ b/plugins/modules/ec2_eip.py @@ -80,20 +80,10 @@ description: Whether to update the reverse DNS record of ec2 Elastic IP Address (eip) required: false type: bool - allocation_id: - description: The allocation ID of ec2 EIP. - required: false - type: str domain_name: description: The domain name to modify for the IP address. required: false type: str - dry_run: - description: - - Checks whether you have the required permissions for the action, without actually making the request, and provides an error response. - - If you have the required permissions, the error response is DryRunOperation. Otherwise, it is UnauthorizedOperation. - required: false - type: bool extends_documentation_fragment: - amazon.aws.common.modules - amazon.aws.region.modules @@ -223,7 +213,7 @@ - name: Modify reverse DNS record of EIP amazon.aws.ec2_eip: update_reverse_dns_record: true - allocation_id: eipalloc-00a61ec1234567890 + public_ip: 11.22.33.44 domain_name: example.com """ @@ -531,15 +521,12 @@ def ensure_present( return result -def update_reverse_dns_record_of_eip(client, module: AnsibleAWSModule): +def update_reverse_dns_record_of_eip(client, module: AnsibleAWSModule, allocation_id, domain_name): changed = False - allocation_id = module.params.get("allocation_id") - domain_name = module.params.get("domain_name") - dry_run = module.params.get("dry_run") try: update_reverse_dns_record_result = client.modify_address_attribute( - AllocationId=allocation_id, DomainName=domain_name, DryRun=dry_run + AllocationId=allocation_id, DomainName=domain_name ) changed = True except AnsibleEC2Error as e: @@ -556,10 +543,8 @@ def update_reverse_dns_record_of_eip(client, module: AnsibleAWSModule): def main(): argument_spec = dict( - allocation_id=dict(required=False, type="str"), device_id=dict(required=False), domain_name=dict(required=False, type="str"), - dry_run=dict(required=False, type="bool"), public_ip=dict(required=False, aliases=["ip"]), state=dict(required=False, default="present", choices=["present", "absent"]), in_vpc=dict(required=False, type="bool", default=False), @@ -583,7 +568,7 @@ def main(): "tag_value": ["tag_name"], }, required_if=[ - ("update_reverse_dns_record", True, ("allocation_id", "domain_name")), + ("update_reverse_dns_record", True, ("public_ip", "domain_name")), ], ) @@ -596,7 +581,9 @@ def main(): is_instance = check_is_instance(module) if module.params.get("update_reverse_dns_record") is True: - result = update_reverse_dns_record_of_eip(ec2, module) + allocation_id = ec2.describe_addresses(PublicIps=[public_ip])["Addresses"][0]["AllocationId"] + domain_name = module.params.get("domain_name") + result = update_reverse_dns_record_of_eip(ec2, module, allocation_id, domain_name) else: try: # Find existing address From cdaa6a3c48533b600e898a3c125798cf1a69ca76 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 17 Sep 2024 15:22:54 -0700 Subject: [PATCH 05/12] remove additional param --- plugins/modules/ec2_eip.py | 59 +++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/plugins/modules/ec2_eip.py b/plugins/modules/ec2_eip.py index f33dab65536..0a014906c21 100644 --- a/plugins/modules/ec2_eip.py +++ b/plugins/modules/ec2_eip.py @@ -76,10 +76,6 @@ - Allocates the new Elastic IP from the provided public IPv4 pool (BYOIP) only applies to newly allocated Elastic IPs, isn't validated when O(reuse_existing_ip_allowed=true). type: str - update_reverse_dns_record: - description: Whether to update the reverse DNS record of ec2 Elastic IP Address (eip) - required: false - type: bool domain_name: description: The domain name to modify for the IP address. required: false @@ -210,11 +206,16 @@ tag_value: "{{ inventory_hostname }}" public_ipv4_pool: ipv4pool-ec2-0588c9b75a25d1a02 -- name: Modify reverse DNS record of EIP +- name: create new IP and modify it's reverse DNS record amazon.aws.ec2_eip: - update_reverse_dns_record: true - public_ip: 11.22.33.44 - domain_name: example.com + state: present + domain_name: test-domain.xyz + +- name: Modify reverse DNS record of an existing EIP + amazon.aws.ec2_eip: + public_ip: 44.224.84.105 + domain_name: test-domain.xyz + state: present """ RETURN = r""" @@ -230,7 +231,7 @@ sample: 52.88.159.209 update_reverse_dns_record_result: description: Information about result of update reverse dns record operation. - returned: When O(update_reverse_dns_record=true). + returned: When O(domain_name) is specified. type: dict contains: address: @@ -465,6 +466,7 @@ def ensure_present( public_ipv4_pool = module.params.get("public_ipv4_pool") tags = module.params.get("tags") purge_tags = module.params.get("purge_tags") + domain_name = module.params.get("domain_name") # Tags for *searching* for an EIP. search_tags = generate_tag_dict(module) @@ -477,6 +479,10 @@ def ensure_present( client, module.check_mode, search_tags, domain, reuse_existing_ip_allowed, tags, public_ipv4_pool ) + if domain_name: + changed, update_reverse_dns_record_result = update_reverse_dns_record_of_eip(client, module, address["AllocationId"], domain_name) + result.update({"update_reverse_dns_record_result": update_reverse_dns_record_result}) + # Associate address to instance if device_id: # Find instance @@ -517,6 +523,7 @@ def ensure_present( client, module, address["AllocationId"], resource_type="elastic-ip", tags=tags, purge_tags=purge_tags ) result.update({"public_ip": address["PublicIp"], "allocation_id": address["AllocationId"]}) + result["changed"] = changed return result @@ -535,10 +542,7 @@ def update_reverse_dns_record_of_eip(client, module: AnsibleAWSModule, allocatio if "ResponseMetadata" in update_reverse_dns_record_result: del update_reverse_dns_record_result["ResponseMetadata"] - return { - "changed": changed, - "update_reverse_dns_record_result": camel_dict_to_snake_dict(update_reverse_dns_record_result), - } + return changed, camel_dict_to_snake_dict(update_reverse_dns_record_result) def main(): @@ -556,7 +560,6 @@ def main(): purge_tags=dict(required=False, type="bool", default=True), tag_name=dict(), tag_value=dict(), - update_reverse_dns_record=dict(required=False, type="bool"), public_ipv4_pool=dict(), ) @@ -567,9 +570,6 @@ def main(): "private_ip_address": ["device_id"], "tag_value": ["tag_name"], }, - required_if=[ - ("update_reverse_dns_record", True, ("public_ip", "domain_name")), - ], ) ec2 = module.client("ec2") @@ -580,21 +580,16 @@ def main(): is_instance = check_is_instance(module) - if module.params.get("update_reverse_dns_record") is True: - allocation_id = ec2.describe_addresses(PublicIps=[public_ip])["Addresses"][0]["AllocationId"] - domain_name = module.params.get("domain_name") - result = update_reverse_dns_record_of_eip(ec2, module, allocation_id, domain_name) - else: - try: - # Find existing address - address = find_address(ec2, public_ip, device_id, is_instance) - if state == "present": - result = ensure_present(ec2, module, address, is_instance) - else: - result = ensure_absent(ec2, module, address, is_instance) - - except AnsibleEC2Error as e: - module.fail_json_aws_error(e) + try: + # Find existing address + address = find_address(ec2, public_ip, device_id, is_instance) + if state == "present": + result = ensure_present(ec2, module, address, is_instance) + else: + result = ensure_absent(ec2, module, address, is_instance) + + except AnsibleEC2Error as e: + module.fail_json_aws_error(e) module.exit_json(**result) From ff496d6bc9bc54addfd0014ca45c06e04822d54c Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 20 Sep 2024 13:16:51 -0500 Subject: [PATCH 06/12] minor fixes --- plugins/modules/ec2_eip.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/modules/ec2_eip.py b/plugins/modules/ec2_eip.py index 0a014906c21..a4a4c8d47bd 100644 --- a/plugins/modules/ec2_eip.py +++ b/plugins/modules/ec2_eip.py @@ -77,9 +77,10 @@ only applies to newly allocated Elastic IPs, isn't validated when O(reuse_existing_ip_allowed=true). type: str domain_name: - description: The domain name to modify for the IP address. + description: The domain name to attach to the IP address. required: false type: str + version_added: 9.0.0 extends_documentation_fragment: - amazon.aws.common.modules - amazon.aws.region.modules From 9422ccfe98bf62390d0db9063764f1ce31f34c8c Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 20 Sep 2024 14:02:58 -0500 Subject: [PATCH 07/12] add check_mode and tests --- plugins/modules/ec2_eip.py | 31 +++++++------ .../targets/ec2_eip/defaults/main.yml | 1 + .../targets/ec2_eip/tasks/main.yml | 1 + .../tasks/update_reverse_dns_record.yml | 43 +++++++++++++++++++ 4 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml diff --git a/plugins/modules/ec2_eip.py b/plugins/modules/ec2_eip.py index a4a4c8d47bd..be659d3b7f9 100644 --- a/plugins/modules/ec2_eip.py +++ b/plugins/modules/ec2_eip.py @@ -481,7 +481,7 @@ def ensure_present( ) if domain_name: - changed, update_reverse_dns_record_result = update_reverse_dns_record_of_eip(client, module, address["AllocationId"], domain_name) + changed, update_reverse_dns_record_result = update_reverse_dns_record_of_eip(client, module, address, domain_name) result.update({"update_reverse_dns_record_result": update_reverse_dns_record_result}) # Associate address to instance @@ -529,21 +529,24 @@ def ensure_present( return result -def update_reverse_dns_record_of_eip(client, module: AnsibleAWSModule, allocation_id, domain_name): +def update_reverse_dns_record_of_eip(client, module: AnsibleAWSModule, address, domain_name): changed = False - try: - update_reverse_dns_record_result = client.modify_address_attribute( - AllocationId=allocation_id, DomainName=domain_name - ) - changed = True - except AnsibleEC2Error as e: - module.fail_json_aws_error(e) - - if "ResponseMetadata" in update_reverse_dns_record_result: - del update_reverse_dns_record_result["ResponseMetadata"] - - return changed, camel_dict_to_snake_dict(update_reverse_dns_record_result) + if module.check_mode: + return True, {} + else: + try: + update_reverse_dns_record_result = client.modify_address_attribute( + AllocationId=address["AllocationId"], DomainName=domain_name + ) + changed = True + except AnsibleEC2Error as e: + module.fail_json_aws_error(e) + + if "ResponseMetadata" in update_reverse_dns_record_result: + del update_reverse_dns_record_result["ResponseMetadata"] + + return changed, camel_dict_to_snake_dict(update_reverse_dns_record_result) def main(): diff --git a/tests/integration/targets/ec2_eip/defaults/main.yml b/tests/integration/targets/ec2_eip/defaults/main.yml index 82c09d949b3..1b82d5c859c 100644 --- a/tests/integration/targets/ec2_eip/defaults/main.yml +++ b/tests/integration/targets/ec2_eip/defaults/main.yml @@ -8,3 +8,4 @@ eip_test_tags: AnsibleEIPTestPrefix: "{{ resource_prefix }}" eip_info_filters: tag:AnsibleEIPTestPrefix: "{{ resource_prefix }}" +test_domain: test-xyz.com diff --git a/tests/integration/targets/ec2_eip/tasks/main.yml b/tests/integration/targets/ec2_eip/tasks/main.yml index 9523fd1ac33..baddcb95bae 100644 --- a/tests/integration/targets/ec2_eip/tasks/main.yml +++ b/tests/integration/targets/ec2_eip/tasks/main.yml @@ -17,6 +17,7 @@ - ansible.builtin.include_tasks: tasks/release.yml - ansible.builtin.include_tasks: tasks/attach_detach_to_eni.yml - ansible.builtin.include_tasks: tasks/attach_detach_to_instance.yml + - ansible.builtin.include_tasks: tasks/update_reverse_dns_record.yml always: - ansible.builtin.include_tasks: tasks/teardown.yml diff --git a/tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml b/tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml new file mode 100644 index 00000000000..c5abc130059 --- /dev/null +++ b/tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml @@ -0,0 +1,43 @@ +- name: Test EIP allocation + block: + # ------------------------------------------------------------------------------------------ + # Allocate EIP with reverse DNS record + # ------------------------------------------------------------------------------------------ + - name: Allocate a new EIP and modify it's reverse DNS record - check_mode + amazon.aws.ec2_eip: + state: present + domain_name: "{{ test_domain }}" + tags: "{{ eip_test_tags }}" + register: eip + check_mode: true + + - ansible.builtin.assert: + that: + - eip is changed + + - name: Ensure no new EIP was created + ansible.builtin.include_tasks: tasks/common.yml + vars: + has_no_new_eip: true + + - name: Allocate a new EIP and modify it's reverse DNS record + amazon.aws.ec2_eip: + state: present + domain_name: "{{ test_domain }}" + tags: "{{ eip_test_tags }}" + register: eip + + - ansible.builtin.assert: + that: + - eip is changed + - eip.public_ip is defined and ( eip.public_ip | ansible.utils.ipaddr ) + - eip.allocation_id is defined and eip.allocation_id.startswith("eipalloc-") + - eip.update_reverse_dns_record_result is defined + - eip.update_reverse_dns_record_result.address.ptr_record_update is defined + - eip.update_reverse_dns_record_result.address.ptr_record_update.value == "{{ test_domain }}" + + always: + - name: Delete EIP + ansible.builtin.include_tasks: tasks/common.yml + vars: + delete_eips: true From 11cf4b07cf8240906a6728506def6dd1609b89aa Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Wed, 25 Sep 2024 17:35:29 -0700 Subject: [PATCH 08/12] update tests, add ability to reset reverse dns record --- plugins/modules/ec2_eip.py | 60 +++++++++---- .../targets/ec2_eip/defaults/main.yml | 2 +- .../targets/ec2_eip/tasks/main.yml | 4 +- .../tasks/update_reverse_dns_record.yml | 84 +++++++++++++++++-- 4 files changed, 127 insertions(+), 23 deletions(-) diff --git a/plugins/modules/ec2_eip.py b/plugins/modules/ec2_eip.py index be659d3b7f9..91790b5cad4 100644 --- a/plugins/modules/ec2_eip.py +++ b/plugins/modules/ec2_eip.py @@ -217,6 +217,12 @@ public_ip: 44.224.84.105 domain_name: test-domain.xyz state: present + +- name: Remove reverse DNS record of an existing EIP + amazon.aws.ec2_eip: + public_ip: 44.224.84.105 + domain_name: "" + state: present """ RETURN = r""" @@ -480,8 +486,10 @@ def ensure_present( client, module.check_mode, search_tags, domain, reuse_existing_ip_allowed, tags, public_ipv4_pool ) - if domain_name: - changed, update_reverse_dns_record_result = update_reverse_dns_record_of_eip(client, module, address, domain_name) + if domain_name is not None: + changed, update_reverse_dns_record_result = update_reverse_dns_record_of_eip( + client, module, address, domain_name + ) result.update({"update_reverse_dns_record_result": update_reverse_dns_record_result}) # Associate address to instance @@ -530,23 +538,43 @@ def ensure_present( def update_reverse_dns_record_of_eip(client, module: AnsibleAWSModule, address, domain_name): - changed = False - if module.check_mode: return True, {} + + current_ptr_record_domain = client.describe_addresses_attribute( + AllocationIds=[address["AllocationId"]], Attribute="domain-name" + ) + + if ( + current_ptr_record_domain["Addresses"] + and current_ptr_record_domain["Addresses"][0]["PtrRecord"] == domain_name + "." + ): + return False, {"ptr_record": domain_name + "."} + + if len(domain_name) == 0: + try: + update_reverse_dns_record_result = client.reset_address_attribute( + AllocationId=address["AllocationId"], Attribute="domain-name" + ) + changed = True + except AnsibleEC2Error as e: + module.fail_json_aws_error(e) + + if "ResponseMetadata" in update_reverse_dns_record_result: + del update_reverse_dns_record_result["ResponseMetadata"] else: - try: - update_reverse_dns_record_result = client.modify_address_attribute( - AllocationId=address["AllocationId"], DomainName=domain_name - ) - changed = True - except AnsibleEC2Error as e: - module.fail_json_aws_error(e) - - if "ResponseMetadata" in update_reverse_dns_record_result: - del update_reverse_dns_record_result["ResponseMetadata"] - - return changed, camel_dict_to_snake_dict(update_reverse_dns_record_result) + try: + update_reverse_dns_record_result = client.modify_address_attribute( + AllocationId=address["AllocationId"], DomainName=domain_name + ) + changed = True + except AnsibleEC2Error as e: + module.fail_json_aws_error(e) + + if "ResponseMetadata" in update_reverse_dns_record_result: + del update_reverse_dns_record_result["ResponseMetadata"] + + return changed, camel_dict_to_snake_dict(update_reverse_dns_record_result) def main(): diff --git a/tests/integration/targets/ec2_eip/defaults/main.yml b/tests/integration/targets/ec2_eip/defaults/main.yml index 1b82d5c859c..76d727e8570 100644 --- a/tests/integration/targets/ec2_eip/defaults/main.yml +++ b/tests/integration/targets/ec2_eip/defaults/main.yml @@ -8,4 +8,4 @@ eip_test_tags: AnsibleEIPTestPrefix: "{{ resource_prefix }}" eip_info_filters: tag:AnsibleEIPTestPrefix: "{{ resource_prefix }}" -test_domain: test-xyz.com +test_domain: "{{ resource_prefix }}.example.xyz" diff --git a/tests/integration/targets/ec2_eip/tasks/main.yml b/tests/integration/targets/ec2_eip/tasks/main.yml index baddcb95bae..667d77b4766 100644 --- a/tests/integration/targets/ec2_eip/tasks/main.yml +++ b/tests/integration/targets/ec2_eip/tasks/main.yml @@ -17,7 +17,9 @@ - ansible.builtin.include_tasks: tasks/release.yml - ansible.builtin.include_tasks: tasks/attach_detach_to_eni.yml - ansible.builtin.include_tasks: tasks/attach_detach_to_instance.yml - - ansible.builtin.include_tasks: tasks/update_reverse_dns_record.yml + + # Disabled as it requires a registered domain, and corresponding hosted zone + # - ansible.builtin.include_tasks: tasks/update_reverse_dns_record.yml always: - ansible.builtin.include_tasks: tasks/teardown.yml diff --git a/tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml b/tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml index c5abc130059..cdc91f047aa 100644 --- a/tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml +++ b/tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml @@ -1,7 +1,7 @@ -- name: Test EIP allocation +- name: Test EIP allocation and reverse DNS record operations block: # ------------------------------------------------------------------------------------------ - # Allocate EIP with reverse DNS record + # Allocate EIP with reverse DNS record - check mode # ------------------------------------------------------------------------------------------ - name: Allocate a new EIP and modify it's reverse DNS record - check_mode amazon.aws.ec2_eip: @@ -11,7 +11,8 @@ register: eip check_mode: true - - ansible.builtin.assert: + - name: Assert that task result was as expected + ansible.builtin.assert: that: - eip is changed @@ -20,6 +21,9 @@ vars: has_no_new_eip: true + # ------------------------------------------------------------------------------------------ + # Allocate EIP with reverse DNS record + # ------------------------------------------------------------------------------------------ - name: Allocate a new EIP and modify it's reverse DNS record amazon.aws.ec2_eip: state: present @@ -27,16 +31,86 @@ tags: "{{ eip_test_tags }}" register: eip - - ansible.builtin.assert: + - name: Add EIP IP address an A record + amazon.aws.route53: + state: present + zone: "ansiblecloud.xyz" + record: "{{ test_domain }}" + type: A + ttl: 7200 + value: "{{ eip.public_ip}}" + identifier: "{{ resource_prefix }}" + wait: true + + - name: Wait for reverse DNS record update to complete + pause: + minutes: 3 + + - name: Assert that task result was as expected + ansible.builtin.assert: that: - eip is changed - eip.public_ip is defined and ( eip.public_ip | ansible.utils.ipaddr ) - eip.allocation_id is defined and eip.allocation_id.startswith("eipalloc-") - eip.update_reverse_dns_record_result is defined - eip.update_reverse_dns_record_result.address.ptr_record_update is defined - - eip.update_reverse_dns_record_result.address.ptr_record_update.value == "{{ test_domain }}" + - eip.update_reverse_dns_record_result.address.ptr_record_update.value == "{{ test_domain }}." + + # ------------------------------------------------------------------------------------------ + # Allocate EIP with reverse DNS record - idempotence + # ------------------------------------------------------------------------------------------ + - name: Try modifying reverse DNS record of EIP to same domain as current - Idempotent + amazon.aws.ec2_eip: + state: present + public_ip: "{{ eip.public_ip }}" + domain_name: "{{ test_domain }}" + tags: "{{ eip_test_tags }}" + register: eip + + - name: Assert that task result was as expected + ansible.builtin.assert: + that: + - eip is not changed + - eip.public_ip is defined and ( eip.public_ip | ansible.utils.ipaddr ) + - eip.allocation_id is defined and eip.allocation_id.startswith("eipalloc-") + - eip.update_reverse_dns_record_result is defined + - eip.update_reverse_dns_record_result.ptr_record == "{{ test_domain }}." + + # ------------------------------------------------------------------------------------------ + # Update reverse DNS record of existing EIP - remove reverse DNS record + # ------------------------------------------------------------------------------------------ + - name: Try modifying reverse DNS record of EIP to different domain than current + amazon.aws.ec2_eip: + state: present + public_ip: "{{ eip.public_ip }}" + domain_name: "" + tags: "{{ eip_test_tags }}" + register: eip + + - name: Assert that changes were applied + ansible.builtin.assert: + that: + - eip is changed + - eip.public_ip is defined and ( eip.public_ip | ansible.utils.ipaddr ) + - eip.allocation_id is defined and eip.allocation_id.startswith("eipalloc-") + + - name: Wait for reverse DNS record update to complete + pause: + minutes: 3 always: + + - name: Delete EIP IP address an A record + amazon.aws.route53: + state: present + zone: "ansiblecloud.xyz" + record: "{{ test_domain }}" + type: A + ttl: 7200 + value: "{{ eip.public_ip}}" + identifier: "{{ resource_prefix }}" + wait: true + - name: Delete EIP ansible.builtin.include_tasks: tasks/common.yml vars: From 3c0d6c0ad3b64eac98fa9c310a4a37075c756c69 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Wed, 25 Sep 2024 17:38:54 -0700 Subject: [PATCH 09/12] minor fix --- tests/integration/targets/ec2_eip/defaults/main.yml | 1 + .../targets/ec2_eip/tasks/update_reverse_dns_record.yml | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/targets/ec2_eip/defaults/main.yml b/tests/integration/targets/ec2_eip/defaults/main.yml index 76d727e8570..736ebbcb767 100644 --- a/tests/integration/targets/ec2_eip/defaults/main.yml +++ b/tests/integration/targets/ec2_eip/defaults/main.yml @@ -9,3 +9,4 @@ eip_test_tags: eip_info_filters: tag:AnsibleEIPTestPrefix: "{{ resource_prefix }}" test_domain: "{{ resource_prefix }}.example.xyz" +test_hosted_zone: "{{ resource_prefix }}.example.xyz" diff --git a/tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml b/tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml index cdc91f047aa..4fc63b55ed9 100644 --- a/tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml +++ b/tests/integration/targets/ec2_eip/tasks/update_reverse_dns_record.yml @@ -34,7 +34,7 @@ - name: Add EIP IP address an A record amazon.aws.route53: state: present - zone: "ansiblecloud.xyz" + zone: "{{ test_hosted_zone }}" record: "{{ test_domain }}" type: A ttl: 7200 @@ -103,7 +103,7 @@ - name: Delete EIP IP address an A record amazon.aws.route53: state: present - zone: "ansiblecloud.xyz" + zone: "{{ test_hosted_zone }}" record: "{{ test_domain }}" type: A ttl: 7200 From 934a22792bdcf850f66286ade38b9a19faeb8633 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Thu, 26 Sep 2024 13:35:14 -0700 Subject: [PATCH 10/12] add unit tests --- .../test_reverse_dns_record_updates.py | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 tests/unit/plugins/modules/ec2_eip/test_reverse_dns_record_updates.py diff --git a/tests/unit/plugins/modules/ec2_eip/test_reverse_dns_record_updates.py b/tests/unit/plugins/modules/ec2_eip/test_reverse_dns_record_updates.py new file mode 100644 index 00000000000..1047bc74416 --- /dev/null +++ b/tests/unit/plugins/modules/ec2_eip/test_reverse_dns_record_updates.py @@ -0,0 +1,187 @@ +# 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 unittest.mock import MagicMock +import pytest + +from ansible_collections.amazon.aws.plugins.modules import ec2_eip + +module_name = "ansible_collections.amazon.aws.plugins.modules.ec2_eip" + + +class FailJsonException(Exception): + def __init__(self): + pass + + +@pytest.fixture +def ansible_module(): + module = MagicMock() + module.fail_json.side_effect = FailJsonException() + module.fail_json_aws.side_effect = FailJsonException() + module.check_mode = False + return module + + +def mock_address(): + return { + "InstanceId": "i-12345678", + "PublicIp": "1.2.3.4", + "AllocationId": "eipalloc-12345678", + "AssociationId": "eipassoc-12345678", + "Domain": "vpc", + "NetworkInterfaceId": "eni-12345678", + "NetworkInterfaceOwnerId": "123456789012", + "PrivateIpAddress": "10.0.0.1", + "Tags": [{"Key": "Name", "Value": "MyElasticIP"}], + "PublicIpv4Pool": "my-ipv4-pool", + "NetworkBorderGroup": "my-border-group", + "CustomerOwnedIp": "192.0.2.0", + "CustomerOwnedIpv4Pool": "my-customer-owned-pool", + "CarrierIp": "203.0.113.0", + } + + +def mock_return_value_describe_addresses_attribute(): + return { + "Addresses": [ + { + "PublicIp": "1.2.3.4", + "AllocationId": "eipalloc-12345678", + "PtrRecord": "current.example.com.", + "PtrRecordUpdate": { + "Value": "current.example.com.", + "Status": "successful", + "Reason": "updated", + }, + } + ] + } + + +def test_update_reverse_dns_record_of_eip_no_change_in_dns_record(ansible_module): + client = MagicMock() + + address = mock_address() + mock_domain_name = "current.example.com" + + client.describe_addresses_attribute.return_value = mock_return_value_describe_addresses_attribute() + + client.modify_address_attribute.return_value = None + client.reset_address_attribute.return_value = None + + assert ec2_eip.update_reverse_dns_record_of_eip(client, ansible_module, address, mock_domain_name) == ( + False, + {"ptr_record": mock_domain_name + "."}, + ) + + assert client.describe_addresses_attribute.call_count == 1 + assert client.modify_address_attribute.call_count == 0 + assert client.reset_address_attribute.call_count == 0 + + client.describe_addresses_attribute.assert_called_once_with( + AllocationIds=["eipalloc-12345678"], Attribute="domain-name" + ) + + +def test_update_reverse_dns_record_of_eip_reset_dns_record(ansible_module): + client = MagicMock() + + address = mock_address() + mock_domain_name = "" + + client.describe_addresses_attribute.return_value = mock_return_value_describe_addresses_attribute() + + client.modify_address_attribute.return_value = None + client.reset_address_attribute.return_value = { + "Addresses": [ + { + "PublicIp": "1.2.3.4", + "AllocationId": "eipalloc-12345678", + "PtrRecord": "current.example.com.", + "PtrRecordUpdate": { + "Value": "", + "Status": "PENDING", + "Reason": "update in progress", + }, + } + ] + } + + assert ec2_eip.update_reverse_dns_record_of_eip(client, ansible_module, address, mock_domain_name) == ( + True, + { + "addresses": [ + { + "public_ip": "1.2.3.4", + "allocation_id": "eipalloc-12345678", + "ptr_record": "current.example.com.", + "ptr_record_update": {"value": "", "status": "PENDING", "reason": "update in progress"}, + } + ] + }, + ) + + assert client.describe_addresses_attribute.call_count == 1 + assert client.modify_address_attribute.call_count == 0 + assert client.reset_address_attribute.call_count == 1 + + client.describe_addresses_attribute.assert_called_once_with( + AllocationIds=["eipalloc-12345678"], Attribute="domain-name" + ) + client.reset_address_attribute.assert_called_once_with(AllocationId="eipalloc-12345678", Attribute="domain-name") + + +def test_update_reverse_dns_record_of_eip_modify_dns_record(ansible_module): + client = MagicMock() + + address = mock_address() + + mock_domain_name = "new.example.com" + + client.describe_addresses_attribute.return_value = mock_return_value_describe_addresses_attribute() + + client.modify_address_attribute.return_value = { + "Addresses": [ + { + "PublicIp": "1.2.3.4", + "AllocationId": "eipalloc-12345678", + "PtrRecord": "current.example.com.", + "PtrRecordUpdate": { + "Value": "new.example.com", + "Status": "PENDING", + "Reason": "update in progress", + }, + } + ] + } + client.reset_address_attribute.return_value = None + + assert ec2_eip.update_reverse_dns_record_of_eip(client, ansible_module, address, mock_domain_name) == ( + True, + { + "addresses": [ + { + "public_ip": "1.2.3.4", + "allocation_id": "eipalloc-12345678", + "ptr_record": "current.example.com.", + "ptr_record_update": { + "value": "new.example.com", + "status": "PENDING", + "reason": "update in progress", + }, + } + ] + }, + ) + + assert client.describe_addresses_attribute.call_count == 1 + assert client.modify_address_attribute.call_count == 1 + assert client.reset_address_attribute.call_count == 0 + + client.describe_addresses_attribute.assert_called_once_with( + AllocationIds=["eipalloc-12345678"], Attribute="domain-name" + ) + client.modify_address_attribute.assert_called_once_with( + AllocationId="eipalloc-12345678", DomainName=mock_domain_name + ) From a262d834a6c95dae920379fc7f5b9354b39356b2 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 27 Sep 2024 10:09:34 -0700 Subject: [PATCH 11/12] linter fix, change version_added to 8.3.0 --- plugins/modules/ec2_eip.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/modules/ec2_eip.py b/plugins/modules/ec2_eip.py index 91790b5cad4..40e83d69d3f 100644 --- a/plugins/modules/ec2_eip.py +++ b/plugins/modules/ec2_eip.py @@ -80,7 +80,7 @@ description: The domain name to attach to the IP address. required: false type: str - version_added: 9.0.0 + version_added: 8.3.0 extends_documentation_fragment: - amazon.aws.common.modules - amazon.aws.region.modules @@ -285,10 +285,11 @@ from typing import Tuple from typing import Union +from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict + from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AnsibleEC2Error from ansible_collections.amazon.aws.plugins.module_utils.ec2 import allocate_address as allocate_ip_address from ansible_collections.amazon.aws.plugins.module_utils.ec2 import associate_address -from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_addresses from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_instances from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_network_interfaces From af97b421d8e483a2a0bd62bbabf295186f080c8a Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 27 Sep 2024 10:16:09 -0700 Subject: [PATCH 12/12] linter fix --- .../plugins/modules/ec2_eip/test_reverse_dns_record_updates.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/plugins/modules/ec2_eip/test_reverse_dns_record_updates.py b/tests/unit/plugins/modules/ec2_eip/test_reverse_dns_record_updates.py index 1047bc74416..143db845724 100644 --- a/tests/unit/plugins/modules/ec2_eip/test_reverse_dns_record_updates.py +++ b/tests/unit/plugins/modules/ec2_eip/test_reverse_dns_record_updates.py @@ -2,6 +2,7 @@ # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from unittest.mock import MagicMock + import pytest from ansible_collections.amazon.aws.plugins.modules import ec2_eip