Do not trigger change for health checks on cname dynamic records

octodns/provider/route53.py

@@ -1253,7 +1253,16 @@ class Route53Provider(BaseProvider):
         return self._gen_mods('DELETE', existing_records, existing_rrsets)
 
     def _extra_changes_update_needed(self, record, rrset):
-        healthcheck_host = record.healthcheck_host
+        value = rrset['ResourceRecords'][0]['Value']
+
+        try:
+            ip_address(text_type(value))
+            # We're working with an IP
+            healthcheck_host = record.healthcheck_host
+        except (AddressValueError, ValueError):
+            # This isn't an IP, host is the value
+            healthcheck_host = value
+
         healthcheck_path = record.healthcheck_path
         healthcheck_protocol = record.healthcheck_protocol
         healthcheck_port = record.healthcheck_port

tests/test_octodns_provider_route53.py

@@ -1962,6 +1962,163 @@ class TestRoute53Provider(TestCase):
         self.assertEquals(1, len(extra))
         stubber.assert_no_pending_responses()
 
+    def test_extra_change_dynamic_has_health_check_cname(self):
+        provider, stubber = self._get_stubbed_provider()
+
+        list_hosted_zones_resp = {
+            'HostedZones': [{
+                'Name': 'unit.tests.',
+                'Id': 'z42',
+                'CallerReference': 'abc',
+            }],
+            'Marker': 'm',
+            'IsTruncated': False,
+            'MaxItems': '100',
+        }
+        stubber.add_response('list_hosted_zones', list_hosted_zones_resp, {})
+
+        # record with geo and no health check returns change
+        desired = Zone('unit.tests.', [])
+        record = Record.new(desired, 'cname', {
+            'ttl': 30,
+            'type': 'CNAME',
+            'value': 'cname.unit.tests.',
+            'dynamic': {
+                'pools': {
+                    'one': {
+                        'values': [{
+                            'value': 'one.cname.unit.tests.',
+                        }],
+                    },
+                },
+                'rules': [{
+                    'pool': 'one',
+                }],
+            },
+        })
+        desired.add_record(record)
+        list_resource_record_sets_resp = {
+            'ResourceRecordSets': [{
+                # Not dynamic value and other name
+                'Name': 'unit.tests.',
+                'Type': 'CNAME',
+                'GeoLocation': {
+                    'CountryCode': '*',
+                },
+                'ResourceRecords': [{
+                    'Value': 'cname.unit.tests.',
+                }],
+                'TTL': 61,
+                # All the non-matches have a different Id so we'll fail if they
+                # match
+                'HealthCheckId': '33',
+            }, {
+                # Not dynamic value, matching name, other type
+                'Name': 'cname.unit.tests.',
+                'Type': 'AAAA',
+                'ResourceRecords': [{
+                    'Value': '2001:0db8:3c4d:0015:0000:0000:1a2f:1a4b'
+                }],
+                'TTL': 61,
+                'HealthCheckId': '33',
+            }, {
+                # default value pool
+                'Name': '_octodns-default-value.cname.unit.tests.',
+                'Type': 'CNAME',
+                'GeoLocation': {
+                    'CountryCode': '*',
+                },
+                'ResourceRecords': [{
+                    'Value': 'cname.unit.tests.',
+                }],
+                'TTL': 61,
+                'HealthCheckId': '33',
+            }, {
+                # different record
+                'Name': '_octodns-two-value.other.unit.tests.',
+                'Type': 'CNAME',
+                'GeoLocation': {
+                    'CountryCode': '*',
+                },
+                'ResourceRecords': [{
+                    'Value': 'cname.unit.tests.',
+                }],
+                'TTL': 61,
+                'HealthCheckId': '33',
+            }, {
+                # same everything, but different type
+                'Name': '_octodns-one-value.cname.unit.tests.',
+                'Type': 'AAAA',
+                'ResourceRecords': [{
+                    'Value': '2001:0db8:3c4d:0015:0000:0000:1a2f:1a4b'
+                }],
+                'TTL': 61,
+                'HealthCheckId': '33',
+            }, {
+                # same everything, sub
+                'Name': '_octodns-one-value.sub.cname.unit.tests.',
+                'Type': 'CNAME',
+                'ResourceRecords': [{
+                    'Value': 'cname.unit.tests.',
+                }],
+                'TTL': 61,
+                'HealthCheckId': '33',
+            }, {
+                # match
+                'Name': '_octodns-one-value.cname.unit.tests.',
+                'Type': 'CNAME',
+                'ResourceRecords': [{
+                    'Value': 'one.cname.unit.tests.',
+                }],
+                'TTL': 61,
+                'HealthCheckId': '42',
+            }],
+            'IsTruncated': False,
+            'MaxItems': '100',
+        }
+        stubber.add_response('list_resource_record_sets',
+                             list_resource_record_sets_resp,
+                             {'HostedZoneId': 'z42'})
+
+        stubber.add_response('list_health_checks', {
+            'HealthChecks': [{
+                'Id': '42',
+                'CallerReference': self.caller_ref,
+                'HealthCheckConfig': {
+                    'Type': 'HTTPS',
+                    'FullyQualifiedDomainName': 'one.cname.unit.tests.',
+                    'ResourcePath': '/_dns',
+                    'Type': 'HTTPS',
+                    'Port': 443,
+                    'MeasureLatency': True,
+                    'RequestInterval': 10,
+                },
+                'HealthCheckVersion': 2,
+            }],
+            'IsTruncated': False,
+            'MaxItems': '100',
+            'Marker': '',
+        })
+        extra = provider._extra_changes(desired=desired, changes=[])
+        self.assertEquals(0, len(extra))
+        stubber.assert_no_pending_responses()
+
+        # change b/c of healthcheck path
+        record._octodns['healthcheck'] = {
+            'path': '/_ready'
+        }
+        extra = provider._extra_changes(desired=desired, changes=[])
+        self.assertEquals(1, len(extra))
+        stubber.assert_no_pending_responses()
+
+        # no change b/c healthcheck host ignored for dynamic cname
+        record._octodns['healthcheck'] = {
+            'host': 'foo.bar.io'
+        }
+        extra = provider._extra_changes(desired=desired, changes=[])
+        self.assertEquals(0, len(extra))
+        stubber.assert_no_pending_responses()
+
     def _get_test_plan(self, max_changes):
 
         provider = Route53Provider('test', 'abc', '123', max_changes)