route53_health_check: Fix "Name" tag key removal idempotentcy issue (a…

…nsible-collections#1253)

route53_health_check: Fix "Name" tag key removal idempotentcy issue

SUMMARY
Depends-On: ansible-collections#1280

Fixes ansible-collections#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 <None>
Reviewed-by: GomathiselviS <None>
Reviewed-by: Mandar Kulkarni <mandar242@gmail.com>
Reviewed-by: Mike Graves <mgraves@redhat.com>
Reviewed-by: Gonéri Le Bouder <goneri@lebouder.net>
Reviewed-by: Alina Buzachis <None>

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).

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

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

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
 

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