From 1e71bce9072838ce0385704502d76813ec8794c0 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 21 Jan 2018 13:47:58 -0800 Subject: [PATCH 01/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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 f62f82496696683112932ca9ee76fb9b426af18d Mon Sep 17 00:00:00 2001 From: Masaki Tagawa Date: Tue, 13 Feb 2018 23:43:30 +0900 Subject: [PATCH 15/18] Escape unescaped semicolons coming out of Google Cloud DNS --- octodns/provider/googlecloud.py | 8 ++++++-- tests/test_octodns_provider_googlecloud.py | 10 ++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index 6ca0794..5ea43ac 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -9,6 +9,7 @@ import shlex import time from logging import getLogger from uuid import uuid4 +import re from google.cloud import dns @@ -269,12 +270,15 @@ class GoogleCloudProvider(BaseProvider): _data_for_PTR = _data_for_CNAME + _fix_semicolons = re.compile(r'(? 1: return { - 'values': gcloud_record.rrdatas} + 'values': [self._fix_semicolons.sub('\;', rr) + for rr in gcloud_record.rrdatas]} return { - 'value': gcloud_record.rrdatas[0]} + 'value': self._fix_semicolons.sub('\;', gcloud_record.rrdatas[0])} def _data_for_SRV(self, gcloud_record): return {'values': [{ diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index adc2112..79fda7f 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -427,3 +427,13 @@ class TestGoogleCloudProvider(TestCase): mock_zone.create.assert_called() provider.gcloud_client.zone.assert_called() + + def test_semicolon_fixup(self): + provider = self._get_provider() + + self.assertEquals({ + 'values': ['abcd\\; ef\\;g', 'hij\\; klm\\;n'] + }, provider._data_for_TXT( + DummyResourceRecordSet( + 'unit.tests.', 'TXT', 0, ['abcd; ef;g', 'hij\\; klm\\;n']) + )) From 7215d80230e4c598eb8caedf429dc142b10b1523 Mon Sep 17 00:00:00 2001 From: Masaki Tagawa Date: Sun, 18 Feb 2018 10:36:02 +0900 Subject: [PATCH 16/18] PEP8 --- tests/test_octodns_provider_googlecloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index 3ad87cb..c2677af 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -451,4 +451,4 @@ class TestGoogleCloudProvider(TestCase): }, provider._data_for_TXT( DummyResourceRecordSet( 'unit.tests.', 'TXT', 0, ['abcd; ef;g', 'hij\\; klm\\;n']) - )) \ No newline at end of file + )) From 5d2ba2e715f770a4ddd90a2b5ca93563a3cdf785 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Tue, 27 Feb 2018 12:09:47 -0500 Subject: [PATCH 17/18] 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 18/18] 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)