From 1e71bce9072838ce0385704502d76813ec8794c0 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 13:47:58 -0800 Subject: [PATCH 01/22] Add create param to Plan --- octodns/manager.py | 2 +- octodns/provider/base.py | 3 ++- octodns/provider/plan.py | 3 ++- tests/test_octodns_plan.py | 4 ++-- tests/test_octodns_provider_azuredns.py | 9 ++++++--- tests/test_octodns_provider_base.py | 22 ++++++++++++---------- tests/test_octodns_provider_cloudflare.py | 4 ++-- tests/test_octodns_provider_dyn.py | 2 +- tests/test_octodns_provider_googlecloud.py | 6 ++++-- 9 files changed, 32 insertions(+), 23 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index d4debf6..bb2c921 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -361,7 +361,7 @@ class Manager(object): plan = target.plan(zone) if plan is None: - plan = Plan(zone, zone, []) + plan = Plan(zone, zone, [], True) target.apply(plan) def validate_configs(self): diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 2d4680f..ddce7a6 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -64,8 +64,9 @@ class BaseProvider(BaseSource): .join([str(c) for c in extra])) changes += extra + create = False if changes: - plan = Plan(existing, desired, changes, + plan = Plan(existing, desired, changes, create, self.update_pcent_threshold, self.delete_pcent_threshold) self.log.info('plan: %s', plan) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 3e86826..8bb72bd 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -21,12 +21,13 @@ class Plan(object): MAX_SAFE_DELETE_PCENT = .3 MIN_EXISTING_RECORDS = 10 - def __init__(self, existing, desired, changes, + def __init__(self, existing, desired, changes, create, update_pcent_threshold=MAX_SAFE_UPDATE_PCENT, delete_pcent_threshold=MAX_SAFE_DELETE_PCENT): self.existing = existing self.desired = desired self.changes = changes + self.create = create self.update_pcent_threshold = update_pcent_threshold self.delete_pcent_threshold = delete_pcent_threshold diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index ea35243..36dee68 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -52,8 +52,8 @@ update = Update(existing, new) delete = Delete(new) changes = [create, delete, update] plans = [ - (simple, Plan(zone, zone, changes)), - (simple, Plan(zone, zone, changes)), + (simple, Plan(zone, zone, changes, False)), + (simple, Plan(zone, zone, changes, False)), ] diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 598fe48..f44d368 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -338,8 +338,10 @@ class TestAzureDnsProvider(TestCase): changes.append(Create(i)) deletes.append(Delete(i)) - self.assertEquals(13, provider.apply(Plan(None, zone, changes))) - self.assertEquals(13, provider.apply(Plan(zone, zone, deletes))) + self.assertEquals(13, provider.apply(Plan(None, zone, + changes, False))) + self.assertEquals(13, provider.apply(Plan(zone, zone, + deletes, False))) def test_create_zone(self): provider = self._get_provider() @@ -354,7 +356,8 @@ class TestAzureDnsProvider(TestCase): _get = provider._dns_client.zones.get _get.side_effect = CloudError(Mock(status=404), err_msg) - self.assertEquals(13, provider.apply(Plan(None, desired, changes))) + self.assertEquals(13, provider.apply(Plan(None, desired, changes, + False))) def test_check_zone_no_create(self): provider = self._get_provider() diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index 472b008..a0ec74e 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -153,7 +153,7 @@ class TestBaseProvider(TestCase): def test_safe_none(self): # No changes is safe - Plan(None, None, []).raise_if_unsafe() + Plan(None, None, [], False).raise_if_unsafe() def test_safe_creates(self): # Creates are safe when existing records is under MIN_EXISTING_RECORDS @@ -164,7 +164,8 @@ class TestBaseProvider(TestCase): 'type': 'A', 'value': '1.2.3.4', }) - Plan(zone, zone, [Create(record) for i in range(10)]).raise_if_unsafe() + Plan(zone, zone, [Create(record) for i in range(10)], False) \ + .raise_if_unsafe() def test_safe_min_existing_creates(self): # Creates are safe when existing records is over MIN_EXISTING_RECORDS @@ -183,7 +184,8 @@ class TestBaseProvider(TestCase): 'value': '2.3.4.5' })) - Plan(zone, zone, [Create(record) for i in range(10)]).raise_if_unsafe() + Plan(zone, zone, [Create(record) for i in range(10)], False) \ + .raise_if_unsafe() def test_safe_no_existing(self): # existing records fewer than MIN_EXISTING_RECORDS is safe @@ -195,7 +197,7 @@ class TestBaseProvider(TestCase): }) updates = [Update(record, record), Update(record, record)] - Plan(zone, zone, updates).raise_if_unsafe() + Plan(zone, zone, updates, False).raise_if_unsafe() def test_safe_updates_min_existing(self): # MAX_SAFE_UPDATE_PCENT+1 fails when more @@ -219,7 +221,7 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_UPDATE_PCENT) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, False).raise_if_unsafe() self.assertTrue('Too many updates' in ctx.exception.message) @@ -243,7 +245,7 @@ class TestBaseProvider(TestCase): for i in range(int(Plan.MIN_EXISTING_RECORDS * Plan.MAX_SAFE_UPDATE_PCENT))] - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, False).raise_if_unsafe() def test_safe_deletes_min_existing(self): # MAX_SAFE_DELETE_PCENT+1 fails when more @@ -267,7 +269,7 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_DELETE_PCENT) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, False).raise_if_unsafe() self.assertTrue('Too many deletes' in ctx.exception.message) @@ -291,7 +293,7 @@ class TestBaseProvider(TestCase): for i in range(int(Plan.MIN_EXISTING_RECORDS * Plan.MAX_SAFE_DELETE_PCENT))] - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, False).raise_if_unsafe() def test_safe_updates_min_existing_override(self): safe_pcent = .4 @@ -316,7 +318,7 @@ class TestBaseProvider(TestCase): safe_pcent) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, + Plan(zone, zone, changes, False, update_pcent_threshold=safe_pcent).raise_if_unsafe() self.assertTrue('Too many updates' in ctx.exception.message) @@ -344,7 +346,7 @@ class TestBaseProvider(TestCase): safe_pcent) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, + Plan(zone, zone, changes, False, delete_pcent_threshold=safe_pcent).raise_if_unsafe() self.assertTrue('Too many deletes' in ctx.exception.message) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 824af9d..7925b94 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -347,7 +347,7 @@ class TestCloudflareProvider(TestCase): 'values': ['2.2.2.2', '3.3.3.3', '4.4.4.4'], }) change = Update(existing, new) - plan = Plan(zone, zone, [change]) + plan = Plan(zone, zone, [change], False) provider._apply(plan) provider._request.assert_has_calls([ @@ -432,7 +432,7 @@ class TestCloudflareProvider(TestCase): 'value': 'ns2.foo.bar.', }) change = Update(existing, new) - plan = Plan(zone, zone, [change]) + plan = Plan(zone, zone, [change], False) provider._apply(plan) provider._request.assert_has_calls([ diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 4415347..9e06a6b 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -913,7 +913,7 @@ class TestDynProviderGeo(TestCase): Delete(geo), Delete(regular), ] - plan = Plan(None, desired, changes) + plan = Plan(None, desired, changes, False) provider._apply(plan) mock.assert_has_calls([ call('/Zone/unit.tests/', 'GET', {}), diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index adc2112..7060355 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -263,7 +263,8 @@ class TestGoogleCloudProvider(TestCase): provider.apply(Plan( existing=[update_existing_r, delete_r], desired=desired, - changes=changes + changes=changes, + create=False )) calls_mock = gcloud_zone_mock.changes.return_value @@ -295,7 +296,8 @@ class TestGoogleCloudProvider(TestCase): provider.apply(Plan( existing=[update_existing_r, delete_r], desired=desired, - changes=changes + changes=changes, + create=False )) unsupported_change = Mock() From 94bfb1e5072beec17f759b15344e3f0892d2955e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:06:20 -0800 Subject: [PATCH 02/22] Switch populate to return exists, cleaner setup --- octodns/manager.py | 2 +- octodns/provider/base.py | 10 +++++++--- octodns/provider/plan.py | 4 ++-- octodns/source/base.py | 5 ++++- tests/test_octodns_plan.py | 4 ++-- tests/test_octodns_provider_azuredns.py | 6 +++--- tests/test_octodns_provider_base.py | 20 ++++++++++---------- tests/test_octodns_provider_cloudflare.py | 4 ++-- tests/test_octodns_provider_dyn.py | 2 +- tests/test_octodns_provider_googlecloud.py | 4 ++-- 10 files changed, 34 insertions(+), 27 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index bb2c921..3f46007 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -361,7 +361,7 @@ class Manager(object): plan = target.plan(zone) if plan is None: - plan = Plan(zone, zone, [], True) + plan = Plan(zone, zone, [], False) target.apply(plan) def validate_configs(self): diff --git a/octodns/provider/base.py b/octodns/provider/base.py index ddce7a6..6bfb16d 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -45,7 +45,12 @@ class BaseProvider(BaseSource): self.log.info('plan: desired=%s', desired.name) existing = Zone(desired.name, desired.sub_zones) - self.populate(existing, target=True, lenient=True) + exists = self.populate(existing, target=True, lenient=True) + if exists is None: + # If your code gets this warning see Source.populate for more + # information + self.log.warn('Provider %s used in target mode did not return ' + 'exists', self.id) # compute the changes at the zone/record level changes = existing.changes(desired, self) @@ -64,9 +69,8 @@ class BaseProvider(BaseSource): .join([str(c) for c in extra])) changes += extra - create = False if changes: - plan = Plan(existing, desired, changes, create, + plan = Plan(existing, desired, changes, exists, self.update_pcent_threshold, self.delete_pcent_threshold) self.log.info('plan: %s', plan) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 8bb72bd..10ab167 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -21,13 +21,13 @@ class Plan(object): MAX_SAFE_DELETE_PCENT = .3 MIN_EXISTING_RECORDS = 10 - def __init__(self, existing, desired, changes, create, + def __init__(self, existing, desired, changes, exists, update_pcent_threshold=MAX_SAFE_UPDATE_PCENT, delete_pcent_threshold=MAX_SAFE_DELETE_PCENT): self.existing = existing self.desired = desired self.changes = changes - self.create = create + self.exists = exists self.update_pcent_threshold = update_pcent_threshold self.delete_pcent_threshold = delete_pcent_threshold diff --git a/octodns/source/base.py b/octodns/source/base.py index 4ace09f..ee33619 100644 --- a/octodns/source/base.py +++ b/octodns/source/base.py @@ -22,7 +22,7 @@ class BaseSource(object): def populate(self, zone, target=False, lenient=False): ''' - Loads all zones the provider knows about + Loads all records the provider knows about for the provided zone When `target` is True the populate call is being made to load the current state of the provider. @@ -31,6 +31,9 @@ class BaseSource(object): do a "best effort" load of data. That will allow through some common, but not best practices stuff that we otherwise would reject. E.g. no trailing . or mising escapes for ;. + + When target is True (loading current state) this method should return + True if the zone exists or False if it does not. ''' raise NotImplementedError('Abstract base class, populate method ' 'missing') diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index 36dee68..91dd948 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -52,8 +52,8 @@ update = Update(existing, new) delete = Delete(new) changes = [create, delete, update] plans = [ - (simple, Plan(zone, zone, changes, False)), - (simple, Plan(zone, zone, changes, False)), + (simple, Plan(zone, zone, changes, True)), + (simple, Plan(zone, zone, changes, True)), ] diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index f44d368..adf394b 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -339,9 +339,9 @@ class TestAzureDnsProvider(TestCase): deletes.append(Delete(i)) self.assertEquals(13, provider.apply(Plan(None, zone, - changes, False))) + changes, True))) self.assertEquals(13, provider.apply(Plan(zone, zone, - deletes, False))) + deletes, True))) def test_create_zone(self): provider = self._get_provider() @@ -357,7 +357,7 @@ class TestAzureDnsProvider(TestCase): _get.side_effect = CloudError(Mock(status=404), err_msg) self.assertEquals(13, provider.apply(Plan(None, desired, changes, - False))) + True))) def test_check_zone_no_create(self): provider = self._get_provider() diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index a0ec74e..3ebf250 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -153,7 +153,7 @@ class TestBaseProvider(TestCase): def test_safe_none(self): # No changes is safe - Plan(None, None, [], False).raise_if_unsafe() + Plan(None, None, [], True).raise_if_unsafe() def test_safe_creates(self): # Creates are safe when existing records is under MIN_EXISTING_RECORDS @@ -164,7 +164,7 @@ class TestBaseProvider(TestCase): 'type': 'A', 'value': '1.2.3.4', }) - Plan(zone, zone, [Create(record) for i in range(10)], False) \ + Plan(zone, zone, [Create(record) for i in range(10)], True) \ .raise_if_unsafe() def test_safe_min_existing_creates(self): @@ -184,7 +184,7 @@ class TestBaseProvider(TestCase): 'value': '2.3.4.5' })) - Plan(zone, zone, [Create(record) for i in range(10)], False) \ + Plan(zone, zone, [Create(record) for i in range(10)], True) \ .raise_if_unsafe() def test_safe_no_existing(self): @@ -197,7 +197,7 @@ class TestBaseProvider(TestCase): }) updates = [Update(record, record), Update(record, record)] - Plan(zone, zone, updates, False).raise_if_unsafe() + Plan(zone, zone, updates, True).raise_if_unsafe() def test_safe_updates_min_existing(self): # MAX_SAFE_UPDATE_PCENT+1 fails when more @@ -221,7 +221,7 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_UPDATE_PCENT) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, False).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() self.assertTrue('Too many updates' in ctx.exception.message) @@ -245,7 +245,7 @@ class TestBaseProvider(TestCase): for i in range(int(Plan.MIN_EXISTING_RECORDS * Plan.MAX_SAFE_UPDATE_PCENT))] - Plan(zone, zone, changes, False).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() def test_safe_deletes_min_existing(self): # MAX_SAFE_DELETE_PCENT+1 fails when more @@ -269,7 +269,7 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_DELETE_PCENT) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, False).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() self.assertTrue('Too many deletes' in ctx.exception.message) @@ -293,7 +293,7 @@ class TestBaseProvider(TestCase): for i in range(int(Plan.MIN_EXISTING_RECORDS * Plan.MAX_SAFE_DELETE_PCENT))] - Plan(zone, zone, changes, False).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() def test_safe_updates_min_existing_override(self): safe_pcent = .4 @@ -318,7 +318,7 @@ class TestBaseProvider(TestCase): safe_pcent) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, False, + Plan(zone, zone, changes, True, update_pcent_threshold=safe_pcent).raise_if_unsafe() self.assertTrue('Too many updates' in ctx.exception.message) @@ -346,7 +346,7 @@ class TestBaseProvider(TestCase): safe_pcent) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, False, + Plan(zone, zone, changes, True, delete_pcent_threshold=safe_pcent).raise_if_unsafe() self.assertTrue('Too many deletes' in ctx.exception.message) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 7925b94..2977d26 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -347,7 +347,7 @@ class TestCloudflareProvider(TestCase): 'values': ['2.2.2.2', '3.3.3.3', '4.4.4.4'], }) change = Update(existing, new) - plan = Plan(zone, zone, [change], False) + plan = Plan(zone, zone, [change], True) provider._apply(plan) provider._request.assert_has_calls([ @@ -432,7 +432,7 @@ class TestCloudflareProvider(TestCase): 'value': 'ns2.foo.bar.', }) change = Update(existing, new) - plan = Plan(zone, zone, [change], False) + plan = Plan(zone, zone, [change], True) provider._apply(plan) provider._request.assert_has_calls([ diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 9e06a6b..0956477 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -913,7 +913,7 @@ class TestDynProviderGeo(TestCase): Delete(geo), Delete(regular), ] - plan = Plan(None, desired, changes, False) + plan = Plan(None, desired, changes, True) provider._apply(plan) mock.assert_has_calls([ call('/Zone/unit.tests/', 'GET', {}), diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index 7060355..dacc369 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -264,7 +264,7 @@ class TestGoogleCloudProvider(TestCase): existing=[update_existing_r, delete_r], desired=desired, changes=changes, - create=False + exists=True )) calls_mock = gcloud_zone_mock.changes.return_value @@ -297,7 +297,7 @@ class TestGoogleCloudProvider(TestCase): existing=[update_existing_r, delete_r], desired=desired, changes=changes, - create=False + exists=True )) unsupported_change = Mock() From 73c002f94c180e97a9b9330caa6b56f2fbb4baab Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:26:51 -0800 Subject: [PATCH 03/22] Implement populate exists for Route53Provider --- octodns/provider/plan.py | 22 ++++++++++++++++++++-- octodns/provider/route53.py | 7 +++++-- tests/test_octodns_provider_route53.py | 2 ++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 10ab167..47a4157 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -124,6 +124,12 @@ class PlanLogger(_PlanOutput): buf.write(' (') buf.write(target) buf.write(')\n* ') + + if plan.exists is False: + buf.write('Create ') + buf.write(str(plan.desired)) + buf.write('\n* ') + for change in plan.changes: buf.write(change.__repr__(leader='* ')) buf.write('\n* ') @@ -169,6 +175,11 @@ class PlanMarkdown(_PlanOutput): fh.write('| Operation | Name | Type | TTL | Value | Source |\n' '|--|--|--|--|--|--|\n') + if plan.exists is False: + fh.write('| Create | ') + fh.write(str(plan.desired)) + fh.write(' | | | | |\n') + for change in plan.changes: existing = change.existing new = change.new @@ -194,7 +205,8 @@ class PlanMarkdown(_PlanOutput): fh.write(' | ') fh.write(_value_stringifier(new, '; ')) fh.write(' | ') - fh.write(new.source.id) + if new.source: + fh.write(new.source.id) fh.write(' |\n') fh.write('\nSummary: ') @@ -230,6 +242,11 @@ class PlanHtml(_PlanOutput): ''') + if plan.exists is False: + fh.write(' \n Create\n ') + fh.write(str(plan.desired)) + fh.write('\n \n') + for change in plan.changes: existing = change.existing new = change.new @@ -257,7 +274,8 @@ class PlanHtml(_PlanOutput): fh.write('\n ') fh.write(_value_stringifier(new, '
')) fh.write('\n ') - fh.write(new.source.id) + if new.source: + fh.write(new.source.id) fh.write('\n \n') fh.write(' \n Summary: ') diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 5bda074..9de1b0c 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -451,9 +451,11 @@ class Route53Provider(BaseProvider): target, lenient) before = len(zone.records) + exists = False zone_id = self._get_zone_id(zone.name) if zone_id: + exists = True records = defaultdict(lambda: defaultdict(list)) for rrset in self._load_records(zone_id): record_name = zone.hostname_from_fqdn(rrset['Name']) @@ -483,8 +485,9 @@ class Route53Provider(BaseProvider): lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _gen_mods(self, action, records): ''' diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 1cd4548..8da7f2e 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -361,6 +361,7 @@ class TestRoute53Provider(TestCase): plan = provider.plan(self.expected) self.assertEquals(9, len(plan.changes)) + self.assertTrue(plan.exists) for change in plan.changes: self.assertIsInstance(change, Create) stubber.assert_no_pending_responses() @@ -593,6 +594,7 @@ class TestRoute53Provider(TestCase): plan = provider.plan(self.expected) self.assertEquals(9, len(plan.changes)) + self.assertFalse(plan.exists) for change in plan.changes: self.assertIsInstance(change, Create) stubber.assert_no_pending_responses() From d03e07c01c25698d0e4522d8e088cd165b98233b Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:27:08 -0800 Subject: [PATCH 04/22] Implement populate exists for PowerDnsProvider --- octodns/provider/powerdns.py | 7 +++++-- tests/test_octodns_provider_powerdns.py | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/octodns/provider/powerdns.py b/octodns/provider/powerdns.py index 4527f8e..3ad0e55 100644 --- a/octodns/provider/powerdns.py +++ b/octodns/provider/powerdns.py @@ -187,8 +187,10 @@ class PowerDnsBaseProvider(BaseProvider): raise before = len(zone.records) + exists = False if resp: + exists = True for rrset in resp.json()['rrsets']: _type = rrset['type'] if _type == 'SOA': @@ -199,8 +201,9 @@ class PowerDnsBaseProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _records_for_multiple(self, record): return [{'content': v, 'disabled': False} diff --git a/tests/test_octodns_provider_powerdns.py b/tests/test_octodns_provider_powerdns.py index 22ccdd6..b0d5dcf 100644 --- a/tests/test_octodns_provider_powerdns.py +++ b/tests/test_octodns_provider_powerdns.py @@ -106,6 +106,7 @@ class TestPowerDnsProvider(TestCase): plan = provider.plan(expected) self.assertEquals(expected_n, len(plan.changes)) self.assertEquals(expected_n, provider.apply(plan)) + self.assertTrue(plan.exists) # Non-existent zone -> creates for every record in expected # OMG this is fucking ugly, probably better to ditch requests_mocks and @@ -124,6 +125,7 @@ class TestPowerDnsProvider(TestCase): plan = provider.plan(expected) self.assertEquals(expected_n, len(plan.changes)) self.assertEquals(expected_n, provider.apply(plan)) + self.assertFalse(plan.exists) with requests_mock() as mock: # get 422's, unknown zone From 3ef91326e8d62f45be7bb869faf0acc8f0e965fc Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:35:32 -0800 Subject: [PATCH 05/22] Implement populate exists for Ns1Provider --- octodns/provider/ns1.py | 7 +++++-- tests/test_octodns_provider_ns1.py | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 5ea68b6..4911082 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -190,11 +190,13 @@ class Ns1Provider(BaseProvider): nsone_zone = self._client.loadZone(zone.name[:-1]) records = nsone_zone.data['records'] geo_records = nsone_zone.search(has_geo=True) + exists = True except ResourceException as e: if e.message != self.ZONE_NOT_FOUND_MESSAGE: raise records = [] geo_records = [] + exists = False before = len(zone.records) # geo information isn't returned from the main endpoint, so we need @@ -208,8 +210,9 @@ class Ns1Provider(BaseProvider): source=self, lenient=lenient) zone_hash[(_type, name)] = record [zone.add_record(r) for r in zone_hash.values()] - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _params_for_A(self, record): params = {'answers': record.values, 'ttl': record.ttl} diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 0e3a286..fa6cf2d 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -196,9 +196,10 @@ class TestNs1Provider(TestCase): load_mock.side_effect = \ ResourceException('server error: zone not found') zone = Zone('unit.tests.', []) - provider.populate(zone) + exists = provider.populate(zone) self.assertEquals(set(), zone.records) self.assertEquals(('unit.tests',), load_mock.call_args[0]) + self.assertFalse(exists) # Existing zone w/o records load_mock.reset_mock() @@ -269,6 +270,7 @@ class TestNs1Provider(TestCase): # everything except the root NS expected_n = len(self.expected) - 1 self.assertEquals(expected_n, len(plan.changes)) + self.assertTrue(plan.exists) # Fails, general error load_mock.reset_mock() From b54630878f465c79aa1281b12ef34b73d0339995 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:37:31 -0800 Subject: [PATCH 06/22] Implement populate exists for DynProvider --- octodns/provider/dyn.py | 8 ++++++-- tests/test_octodns_provider_dyn.py | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 010fd31..beb84d5 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -353,6 +353,7 @@ class DynProvider(BaseProvider): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) + exists = False before = len(zone.records) self._check_dyn_sess() @@ -360,10 +361,12 @@ class DynProvider(BaseProvider): td_records = set() if self.traffic_directors_enabled: td_records = self._populate_traffic_directors(zone) + exists = True dyn_zone = _CachingDynZone.get(zone.name[:-1]) if dyn_zone: + exists = True values = defaultdict(lambda: defaultdict(list)) for _type, records in dyn_zone.get_all_records().items(): if _type == 'soa_records': @@ -382,8 +385,9 @@ class DynProvider(BaseProvider): if record not in td_records: zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _kwargs_for_A(self, record): return [{ diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 0956477..3f95980 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -430,6 +430,7 @@ class TestDynProvider(TestCase): update_mock.assert_not_called() provider.apply(plan) update_mock.assert_called() + self.assertFalse(plan.exists) add_mock.assert_called() # Once for each dyn record (8 Records, 2 of which have dual values) self.assertEquals(15, len(add_mock.call_args_list)) @@ -474,6 +475,7 @@ class TestDynProvider(TestCase): plan = provider.plan(new) provider.apply(plan) update_mock.assert_called() + self.assertTrue(plan.exists) # we expect 4 deletes, 2 from actual deletes and 2 from # updates which delete and recreate self.assertEquals(4, len(delete_mock.call_args_list)) From 1f40b98889a51c88f7dd3e030617f887399bfd76 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:40:07 -0800 Subject: [PATCH 07/22] Implement populate exists for CloudflareProvider --- octodns/provider/cloudflare.py | 7 +++++-- tests/test_octodns_provider_cloudflare.py | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 9dfef6d..b1dee2b 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -173,9 +173,11 @@ class CloudflareProvider(BaseProvider): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) + exists = False before = len(zone.records) records = self.zone_records(zone) if records: + exists = True values = defaultdict(lambda: defaultdict(list)) for record in records: name = zone.hostname_from_fqdn(record['name']) @@ -196,8 +198,9 @@ class CloudflareProvider(BaseProvider): lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _include_change(self, change): if isinstance(change, Update): diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 2977d26..328bd6f 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -147,6 +147,7 @@ class TestCloudflareProvider(TestCase): plan = provider.plan(self.expected) self.assertEquals(11, len(plan.changes)) self.assertEquals(11, provider.apply(plan)) + self.assertFalse(plan.exists) provider._request.assert_has_calls([ # created the domain @@ -266,6 +267,7 @@ class TestCloudflareProvider(TestCase): # only see the delete & ttl update, below min-ttl is filtered out self.assertEquals(2, len(plan.changes)) self.assertEquals(2, provider.apply(plan)) + self.assertTrue(plan.exists) # recreate for update, and deletes for the 2 parts of the other provider._request.assert_has_calls([ call('PUT', '/zones/ff12ab34cd5611334422ab3322997650/dns_records/' From 4d180ed991ce956254b612190f3c21cc1eb3dd95 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:41:54 -0800 Subject: [PATCH 08/22] Implement populate exists for YamlProvider --- octodns/provider/yaml.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 752e793..0241d50 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -52,7 +52,7 @@ class YamlProvider(BaseProvider): if target: # When acting as a target we ignore any existing records so that we # create a completely new copy - return + return False before = len(zone.records) filename = join(self.directory, '{}yaml'.format(zone.name)) @@ -69,8 +69,9 @@ class YamlProvider(BaseProvider): lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', + self.log.info('populate: found %s records, exists=False', len(zone.records) - before) + return False def _apply(self, plan): desired = plan.desired From d35fcd319aa5278cad2f0df2656f2bb0c89684a9 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:44:36 -0800 Subject: [PATCH 09/22] Implement populate exists for RackspaceProvider --- octodns/provider/rackspace.py | 5 +++-- tests/test_octodns_provider_rackspace.py | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/octodns/provider/rackspace.py b/octodns/provider/rackspace.py index 12b2c54..02c833d 100644 --- a/octodns/provider/rackspace.py +++ b/octodns/provider/rackspace.py @@ -208,7 +208,7 @@ class RackspaceProvider(BaseProvider): raise Exception('Rackspace request unauthorized') elif e.response.status_code == 404: # Zone not found leaves the zone empty instead of failing. - return + return False raise before = len(zone.records) @@ -225,8 +225,9 @@ class RackspaceProvider(BaseProvider): source=self) zone.add_record(record) - self.log.info('populate: found %s records', + self.log.info('populate: found %s records, exists=True', len(zone.records) - before) + return True def _group_records(self, all_records): records = defaultdict(lambda: defaultdict(list)) diff --git a/tests/test_octodns_provider_rackspace.py b/tests/test_octodns_provider_rackspace.py index 274d63e..c467dec 100644 --- a/tests/test_octodns_provider_rackspace.py +++ b/tests/test_octodns_provider_rackspace.py @@ -73,9 +73,10 @@ class TestRackspaceProvider(TestCase): json={'error': "Could not find domain 'unit.tests.'"}) zone = Zone('unit.tests.', []) - self.provider.populate(zone) + exists = self.provider.populate(zone) self.assertEquals(set(), zone.records) self.assertTrue(mock.called_once) + self.assertFalse(exists) def test_multipage_populate(self): with requests_mock() as mock: @@ -109,6 +110,7 @@ class TestRackspaceProvider(TestCase): plan = self.provider.plan(expected) self.assertTrue(mock.called) + self.assertTrue(plan.exists) # OctoDNS does not propagate top-level NS records. self.assertEquals(1, len(plan.changes)) From d693d2e99e0cec859ed53fe37ef7bc4731c0bbf1 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:46:49 -0800 Subject: [PATCH 10/22] Implement populate exists for GoogleCloudProvider --- octodns/provider/googlecloud.py | 7 ++++++- tests/test_octodns_provider_googlecloud.py | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index 6ca0794..1dc5393 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -202,11 +202,14 @@ class GoogleCloudProvider(BaseProvider): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) + + exists = False before = len(zone.records) gcloud_zone = self.gcloud_zones.get(zone.name) if gcloud_zone: + exists = True for gcloud_record in self._get_gcloud_records(gcloud_zone): if gcloud_record.record_type.upper() not in self.SUPPORTS: continue @@ -227,7 +230,9 @@ class GoogleCloudProvider(BaseProvider): record = Record.new(zone, record_name, data, source=self) zone.add_record(record) - self.log.info('populate: found %s records', len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _data_for_A(self, gcloud_record): return { diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index dacc369..fc7fb03 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -359,7 +359,8 @@ class TestGoogleCloudProvider(TestCase): "unit.tests.") test_zone = Zone('unit.tests.', []) - provider.populate(test_zone) + exists = provider.populate(test_zone) + self.assertTrue(exists) # test_zone gets fed the same records as zone does, except it's in # the format returned by google API, so after populate they should look @@ -367,7 +368,8 @@ class TestGoogleCloudProvider(TestCase): self.assertEqual(test_zone.records, zone.records) test_zone2 = Zone('nonexistant.zone.', []) - provider.populate(test_zone2, False, False) + exists = provider.populate(test_zone2, False, False) + self.assertFalse(exists) self.assertEqual(len(test_zone2.records), 0, msg="Zone should not get records from wrong domain") From 720e8eb4349d4ee598c452f1345b6b6e46a2d051 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:49:20 -0800 Subject: [PATCH 11/22] Implement populate exists for AzureProvider --- octodns/provider/azuredns.py | 7 ++++++- tests/test_octodns_provider_azuredns.py | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 1757274..af23ef7 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -322,6 +322,8 @@ class AzureProvider(BaseProvider): :type return: void ''' self.log.debug('populate: name=%s', zone.name) + + exists = False before = len(zone.records) zone_name = zone.name[:len(zone.name) - 1] @@ -331,6 +333,7 @@ class AzureProvider(BaseProvider): _records = set() records = self._dns_client.record_sets.list_by_dns_zone if self._check_zone(zone_name): + exists = True for azrecord in records(self._resource_group, zone_name): if _parse_azure_type(azrecord.type) in self.SUPPORTS: _records.add(azrecord) @@ -344,7 +347,9 @@ class AzureProvider(BaseProvider): record = Record.new(zone, record_name, data, source=self) zone.add_record(record) - self.log.info('populate: found %s records', len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _data_for_A(self, azrecord): return {'values': [ar.ipv4_address for ar in azrecord.arecords]} diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index adf394b..9784945 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -302,7 +302,8 @@ class TestAzureDnsProvider(TestCase): record_list = provider._dns_client.record_sets.list_by_dns_zone record_list.return_value = rs - provider.populate(zone) + exists = provider.populate(zone) + self.assertTrue(exists) self.assertEquals(len(zone.records), 16) @@ -377,6 +378,7 @@ class TestAzureDnsProvider(TestCase): _get = provider._dns_client.zones.get _get.side_effect = CloudError(Mock(status=404), err_msg) - provider.populate(Zone('unit3.test.', [])) + exists = provider.populate(Zone('unit3.test.', [])) + self.assertFalse(exists) self.assertEquals(len(zone.records), 0) From 88ff1729ab42b38c97ea1d81371b8ccb413527cf Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:55:53 -0800 Subject: [PATCH 12/22] Implement populate exists for DigitalOceanProvider --- octodns/provider/digitalocean.py | 6 ++++-- tests/test_octodns_provider_digitalocean.py | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/octodns/provider/digitalocean.py b/octodns/provider/digitalocean.py index c68e7d6..b1df187 100644 --- a/octodns/provider/digitalocean.py +++ b/octodns/provider/digitalocean.py @@ -232,8 +232,10 @@ class DigitalOceanProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + exists = zone.name in self._zone_records + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _params_for_multiple(self, record): for value in record.values: diff --git a/tests/test_octodns_provider_digitalocean.py b/tests/test_octodns_provider_digitalocean.py index 5fb47c5..ddc6bc2 100644 --- a/tests/test_octodns_provider_digitalocean.py +++ b/tests/test_octodns_provider_digitalocean.py @@ -165,6 +165,7 @@ class TestDigitalOceanProvider(TestCase): n = len(self.expected.records) - 7 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) + self.assertFalse(plan.exists) provider._client._request.assert_has_calls([ # created the domain @@ -225,6 +226,7 @@ class TestDigitalOceanProvider(TestCase): })) plan = provider.plan(wanted) + self.assertTrue(plan.exists) self.assertEquals(2, len(plan.changes)) self.assertEquals(2, provider.apply(plan)) # recreate for update, and delete for the 2 parts of the other From 7566250f96a2eb1dc30b5e6a48ab72b7c664dd98 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 14:58:33 -0800 Subject: [PATCH 13/22] Implement populate exists for DnsimpleProvider --- octodns/provider/dnsimple.py | 6 ++++-- tests/test_octodns_provider_dnsimple.py | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py index 43b5b9b..304780d 100644 --- a/octodns/provider/dnsimple.py +++ b/octodns/provider/dnsimple.py @@ -272,8 +272,10 @@ class DnsimpleProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + exists = zone.name in self._zone_records + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _params_for_multiple(self, record): for value in record.values: diff --git a/tests/test_octodns_provider_dnsimple.py b/tests/test_octodns_provider_dnsimple.py index 60dacf1..896425e 100644 --- a/tests/test_octodns_provider_dnsimple.py +++ b/tests/test_octodns_provider_dnsimple.py @@ -133,6 +133,7 @@ class TestDnsimpleProvider(TestCase): n = len(self.expected.records) - 3 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) + self.assertFalse(plan.exists) provider._client._request.assert_has_calls([ # created the domain @@ -186,6 +187,7 @@ class TestDnsimpleProvider(TestCase): })) plan = provider.plan(wanted) + self.assertTrue(plan.exists) self.assertEquals(2, len(plan.changes)) self.assertEquals(2, provider.apply(plan)) # recreate for update, and deletes for the 2 parts of the other From ff305ca1bbc60018333a71321de14c98d7adb299 Mon Sep 17 00:00:00 2001 From: Eric Vergne Date: Mon, 22 Jan 2018 17:12:06 +0100 Subject: [PATCH 14/22] Implement populate exists for OvhProvider --- octodns/provider/ovh.py | 16 +++++++++++++--- tests/test_octodns_provider_ovh.py | 20 ++++++++++++++++---- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py index 5c2fe0d..93e7594 100644 --- a/octodns/provider/ovh.py +++ b/octodns/provider/ovh.py @@ -11,6 +11,7 @@ import logging from collections import defaultdict import ovh +from ovh import ResourceNotFoundError from octodns.record import Record from .base import BaseProvider @@ -33,6 +34,7 @@ class OvhProvider(BaseProvider): """ SUPPORTS_GEO = False + ZONE_NOT_FOUND_MESSAGE = 'This service does not exist' # This variable is also used in populate method to filter which OVH record # types are supported by octodns @@ -57,7 +59,14 @@ class OvhProvider(BaseProvider): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) zone_name = zone.name[:-1] - records = self.get_records(zone_name=zone_name) + try: + records = self.get_records(zone_name=zone_name) + exists = True + except ResourceNotFoundError as e: + if e.message != self.ZONE_NOT_FOUND_MESSAGE: + raise + exists = False + records = [] values = defaultdict(lambda: defaultdict(list)) for record in records: @@ -75,8 +84,9 @@ class OvhProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _apply(self, plan): desired = plan.desired diff --git a/tests/test_octodns_provider_ovh.py b/tests/test_octodns_provider_ovh.py index 6a44e25..472f03d 100644 --- a/tests/test_octodns_provider_ovh.py +++ b/tests/test_octodns_provider_ovh.py @@ -8,7 +8,7 @@ from __future__ import absolute_import, division, print_function, \ from unittest import TestCase from mock import patch, call -from ovh import APIError +from ovh import APIError, ResourceNotFoundError, InvalidCredential from octodns.provider.ovh import OvhProvider from octodns.record import Record @@ -307,18 +307,30 @@ class TestOvhProvider(TestCase): with patch.object(provider._client, 'get') as get_mock: zone = Zone('unit.tests.', []) - get_mock.side_effect = APIError('boom') + get_mock.side_effect = ResourceNotFoundError('boom') with self.assertRaises(APIError) as ctx: provider.populate(zone) self.assertEquals(get_mock.side_effect, ctx.exception) - with patch.object(provider._client, 'get') as get_mock: + get_mock.side_effect = InvalidCredential('boom') + with self.assertRaises(APIError) as ctx: + provider.populate(zone) + self.assertEquals(get_mock.side_effect, ctx.exception) + + zone = Zone('unit.tests.', []) + get_mock.side_effect = ResourceNotFoundError('This service does ' + 'not exist') + exists = provider.populate(zone) + self.assertEquals(set(), zone.records) + self.assertFalse(exists) + zone = Zone('unit.tests.', []) get_returns = [[record['id'] for record in self.api_record]] get_returns += self.api_record get_mock.side_effect = get_returns - provider.populate(zone) + exists = provider.populate(zone) self.assertEquals(self.expected, zone.records) + self.assertTrue(exists) @patch('ovh.Client') def test_is_valid_dkim(self, client_mock): From 886a26bc6f0ffd78bc0d41bbf88e9fc26e686b21 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 19 Feb 2018 11:30:35 -0800 Subject: [PATCH 15/22] Extract _record_for from populate, use round trip in _apply_Update This will ensure that we have exactly the same logic/behavior across the board when turning records into content and prevent the :-( hack that was in here before. It was missing the max(min, ttl) bit we throw everything else through and this makes that consistent. Most importantly it'll prevent us from having to fix bugs or make improvements in multiple code paths in the future. --- octodns/provider/cloudflare.py | 56 ++++++++++++++++------------------ 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 8df146d..0d058dc 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -210,6 +210,20 @@ class CloudflareProvider(BaseProvider): return self._zone_records[zone.name] + def _record_for(self, zone, name, _type, records, lenient): + # rewrite Cloudflare proxied records + if self.cdn and records[0]['proxied']: + data = self._data_for_cdn(name, _type, records) + else: + # Cloudflare supports ALIAS semantics with root CNAMEs + if _type == 'CNAME' and name == '': + _type = 'ALIAS' + + data_for = getattr(self, '_data_for_{}'.format(_type)) + data = data_for(_type, records) + + return Record.new(zone, name, data, source=self, lenient=lenient) + def populate(self, zone, target=False, lenient=False): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) @@ -226,21 +240,8 @@ class CloudflareProvider(BaseProvider): for name, types in values.items(): for _type, records in types.items(): - - # rewrite Cloudflare proxied records - if self.cdn and records[0]['proxied']: - data = self._data_for_cdn(name, _type, records) - - else: - # Cloudflare supports ALIAS semantics with root CNAMEs - if _type == 'CNAME' and name == '': - _type = 'ALIAS' - - data_for = getattr(self, '_data_for_{}'.format(_type)) - data = data_for(_type, records) - - record = Record.new(zone, name, data, source=self, - lenient=lenient) + record = self._record_for(zone, name, _type, records, + lenient) # only one rewrite is needed for names where the proxy is # enabled at multiple records with a different type but @@ -374,10 +375,6 @@ class CloudflareProvider(BaseProvider): for c in self._gen_contents(change.new) } - # We need a list of keys to consider for diffs, use the first content - # before we muck with anything - keys = existing_contents.values()[0].keys() - # Find the things we need to add adds = [] for k, content in new_contents.items(): @@ -387,22 +384,23 @@ class CloudflareProvider(BaseProvider): except KeyError: adds.append(content) - zone_id = self.zones[change.new.zone.name] + zone = change.new.zone + zone_id = self.zones[zone.name] # Find things we need to remove name = change.new.fqdn[:-1] + hostname = zone.hostname_from_fqdn(name) _type = change.new._type # OK, work through each record from the zone - for record in self.zone_records(change.new.zone): + for record in self.zone_records(zone): if name == record['name'] and _type == record['type']: - # This is match for our name and type, we need to look at - # contents now, build a dict of the relevant keys and vals - content = {} - for k in keys: - content[k] = record[k] - # :-( - if _type in ('CNAME', 'MX', 'NS'): - content['content'] += '.' + + # Round trip the single value through a record to contents flow + # to get a consistent _gen_contents result that matches what + # went in to new_contents + r = self._record_for(zone, hostname, _type, [record], True) + content = self._gen_contents(r).next() + # If the hash of that dict isn't in new this record isn't # needed if self._hash_content(content) not in new_contents: From 4b44ab14b1f0a52f1051c67656d6e3dd6f0ba903 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 19 Feb 2018 11:40:32 -0800 Subject: [PATCH 16/22] Use MIN_TTL, not 120 literal --- octodns/provider/cloudflare.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 0d058dc..eb08952 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -260,7 +260,7 @@ class CloudflareProvider(BaseProvider): if isinstance(change, Update): existing = change.existing.data new = change.new.data - new['ttl'] = max(120, new['ttl']) + new['ttl'] = max(self.MIN_TTL, new['ttl']) if new == existing: return False From 29bc1f3af3cb68c0c8c851c8169f842e81a749a6 Mon Sep 17 00:00:00 2001 From: Sergei Shmanko Date: Thu, 22 Feb 2018 11:36:25 +0200 Subject: [PATCH 17/22] fix logging for update/delete_pcent_threshold --- octodns/provider/base.py | 3 ++- octodns/provider/plan.py | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 2d4680f..3f8a5b8 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -17,7 +17,8 @@ class BaseProvider(BaseSource): delete_pcent_threshold=Plan.MAX_SAFE_DELETE_PCENT): super(BaseProvider, self).__init__(id) self.log.debug('__init__: id=%s, apply_disabled=%s, ' - 'update_pcent_threshold=%d, delete_pcent_threshold=%d', + 'update_pcent_threshold=%.2f' + 'delete_pcent_threshold=%.2f', id, apply_disabled, update_pcent_threshold, diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 3e86826..9613809 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -60,17 +60,17 @@ class Plan(object): delete_pcent = self.change_counts['Delete'] / existing_record_count if update_pcent > self.update_pcent_threshold: - raise UnsafePlan('Too many updates, {} is over {} percent' + raise UnsafePlan('Too many updates, {:.2f} is over {:.2f} %' '({}/{})'.format( - update_pcent, - self.MAX_SAFE_UPDATE_PCENT * 100, + update_pcent * 100, + self.update_pcent_threshold * 100, self.change_counts['Update'], existing_record_count)) if delete_pcent > self.delete_pcent_threshold: - raise UnsafePlan('Too many deletes, {} is over {} percent' + raise UnsafePlan('Too many deletes, {:.2f} is over {:.2f} %' '({}/{})'.format( - delete_pcent, - self.MAX_SAFE_DELETE_PCENT * 100, + delete_pcent * 100, + self.delete_pcent_threshold * 100, self.change_counts['Delete'], existing_record_count)) From 543b1c9dbdd0810aad851adf53f99fc4b6cc72d8 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 24 Feb 2018 09:10:57 -0800 Subject: [PATCH 18/22] Fix handling of Cloudflare ALIAS updates --- octodns/provider/cloudflare.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index eb08952..688b10e 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -388,17 +388,19 @@ class CloudflareProvider(BaseProvider): zone_id = self.zones[zone.name] # Find things we need to remove - name = change.new.fqdn[:-1] - hostname = zone.hostname_from_fqdn(name) + hostname = zone.hostname_from_fqdn(change.new.fqdn[:-1]) _type = change.new._type # OK, work through each record from the zone for record in self.zone_records(zone): - if name == record['name'] and _type == record['type']: + name = zone.hostname_from_fqdn(record['name']) + # Use the _record_for so that we include all of standard + # converstion logic + r = self._record_for(zone, name, record['type'], [record], True) + if hostname == r.name and _type == r._type: # Round trip the single value through a record to contents flow # to get a consistent _gen_contents result that matches what # went in to new_contents - r = self._record_for(zone, hostname, _type, [record], True) content = self._gen_contents(r).next() # If the hash of that dict isn't in new this record isn't From 9f2b65ec83aae71220ef0367d864ab444f3ed8b3 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 24 Feb 2018 09:19:23 -0800 Subject: [PATCH 19/22] Change str() to unicode() to avoid encoding problems --- README.md | 1 + octodns/cmds/report.py | 6 +++--- octodns/provider/base.py | 2 +- octodns/provider/ns1.py | 4 ++-- octodns/provider/plan.py | 16 ++++++++-------- octodns/record.py | 5 +++-- octodns/zone.py | 2 +- tests/test_octodns_provider_base.py | 14 +++++++------- 8 files changed, 26 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index b99e836..0abf9c6 100644 --- a/README.md +++ b/README.md @@ -169,6 +169,7 @@ The above command pulled the existing data out of Route53 and placed the results * ALIAS support varies a lot from provider to provider care should be taken to verify that your needs are met in detail. * Dyn's UI doesn't allow editing or view of TTL, but the API accepts and stores the value provided, this value does not appear to be used when served * Dnsimple's uses the configured TTL when serving things through the ALIAS, there's also a secondary TXT record created alongside the ALIAS that octoDNS ignores +* octoDNS itself supports non-ASCII character sets, but in testing Cloudflare is the only provider where that is currently functional end-to-end. Others have failures either in the client libraries or API calls ## Custom Sources and Providers diff --git a/octodns/cmds/report.py b/octodns/cmds/report.py index 06a4484..2b32e77 100755 --- a/octodns/cmds/report.py +++ b/octodns/cmds/report.py @@ -65,7 +65,7 @@ def main(): resolver = AsyncResolver(configure=False, num_workers=int(args.num_workers)) if not ip_addr_re.match(server): - server = str(query(server, 'A')[0]) + server = unicode(query(server, 'A')[0]) log.info('server=%s', server) resolver.nameservers = [server] resolver.lifetime = int(args.timeout) @@ -81,12 +81,12 @@ def main(): stdout.write(',') stdout.write(record._type) stdout.write(',') - stdout.write(str(record.ttl)) + stdout.write(unicode(record.ttl)) compare = {} for future in futures: stdout.write(',') try: - answers = [str(r) for r in future.result()] + answers = [unicode(r) for r in future.result()] except (NoAnswer, NoNameservers): answers = ['*no answer*'] except NXDOMAIN: diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 3f8a5b8..6f67f1c 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -62,7 +62,7 @@ class BaseProvider(BaseSource): extra = self._extra_changes(existing, changes) if extra: self.log.info('plan: extra changes\n %s', '\n ' - .join([str(c) for c in extra])) + .join([unicode(c) for c in extra])) changes += extra if changes: diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 80797d8..d214062 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -75,9 +75,9 @@ class Ns1Provider(BaseProvider): else: values.extend(answer['answer']) codes.append([]) - values = [str(x) for x in values] + values = [unicode(x) for x in values] geo = OrderedDict( - {str(k): [str(x) for x in v] for k, v in geo.items()} + {unicode(k): [unicode(x) for x in v] for k, v in geo.items()} ) data['values'] = values data['geo'] = geo diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 9613809..5944d6e 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -140,11 +140,11 @@ class PlanLogger(_PlanOutput): def _value_stringifier(record, sep): try: - values = [str(v) for v in record.values] + values = [unicode(v) for v in record.values] except AttributeError: values = [record.value] for code, gv in sorted(getattr(record, 'geo', {}).items()): - vs = ', '.join([str(v) for v in gv.values]) + vs = ', '.join([unicode(v) for v in gv.values]) values.append('{}: {}'.format(code, vs)) return sep.join(values) @@ -181,7 +181,7 @@ class PlanMarkdown(_PlanOutput): fh.write(' | ') # TTL if existing: - fh.write(str(existing.ttl)) + fh.write(unicode(existing.ttl)) fh.write(' | ') fh.write(_value_stringifier(existing, '; ')) fh.write(' | |\n') @@ -189,7 +189,7 @@ class PlanMarkdown(_PlanOutput): fh.write('| | | | ') if new: - fh.write(str(new.ttl)) + fh.write(unicode(new.ttl)) fh.write(' | ') fh.write(_value_stringifier(new, '; ')) fh.write(' | ') @@ -197,7 +197,7 @@ class PlanMarkdown(_PlanOutput): fh.write(' |\n') fh.write('\nSummary: ') - fh.write(str(plan)) + fh.write(unicode(plan)) fh.write('\n\n') else: fh.write('## No changes were planned\n') @@ -243,7 +243,7 @@ class PlanHtml(_PlanOutput): # TTL if existing: fh.write(' ') - fh.write(str(existing.ttl)) + fh.write(unicode(existing.ttl)) fh.write('\n ') fh.write(_value_stringifier(existing, '
')) fh.write('\n \n \n') @@ -252,7 +252,7 @@ class PlanHtml(_PlanOutput): if new: fh.write(' ') - fh.write(str(new.ttl)) + fh.write(unicode(new.ttl)) fh.write('\n ') fh.write(_value_stringifier(new, '
')) fh.write('\n ') @@ -260,7 +260,7 @@ class PlanHtml(_PlanOutput): fh.write('\n \n') fh.write(' \n Summary: ') - fh.write(str(plan)) + fh.write(unicode(plan)) fh.write('\n \n\n') else: fh.write('No changes were planned') diff --git a/octodns/record.py b/octodns/record.py index b608ef6..82ee191 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -122,7 +122,7 @@ class Record(object): self.__class__.__name__, name) self.zone = zone # force everything lower-case just to be safe - self.name = str(name).lower() if name else name + self.name = unicode(name).lower() if name else name self.source = source self.ttl = int(data['ttl']) @@ -274,7 +274,8 @@ class _ValuesMixin(object): return ret def __repr__(self): - values = "['{}']".format("', '".join([str(v) for v in self.values])) + values = "['{}']".format("', '".join([unicode(v) + for v in self.values])) return '<{} {} {}, {}, {}>'.format(self.__class__.__name__, self._type, self.ttl, self.fqdn, values) diff --git a/octodns/zone.py b/octodns/zone.py index bbc38d0..a8a91ca 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -38,7 +38,7 @@ class Zone(object): raise Exception('Invalid zone name {}, missing ending dot' .format(name)) # Force everyting to lowercase just to be safe - self.name = str(name).lower() if name else name + self.name = unicode(name).lower() if name else name self.sub_zones = sub_zones # We're grouping by node, it allows us to efficently search for # duplicates and detect when CNAMEs co-exist with other records diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index 472b008..855efaf 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -177,7 +177,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -208,7 +208,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -234,7 +234,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -256,7 +256,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -282,7 +282,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -305,7 +305,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -333,7 +333,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' From f5c17638a4a2fa4ad596fa7f45ba92e87f71e83e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 24 Feb 2018 09:27:43 -0800 Subject: [PATCH 20/22] Remove Rackspace's _as_unicode, no longer necessary --- octodns/provider/rackspace.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/octodns/provider/rackspace.py b/octodns/provider/rackspace.py index 12b2c54..e370315 100644 --- a/octodns/provider/rackspace.py +++ b/octodns/provider/rackspace.py @@ -130,17 +130,9 @@ class RackspaceProvider(BaseProvider): def _delete(self, path, data=None): return self._request('DELETE', path, data=data) - @staticmethod - def _as_unicode(s, codec): - if not isinstance(s, unicode): - return unicode(s, codec) - return s - @classmethod def _key_for_record(cls, rs_record): - return cls._as_unicode(rs_record['type'], 'ascii'), \ - cls._as_unicode(rs_record['name'], 'utf-8'), \ - cls._as_unicode(rs_record['data'], 'utf-8') + return rs_record['type'], rs_record['name'], rs_record['data'] def _data_for_multiple(self, rrset): return { From 5d2ba2e715f770a4ddd90a2b5ca93563a3cdf785 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Tue, 27 Feb 2018 12:09:47 -0500 Subject: [PATCH 21/22] Spelling (#214) * spelling: ancillary * spelling: antarctica * spelling: australia * spelling: authentication * spelling: continental * spelling: constructor * spelling: conversion * spelling: creation * spelling: doesn't * spelling: easily * spelling: efficiently * spelling: equivalent * spelling: essentially * spelling: everything * spelling: exactly * spelling: be * spelling: expensive * spelling: supports * spelling: healthcheck * spelling: immediately * spelling: ignored * spelling: invocation * spelling: itself * spelling: leftovers * spelling: missing * spelling: natural * spelling: nonexistent * spelling: peculiarities * spelling: pointing This change hit a line length limitation, so I'm wrapping it and adding a period which appears to match local style... * spelling: quicker * spelling: response * spelling: requested * spelling: redirect * spelling: traffic * spelling: unknown * spelling: uploaded * spelling: useful * spelling: separately * spelling: zone --- CHANGELOG.md | 2 +- docs/records.md | 8 ++++---- octodns/cmds/sync.py | 2 +- octodns/manager.py | 2 +- octodns/provider/azuredns.py | 4 ++-- octodns/provider/base.py | 4 ++-- octodns/provider/cloudflare.py | 2 +- octodns/provider/digitalocean.py | 2 +- octodns/provider/dnsimple.py | 2 +- octodns/provider/dyn.py | 14 +++++++------- octodns/provider/powerdns.py | 8 ++++---- octodns/provider/route53.py | 6 +++--- octodns/record.py | 2 +- octodns/source/base.py | 2 +- octodns/zone.py | 4 ++-- script/release | 2 +- tests/test_octodns_provider_base.py | 6 +++--- tests/test_octodns_provider_cloudflare.py | 7 ++++--- tests/test_octodns_provider_dyn.py | 16 ++++++++-------- tests/test_octodns_provider_googlecloud.py | 10 +++++----- tests/test_octodns_provider_ovh.py | 6 +++--- tests/test_octodns_provider_powerdns.py | 4 ++-- tests/test_octodns_provider_route53.py | 6 +++--- tests/test_octodns_record.py | 4 ++-- 24 files changed, 63 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6777ea9..3d8b441 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,7 @@ better in the future :fingers_crossed: #### Miscellaneous -* Use a 3rd party lib for nautrual sorting of keys, rather than my old +* Use a 3rd party lib for natural sorting of keys, rather than my old implementation. Sorting can be disabled in the YamlProvider with `enforce_order: False`. * Semi-colon/escaping fixes and improvements. diff --git a/docs/records.md b/docs/records.md index 82d9a37..75f4d0b 100644 --- a/docs/records.md +++ b/docs/records.md @@ -53,11 +53,11 @@ The geo labels breakdown based on: 1. - 'AF': 14, # Continental Africa - - 'AN': 17, # Continental Antartica - - 'AS': 15, # Contentinal Asia - - 'EU': 13, # Contentinal Europe + - 'AN': 17, # Continental Antarctica + - 'AS': 15, # Continental Asia + - 'EU': 13, # Continental Europe - 'NA': 11, # Continental North America - - 'OC': 16, # Contentinal Austrailia/Oceania + - 'OC': 16, # Continental Australia/Oceania - 'SA': 12, # Continental South America 2. ISO Country Code https://en.wikipedia.org/wiki/ISO_3166-2 diff --git a/octodns/cmds/sync.py b/octodns/cmds/sync.py index 4dd3e87..60793e7 100755 --- a/octodns/cmds/sync.py +++ b/octodns/cmds/sync.py @@ -26,7 +26,7 @@ def main(): help='Limit sync to the specified zone(s)') # --sources isn't an option here b/c filtering sources out would be super - # dangerous since you could eaily end up with an empty zone and delete + # dangerous since you could easily end up with an empty zone and delete # everything, or even just part of things when there are multiple sources parser.add_argument('--target', default=[], action='append', diff --git a/octodns/manager.py b/octodns/manager.py index d4debf6..c9c6a13 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -51,7 +51,7 @@ class MakeThreadFuture(object): class MainThreadExecutor(object): ''' - Dummy executor that runs things on the main thread during the involcation + Dummy executor that runs things on the main thread during the invocation of submit, but still returns a future object with the result. This allows code to be written to handle async, even in the case where we don't want to use multiple threads/workers and would prefer that things flow as if diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 1757274..65160cc 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -39,7 +39,7 @@ class _AzureRecord(object): } def __init__(self, resource_group, record, delete=False): - '''Contructor for _AzureRecord. + '''Constructor for _AzureRecord. Notes on Azure records: An Azure record set has the form RecordSet(name=<...>, type=<...>, arecords=[...], aaaa_records, ..) @@ -222,7 +222,7 @@ class AzureProvider(BaseProvider): azuredns: class: octodns.provider.azuredns.AzureProvider client_id: env/AZURE_APPLICATION_ID - key: env/AZURE_AUTHENICATION_KEY + key: env/AZURE_AUTHENTICATION_KEY directory_id: env/AZURE_DIRECTORY_ID sub_id: env/AZURE_SUBSCRIPTION_ID resource_group: 'TestResource1' diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 6f67f1c..4e99651 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -30,14 +30,14 @@ class BaseProvider(BaseSource): def _include_change(self, change): ''' An opportunity for providers to filter out false positives due to - pecularities in their implementation. E.g. minimum TTLs. + peculiarities in their implementation. E.g. minimum TTLs. ''' return True def _extra_changes(self, existing, changes): ''' An opportunity for providers to add extra changes to the plan that are - necessary to update ancilary record data or configure the zone. E.g. + necessary to update ancillary record data or configure the zone. E.g. base NS records. ''' return [] diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 688b10e..b66fcfa 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -394,7 +394,7 @@ class CloudflareProvider(BaseProvider): for record in self.zone_records(zone): name = zone.hostname_from_fqdn(record['name']) # Use the _record_for so that we include all of standard - # converstion logic + # conversion logic r = self._record_for(zone, name, record['type'], [record], True) if hostname == r.name and _type == r._type: diff --git a/octodns/provider/digitalocean.py b/octodns/provider/digitalocean.py index c68e7d6..5de8aee 100644 --- a/octodns/provider/digitalocean.py +++ b/octodns/provider/digitalocean.py @@ -56,7 +56,7 @@ class DigitalOceanClient(object): self._request('POST', '/domains', data={'name': name, 'ip_address': '192.0.2.1'}) - # After the zone is created, immeadiately delete the record + # After the zone is created, immediately delete the record records = self.records(name) for record in records: if record['name'] == '' and record['type'] == 'A': diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py index 43b5b9b..ca95706 100644 --- a/octodns/provider/dnsimple.py +++ b/octodns/provider/dnsimple.py @@ -160,7 +160,7 @@ class DnsimpleProvider(BaseProvider): record['content'].split(' ', 5) except ValueError: # their api will let you create invalid records, this - # essnetially handles that by ignoring them for values + # essentially handles that by ignoring them for values # purposes. That will cause updates to happen to delete them if # they shouldn't exist or update them if they're wrong continue diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 010fd31..bcd8308 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -40,7 +40,7 @@ class _CachingDynZone(DynZone): cls.log.debug('get: fetched') except DynectGetError: if not create: - cls.log.debug("get: does't exist") + cls.log.debug("get: doesn't exist") return None # this value shouldn't really matter, it's not tied to # whois or anything @@ -129,11 +129,11 @@ class DynProvider(BaseProvider): REGION_CODES = { 'NA': 11, # Continental North America 'SA': 12, # Continental South America - 'EU': 13, # Contentinal Europe + 'EU': 13, # Continental Europe 'AF': 14, # Continental Africa - 'AS': 15, # Contentinal Asia - 'OC': 16, # Contentinal Austrailia/Oceania - 'AN': 17, # Continental Antartica + 'AS': 15, # Continental Asia + 'OC': 16, # Continental Australia/Oceania + 'AN': 17, # Continental Antarctica } _sess_create_lock = Lock() @@ -166,7 +166,7 @@ class DynProvider(BaseProvider): if DynectSession.get_session() is None: # We need to create a new session for this thread and DynectSession # creation is not thread-safe so we have to do the locking. If we - # don't and multiple sessions start creattion before the the first + # don't and multiple sessions start creation before the the first # has finished (long time b/c it makes http calls) the subsequent # creates will blow away DynectSession._instances, potentially # multiple times if there are multiple creates in flight. Only the @@ -291,7 +291,7 @@ class DynProvider(BaseProvider): try: fqdn, _type = td.label.split(':', 1) except ValueError as e: - self.log.warn("Failed to load TraficDirector '%s': %s", + self.log.warn("Failed to load TrafficDirector '%s': %s", td.label, e.message) continue tds[fqdn][_type] = td diff --git a/octodns/provider/powerdns.py b/octodns/provider/powerdns.py index 4527f8e..aafb64e 100644 --- a/octodns/provider/powerdns.py +++ b/octodns/provider/powerdns.py @@ -178,7 +178,7 @@ class PowerDnsBaseProvider(BaseProvider): raise Exception('PowerDNS unauthorized host={}' .format(self.host)) elif e.response.status_code == 422: - # 422 means powerdns doesn't know anything about the requsted + # 422 means powerdns doesn't know anything about the requested # domain. We'll just ignore it here and leave the zone # untouched. pass @@ -294,8 +294,8 @@ class PowerDnsBaseProvider(BaseProvider): return [] # sorting mostly to make things deterministic for testing, but in - # theory it let us find what we're after quickier (though sorting would - # ve more exepensive.) + # theory it let us find what we're after quicker (though sorting would + # be more expensive.) for record in sorted(existing.records): if record == ns: # We've found the top-level NS record, return any changes @@ -341,7 +341,7 @@ class PowerDnsBaseProvider(BaseProvider): e.response.text) raise self.log.info('_apply: creating zone=%s', desired.name) - # 422 means powerdns doesn't know anything about the requsted + # 422 means powerdns doesn't know anything about the requested # domain. We'll try to create it with the correct records instead # of update. Hopefully all the mods are creates :-) data = { diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 5bda074..cf9c3c4 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -61,7 +61,7 @@ class _Route53Record(object): # NOTE: we're using __hash__ and __cmp__ methods that consider # _Route53Records equivalent if they have the same class, fqdn, and _type. - # Values are ignored. This is usful when computing diffs/changes. + # Values are ignored. This is useful when computing diffs/changes. def __hash__(self): 'sub-classes should never use this method' @@ -679,7 +679,7 @@ class Route53Provider(BaseProvider): .get('CountryCode', False) == '*': # it's a default record continue - # we expect a healtcheck now + # we expect a healthcheck now try: health_check_id = rrset['HealthCheckId'] caller_ref = \ @@ -730,7 +730,7 @@ class Route53Provider(BaseProvider): batch_rs_count) # send the batch self._really_apply(batch, zone_id) - # start a new batch with the lefovers + # start a new batch with the leftovers batch = mods batch_rs_count = mods_rs_count diff --git a/octodns/record.py b/octodns/record.py index 82ee191..6d1e25b 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -151,7 +151,7 @@ class Record(object): # NOTE: we're using __hash__ and __cmp__ methods that consider Records # equivalent if they have the same name & _type. Values are ignored. This - # is usful when computing diffs/changes. + # is useful when computing diffs/changes. def __hash__(self): return '{}:{}'.format(self.name, self._type).__hash__() diff --git a/octodns/source/base.py b/octodns/source/base.py index 4ace09f..15455b8 100644 --- a/octodns/source/base.py +++ b/octodns/source/base.py @@ -30,7 +30,7 @@ class BaseSource(object): When `lenient` is True the populate call may skip record validation and do a "best effort" load of data. That will allow through some common, but not best practices stuff that we otherwise would reject. E.g. no - trailing . or mising escapes for ;. + trailing . or missing escapes for ;. ''' raise NotImplementedError('Abstract base class, populate method ' 'missing') diff --git a/octodns/zone.py b/octodns/zone.py index a8a91ca..bed3a59 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -37,10 +37,10 @@ class Zone(object): if not name[-1] == '.': raise Exception('Invalid zone name {}, missing ending dot' .format(name)) - # Force everyting to lowercase just to be safe + # Force everything to lowercase just to be safe self.name = unicode(name).lower() if name else name self.sub_zones = sub_zones - # We're grouping by node, it allows us to efficently search for + # We're grouping by node, it allows us to efficiently search for # duplicates and detect when CNAMEs co-exist with other records self._records = defaultdict(set) # optional leading . to match empty hostname diff --git a/script/release b/script/release index d8fabf2..bcc0ba3 100755 --- a/script/release +++ b/script/release @@ -11,4 +11,4 @@ git tag -s v$VERSION -m "Release $VERSION" git push origin v$VERSION echo "Tagged and pushed v$VERSION" python setup.py sdist upload -echo "Updloaded $VERSION" +echo "Uploaded $VERSION" diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index 855efaf..22544e1 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -63,14 +63,14 @@ class TestBaseProvider(TestCase): zone = Zone('unit.tests.', []) with self.assertRaises(NotImplementedError) as ctx: - HasSupportsGeo('hassupportesgeo').populate(zone) + HasSupportsGeo('hassupportsgeo').populate(zone) self.assertEquals('Abstract base class, SUPPORTS property missing', ctx.exception.message) class HasSupports(HasSupportsGeo): SUPPORTS = set(('A',)) with self.assertRaises(NotImplementedError) as ctx: - HasSupports('hassupportes').populate(zone) + HasSupports('hassupports').populate(zone) self.assertEquals('Abstract base class, populate method missing', ctx.exception.message) @@ -94,7 +94,7 @@ class TestBaseProvider(TestCase): 'value': '1.2.3.4' })) - self.assertTrue(HasSupports('hassupportesgeo') + self.assertTrue(HasSupports('hassupportsgeo') .supports(list(zone.records)[0])) plan = HasPopulate('haspopulate').plan(zone) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 7bb1b6a..36a6d35 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -599,7 +599,8 @@ class TestCloudflareProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) - # the two A records get merged into one CNAME record poining to the CDN + # the two A records get merged into one CNAME record pointing to + # the CDN. self.assertEquals(3, len(zone.records)) record = list(zone.records)[0] @@ -621,7 +622,7 @@ class TestCloudflareProvider(TestCase): self.assertEquals('a.unit.tests.cdn.cloudflare.net.', record.value) # CDN enabled records can't be updated, we don't know the real values - # never point a Cloudflare record to itsself. + # never point a Cloudflare record to itself. wanted = Zone('unit.tests.', []) wanted.add_record(Record.new(wanted, 'cname', { 'ttl': 300, @@ -676,7 +677,7 @@ class TestCloudflareProvider(TestCase): self.assertEquals('unit.tests.cdn.cloudflare.net.', record.value) # CDN enabled records can't be updated, we don't know the real values - # never point a Cloudflare record to itsself. + # never point a Cloudflare record to itself. wanted = Zone('unit.tests.', []) wanted.add_record(Record.new(wanted, '', { 'ttl': 300, diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 4415347..d1be606 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -491,7 +491,7 @@ class TestDynProviderGeo(TestCase): traffic_director_response = loads(fh.read()) @property - def traffic_directors_reponse(self): + def traffic_directors_response(self): return { 'data': [{ 'active': 'Y', @@ -607,7 +607,7 @@ class TestDynProviderGeo(TestCase): mock.side_effect = [{'data': []}] self.assertEquals({}, provider.traffic_directors) - # a supported td and an ingored one + # a supported td and an ignored one response = { 'data': [{ 'active': 'Y', @@ -650,7 +650,7 @@ class TestDynProviderGeo(TestCase): set(tds.keys())) self.assertEquals(['A'], tds['unit.tests.'].keys()) self.assertEquals(['A'], tds['geo.unit.tests.'].keys()) - provider.log.warn.assert_called_with("Failed to load TraficDirector " + provider.log.warn.assert_called_with("Failed to load TrafficDirector " "'%s': %s", 'something else', 'need more than 1 value to ' 'unpack') @@ -758,7 +758,7 @@ class TestDynProviderGeo(TestCase): # only traffic director mock.side_effect = [ # get traffic directors - self.traffic_directors_reponse, + self.traffic_directors_response, # get traffic director self.traffic_director_response, # get zone @@ -809,7 +809,7 @@ class TestDynProviderGeo(TestCase): # both traffic director and regular, regular is ignored mock.side_effect = [ # get traffic directors - self.traffic_directors_reponse, + self.traffic_directors_response, # get traffic director self.traffic_director_response, # get zone @@ -859,7 +859,7 @@ class TestDynProviderGeo(TestCase): # busted traffic director mock.side_effect = [ # get traffic directors - self.traffic_directors_reponse, + self.traffic_directors_response, # get traffic director busted_traffic_director_response, # get zone @@ -932,14 +932,14 @@ class TestDynProviderGeo(TestCase): provider = DynProvider('test', 'cust', 'user', 'pass', traffic_directors_enabled=True) - # will be tested seperately + # will be tested separately provider._mod_rulesets = MagicMock() mock.side_effect = [ # create traffic director self.traffic_director_response, # get traffic directors - self.traffic_directors_reponse + self.traffic_directors_response ] provider._mod_geo_Create(None, Create(self.geo_record)) # td now lives in cache diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index 6c6c2aa..127cb53 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -361,10 +361,10 @@ class TestGoogleCloudProvider(TestCase): # test_zone gets fed the same records as zone does, except it's in # the format returned by google API, so after populate they should look - # excactly the same. + # exactly the same. self.assertEqual(test_zone.records, zone.records) - test_zone2 = Zone('nonexistant.zone.', []) + test_zone2 = Zone('nonexistent.zone.', []) provider.populate(test_zone2, False, False) self.assertEqual(len(test_zone2.records), 0, @@ -401,8 +401,8 @@ class TestGoogleCloudProvider(TestCase): provider.gcloud_client.list_zones = Mock( return_value=DummyIterator([])) - self.assertIsNone(provider.gcloud_zones.get("nonexistant.xone"), - msg="Check that nonexistant zones return None when" + self.assertIsNone(provider.gcloud_zones.get("nonexistent.zone"), + msg="Check that nonexistent zones return None when" "there's no create=True flag") def test__get_rrsets(self): @@ -423,7 +423,7 @@ class TestGoogleCloudProvider(TestCase): provider.gcloud_client.list_zones = Mock( return_value=DummyIterator([])) - mock_zone = provider._create_gcloud_zone("nonexistant.zone.mock") + mock_zone = provider._create_gcloud_zone("nonexistent.zone.mock") mock_zone.create.assert_called() provider.gcloud_client.zone.assert_called() diff --git a/tests/test_octodns_provider_ovh.py b/tests/test_octodns_provider_ovh.py index 9404f9e..3e7affe 100644 --- a/tests/test_octodns_provider_ovh.py +++ b/tests/test_octodns_provider_ovh.py @@ -199,14 +199,14 @@ class TestOvhProvider(TestCase): api_record.append({ 'fieldType': 'SPF', 'ttl': 1000, - 'target': 'v=spf1 include:unit.texts.rerirect ~all', + 'target': 'v=spf1 include:unit.texts.redirect ~all', 'subDomain': '', 'id': 13 }) expected.add(Record.new(zone, '', { 'ttl': 1000, 'type': 'SPF', - 'value': 'v=spf1 include:unit.texts.rerirect ~all' + 'value': 'v=spf1 include:unit.texts.redirect ~all' })) # SSHFP @@ -404,7 +404,7 @@ class TestOvhProvider(TestCase): call(u'/domain/zone/unit.tests/record', fieldType=u'SPF', subDomain=u'', ttl=1000, target=u'v=spf1 include:unit.texts.' - u'rerirect ~all', + u'redirect ~all', ), call(u'/domain/zone/unit.tests/record', fieldType=u'A', subDomain='sub', target=u'1.2.3.4', ttl=200), diff --git a/tests/test_octodns_provider_powerdns.py b/tests/test_octodns_provider_powerdns.py index 22ccdd6..5d89c3e 100644 --- a/tests/test_octodns_provider_powerdns.py +++ b/tests/test_octodns_provider_powerdns.py @@ -100,7 +100,7 @@ class TestPowerDnsProvider(TestCase): # No existing records -> creates for every record in expected with requests_mock() as mock: mock.get(ANY, status_code=200, text=EMPTY_TEXT) - # post 201, is reponse to the create with data + # post 201, is response to the create with data mock.patch(ANY, status_code=201, text=assert_rrsets_callback) plan = provider.plan(expected) @@ -118,7 +118,7 @@ class TestPowerDnsProvider(TestCase): mock.get(ANY, status_code=422, text='') # patch 422's, unknown zone mock.patch(ANY, status_code=422, text=dumps(not_found)) - # post 201, is reponse to the create with data + # post 201, is response to the create with data mock.post(ANY, status_code=201, text=assert_rrsets_callback) plan = provider.plan(expected) diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 1cd4548..c2afcd2 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -331,9 +331,9 @@ class TestRoute53Provider(TestCase): stubber.assert_no_pending_responses() # Populate a zone that doesn't exist - noexist = Zone('does.not.exist.', []) - provider.populate(noexist) - self.assertEquals(set(), noexist.records) + nonexistent = Zone('does.not.exist.', []) + provider.populate(nonexistent) + self.assertEquals(set(), nonexistent.records) def test_sync(self): provider, stubber = self._get_stubbed_provider() diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 9f3dc33..56502a0 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -430,7 +430,7 @@ class TestRecord(TestCase): self.assertEqual(change.new, other) # full sorting - # equivilent + # equivalent b_naptr_value = b.values[0] self.assertEquals(0, b_naptr_value.__cmp__(b_naptr_value)) # by order @@ -710,7 +710,7 @@ class TestRecord(TestCase): Record.new(self.zone, 'unknown', {}) self.assertTrue('missing type' in ctx.exception.message) - # Unkown type + # Unknown type with self.assertRaises(Exception) as ctx: Record.new(self.zone, 'unknown', { 'type': 'XXX', From 876c09dcc07efb721f3ac08262d23bb2d9861a4e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 3 Mar 2018 10:12:34 -0800 Subject: [PATCH 22/22] Flesh out UT for new Plan.exists messaging --- tests/test_octodns_plan.py | 45 ++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index 91dd948..7d849be 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -16,15 +16,6 @@ from octodns.zone import Zone from helpers import SimpleProvider -class TestPlanLogger(TestCase): - - def test_invalid_level(self): - with self.assertRaises(Exception) as ctx: - PlanLogger('invalid', 'not-a-level') - self.assertEquals('Unsupported level: not-a-level', - ctx.exception.message) - - simple = SimpleProvider() zone = Zone('unit.tests.', []) existing = Record.new(zone, 'a', { @@ -48,15 +39,45 @@ create = Create(Record.new(zone, 'b', { 'type': 'CNAME', 'value': 'foo.unit.tests.' }, simple)) +create2 = Create(Record.new(zone, 'c', { + 'ttl': 60, + 'type': 'CNAME', + 'value': 'foo.unit.tests.' +})) update = Update(existing, new) delete = Delete(new) -changes = [create, delete, update] +changes = [create, create2, delete, update] plans = [ (simple, Plan(zone, zone, changes, True)), - (simple, Plan(zone, zone, changes, True)), + (simple, Plan(zone, zone, changes, False)), ] +class TestPlanLogger(TestCase): + + def test_invalid_level(self): + with self.assertRaises(Exception) as ctx: + PlanLogger('invalid', 'not-a-level') + self.assertEquals('Unsupported level: not-a-level', + ctx.exception.message) + + def test_create(self): + + class MockLogger(object): + + def __init__(self): + self.out = StringIO() + + def log(self, level, msg): + self.out.write(msg) + + log = MockLogger() + PlanLogger('logger').run(log, plans) + out = log.out.getvalue() + self.assertTrue('Summary: Creates=2, Updates=1, ' + 'Deletes=1, Existing Records=0' in out) + + class TestPlanHtml(TestCase): log = getLogger('TestPlanHtml') @@ -69,7 +90,7 @@ class TestPlanHtml(TestCase): out = StringIO() PlanHtml('html').run(plans, fh=out) out = out.getvalue() - self.assertTrue(' Summary: Creates=1, Updates=1, ' + self.assertTrue(' Summary: Creates=2, Updates=1, ' 'Deletes=1, Existing Records=0' in out)