diff --git a/octodns/provider/base.py b/octodns/provider/base.py index d839d59..e4f8139 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -7,6 +7,7 @@ from __future__ import absolute_import, division, print_function, \ from ..source.base import BaseSource from ..zone import Zone +from logging import getLogger class UnsafePlan(Exception): @@ -14,8 +15,11 @@ class UnsafePlan(Exception): class Plan(object): - MAX_SAFE_UPDATES = 4 - MAX_SAFE_DELETES = 4 + log = getLogger('Plan') + + MAX_SAFE_UPDATE_PCENT = .3 + MAX_SAFE_DELETE_PCENT = .3 + MIN_EXISTING_RECORDS = 10 def __init__(self, existing, desired, changes): self.existing = existing @@ -31,12 +35,42 @@ class Plan(object): change_counts[change.__class__.__name__] += 1 self.change_counts = change_counts + if self.existing: + self.log.debug('__init__: Creates=%s, Updates=%s, Deletes=%s' + 'Existing=%s', + self.change_counts['Create'], + self.change_counts['Update'], + self.change_counts['Delete'], + len(self.existing.records)) + else: + self.log.debug('__init__: Creates=%s, Updates=%s, Deletes=%s', + self.change_counts['Create'], + self.change_counts['Update'], + self.change_counts['Delete']) + def raise_if_unsafe(self): # TODO: what is safe really? - if self.change_counts['Update'] > self.MAX_SAFE_UPDATES: - raise UnsafePlan('Too many updates') - if self.change_counts['Delete'] > self.MAX_SAFE_DELETES: - raise UnsafePlan('Too many deletes') + if self.existing and \ + len(self.existing.records) >= self.MIN_EXISTING_RECORDS: + + existing_record_count = len(self.existing.records) + update_pcent = self.change_counts['Update'] / existing_record_count + delete_pcent = self.change_counts['Delete'] / existing_record_count + + if update_pcent > self.MAX_SAFE_UPDATE_PCENT: + raise UnsafePlan('Too many updates, %s is over %s percent' + '(%s/%s)', + update_pcent, + self.MAX_SAFE_UPDATE_PCENT * 100, + self.change_counts['Update'], + existing_record_count) + if delete_pcent > self.MAX_SAFE_DELETE_PCENT: + raise UnsafePlan('Too many deletes, %s is over %s percent' + '(%s/%s)', + delete_pcent, + self.MAX_SAFE_DELETE_PCENT * 100, + self.change_counts['Delete'], + existing_record_count) def __repr__(self): return 'Creates={}, Updates={}, Deletes={}, Existing Records={}' \ diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index 22ea935..c7836c8 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -138,33 +138,140 @@ class TestBaseProvider(TestCase): # We filtered out the only change self.assertFalse(plan) - def test_safe(self): - ignored = Zone('unit.tests.', []) - + def test_safe_none(self): # No changes is safe Plan(None, None, []).raise_if_unsafe() - # Creates are safe - record = Record.new(ignored, 'a', { + def test_safe_creates(self): + # Creates are safe when existing records is under MIN_EXISTING_RECORDS + zone = Zone('unit.tests.', []) + + record = Record.new(zone, 'a', { + 'ttl': 30, + 'type': 'A', + 'value': '1.2.3.4', + }) + Plan(zone, zone, [Create(record) for i in range(10)]).raise_if_unsafe() + + def test_safe_min_existing_creates(self): + # Creates are safe when existing records is over MIN_EXISTING_RECORDS + zone = Zone('unit.tests.', []) + + record = Record.new(zone, 'a', { + 'ttl': 30, + 'type': 'A', + 'value': '1.2.3.4', + }) + + for i in range(int(Plan.MIN_EXISTING_RECORDS)): + zone.add_record(Record.new(zone, str(i), { + 'ttl': 60, + 'type': 'A', + 'value': '2.3.4.5' + })) + + Plan(zone, zone, [Create(record) for i in range(10)]).raise_if_unsafe() + + def test_safe_no_existing(self): + # existing records fewer than MIN_EXISTING_RECORDS is safe + zone = Zone('unit.tests.', []) + record = Record.new(zone, 'a', { + 'ttl': 30, + 'type': 'A', + 'value': '1.2.3.4', + }) + + updates = [Update(record, record), Update(record, record)] + Plan(zone, zone, updates).raise_if_unsafe() + + def test_safe_updates_min_existing(self): + # MAX_SAFE_UPDATE_PCENT+1 fails when more + # than MIN_EXISTING_RECORDS exist + zone = Zone('unit.tests.', []) + record = Record.new(zone, 'a', { 'ttl': 30, 'type': 'A', 'value': '1.2.3.4', }) - Plan(None, None, [Create(record) for i in range(10)]).raise_if_unsafe() - # max Updates is safe + for i in range(int(Plan.MIN_EXISTING_RECORDS)): + zone.add_record(Record.new(zone, str(i), { + 'ttl': 60, + 'type': 'A', + 'value': '2.3.4.5' + })) + changes = [Update(record, record) - for i in range(Plan.MAX_SAFE_UPDATES)] - Plan(None, None, changes).raise_if_unsafe() - # but max + 1 isn't + for i in range(int(Plan.MIN_EXISTING_RECORDS * + Plan.MAX_SAFE_UPDATE_PCENT) + 1)] + with self.assertRaises(UnsafePlan): - changes.append(Update(record, record)) - Plan(None, None, changes).raise_if_unsafe() + Plan(zone, zone, changes).raise_if_unsafe() + + def test_safe_updates_min_existing_pcent(self): + # MAX_SAFE_UPDATE_PCENT is safe when more + # than MIN_EXISTING_RECORDS exist + zone = Zone('unit.tests.', []) + record = Record.new(zone, 'a', { + 'ttl': 30, + 'type': 'A', + 'value': '1.2.3.4', + }) + + for i in range(int(Plan.MIN_EXISTING_RECORDS)): + zone.add_record(Record.new(zone, str(i), { + 'ttl': 60, + 'type': 'A', + 'value': '2.3.4.5' + })) + changes = [Update(record, record) + for i in range(int(Plan.MIN_EXISTING_RECORDS * + Plan.MAX_SAFE_UPDATE_PCENT))] + + Plan(zone, zone, changes).raise_if_unsafe() + + def test_safe_deletes_min_existing(self): + # MAX_SAFE_DELETE_PCENT+1 fails when more + # than MIN_EXISTING_RECORDS exist + zone = Zone('unit.tests.', []) + record = Record.new(zone, 'a', { + 'ttl': 30, + 'type': 'A', + 'value': '1.2.3.4', + }) + + for i in range(int(Plan.MIN_EXISTING_RECORDS)): + zone.add_record(Record.new(zone, str(i), { + 'ttl': 60, + 'type': 'A', + 'value': '2.3.4.5' + })) + + changes = [Delete(record) + for i in range(int(Plan.MIN_EXISTING_RECORDS * + Plan.MAX_SAFE_DELETE_PCENT) + 1)] - # max Deletes is safe - changes = [Delete(record) for i in range(Plan.MAX_SAFE_DELETES)] - Plan(None, None, changes).raise_if_unsafe() - # but max + 1 isn't with self.assertRaises(UnsafePlan): - changes.append(Delete(record)) - Plan(None, None, changes).raise_if_unsafe() + Plan(zone, zone, changes).raise_if_unsafe() + + def test_safe_deletes_min_existing_pcent(self): + # MAX_SAFE_DELETE_PCENT is safe when more + # than MIN_EXISTING_RECORDS exist + zone = Zone('unit.tests.', []) + record = Record.new(zone, 'a', { + 'ttl': 30, + 'type': 'A', + 'value': '1.2.3.4', + }) + + for i in range(int(Plan.MIN_EXISTING_RECORDS)): + zone.add_record(Record.new(zone, str(i), { + 'ttl': 60, + 'type': 'A', + 'value': '2.3.4.5' + })) + changes = [Delete(record) + for i in range(int(Plan.MIN_EXISTING_RECORDS * + Plan.MAX_SAFE_DELETE_PCENT))] + + Plan(zone, zone, changes).raise_if_unsafe()