route53: optimize zone_id lookup when hosted_zone_id is provided (#60437)

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

@@ -31,10 +31,11 @@ options:
   zone:
     description:
       - The DNS zone to modify
-    required: true
+      - This is a required parameter, if parameter C(hosted_zone_id) is not supplied.
   hosted_zone_id:
     description:
       - The Hosted Zone ID of the DNS zone to modify
+      - This is a required parameter, if parameter C(zone) is not supplied.
     version_added: "2.0"
   record:
     description:
@@ -386,13 +387,13 @@ class TimeoutError(Exception):
     pass
 
 
-def get_zone_by_name(conn, module, zone_name, want_private, zone_id, want_vpc_id):
+def get_zone_id_by_name(conn, module, zone_name, want_private, want_vpc_id):
     """Finds a zone by name or zone_id"""
     for zone in invoke_with_throttling_retries(conn.get_zones):
         # only save this zone id if the private status of the zone matches
         # the private_zone_in boolean specified in the params
         private_zone = module.boolean(zone.config.get('PrivateZone', False))
-        if private_zone == want_private and ((zone.name == zone_name and zone_id is None) or zone.id.replace('/hostedzone/', '') == zone_id):
+        if private_zone == want_private and zone.name == zone_name:
             if want_vpc_id:
                 # NOTE: These details aren't available in other boto methods, hence the necessary
                 # extra API call
@@ -401,12 +402,12 @@ def get_zone_by_name(conn, module, zone_name, want_private, zone_id, want_vpc_id
                 # this is to deal with this boto bug: https://github.com/boto/boto/pull/2882
                 if isinstance(zone_details['VPCs'], dict):
                     if zone_details['VPCs']['VPC']['VPCId'] == want_vpc_id:
-                        return zone
+                        return zone.id
                 else:  # Forward compatibility for when boto fixes that bug
                     if want_vpc_id in [v['VPCId'] for v in zone_details['VPCs']]:
-                        return zone
+                        return zone.id
             else:
-                return zone
+                return zone.id
     return None
 
 
@@ -461,8 +462,8 @@ def main():
     argument_spec = ec2_argument_spec()
     argument_spec.update(dict(
         state=dict(type='str', required=True, choices=['absent', 'create', 'delete', 'get', 'present'], aliases=['command']),
-        zone=dict(type='str', required=True),
+        zone=dict(type='str'),
-        hosted_zone_id=dict(type='str', ),
+        hosted_zone_id=dict(type='str'),
         record=dict(type='str', required=True),
         ttl=dict(type='int', default=3600),
         type=dict(type='str', required=True, choices=['A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'PTR', 'SOA', 'SPF', 'SRV', 'TXT']),
@@ -486,6 +487,7 @@ def main():
     module = AnsibleModule(
         argument_spec=argument_spec,
         supports_check_mode=True,
+        required_one_of=[['zone', 'hosted_zone_id']],
         # If alias is True then you must specify alias_hosted_zone as well
         required_together=[['alias', 'alias_hosted_zone_id']],
         # state=present, absent, create, delete THEN value is required
@@ -518,7 +520,7 @@ def main():
     elif module.params['state'] == 'get':
         command_in = 'get'
 
-    zone_in = module.params.get('zone').lower()
+    zone_in = (module.params.get('zone') or '').lower()
     hosted_zone_id_in = module.params.get('hosted_zone_id')
     ttl_in = module.params.get('ttl')
     record_in = module.params.get('record').lower()
@@ -564,11 +566,11 @@ def main():
         module.fail_json(msg=e.error_message)
 
     # Find the named zone ID
-    zone = get_zone_by_name(conn, module, zone_in, private_zone_in, hosted_zone_id_in, vpc_id_in)
+    zone_id = hosted_zone_id_in or get_zone_id_by_name(conn, module, zone_in, private_zone_in, vpc_id_in)
 
     # Verify that the requested zone is already defined in Route53
-    if zone is None:
+    if zone_id is None:
-        errmsg = "Zone %s does not exist in Route53" % zone_in
+        errmsg = "Zone %s does not exist in Route53" % (zone_in or hosted_zone_id_in)
         module.fail_json(msg=errmsg)
 
     record = {}
@@ -591,7 +593,7 @@ def main():
     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,
+    sets = invoke_with_throttling_retries(conn.get_all_rrsets, zone_id, name=record_in,
                                           type=type_in, identifier=identifier_in)
     sets_iter = iter(sets)
     while True:
@@ -618,15 +620,12 @@ def main():
             record['type'] = rset.type
             record['record'] = decoded_name
             record['ttl'] = rset.ttl
-            if hosted_zone_id_in:
-                record['hosted_zone_id'] = hosted_zone_id_in
             record['identifier'] = rset.identifier
             record['weight'] = rset.weight
             record['region'] = rset.region
             record['failover'] = rset.failover
             record['health_check'] = rset.health_check
-            if hosted_zone_id_in:
+            record['hosted_zone_id'] = zone_id
-                record['hosted_zone_id'] = hosted_zone_id_in
             if rset.alias_dns_name:
                 record['alias'] = True
                 record['value'] = rset.alias_dns_name
@@ -661,7 +660,7 @@ def main():
     if command_in == 'delete' and not found_record:
         module.exit_json(changed=False)
 
-    changes = ResourceRecordSets(conn, zone.id)
+    changes = ResourceRecordSets(conn, zone_id)
 
     if command_in == 'create' or command_in == 'delete':
         if command_in == 'create' and found_record:

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

@@ -50,6 +50,19 @@
         - non_qdn is not failed
         - non_qdn is not changed
 
+  - name: Create A record using zone ID
+    route53:
+      state: present
+      hosted_zone_id: '{{ z1.zone_id }}'
+      record: 'zid_test.{{ zone_one }}'
+      type: A
+      value: 1.2.3.4
+    register: zid
+  - assert:
+      that:
+        - zid is not failed
+        - zid is changed
+
   - name: Create a multi-value A record with values in different order
     route53:
       state: present