From b913b7ed9586642704e7af079a3ded9d53129ec9 Mon Sep 17 00:00:00 2001 From: Mathurin Gagnon Date: Wed, 23 Apr 2025 16:25:47 -0700 Subject: [PATCH 1/5] added patch so when a record has an empty value it does not crash --- octodns/processor/filter.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/octodns/processor/filter.py b/octodns/processor/filter.py index 848cb6f..0ac152f 100644 --- a/octodns/processor/filter.py +++ b/octodns/processor/filter.py @@ -234,8 +234,10 @@ class _ValueBaseFilter(_FilterProcessor): if hasattr(record, 'values'): values = [value.rdata_text for value in record.values] else: - values = [record.value.rdata_text] - + try: + values = [record.value.rdata_text] + except AttributeError: + self.log.warning(f"Record value is NoneType: {record.fqdn}") if any(value in self.exact for value in values): self.matches(zone, record) continue @@ -317,6 +319,7 @@ class ValueRejectlistFilter(_ValueBaseFilter, RejectsMixin): ''' def __init__(self, name, rejectlist): + self.log = getLogger(f'ValueRejectlistFilter[{name}]') super().__init__(name, rejectlist) From cb01f041c19bba05ac695e9fcdd5b295d2ed7d34 Mon Sep 17 00:00:00 2001 From: Mathurin Gagnon Date: Fri, 25 Apr 2025 16:09:09 -0700 Subject: [PATCH 2/5] updated tests and filter to cover all new code changes in value filter reject and allow --- octodns/processor/filter.py | 1 + tests/test_octodns_processor_filter.py | 30 +++++++++++++++++++------- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/octodns/processor/filter.py b/octodns/processor/filter.py index 0ac152f..8b15682 100644 --- a/octodns/processor/filter.py +++ b/octodns/processor/filter.py @@ -283,6 +283,7 @@ class ValueAllowlistFilter(_ValueBaseFilter, AllowsMixin): ''' def __init__(self, name, allowlist): + self.log = getLogger(f'ValueAllowlistFilter[{name}]') super().__init__(name, allowlist) diff --git a/tests/test_octodns_processor_filter.py b/tests/test_octodns_processor_filter.py index 29e1ece..9028804 100644 --- a/tests/test_octodns_processor_filter.py +++ b/tests/test_octodns_processor_filter.py @@ -227,11 +227,18 @@ class TestValueAllowListFilter(TestCase): {'type': 'CNAME', 'ttl': 42, 'value': 'start.a3b444c.end.'}, ) zone.add_record(matchable2) + empty_val = Record.new( + zone, + 'no.values', + {'type': 'CNAME', 'ttl': 42, 'value': None}, + lenient=True, + ) + zone.add_record(empty_val) def test_exact(self): allows = ValueAllowlistFilter('exact', ('matches.example.com.',)) - self.assertEqual(6, len(self.zone.records)) + self.assertEqual(7, len(self.zone.records)) filtered = allows.process_source_zone(self.zone.copy()) self.assertEqual(2, len(filtered.records)) self.assertEqual( @@ -242,7 +249,7 @@ class TestValueAllowListFilter(TestCase): def test_regex(self): allows = ValueAllowlistFilter('exact', ('/^start\\..+\\.end\\.$/',)) - self.assertEqual(6, len(self.zone.records)) + self.assertEqual(7, len(self.zone.records)) filtered = allows.process_source_zone(self.zone.copy()) self.assertEqual(2, len(filtered.records)) self.assertEqual( @@ -297,26 +304,33 @@ class TestValueRejectListFilter(TestCase): {'type': 'CNAME', 'ttl': 42, 'value': 'start.a3b444c.end.'}, ) zone.add_record(matchable2) + empty_val = Record.new( + zone, + 'no.values', + {'type': 'CNAME', 'ttl': 42, 'value': None}, + lenient=True, + ) + zone.add_record(empty_val) def test_exact(self): rejects = ValueRejectlistFilter('exact', ('matches.example.com.',)) - self.assertEqual(6, len(self.zone.records)) + self.assertEqual(7, len(self.zone.records)) filtered = rejects.process_source_zone(self.zone.copy()) - self.assertEqual(4, len(filtered.records)) + self.assertEqual(5, len(filtered.records)) self.assertEqual( - ['bad.compare', 'bad.values', 'first.regex', 'second.regex'], + ['bad.compare', 'bad.values', 'first.regex', 'no.values', 'second.regex'], sorted([r.name for r in filtered.records]), ) def test_regex(self): rejects = ValueRejectlistFilter('exact', ('/^start\\..+\\.end\\.$/',)) - self.assertEqual(6, len(self.zone.records)) + self.assertEqual(7, len(self.zone.records)) filtered = rejects.process_source_zone(self.zone.copy()) - self.assertEqual(4, len(filtered.records)) + self.assertEqual(5, len(filtered.records)) self.assertEqual( - ['bad.compare', 'bad.values', 'good.compare', 'good.values'], + ['bad.compare', 'bad.values', 'good.compare', 'good.values', 'no.values'], sorted([r.name for r in filtered.records]), ) From 1a568cc0cf5379d3f9fd23dcd5003eace669fabc Mon Sep 17 00:00:00 2001 From: Mathurin Gagnon <91029118+mtgagnon@users.noreply.github.com> Date: Mon, 28 Apr 2025 11:33:31 -0700 Subject: [PATCH 3/5] lint check --- tests/test_octodns_processor_filter.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/test_octodns_processor_filter.py b/tests/test_octodns_processor_filter.py index 9028804..84414cf 100644 --- a/tests/test_octodns_processor_filter.py +++ b/tests/test_octodns_processor_filter.py @@ -319,7 +319,13 @@ class TestValueRejectListFilter(TestCase): filtered = rejects.process_source_zone(self.zone.copy()) self.assertEqual(5, len(filtered.records)) self.assertEqual( - ['bad.compare', 'bad.values', 'first.regex', 'no.values', 'second.regex'], + [ + 'bad.compare', + 'bad.values', + 'first.regex', + 'no.values', + 'second.regex', + ], sorted([r.name for r in filtered.records]), ) @@ -330,7 +336,13 @@ class TestValueRejectListFilter(TestCase): filtered = rejects.process_source_zone(self.zone.copy()) self.assertEqual(5, len(filtered.records)) self.assertEqual( - ['bad.compare', 'bad.values', 'good.compare', 'good.values', 'no.values'], + [ + 'bad.compare', + 'bad.values', + 'good.compare', + 'good.values', + 'no.values', + ], sorted([r.name for r in filtered.records]), ) From 4c8442391765b4d6c872e3f785acbb11fdf0ca8e Mon Sep 17 00:00:00 2001 From: Mathurin Gagnon <91029118+mtgagnon@users.noreply.github.com> Date: Mon, 28 Apr 2025 11:38:32 -0700 Subject: [PATCH 4/5] updated warning message --- octodns/processor/filter.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/octodns/processor/filter.py b/octodns/processor/filter.py index 8b15682..5619b1c 100644 --- a/octodns/processor/filter.py +++ b/octodns/processor/filter.py @@ -237,7 +237,9 @@ class _ValueBaseFilter(_FilterProcessor): try: values = [record.value.rdata_text] except AttributeError: - self.log.warning(f"Record value is NoneType: {record.fqdn}") + self.log.warning( + f"Value processor ignoring record, value is NoneType: {record.fqdn}" + ) if any(value in self.exact for value in values): self.matches(zone, record) continue From 0d9ec583b5549cf6feb6d42d307619c9c3aa2aaf Mon Sep 17 00:00:00 2001 From: Mathurin Gagnon <91029118+mtgagnon@users.noreply.github.com> Date: Mon, 28 Apr 2025 14:47:20 -0700 Subject: [PATCH 5/5] changed try except to elif and else, updated log line to octodns standard --- octodns/processor/filter.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/octodns/processor/filter.py b/octodns/processor/filter.py index 5619b1c..638823d 100644 --- a/octodns/processor/filter.py +++ b/octodns/processor/filter.py @@ -233,13 +233,13 @@ class _ValueBaseFilter(_FilterProcessor): values = [] if hasattr(record, 'values'): values = [value.rdata_text for value in record.values] + elif record.value is not None: + values = [record.value.rdata_text] else: - try: - values = [record.value.rdata_text] - except AttributeError: - self.log.warning( - f"Value processor ignoring record, value is NoneType: {record.fqdn}" - ) + self.log.warning( + 'value for %s is NoneType, ignoring', record.fqdn + ) + if any(value in self.exact for value in values): self.matches(zone, record) continue