[aws] route53 module: fix idempotency for CAA records (#46049)

* Fixing record order for CAA records to properly handle idempotency.

* Add integration tests that reproduce CAA failure

lib/ansible/modules/cloud/amazon/route53.py

@@ -574,6 +574,13 @@ def main():
         else:
             wanted_rset.add_value(v)
 
+    need_to_sort_records = (type_in == 'CAA')
+
+    # Sort records for wanted_rset if necessary (keep original list)
+    unsorted_records = wanted_rset.resource_records
+    if need_to_sort_records:
+        wanted_rset.resource_records = sorted(unsorted_records)
+
     sets = invoke_with_throttling_retries(conn.get_all_rrsets, zone.id, name=record_in,
                                           type=type_in, identifier=identifier_in)
     sets_iter = iter(sets)
@@ -593,13 +600,14 @@ def main():
             identifier_in = str(identifier_in)
 
         if rset.type == type_in and decoded_name.lower() == record_in.lower() and rset.identifier == identifier_in:
+            if need_to_sort_records:
+                # Sort records
+                rset.resource_records = sorted(rset.resource_records)
             found_record = True
             record['zone'] = zone_in
             record['type'] = rset.type
             record['record'] = decoded_name
             record['ttl'] = rset.ttl
-            record['value'] = ','.join(sorted(rset.resource_records))
-            record['values'] = sorted(rset.resource_records)
             if hosted_zone_id_in:
                 record['hosted_zone_id'] = hosted_zone_id_in
             record['identifier'] = rset.identifier
@@ -652,6 +660,8 @@ def main():
             command = 'UPSERT'
         else:
             command = command_in.upper()
+        # Restore original order of records
+        wanted_rset.resource_records = unsorted_records
         changes.add_change_record(command, wanted_rset)
 
     if not module.check_mode:

test/integration/targets/route53/aliases

@@ -0,0 +1,2 @@
+cloud/aws
+unsupported

test/integration/targets/route53/defaults/main.yml

@@ -0,0 +1,2 @@
+---
+# defaults file for route53 tests

test/integration/targets/route53/tasks/main.yml

@@ -0,0 +1,135 @@
+---
+# tasks file for Route53
+
+- set_fact:
+    zone_one: '{{ resource_prefix | replace("-", "") }}.one.fakeansible.com.'
+    zone_two: '{{ resource_prefix | replace("-", "") }}.two.fakeansible.com.'
+- debug: msg='Set zones {{ zone_one }} and {{ zone_two }}'
+
+- name: Test basics (new zone, A and AAAA records)
+  module_defaults:
+    group/aws:
+      aws_access_key: "{{ aws_access_key }}"
+      aws_secret_key: "{{ aws_secret_key }}"
+      security_token: "{{ security_token }}"
+      region: "{{ aws_region }}"
+    route53:
+      region: null
+  block:
+  - route53_zone:
+      zone: '{{ zone_one }}'
+      comment: Created in Ansible test {{ resource_prefix }}
+    register: z1
+
+  - debug: msg='TODO write tests'
+  - debug: var=z1
+
+  - name: Create A record using zone fqdn
+    route53:
+      state: present
+      zone: '{{ zone_one }}'
+      record: 'qdn_test.{{ zone_one }}'
+      type: A
+      value: 1.2.3.4
+    register: qdn
+  - assert:
+      that:
+        - qdn is not failed
+        - qdn is changed
+
+  - name: Create same A record using zone non-qualified domain
+    route53:
+      state: present
+      zone: '{{ zone_one[:-1] }}'
+      record: 'qdn_test.{{ zone_one[:-1] }}'
+      type: A
+      value: 1.2.3.4
+    register: non_qdn
+  - assert:
+      that:
+        - non_qdn is not failed
+        - non_qdn is not changed
+
+  - name: Create a LetsEncrypt CAA record
+    route53:
+      state: present
+      zone: '{{ zone_one }}'
+      record: '{{ zone_one }}'
+      type: CAA
+      value:
+      - 0 issue "letsencrypt.org;"
+      - 0 issuewild "letsencrypt.org;"
+      overwrite: true
+    register: caa
+  - assert:
+      that:
+        - caa is not failed
+        - caa is changed
+
+  - name: Re-create the same LetsEncrypt CAA record
+    route53:
+      state: present
+      zone: '{{ zone_one }}'
+      record: '{{ zone_one }}'
+      type: CAA
+      value:
+      - 0 issue "letsencrypt.org;"
+      - 0 issuewild "letsencrypt.org;"
+      overwrite: true
+    register: caa
+  - assert:
+      that:
+        - caa is not failed
+        - caa is not changed
+
+  - name: Re-create the same LetsEncrypt CAA record in opposite-order
+    route53:
+      state: present
+      zone: '{{ zone_one }}'
+      record: '{{ zone_one }}'
+      type: CAA
+      value:
+      - 0 issuewild "letsencrypt.org;"
+      - 0 issue "letsencrypt.org;"
+      overwrite: true
+    register: caa
+  - name: This should not be changed, as CAA records are not order sensitive
+    assert:
+      that:
+        - caa is not failed
+        - caa is not changed
+  always:
+  - route53_facts:
+      query: record_sets
+      hosted_zone_id: '{{ z1.zone_id }}'
+    register: z1_records
+  - debug: var=z1_records
+  - name: Loop over A/AAAA/CNAME records and delete them
+    route53:
+      state: absent
+      zone: '{{ zone_one }}'
+      record: '{{ item.Name }}'
+      type: '{{ item.Type }}'
+      value: '{{ item.ResourceRecords | map(attribute="Value") | join(",") }}'
+    loop: '{{ z1_records.ResourceRecordSets | selectattr("Type", "in", ["A", "AAAA", "CNAME", "CAA"]) | list }}'
+  - name: Delete test zone one '{{ zone_one }}'
+    route53_zone:
+      state: absent
+      zone: '{{ zone_one }}'
+    register: delete_one
+    ignore_errors: yes
+    retries: 10
+    until: delete_one is not failed
+  - name: Delete test zone two '{{ zone_two }}'
+    route53_zone:
+      state: absent
+      zone: '{{ zone_two }}'
+    register: delete_two
+    ignore_errors: yes
+    retries: 10
+    until: delete_two is not failed
+    when: false
+
+
+#TODO(ryansb) build internal-vpc integration tests
+#- include_tasks: internal_zone.yml