Merge pull request #1159 from cholick/main

Add Zone Specific Threshold Config

CHANGELOG.md

@@ -1,3 +1,9 @@
+## v1.?.? - 2024-??-?? - ????
+
+* Support for specifying per-zone change thresholds, to allow for zones
+  where lots of changes are expected frequently to live along side zones
+  where little or no churn is expected.
+
 ## v1.6.1 - 2024-03-17 - Didn't we do this already
 
 * Fix env var type handling that was previously fixed in 1.5.1 and then

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

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)}')

octodns/provider/plan.py

@@ -57,9 +57,16 @@ class Plan(object):
         # them and/or is as safe as possible.
         self.changes = sorted(changes)
         self.exists = exists
-        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
+        else:
+            self.update_pcent_threshold = update_pcent_threshold
+        if existing and existing.delete_pcent_threshold is not None:
+            self.delete_pcent_threshold = existing.delete_pcent_threshold
+        else:
+            self.delete_pcent_threshold = delete_pcent_threshold
         change_counts = {'Create': 0, 'Delete': 0, 'Update': 0}
         for change in changes:
             change_counts[change.__class__.__name__] += 1

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
 

tests/config/zone-threshold.yaml

@@ -0,0 +1,34 @@
+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
+
+  defaultthresholds.tests.:
+    sources:
+    - in
+    targets:
+    - dump

tests/test_octodns_manager.py

@@ -1294,6 +1294,29 @@ 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)
+
+            # test default of None to ensure Provider precedence
+            zone_with_defaults = manager.get_zone('defaultthresholds.tests.')
+
+            self.assertIsNone(zone_with_defaults.update_pcent_threshold)
+            self.assertIsNone(zone_with_defaults.delete_pcent_threshold)
+
 
 class TestMainThreadExecutor(TestCase):
     def test_success(self):

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 = []