From cfa7abaee5f9d6262a807f8552478b41716a9e55 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 19 May 2023 09:57:56 -0700 Subject: [PATCH 1/6] Validations to ensure Record.name and Zone.name have no whitespace --- CHANGELOG.md | 4 ++++ octodns/record/base.py | 4 ++++ octodns/record/exception.py | 2 +- octodns/zone.py | 3 +++ tests/test_octodns_record.py | 22 ++++++++++++++++++++++ tests/test_octodns_zone.py | 5 +++++ 6 files changed, 39 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc478da..5c78c4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## v1.0.0.rc2 - 2023-??-?? - + +* Record and Zone validation now ensures there's no whitespace in names + ## v1.0.0.rc0 - 2023-05-16 - First of the ones #### Noteworthy changes diff --git a/octodns/record/base.py b/octodns/record/base.py index e9f9599..b6e3ed3 100644 --- a/octodns/record/base.py +++ b/octodns/record/base.py @@ -41,6 +41,10 @@ class Record(EqualityTupleMixin): # convert the error into a reason reasons.append(str(e)) name = str(name) + + if ' ' in name or '\t' in name: + reasons.append('invalid record, whitespace is not allowed') + fqdn = f'{name}.{zone.name}' if name else zone.name try: _type = data['type'] diff --git a/octodns/record/exception.py b/octodns/record/exception.py index 2d27c8d..34aea0b 100644 --- a/octodns/record/exception.py +++ b/octodns/record/exception.py @@ -13,7 +13,7 @@ class ValidationError(RecordException): @classmethod def build_message(cls, fqdn, reasons): reasons = '\n - '.join(reasons) - return f'Invalid record {idna_decode(fqdn)}\n - {reasons}' + return f'Invalid record "{idna_decode(fqdn)}"\n - {reasons}' def __init__(self, fqdn, reasons): super().__init__(self.build_message(fqdn, reasons)) diff --git a/octodns/zone.py b/octodns/zone.py index 82596d5..c35e94c 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -28,6 +28,9 @@ class Zone(object): def __init__(self, name, sub_zones): if not name[-1] == '.': raise Exception(f'Invalid zone name {name}, missing ending dot') + elif ' ' in name or '\t' in name: + raise Exception(f'Invalid zone name {name}, whitespace not allowed') + # internally everything is idna self.name = idna_encode(str(name)) if name else name # we'll keep a decoded version around for logs and errors diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 98b5f9f..88a7dfc 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -398,6 +398,28 @@ class TestRecordValidation(TestCase): zone = Zone('unit.tests.', []) def test_base(self): + # no spaces + for name in ( + ' ', + ' leading', + 'trailing ', + 'in the middle', + '\t', + '\tleading', + 'trailing\t', + 'in\tthe\tmiddle', + ): + with self.assertRaises(ValidationError) as ctx: + Record.new( + self.zone, + name, + {'ttl': 300, 'type': 'A', 'value': '1.2.3.4'}, + ) + reason = ctx.exception.reasons[0] + self.assertEqual( + 'invalid record, whitespace is not allowed', reason + ) + # name = '@' with self.assertRaises(ValidationError) as ctx: name = '@' diff --git a/tests/test_octodns_zone.py b/tests/test_octodns_zone.py index cacfe27..07e94ea 100644 --- a/tests/test_octodns_zone.py +++ b/tests/test_octodns_zone.py @@ -186,6 +186,11 @@ class TestZone(TestCase): Zone('not.allowed', []) self.assertTrue('missing ending dot' in str(ctx.exception)) + def test_whitespace(self): + with self.assertRaises(Exception) as ctx: + Zone('space not allowed.', []) + self.assertTrue('whitespace not allowed' in str(ctx.exception)) + def test_sub_zones(self): # NS for exactly the sub is allowed From 5b3bcb704a18558aa4dcd6eb514bd5bd9bf10bbf Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Sat, 20 May 2023 22:24:56 -0400 Subject: [PATCH 2/6] add Python 3.11 to test matrix --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 76decff..a62cd00 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -8,7 +8,7 @@ jobs: fail-fast: false matrix: # Tested versions based on dates in https://devguide.python.org/devcycle/#end-of-life-branches, - python-version: ['3.7', '3.8', '3.9', '3.10'] + python-version: ['3.7', '3.8', '3.9', '3.10', '3.11'] steps: - uses: actions/checkout@master - name: Setup python From 193d004d65450e6799148cb43f1a842173b85ce2 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 23 May 2023 10:20:32 -0700 Subject: [PATCH 3/6] update requirements*.txt --- requirements-dev.txt | 62 +++++++++++++++++++++----------------------- requirements.txt | 2 +- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 068d690..3465a00 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,50 +1,46 @@ # DO NOT EDIT THIS FILE DIRECTLY - use ./script/update-requirements to update -Pygments==2.13.0 -attrs==22.1.0 -black==22.10.0 -bleach==5.0.1 -build==0.9.0 -certifi==2022.12.7 +Pygments==2.15.1 +black==23.3.0 +bleach==6.0.0 +build==0.10.0 +certifi==2023.5.7 cffi==1.15.1 -charset-normalizer==2.1.1 +charset-normalizer==3.1.0 click==8.1.3 cmarkgfm==2022.10.27 -commonmark==0.9.1 -coverage==6.5.0 -docutils==0.19 -exceptiongroup==1.0.0 -importlib-metadata==5.0.0 -iniconfig==1.1.1 -isort==5.11.4 +coverage==7.2.5 +docutils==0.20.1 +importlib-metadata==6.6.0 +iniconfig==2.0.0 +isort==5.12.0 jaraco.classes==3.2.3 -keyring==23.9.3 -more-itertools==9.0.0 -mypy-extensions==0.4.3 -packaging==21.3 -pathspec==0.10.1 -pep517==0.13.0 -pkginfo==1.8.3 -platformdirs==2.5.2 +keyring==23.13.1 +markdown-it-py==2.2.0 +mdurl==0.1.2 +more-itertools==9.1.0 +mypy-extensions==1.0.0 +packaging==23.1 +pathspec==0.11.1 +pkginfo==1.9.6 +platformdirs==3.5.1 pluggy==1.0.0 pprintpp==0.4.0 pycountry-convert==0.7.2 pycountry==22.3.5 pycparser==2.21 -pyflakes==2.5.0 -pyparsing==3.0.9 +pyflakes==3.0.1 +pyproject_hooks==1.0.0 pytest-cov==4.0.0 pytest-mock==3.10.0 pytest-network==0.0.1 -pytest==7.2.0 +pytest==7.3.1 readme-renderer==37.3 repoze.lru==0.7 -requests-toolbelt==0.10.1 -requests==2.28.1 +requests-toolbelt==1.0.0 +requests==2.31.0 rfc3986==2.0.0 -rich==12.6.0 -tomli==2.0.1 -twine==4.0.1 -typing_extensions==4.4.0 -urllib3==1.26.12 +rich==13.3.5 +twine==4.0.2 +urllib3==2.0.2 webencodings==0.5.1 -zipp==3.10.0 +zipp==3.15.0 diff --git a/requirements.txt b/requirements.txt index 22f9237..ecc8879 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,6 +3,6 @@ PyYAML==6.0 dnspython==2.3.0 fqdn==1.5.1 idna==3.4 -natsort==8.2.0 +natsort==8.3.1 python-dateutil==2.8.2 six==1.16.0 From 98f51db028127d927f465548c16bf4dfc4cf148c Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 23 May 2023 10:27:23 -0700 Subject: [PATCH 4/6] updated black formatting --- octodns/manager.py | 1 - octodns/provider/plan.py | 1 - octodns/source/base.py | 1 - tests/test_octodns_processor_arpa.py | 1 - tests/test_octodns_record_ip.py | 1 - tests/test_octodns_record_mx.py | 1 - tests/test_octodns_record_ptr.py | 1 - tests/test_octodns_record_srv.py | 1 - tests/test_octodns_record_sshfp.py | 1 - tests/test_octodns_record_target.py | 1 - tests/test_octodns_record_tlsa.py | 1 - tests/test_octodns_source_tinydns.py | 1 - tests/test_octodns_zone.py | 1 - 13 files changed, 13 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index df64b2e..b8fb0ff 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -475,7 +475,6 @@ class Manager(object): force=False, plan_output_fh=stdout, ): - self.log.info( 'sync: eligible_zones=%s, eligible_targets=%s, dry_run=%s, ' 'force=%s, plan_output_fh=%s', diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index bdd47a4..e27b1e3 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -78,7 +78,6 @@ class Plan(object): self.existing and len(self.existing.records) >= self.MIN_EXISTING_RECORDS ): - existing_record_count = len(self.existing.records) if existing_record_count > 0: update_pcent = ( diff --git a/octodns/source/base.py b/octodns/source/base.py index 1c3ff15..0f0b2dd 100644 --- a/octodns/source/base.py +++ b/octodns/source/base.py @@ -4,7 +4,6 @@ class BaseSource(object): - SUPPORTS_MULTIVALUE_PTR = False SUPPORTS_POOL_VALUE_STATUS = False SUPPORTS_ROOT_NS = False diff --git a/tests/test_octodns_processor_arpa.py b/tests/test_octodns_processor_arpa.py index b3baa43..b2038b1 100644 --- a/tests/test_octodns_processor_arpa.py +++ b/tests/test_octodns_processor_arpa.py @@ -11,7 +11,6 @@ from octodns.zone import Zone class TestAutoArpa(TestCase): def test_empty_zone(self): - # empty zone no records zone = Zone('unit.tests.', []) aa = AutoArpa('auto-arpa') diff --git a/tests/test_octodns_record_ip.py b/tests/test_octodns_record_ip.py index 29d576f..f9ba62d 100644 --- a/tests/test_octodns_record_ip.py +++ b/tests/test_octodns_record_ip.py @@ -10,7 +10,6 @@ from octodns.zone import Zone class TestRecordIp(TestCase): def test_ipv4_value_rdata_text(self): - # anything goes, we're a noop for s in ( None, diff --git a/tests/test_octodns_record_mx.py b/tests/test_octodns_record_mx.py index d3da73d..1ae37e6 100644 --- a/tests/test_octodns_record_mx.py +++ b/tests/test_octodns_record_mx.py @@ -68,7 +68,6 @@ class TestRecordMx(TestCase): a.__repr__() def test_mx_value_rdata_text(self): - # empty string won't parse with self.assertRaises(RrParseError): MxValue.parse_rdata_text('') diff --git a/tests/test_octodns_record_ptr.py b/tests/test_octodns_record_ptr.py index 14f744c..7a380e6 100644 --- a/tests/test_octodns_record_ptr.py +++ b/tests/test_octodns_record_ptr.py @@ -63,7 +63,6 @@ class TestRecordPtr(TestCase): ) def test_ptr_rdata_text(self): - # anything goes, we're a noop for s in ( None, diff --git a/tests/test_octodns_record_srv.py b/tests/test_octodns_record_srv.py index f86d406..3cc39b5 100644 --- a/tests/test_octodns_record_srv.py +++ b/tests/test_octodns_record_srv.py @@ -81,7 +81,6 @@ class TestRecordSrv(TestCase): a.__repr__() def test_srv_value_rdata_text(self): - # empty string won't parse with self.assertRaises(RrParseError): SrvValue.parse_rdata_text('') diff --git a/tests/test_octodns_record_sshfp.py b/tests/test_octodns_record_sshfp.py index f026514..251efca 100644 --- a/tests/test_octodns_record_sshfp.py +++ b/tests/test_octodns_record_sshfp.py @@ -85,7 +85,6 @@ class TestRecordSshfp(TestCase): a.__repr__() def test_sshfp_value_rdata_text(self): - # empty string won't parse with self.assertRaises(RrParseError): SshfpValue.parse_rdata_text('') diff --git a/tests/test_octodns_record_target.py b/tests/test_octodns_record_target.py index 728c349..715cd4a 100644 --- a/tests/test_octodns_record_target.py +++ b/tests/test_octodns_record_target.py @@ -11,7 +11,6 @@ from octodns.zone import Zone class TestRecordTarget(TestCase): def test_target_rdata_text(self): - # anything goes, we're a noop for s in ( None, diff --git a/tests/test_octodns_record_tlsa.py b/tests/test_octodns_record_tlsa.py index 6554bbc..9739017 100644 --- a/tests/test_octodns_record_tlsa.py +++ b/tests/test_octodns_record_tlsa.py @@ -118,7 +118,6 @@ class TestRecordTlsa(TestCase): a.__repr__() def test_tsla_value_rdata_text(self): - # empty string won't parse with self.assertRaises(RrParseError): TlsaValue.parse_rdata_text('') diff --git a/tests/test_octodns_source_tinydns.py b/tests/test_octodns_source_tinydns.py index 717d46b..40ce9f5 100644 --- a/tests/test_octodns_source_tinydns.py +++ b/tests/test_octodns_source_tinydns.py @@ -150,7 +150,6 @@ class TestTinyDnsFileSource(TestCase): self.assertEqual([], changes) def test_populate_in_addr_arpa(self): - got = Zone('3.2.10.in-addr.arpa.', []) self.source.populate(got) diff --git a/tests/test_octodns_zone.py b/tests/test_octodns_zone.py index 07e94ea..0ee01b0 100644 --- a/tests/test_octodns_zone.py +++ b/tests/test_octodns_zone.py @@ -192,7 +192,6 @@ class TestZone(TestCase): self.assertTrue('whitespace not allowed' in str(ctx.exception)) def test_sub_zones(self): - # NS for exactly the sub is allowed zone = Zone('unit.tests.', set(['sub', 'barred'])) record = Record.new( From 4ac0f06673c3b6ac965724d2633b6b52fbff14eb Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 23 May 2023 10:33:18 -0700 Subject: [PATCH 5/6] stick with isort 5.11.4, newer doesn't support 3.7 --- requirements-dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 3465a00..0fb17f8 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -12,7 +12,7 @@ coverage==7.2.5 docutils==0.20.1 importlib-metadata==6.6.0 iniconfig==2.0.0 -isort==5.12.0 +isort==5.11.4 jaraco.classes==3.2.3 keyring==23.13.1 markdown-it-py==2.2.0 From 7e931080f47cb06b93ae9c9f155041ebfd061ef5 Mon Sep 17 00:00:00 2001 From: Nathan Tat Date: Tue, 30 May 2023 18:14:36 -0700 Subject: [PATCH 6/6] Pass lenient through to arpa processor --- octodns/processor/arpa.py | 7 +++++- tests/test_octodns_processor_arpa.py | 37 ++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/octodns/processor/arpa.py b/octodns/processor/arpa.py index a2dfa0e..7de4c1c 100644 --- a/octodns/processor/arpa.py +++ b/octodns/processor/arpa.py @@ -61,8 +61,13 @@ class AutoArpa(BaseProcessor): zone, name, {'ttl': self.ttl, 'type': 'PTR', 'values': fqdns}, + lenient=lenient, + ) + zone.add_record( + record, + replace=self.populate_should_replace, + lenient=lenient, ) - zone.add_record(record, replace=self.populate_should_replace) self.log.info( 'populate: found %s records', len(zone.records) - before diff --git a/tests/test_octodns_processor_arpa.py b/tests/test_octodns_processor_arpa.py index b2038b1..32fc49d 100644 --- a/tests/test_octodns_processor_arpa.py +++ b/tests/test_octodns_processor_arpa.py @@ -6,6 +6,7 @@ from unittest import TestCase from octodns.processor.arpa import AutoArpa from octodns.record import Record +from octodns.record.exception import ValidationError from octodns.zone import Zone @@ -226,3 +227,39 @@ class TestAutoArpa(TestCase): arpa = Zone('0.10.in-addr.arpa.', []) aa.populate(arpa) self.assertEqual(0, len(arpa.records)) + + def test_single_value_A_with_space(self): + zone = Zone('unit.tests.', []) + + # invalid record without lenient + with self.assertRaises(ValidationError): + Record.new( + zone, + 'a with spaces', + {'ttl': 32, 'type': 'A', 'value': '1.2.3.4'}, + ) + + # invalid record with lenient + lenient = True + record = Record.new( + zone, + 'a with spaces', + {'ttl': 32, 'type': 'A', 'value': '1.2.3.4'}, + lenient=lenient, + ) + zone.add_record(record) + aa = AutoArpa('auto-arpa') + aa.process_source_zone(zone, []) + self.assertEqual( + {'4.3.2.1.in-addr.arpa.': {'a with spaces.unit.tests.'}}, + aa._records, + ) + + # matching zone + arpa = Zone('3.2.1.in-addr.arpa.', []) + aa.populate(arpa, lenient=lenient) + self.assertEqual(1, len(arpa.records)) + (ptr,) = arpa.records + self.assertEqual('4.3.2.1.in-addr.arpa.', ptr.fqdn) + self.assertEqual(record.fqdn, ptr.value) + self.assertEqual(3600, ptr.ttl)