From a727a1ee6779f92ad4e532dc3934754384e1bc73 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 27 Sep 2018 21:08:47 +0200 Subject: [PATCH] [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 | 14 +- test/integration/targets/route53/aliases | 2 + .../targets/route53/defaults/main.yml | 2 + .../targets/route53/tasks/main.yml | 135 ++++++++++++++++++ .../integration/targets/route53/vars/main.yml | 0 5 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 test/integration/targets/route53/aliases create mode 100644 test/integration/targets/route53/defaults/main.yml create mode 100644 test/integration/targets/route53/tasks/main.yml create mode 100644 test/integration/targets/route53/vars/main.yml diff --git a/lib/ansible/modules/cloud/amazon/route53.py b/lib/ansible/modules/cloud/amazon/route53.py index 85890ac73c2..a50d1978eb9 100644 --- a/lib/ansible/modules/cloud/amazon/route53.py +++ b/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: diff --git a/test/integration/targets/route53/aliases b/test/integration/targets/route53/aliases new file mode 100644 index 00000000000..56927195182 --- /dev/null +++ b/test/integration/targets/route53/aliases @@ -0,0 +1,2 @@ +cloud/aws +unsupported diff --git a/test/integration/targets/route53/defaults/main.yml b/test/integration/targets/route53/defaults/main.yml new file mode 100644 index 00000000000..cc0d3b78d04 --- /dev/null +++ b/test/integration/targets/route53/defaults/main.yml @@ -0,0 +1,2 @@ +--- +# defaults file for route53 tests diff --git a/test/integration/targets/route53/tasks/main.yml b/test/integration/targets/route53/tasks/main.yml new file mode 100644 index 00000000000..fd821c1ab7e --- /dev/null +++ b/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 diff --git a/test/integration/targets/route53/vars/main.yml b/test/integration/targets/route53/vars/main.yml new file mode 100644 index 00000000000..e69de29bb2d