From d73b9e487a7aedf062da89de7775c3358fe5eacb Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 29 Nov 2022 10:01:35 -0800 Subject: [PATCH] route53_health_check: Fix "Name" tag key removal idempotentcy issue (#1253) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit route53_health_check: Fix "Name" tag key removal idempotentcy issue SUMMARY Depends-On: #1280 Fixes #1188 When using health_check_name as unique identifier (setting use_unique_names: True and providing a health_check_name) and health_check tags are set, Current logic for adding name to a health_check causes an issue when rerunning the create/update task. While ideally it should be idempotent, it removes the 'Name' tag (used for health_check_name) causing removal of health check name. ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_health_check ADDITIONAL INFORMATION To test, run the following sample playbook task twice Expected output: Health check name should not disapper (i.e. 'Name' tag should not get removed on rerun) --- - hosts: localhost gather_facts: False tasks: - name: Create a health-check amazon.aws.route53_health_check: health_check_name: my-test-hc use_unique_names: true fqdn: my-test-xyz.com type: HTTPS resource_path: / request_interval: 30 failure_threshold: 3 tags: Service: my-service Owner: my-test-xyz Lifecycle: dev Reviewed-by: Mark Chappell Reviewed-by: GomathiselviS Reviewed-by: Mandar Kulkarni Reviewed-by: Mike Graves Reviewed-by: Gonéri Le Bouder Reviewed-by: Alina Buzachis --- ...ame-tag-key-removal-idempotentcy-issue.yml | 3 + plugins/modules/route53_health_check.py | 13 +- .../tasks/create_multiple_health_checks.yml | 3 +- .../route53_health_check/tasks/main.yml | 3 + .../named_health_check_tag_operations.yml | 271 ++++++++++++++++++ 5 files changed, 285 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/1253-route53_health_check-fix-name-tag-key-removal-idempotentcy-issue.yml create mode 100644 tests/integration/targets/route53_health_check/tasks/named_health_check_tag_operations.yml diff --git a/changelogs/fragments/1253-route53_health_check-fix-name-tag-key-removal-idempotentcy-issue.yml b/changelogs/fragments/1253-route53_health_check-fix-name-tag-key-removal-idempotentcy-issue.yml new file mode 100644 index 00000000000..3bff5eaf2b7 --- /dev/null +++ b/changelogs/fragments/1253-route53_health_check-fix-name-tag-key-removal-idempotentcy-issue.yml @@ -0,0 +1,3 @@ +--- +bugfixes: +- route53_health_check - Fix "Name" tag key removal idempotentcy issue when creating health_check with `use_unique_names` and `tags` set (https://github.com/ansible-collections/amazon.aws/pull/1253). diff --git a/plugins/modules/route53_health_check.py b/plugins/modules/route53_health_check.py index a16ef8bd549..472e9d7b376 100644 --- a/plugins/modules/route53_health_check.py +++ b/plugins/modules/route53_health_check.py @@ -573,6 +573,7 @@ def main(): request_interval_in = module.params.get('request_interval') health_check_name = module.params.get('health_check_name') tags = module.params.get('tags') + purge_tags = module.params.get('purge_tags') # Default port if port_in is None: @@ -632,24 +633,24 @@ def main(): # If health_check_name is a unique identifier if module.params.get('use_unique_names'): existing_checks_with_name = get_existing_checks_with_name() + if tags is None: + purge_tags = False + tags = {} + tags['Name'] = health_check_name + # update the health_check if another health check with same name exists if health_check_name in existing_checks_with_name: changed, action, check_id = update_health_check(existing_checks_with_name[health_check_name]) else: # create a new health_check if another health check with same name does not exists changed, action, check_id = create_health_check(ip_addr_in, fqdn_in, type_in, request_interval_in, port_in) - # Add tag to add name to health check - if check_id: - if not tags: - tags = {} - tags['Name'] = health_check_name else: changed, action, check_id = update_health_check(existing_check) if check_id: changed |= manage_tags(module, client, 'healthcheck', check_id, - tags, module.params.get('purge_tags')) + tags, purge_tags) health_check = describe_health_check(id=check_id) health_check['action'] = action diff --git a/tests/integration/targets/route53_health_check/tasks/create_multiple_health_checks.yml b/tests/integration/targets/route53_health_check/tasks/create_multiple_health_checks.yml index 9cd1dd53446..c20be6044e6 100644 --- a/tests/integration/targets/route53_health_check/tasks/create_multiple_health_checks.yml +++ b/tests/integration/targets/route53_health_check/tasks/create_multiple_health_checks.yml @@ -102,12 +102,11 @@ resource_path: '{{ item }}' use_unique_names: true register: create_idem - check_mode: true with_items: - '{{ resource_path }}' - '{{ resource_path_1 }}' - - name: 'Check result - Create multiple HTTP health check - idempotency - check_mode' + - name: 'Check result - Create multiple HTTP health check - idempotency' assert: that: - create_idem is not failed diff --git a/tests/integration/targets/route53_health_check/tasks/main.yml b/tests/integration/targets/route53_health_check/tasks/main.yml index 05305c25890..1cd73f6a4bf 100644 --- a/tests/integration/targets/route53_health_check/tasks/main.yml +++ b/tests/integration/targets/route53_health_check/tasks/main.yml @@ -32,6 +32,9 @@ - set_fact: ip_address: '{{ eip.public_ip }}' + - name: Run tests for create and delete health check with tags and name as unique identifier + include_tasks: named_health_check_tag_operations.yml + - name: Run tests for creating multiple health checks with name as unique identifier include_tasks: create_multiple_health_checks.yml diff --git a/tests/integration/targets/route53_health_check/tasks/named_health_check_tag_operations.yml b/tests/integration/targets/route53_health_check/tasks/named_health_check_tag_operations.yml new file mode 100644 index 00000000000..aa2d537c366 --- /dev/null +++ b/tests/integration/targets/route53_health_check/tasks/named_health_check_tag_operations.yml @@ -0,0 +1,271 @@ +--- +- block: + # Create Health Check ================================================================= + - name: 'Create Health Check with name and tags' + amazon.aws.route53_health_check: + state: present + name: '{{ tiny_prefix }}-{{ resource_path }}-test-hc-tag-operations' + ip_address: '{{ ip_address }}' + port: '{{ port }}' + type: '{{ type_http }}' + resource_path: '{{ resource_path }}' + use_unique_names: true + fqdn: '{{ fqdn }}' + tags: + Service: my-service + Owner: my-test-xyz + Lifecycle: dev + register: create_result + + - name: Get Health Check tags + amazon.aws.route53_info: + query: health_check + resource_id: "{{ create_result.health_check.id }}" + health_check_method: tags + register: health_check_tags + - set_fact: + tags_keys_list: "{{ health_check_tags.ResourceTagSets[0].Tags | map(attribute='Key') | list }}" + + - name: 'Check result - Create HTTP health check' + assert: + that: + - create_result is not failed + - create_result is changed + - tags_keys_list | length == 4 + - '"Service" in tags_keys_list' + - '"Owner" in tags_keys_list' + - '"Lifecycle" in tags_keys_list' + - '"Name" in tags_keys_list' + + # Create Health Check - check Idempotenty ================================================================= + - name: 'Create Health Check with name and tags - idempotency' + amazon.aws.route53_health_check: + state: present + name: '{{ tiny_prefix }}-{{ resource_path }}-test-hc-tag-operations' + ip_address: '{{ ip_address }}' + port: '{{ port }}' + type: '{{ type_http }}' + resource_path: '{{ resource_path }}' + use_unique_names: true + tags: + Service: my-service + Owner: my-test-xyz + Lifecycle: dev + fqdn: '{{ fqdn }}' + register: create_idem + + - name: Get Health Check tags + amazon.aws.route53_info: + query: health_check + resource_id: "{{ create_idem.health_check.id }}" + health_check_method: tags + register: health_check_tags + - set_fact: + tags_keys_list: "{{ health_check_tags.ResourceTagSets[0].Tags | map(attribute='Key') | list }}" + + - name: 'Check result - Create HTTP health check - idempotency' + assert: + that: + - create_idem is not failed + - create_idem is not changed + - tags_keys_list | length == 4 + - '"Service" in tags_keys_list' + - '"Owner" in tags_keys_list' + - '"Lifecycle" in tags_keys_list' + - '"Name" in tags_keys_list' + + # Create Health Check - Update Tags ================================================================= + - name: 'Create Health Check with name and tags' + amazon.aws.route53_health_check: + state: present + name: '{{ tiny_prefix }}-{{ resource_path }}-test-hc-tag-operations' + ip_address: '{{ ip_address }}' + port: '{{ port }}' + type: '{{ type_http }}' + resource_path: '{{ resource_path }}' + use_unique_names: true + tags: + Service: my-service + NewOwner: my-test-abcd + fqdn: '{{ fqdn }}' + register: create_hc_update_tags + + - name: Get Health Check tags + amazon.aws.route53_info: + query: health_check + resource_id: "{{ create_hc_update_tags.health_check.id }}" + health_check_method: tags + register: health_check_tags + - set_fact: + tags_keys_list: "{{ health_check_tags.ResourceTagSets[0].Tags | map(attribute='Key') | list }}" + + - name: 'Check result - Create HTTP health check' + assert: + that: + - create_hc_update_tags is not failed + - create_hc_update_tags is changed + - tags_keys_list | length == 3 + - '"Service" in tags_keys_list' + - '"NewOwner" in tags_keys_list' + - '"Owner" not in tags_keys_list' + - '"Lifecycle" not in tags_keys_list' + - '"Name" in tags_keys_list' + + # Create Health Check - Update Tags - Idempotency ================================================================= + - name: 'Create Health Check with name and tags - Idempotency' + amazon.aws.route53_health_check: + state: present + name: '{{ tiny_prefix }}-{{ resource_path }}-test-hc-tag-operations' + ip_address: '{{ ip_address }}' + port: '{{ port }}' + type: '{{ type_http }}' + resource_path: '{{ resource_path }}' + use_unique_names: true + tags: + Service: my-service + NewOwner: my-test-abcd + fqdn: '{{ fqdn }}' + register: create_hc_update_tags_idem + + - name: Get Health Check tags + amazon.aws.route53_info: + query: health_check + resource_id: "{{ create_hc_update_tags_idem.health_check.id }}" + health_check_method: tags + register: health_check_tags + - set_fact: + tags_keys_list: "{{ health_check_tags.ResourceTagSets[0].Tags | map(attribute='Key') | list }}" + + - name: 'Check result - Create HTTP health check' + assert: + that: + - create_hc_update_tags_idem is not failed + - create_hc_update_tags_idem is not changed + - tags_keys_list | length == 3 + - '"Service" in tags_keys_list' + - '"NewOwner" in tags_keys_list' + - '"Owner" not in tags_keys_list' + - '"Lifecycle" not in tags_keys_list' + - '"Name" in tags_keys_list' + + # Create Health Check - test purge_tags behavior ================================================================= + + - name: 'Create Health Check with name with tags={} and purge_tags=false (should not remove existing tags)' + amazon.aws.route53_health_check: + state: present + name: '{{ tiny_prefix }}-{{ resource_path }}-test-hc-tag-operations' + ip_address: '{{ ip_address }}' + port: '{{ port }}' + type: '{{ type_http }}' + resource_path: '{{ resource_path }}' + use_unique_names: true + fqdn: '{{ fqdn }}' + tags: {} + purge_tags: false + register: create_hc_update_tags + + - name: Get Health Check tags + amazon.aws.route53_info: + query: health_check + resource_id: "{{ create_hc_update_tags.health_check.id }}" + health_check_method: tags + register: health_check_tags + - set_fact: + tags_keys_list: "{{ health_check_tags.ResourceTagSets[0].Tags | map(attribute='Key') | list }}" + + - name: 'Check result - Create HTTP health check' + assert: + that: + - create_hc_update_tags is not failed + - create_hc_update_tags is not changed + - tags_keys_list | length == 3 + - '"Service" in tags_keys_list' + - '"NewOwner" in tags_keys_list' + - '"Name" in tags_keys_list' + + - name: 'Create Health Check with name with tags=None with purge_tags=true (should not remove existing tags)' + amazon.aws.route53_health_check: + state: present + name: '{{ tiny_prefix }}-{{ resource_path }}-test-hc-tag-operations' + ip_address: '{{ ip_address }}' + port: '{{ port }}' + type: '{{ type_http }}' + resource_path: '{{ resource_path }}' + use_unique_names: true + fqdn: '{{ fqdn }}' + purge_tags: true + register: create_hc_update_tags + + - name: Get Health Check tags + amazon.aws.route53_info: + query: health_check + resource_id: "{{ create_hc_update_tags.health_check.id }}" + health_check_method: tags + register: health_check_tags + - set_fact: + tags_keys_list: "{{ health_check_tags.ResourceTagSets[0].Tags | map(attribute='Key') | list }}" + + - name: 'Check result - Create HTTP health check' + assert: + that: + - create_hc_update_tags is not failed + - create_hc_update_tags is not changed + - tags_keys_list | length == 3 + - '"Service" in tags_keys_list' + - '"NewOwner" in tags_keys_list' + - '"Name" in tags_keys_list' + + - name: 'Create Health Check with name with tags={} with purge_tags=true (should remove existing tags except Name)' + amazon.aws.route53_health_check: + state: present + name: '{{ tiny_prefix }}-{{ resource_path }}-test-hc-tag-operations' + ip_address: '{{ ip_address }}' + port: '{{ port }}' + type: '{{ type_http }}' + resource_path: '{{ resource_path }}' + use_unique_names: true + fqdn: '{{ fqdn }}' + tags: {} + purge_tags: true + register: create_hc_update_tags + + - name: Get Health Check tags + amazon.aws.route53_info: + query: health_check + resource_id: "{{ create_hc_update_tags.health_check.id }}" + health_check_method: tags + register: health_check_tags + - set_fact: + tags_keys_list: "{{ health_check_tags.ResourceTagSets[0].Tags | map(attribute='Key') | list }}" + + - name: 'Check result - Create HTTP health check' + assert: + that: + - create_hc_update_tags is not failed + - create_hc_update_tags is changed + - tags_keys_list | length == 1 + - '"Service" not in tags_keys_list' + - '"NewOwner" not in tags_keys_list' + - '"Name" in tags_keys_list' + + # Cleanup starts here ================================================================= + always: + - name: 'Delete HTTP health check with use_unique_names' + amazon.aws.route53_health_check: + state: absent + name: '{{ tiny_prefix }}-{{ resource_path }}-test-hc-tag-operations' + ip_address: '{{ ip_address }}' + port: '{{ port }}' + type: '{{ type_http }}' + resource_path: '{{ resource_path }}' + use_unique_names: true + fqdn: '{{ fqdn }}' + tags: {} + register: delete_result + with_items: + - '{{ resource_path }}' + + - assert: + that: + - delete_result is changed + - delete_result is not failed