From adf7154f6b8fe248757da490e7594c3b36f6055b Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 30 May 2018 12:22:35 -0700 Subject: [PATCH 1/4] TDD lenient add_record --- tests/test_octodns_zone.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/test_octodns_zone.py b/tests/test_octodns_zone.py index 94faef3..e90f94c 100644 --- a/tests/test_octodns_zone.py +++ b/tests/test_octodns_zone.py @@ -139,9 +139,9 @@ class TestZone(TestCase): self.assertTrue('missing ending dot' in ctx.exception.message) def test_sub_zones(self): - zone = Zone('unit.tests.', set(['sub', 'barred'])) # NS for exactly the sub is allowed + zone = Zone('unit.tests.', set(['sub', 'barred'])) record = Record.new(zone, 'sub', { 'ttl': 3600, 'type': 'NS', @@ -151,6 +151,7 @@ class TestZone(TestCase): self.assertEquals(set([record]), zone.records) # non-NS for exactly the sub is rejected + zone = Zone('unit.tests.', set(['sub', 'barred'])) record = Record.new(zone, 'sub', { 'ttl': 3600, 'type': 'A', @@ -159,8 +160,12 @@ class TestZone(TestCase): with self.assertRaises(SubzoneRecordException) as ctx: zone.add_record(record) self.assertTrue('not of type NS', ctx.exception.message) + # Can add it w/lenient + zone.add_record(record, lenient=True) + self.assertEquals(set([record]), zone.records) # NS for something below the sub is rejected + zone = Zone('unit.tests.', set(['sub', 'barred'])) record = Record.new(zone, 'foo.sub', { 'ttl': 3600, 'type': 'NS', @@ -169,8 +174,12 @@ class TestZone(TestCase): with self.assertRaises(SubzoneRecordException) as ctx: zone.add_record(record) self.assertTrue('under a managed sub-zone', ctx.exception.message) + # Can add it w/lenient + zone.add_record(record, lenient=True) + self.assertEquals(set([record]), zone.records) # A for something below the sub is rejected + zone = Zone('unit.tests.', set(['sub', 'barred'])) record = Record.new(zone, 'foo.bar.sub', { 'ttl': 3600, 'type': 'A', @@ -179,6 +188,9 @@ class TestZone(TestCase): with self.assertRaises(SubzoneRecordException) as ctx: zone.add_record(record) self.assertTrue('under a managed sub-zone', ctx.exception.message) + # Can add it w/lenient + zone.add_record(record, lenient=True) + self.assertEquals(set([record]), zone.records) def test_ignored_records(self): zone_normal = Zone('unit.tests.', []) From 1103b4c383646178727382e9eef895ad07b32b71 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 30 May 2018 12:30:20 -0700 Subject: [PATCH 2/4] Implement Zone.add_record lenient param/support and more tests --- CHANGELOG.md | 6 ++++++ octodns/zone.py | 8 ++++---- tests/test_octodns_zone.py | 6 ++++++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6365545..eebf7bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## v0.9.2 - Unreleased + +* Add lenient support to Zone.add_record, allows populate from providers that + have allowed/created invalid data and situations where a sub-zone is being + extracted from a parent, but the records still exist in the remote provider. + ## v0.9.1 - 2018-05-21 - Going backwards with setup.py ### NOTICE diff --git a/octodns/zone.py b/octodns/zone.py index e4dc859..916f81b 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -56,11 +56,11 @@ class Zone(object): def hostname_from_fqdn(self, fqdn): return self._name_re.sub('', fqdn) - def add_record(self, record, replace=False): + def add_record(self, record, replace=False, lenient=False): name = record.name last = name.split('.')[-1] - if last in self.sub_zones: + if not lenient and last in self.sub_zones: if name != last: # it's a record for something under a sub-zone raise SubzoneRecordException('Record {} is under a ' @@ -82,8 +82,8 @@ class Zone(object): raise DuplicateRecordException('Duplicate record {}, type {}' .format(record.fqdn, record._type)) - elif ((record._type == 'CNAME' and len(node) > 0) or - ('CNAME' in map(lambda r: r._type, node))): + elif not lenient and (((record._type == 'CNAME' and len(node) > 0) or + ('CNAME' in map(lambda r: r._type, node)))): # We're adding a CNAME to existing records or adding to an existing # CNAME raise InvalidNodeException('Invalid state, CNAME at {} cannot ' diff --git a/tests/test_octodns_zone.py b/tests/test_octodns_zone.py index e90f94c..b371590 100644 --- a/tests/test_octodns_zone.py +++ b/tests/test_octodns_zone.py @@ -242,12 +242,18 @@ class TestZone(TestCase): zone.add_record(a) with self.assertRaises(InvalidNodeException): zone.add_record(cname) + self.assertEquals(set([a]), zone.records) + zone.add_record(cname, lenient=True) + self.assertEquals(set([a, cname]), zone.records) # add a to cname zone = Zone('unit.tests.', []) zone.add_record(cname) with self.assertRaises(InvalidNodeException): zone.add_record(a) + self.assertEquals(set([cname]), zone.records) + zone.add_record(a, lenient=True) + self.assertEquals(set([a, cname]), zone.records) def test_excluded_records(self): zone_normal = Zone('unit.tests.', []) From cee7677ae4738f2f5518dd1d13c0c98047b82f68 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 30 May 2018 12:41:59 -0700 Subject: [PATCH 3/4] Implement Zone.add_record lenient across our providers --- octodns/provider/azuredns.py | 2 +- octodns/provider/cloudflare.py | 2 +- octodns/provider/digitalocean.py | 2 +- octodns/provider/dnsimple.py | 2 +- octodns/provider/dnsmadeeasy.py | 2 +- octodns/provider/dyn.py | 8 ++++---- octodns/provider/googlecloud.py | 2 +- octodns/provider/ns1.py | 2 +- octodns/provider/ovh.py | 2 +- octodns/provider/powerdns.py | 2 +- octodns/provider/rackspace.py | 2 +- octodns/provider/route53.py | 2 +- octodns/provider/yaml.py | 2 +- octodns/source/tinydns.py | 4 ++-- 14 files changed, 18 insertions(+), 18 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 1f14c4b..b3bff6c 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -345,7 +345,7 @@ class AzureProvider(BaseProvider): data['type'] = typ data['ttl'] = azrecord.ttl record = Record.new(zone, record_name, data, source=self) - zone.add_record(record) + zone.add_record(record, lenient=lenient) self.log.info('populate: found %s records, exists=%s', len(zone.records) - before, exists) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index c51e61c..ecf3e3f 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -253,7 +253,7 @@ class CloudflareProvider(BaseProvider): self.log.info('CDN rewrite %s already in zone', name) continue - zone.add_record(record) + zone.add_record(record, lenient=lenient) self.log.info('populate: found %s records, exists=%s', len(zone.records) - before, exists) diff --git a/octodns/provider/digitalocean.py b/octodns/provider/digitalocean.py index e71e87f..84116a0 100644 --- a/octodns/provider/digitalocean.py +++ b/octodns/provider/digitalocean.py @@ -230,7 +230,7 @@ class DigitalOceanProvider(BaseProvider): data_for = getattr(self, '_data_for_{}'.format(_type)) record = Record.new(zone, name, data_for(_type, records), source=self, lenient=lenient) - zone.add_record(record) + zone.add_record(record, lenient=lenient) exists = zone.name in self._zone_records self.log.info('populate: found %s records, exists=%s', diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py index b696f75..7a9db50 100644 --- a/octodns/provider/dnsimple.py +++ b/octodns/provider/dnsimple.py @@ -270,7 +270,7 @@ class DnsimpleProvider(BaseProvider): data_for = getattr(self, '_data_for_{}'.format(_type)) record = Record.new(zone, name, data_for(_type, records), source=self, lenient=lenient) - zone.add_record(record) + zone.add_record(record, lenient=lenient) exists = zone.name in self._zone_records self.log.info('populate: found %s records, exists=%s', diff --git a/octodns/provider/dnsmadeeasy.py b/octodns/provider/dnsmadeeasy.py index b439931..f8cee1c 100644 --- a/octodns/provider/dnsmadeeasy.py +++ b/octodns/provider/dnsmadeeasy.py @@ -263,7 +263,7 @@ class DnsMadeEasyProvider(BaseProvider): data_for = getattr(self, '_data_for_{}'.format(_type)) record = Record.new(zone, name, data_for(_type, records), source=self, lenient=lenient) - zone.add_record(record) + zone.add_record(record, lenient=lenient) exists = zone.name in self._zone_records self.log.info('populate: found %s records, exists=%s', diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index de430db..166227b 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -399,7 +399,7 @@ class DynProvider(BaseProvider): return self._traffic_directors - def _populate_traffic_directors(self, zone): + def _populate_traffic_directors(self, zone, lenient): self.log.debug('_populate_traffic_directors: zone=%s', zone.name) td_records = set() for fqdn, types in self.traffic_directors.items(): @@ -444,7 +444,7 @@ class DynProvider(BaseProvider): name = zone.hostname_from_fqdn(fqdn) record = Record.new(zone, name, data, source=self) - zone.add_record(record) + zone.add_record(record, lenient=lenient) td_records.add(record) return td_records @@ -460,7 +460,7 @@ class DynProvider(BaseProvider): td_records = set() if self.traffic_directors_enabled: - td_records = self._populate_traffic_directors(zone) + td_records = self._populate_traffic_directors(zone, lenient) exists = True dyn_zone = _CachingDynZone.get(zone.name[:-1]) @@ -483,7 +483,7 @@ class DynProvider(BaseProvider): record = Record.new(zone, name, data, source=self, lenient=lenient) if record not in td_records: - zone.add_record(record) + zone.add_record(record, lenient=lenient) self.log.info('populate: found %s records, exists=%s', len(zone.records) - before, exists) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index 82a4fcf..74c17a4 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -230,7 +230,7 @@ class GoogleCloudProvider(BaseProvider): self.log.debug('populate: adding record {} records: {!s}' .format(record_name, data)) record = Record.new(zone, record_name, data, source=self) - zone.add_record(record) + zone.add_record(record, lenient=lenient) self.log.info('populate: found %s records, exists=%s', len(zone.records) - before, exists) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 37b8d77..31aff47 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -211,7 +211,7 @@ class Ns1Provider(BaseProvider): record = Record.new(zone, name, data_for(_type, record), source=self, lenient=lenient) zone_hash[(_type, name)] = record - [zone.add_record(r) for r in zone_hash.values()] + [zone.add_record(r, lenient=lenient) for r in zone_hash.values()] self.log.info('populate: found %s records, exists=%s', len(zone.records) - before, exists) return exists diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py index ee1cef9..a74a3cd 100644 --- a/octodns/provider/ovh.py +++ b/octodns/provider/ovh.py @@ -82,7 +82,7 @@ class OvhProvider(BaseProvider): data_for = getattr(self, '_data_for_{}'.format(_type)) record = Record.new(zone, name, data_for(_type, records), source=self, lenient=lenient) - zone.add_record(record) + zone.add_record(record, lenient=lenient) self.log.info('populate: found %s records, exists=%s', len(zone.records) - before, exists) diff --git a/octodns/provider/powerdns.py b/octodns/provider/powerdns.py index 02319e5..30cd01e 100644 --- a/octodns/provider/powerdns.py +++ b/octodns/provider/powerdns.py @@ -199,7 +199,7 @@ class PowerDnsBaseProvider(BaseProvider): record_name = zone.hostname_from_fqdn(rrset['name']) record = Record.new(zone, record_name, data_for(rrset), source=self, lenient=lenient) - zone.add_record(record) + zone.add_record(record, lenient=lenient) self.log.info('populate: found %s records, exists=%s', len(zone.records) - before, exists) diff --git a/octodns/provider/rackspace.py b/octodns/provider/rackspace.py index b9912a3..d2b85f3 100644 --- a/octodns/provider/rackspace.py +++ b/octodns/provider/rackspace.py @@ -215,7 +215,7 @@ class RackspaceProvider(BaseProvider): record = Record.new(zone, record_name, data_for(record_set), source=self) - zone.add_record(record) + zone.add_record(record, lenient=lenient) self.log.info('populate: found %s records, exists=True', len(zone.records) - before) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 5fc0faa..502fa9f 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -489,7 +489,7 @@ class Route53Provider(BaseProvider): data = data[0] record = Record.new(zone, name, data, source=self, lenient=lenient) - zone.add_record(record) + zone.add_record(record, lenient=lenient) self.log.info('populate: found %s records, exists=%s', len(zone.records) - before, exists) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 0241d50..287fd3b 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -67,7 +67,7 @@ class YamlProvider(BaseProvider): d['ttl'] = self.default_ttl record = Record.new(zone, name, d, source=self, lenient=lenient) - zone.add_record(record) + zone.add_record(record, lenient=lenient) self.log.info('populate: found %s records, exists=False', len(zone.records) - before) diff --git a/octodns/source/tinydns.py b/octodns/source/tinydns.py index 8013cd4..7b06527 100644 --- a/octodns/source/tinydns.py +++ b/octodns/source/tinydns.py @@ -134,7 +134,7 @@ class TinyDnsBaseSource(BaseSource): record = Record.new(zone, name, data, source=self, lenient=lenient) try: - zone.add_record(record) + zone.add_record(record, lenient=lenient) except SubzoneRecordException: self.log.debug('_populate_normal: skipping subzone ' 'record=%s', record) @@ -175,7 +175,7 @@ class TinyDnsBaseSource(BaseSource): 'value': value }, source=self, lenient=lenient) try: - zone.add_record(record) + zone.add_record(record, lenient=lenient) except DuplicateRecordException: self.log.warn('Duplicate PTR record for {}, ' 'skipping'.format(addr)) From 206d77d5a6a9ac3e684a1e17c48e71148b28dc85 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 30 May 2018 12:56:25 -0700 Subject: [PATCH 4/4] Include a provider test of populate w/lenient=True --- tests/test_octodns_provider_base.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index d5ac5b3..22a0ee6 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -61,7 +61,7 @@ class TestBaseProvider(TestCase): class HasSupportsGeo(HasLog): SUPPORTS_GEO = False - zone = Zone('unit.tests.', []) + zone = Zone('unit.tests.', ['sub']) with self.assertRaises(NotImplementedError) as ctx: HasSupportsGeo('hassupportsgeo').populate(zone) self.assertEquals('Abstract base class, SUPPORTS property missing', @@ -81,12 +81,17 @@ class TestBaseProvider(TestCase): 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' - })) + }), lenient=lenient) zone.add_record(Record.new(zone, 'going', { 'ttl': 60, 'type': 'A', 'value': '3.4.5.6' - })) + }), lenient=lenient) + zone.add_record(Record.new(zone, 'foo.sub', { + 'ttl': 61, + 'type': 'A', + 'value': '4.5.6.7' + }), lenient=lenient) zone.add_record(Record.new(zone, '', { 'ttl': 60, @@ -98,7 +103,7 @@ class TestBaseProvider(TestCase): .supports(list(zone.records)[0])) plan = HasPopulate('haspopulate').plan(zone) - self.assertEquals(2, len(plan.changes)) + self.assertEquals(3, len(plan.changes)) with self.assertRaises(NotImplementedError) as ctx: HasPopulate('haspopulate').apply(plan)