diff --git a/docs/records.md b/docs/records.md index b7cbbe9..d0100d3 100644 --- a/docs/records.md +++ b/docs/records.md @@ -126,7 +126,7 @@ If you'd like to enable lenience for a whole zone you can do so with the followi #### Restrict Record manipulations octoDNS currently provides the ability to limit the number of updates/deletes on -DNS records by configuring a percentage of allowed operations as a threshold. +DNS records by configuring a percentage of allowed operations as a provider threshold. If left unconfigured, suitable defaults take over instead. In the below example, the Dyn provider is configured with limits of 40% on both update and delete operations over all the records present. @@ -138,6 +138,17 @@ dyn: delete_pcent_threshold: 0.4 ```` +Additionally, thresholds can be configured at the zone level. Zone thresholds +take precedence over any provider default or explicit configuration. Zone +thresholds do not have a default. + +```yaml +zones: + example.com.: + update_pcent_threshold: 0.2 + delete_pcent_threshold: 0.1 +``` + ## Provider specific record types ### Creating and registering diff --git a/octodns/manager.py b/octodns/manager.py index 3ea0601..e74487c 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -1052,6 +1052,13 @@ class Manager(object): zone = self.config['zones'].get(zone_name) if zone is not None: sub_zones = self.configured_sub_zones(zone_name) - return Zone(idna_encode(zone_name), sub_zones) + update_pcent_threshold = zone.get("update_pcent_threshold", None) + delete_pcent_threshold = zone.get("delete_pcent_threshold", None) + return Zone( + idna_encode(zone_name), + sub_zones, + update_pcent_threshold, + delete_pcent_threshold, + ) raise ManagerException(f'Unknown zone name {idna_decode(zone_name)}') diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 49b7156..df4cb94 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -60,6 +60,12 @@ class Plan(object): self.update_pcent_threshold = update_pcent_threshold self.delete_pcent_threshold = delete_pcent_threshold + # Zone thresholds take precedence over provider + if existing and existing.update_pcent_threshold is not None: + self.update_pcent_threshold = existing.update_pcent_threshold + if existing and existing.delete_pcent_threshold is not None: + self.delete_pcent_threshold = existing.delete_pcent_threshold + change_counts = {'Create': 0, 'Delete': 0, 'Update': 0} for change in changes: change_counts[change.__class__.__name__] += 1 diff --git a/octodns/zone.py b/octodns/zone.py index fe604f7..4ac7823 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -56,7 +56,13 @@ class InvalidNodeException(Exception): class Zone(object): log = getLogger('Zone') - def __init__(self, name, sub_zones): + def __init__( + self, + name, + sub_zones, + update_pcent_threshold=None, + delete_pcent_threshold=None, + ): if not name[-1] == '.': raise Exception(f'Invalid zone name {name}, missing ending dot') elif ' ' in name or '\t' in name: @@ -78,6 +84,9 @@ class Zone(object): self._utf8_name_re = re.compile(fr'\.?{idna_decode(name)}?$') self._idna_name_re = re.compile(fr'\.?{self.name}?$') + self.update_pcent_threshold = update_pcent_threshold + self.delete_pcent_threshold = delete_pcent_threshold + # Copy-on-write semantics support, when `not None` this property will # point to a location with records for this `Zone`. Once `hydrated` # this property will be set to None @@ -352,7 +361,12 @@ class Zone(object): copying the records when required. The actual record copy will not be "deep" meaning that records should not be modified directly. ''' - copy = Zone(self.name, self.sub_zones) + copy = Zone( + self.name, + self.sub_zones, + self.update_pcent_threshold, + self.delete_pcent_threshold, + ) copy._origin = self return copy diff --git a/tests/config/zone-threshold.yaml b/tests/config/zone-threshold.yaml new file mode 100644 index 0000000..0598555 --- /dev/null +++ b/tests/config/zone-threshold.yaml @@ -0,0 +1,28 @@ +manager: + max_workers: 2 +providers: + in: + class: octodns.provider.yaml.YamlProvider + directory: tests/config + strict_supports: False + dump: + class: octodns.provider.yaml.YamlProvider + directory: env/YAML_TMP_DIR + supports_root_ns: False + strict_supports: False +zones: + unit.tests.: + update_pcent_threshold: 0.2 + delete_pcent_threshold: 0.1 + sources: + - in + targets: + - dump + + subzone.unit.tests.: + update_pcent_threshold: 0.02 + delete_pcent_threshold: 0.01 + sources: + - in + targets: + - dump diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 05d20aa..6ed972f 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -1294,6 +1294,23 @@ class TestManager(TestCase): requires_dummy.fetch(':hello', None), ) + def test_zone_threshold(self): + with TemporaryDirectory() as tmpdir: + environ['YAML_TMP_DIR'] = tmpdir.dirname + + manager = Manager(get_config_filename('zone-threshold.yaml')) + + zone = manager.get_zone('unit.tests.') + + self.assertEqual(0.2, zone.update_pcent_threshold) + self.assertEqual(0.1, zone.delete_pcent_threshold) + + # subzone has different threshold + subzone = manager.get_zone('subzone.unit.tests.') + + self.assertEqual(0.02, subzone.update_pcent_threshold) + self.assertEqual(0.01, subzone.delete_pcent_threshold) + class TestMainThreadExecutor(TestCase): def test_success(self): diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index 1942666..2fda278 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -165,8 +165,27 @@ class TestPlanSafety(TestCase): record_4 = Record.new( existing, '4', data={'type': 'A', 'ttl': 42, 'value': '1.2.3.4'} ) + record_5 = Record.new( + existing, '5', data={'type': 'A', 'ttl': 42, 'value': '1.2.3.4'} + ) + record_6 = Record.new( + existing, '6', data={'type': 'A', 'ttl': 42, 'value': '1.2.3.4'} + ) + record_7 = Record.new( + existing, '7', data={'type': 'A', 'ttl': 42, 'value': '1.2.3.4'} + ) + record_8 = Record.new( + existing, '8', data={'type': 'A', 'ttl': 42, 'value': '1.2.3.4'} + ) - def test_too_many_updates(self): + # manager loads the zone's config, so existing also holds providers & other config + update_threshold = 0.2 + delete_threshold = 0.1 + existing_with_thresholds = Zone( + 'cautious.tests.', [], update_threshold, delete_threshold + ) + + def test_too_many_provider_updates(self): existing = self.existing.copy() changes = [] @@ -206,7 +225,46 @@ class TestPlanSafety(TestCase): plan = HelperPlan(existing, None, changes, True, min_existing=10) plan.raise_if_unsafe() - def test_too_many_deletes(self): + def test_too_many_zone_updates(self): + existing = self.existing_with_thresholds.copy() + changes = [] + + # No records, no changes, we're good + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # Setup quite a few records, so that + # zone can be more cautious than provider's default + existing.add_record(self.record_1) + existing.add_record(self.record_2) + existing.add_record(self.record_3) + existing.add_record(self.record_4) + existing.add_record(self.record_5) + existing.add_record(self.record_6) + existing.add_record(self.record_7) + existing.add_record(self.record_8) + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # One update is ok: 12.5% < 20% + changes.append(Update(self.record_1, self.record_1)) + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # Still ok, zone takes precedence + plan = HelperPlan( + existing, None, changes, True, update_pcent_threshold=0 + ) + plan.raise_if_unsafe() + + # Two exceeds threshold, 25% > 20% + changes.append(Update(self.record_2, self.record_2)) + plan = HelperPlan(existing, None, changes, True) + with self.assertRaises(TooMuchChange) as ctx: + plan.raise_if_unsafe() + self.assertTrue('Too many updates', str(ctx.exception)) + + def test_too_many_provider_deletes(self): existing = self.existing.copy() changes = [] @@ -246,6 +304,34 @@ class TestPlanSafety(TestCase): plan = HelperPlan(existing, None, changes, True, min_existing=10) plan.raise_if_unsafe() + def test_too_many_zone_deletes(self): + existing = self.existing_with_thresholds.copy() + changes = [] + + # No records, no changes, we're good + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # Setup quite a few records, so that + # zone can be more cautious than provider's default + existing.add_record(self.record_1) + existing.add_record(self.record_2) + existing.add_record(self.record_3) + existing.add_record(self.record_4) + existing.add_record(self.record_5) + existing.add_record(self.record_6) + existing.add_record(self.record_7) + existing.add_record(self.record_8) + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # One delete exceeds Zone threshold + changes.append(Delete(self.record_1)) + plan = HelperPlan(existing, None, changes, True) + with self.assertRaises(TooMuchChange) as ctx: + plan.raise_if_unsafe() + self.assertTrue('Too many deletes', str(ctx.exception)) + def test_root_ns_change(self): existing = self.existing.copy() changes = []