From 2bb2d5643b72130ed049fb674f718199238ff858 Mon Sep 17 00:00:00 2001 From: Matt Cholick Date: Wed, 10 Apr 2024 11:44:37 -0700 Subject: [PATCH 1/4] Add zone specific threshold config --- docs/records.md | 13 ++++- octodns/manager.py | 9 +++- octodns/provider/plan.py | 6 +++ octodns/zone.py | 18 ++++++- tests/config/zone-threshold.yaml | 28 ++++++++++ tests/test_octodns_manager.py | 17 ++++++ tests/test_octodns_plan.py | 90 +++++++++++++++++++++++++++++++- 7 files changed, 175 insertions(+), 6 deletions(-) create mode 100644 tests/config/zone-threshold.yaml 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 = [] From 2bc961a66dd871d1fc5e1289bb230f9fc9863b3c Mon Sep 17 00:00:00 2001 From: Matt Cholick Date: Wed, 10 Apr 2024 12:34:54 -0700 Subject: [PATCH 2/4] Update octodns/provider/plan.py Refactor zone vs provider choice to read more cleanly Co-authored-by: Ross McFarland --- octodns/provider/plan.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index df4cb94..825204c 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -57,15 +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 From 885126d48fa2f9b21784048281656ce33d50e06c Mon Sep 17 00:00:00 2001 From: Matt Cholick Date: Wed, 10 Apr 2024 12:42:40 -0700 Subject: [PATCH 3/4] Additional test checking Zone threshold defaults to None --- tests/config/zone-threshold.yaml | 6 ++++++ tests/test_octodns_manager.py | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/tests/config/zone-threshold.yaml b/tests/config/zone-threshold.yaml index 0598555..3479841 100644 --- a/tests/config/zone-threshold.yaml +++ b/tests/config/zone-threshold.yaml @@ -26,3 +26,9 @@ zones: - in targets: - dump + + defaultthresholds.tests.: + sources: + - in + targets: + - dump diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 6ed972f..271268d 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -1311,6 +1311,12 @@ class TestManager(TestCase): 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): From b82c3dc9215354319dfbf48177b905b965d14318 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 10 Apr 2024 12:48:29 -0700 Subject: [PATCH 4/4] Update CHANGELOG.md to mention per-zone thresholds --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 818c037..e10c072 100644 --- a/CHANGELOG.md +++ b/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