From 49bff426b7ee4d5a4554b2c6e1603e6384d32949 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Thu, 12 Aug 2021 21:43:01 -0700 Subject: [PATCH] Multi-value PTR records --- octodns/provider/azuredns.py | 4 +-- octodns/provider/ns1.py | 24 ++++++++++++----- octodns/record/__init__.py | 31 ++++++++++++++++++++-- tests/config/split/unit.tests.tst/ptr.yaml | 2 +- tests/config/unit.tests.yaml | 2 +- tests/test_octodns_provider_ns1.py | 17 ++++++++++-- tests/test_octodns_record.py | 12 ++++++++- 7 files changed, 76 insertions(+), 16 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 6edc1ba..c75ef4b 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -707,8 +707,8 @@ class AzureProvider(BaseProvider): return {'values': [_check_endswith_dot(val) for val in vals]} def _data_for_PTR(self, azrecord): - ptrdname = azrecord.ptr_records[0].ptrdname - return {'value': _check_endswith_dot(ptrdname)} + vals = [ar.ptrdname for ar in azrecord.ptr_records] + return {'values': [_check_endswith_dot(val) for val in vals]} def _data_for_SRV(self, azrecord): return {'values': [{'priority': ar.priority, 'weight': ar.weight, diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 0ebb36b..72e0bc9 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -20,6 +20,10 @@ from ..record import Record, Update from .base import BaseProvider +def _check_endswith_dot(string): + return string if string.endswith('.') else '{}.'.format(string) + + class Ns1Exception(Exception): pass @@ -717,7 +721,6 @@ class Ns1Provider(BaseProvider): } _data_for_ALIAS = _data_for_CNAME - _data_for_PTR = _data_for_CNAME def _data_for_MX(self, _type, record): values = [] @@ -756,10 +759,11 @@ class Ns1Provider(BaseProvider): return { 'ttl': record['ttl'], 'type': _type, - 'values': [a if a.endswith('.') else '{}.'.format(a) - for a in record['short_answers']], + 'values': record['short_answers'], } + _data_for_PTR = _data_for_NS + def _data_for_SRV(self, _type, record): values = [] for answer in record['short_answers']: @@ -809,9 +813,10 @@ class Ns1Provider(BaseProvider): for record in ns1_zone['records']: if record['type'] in ['ALIAS', 'CNAME', 'MX', 'NS', 'PTR', 'SRV']: - for i, a in enumerate(record['short_answers']): - if not a.endswith('.'): - record['short_answers'][i] = '{}.'.format(a) + record['short_answers'] = [ + _check_endswith_dot(a) + for a in record['short_answers'] + ] if record.get('tier', 1) > 1: # Need to get the full record data for geo records @@ -1297,7 +1302,6 @@ class Ns1Provider(BaseProvider): return {'answers': [record.value], 'ttl': record.ttl}, None _params_for_ALIAS = _params_for_CNAME - _params_for_PTR = _params_for_CNAME def _params_for_MX(self, record): values = [(v.preference, v.exchange) for v in record.values] @@ -1308,6 +1312,12 @@ class Ns1Provider(BaseProvider): v.replacement) for v in record.values] return {'answers': values, 'ttl': record.ttl}, None + def _params_for_PTR(self, record): + return { + 'answers': record.values, + 'ttl': record.ttl, + }, None + def _params_for_SRV(self, record): values = [(v.priority, v.weight, v.port, v.target) for v in record.values] diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 7714b27..93f6660 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -1256,13 +1256,40 @@ class NsRecord(_ValuesMixin, Record): class PtrValue(_TargetValue): - pass + @classmethod + def validate(cls, values, _type): + if not isinstance(values, list): + values = [values] + + reasons = [] + + if not values: + reasons.append('missing values') + + for value in values: + reasons.extend(super(PtrValue, cls).validate(value, _type)) + + return reasons -class PtrRecord(_ValueMixin, Record): + @classmethod + def process(cls, values): + return [super(PtrValue, cls).process(v) for v in values] + + +class PtrRecord(_ValuesMixin, Record): _type = 'PTR' _value_type = PtrValue + # This is for backward compatibility with providers that don't support + # multi-value PTR records. + @property + def value(self): + if len(self.values) == 1: + return self.values[0] + + raise AttributeError("Multi-value PTR record has no attribute 'value'") + class SshfpValue(EqualityTupleMixin): VALID_ALGORITHMS = (1, 2, 3, 4) diff --git a/tests/config/split/unit.tests.tst/ptr.yaml b/tests/config/split/unit.tests.tst/ptr.yaml index 0098b57..cffb50b 100644 --- a/tests/config/split/unit.tests.tst/ptr.yaml +++ b/tests/config/split/unit.tests.tst/ptr.yaml @@ -2,4 +2,4 @@ ptr: ttl: 300 type: PTR - value: foo.bar.com. + values: [foo.bar.com.] diff --git a/tests/config/unit.tests.yaml b/tests/config/unit.tests.yaml index c70b20c..aa28ee5 100644 --- a/tests/config/unit.tests.yaml +++ b/tests/config/unit.tests.yaml @@ -152,7 +152,7 @@ naptr: ptr: ttl: 300 type: PTR - value: foo.bar.com. + values: [foo.bar.com.] spf: ttl: 600 type: SPF diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 932e4c4..3e239b3 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -120,6 +120,11 @@ class TestNs1Provider(TestCase): 'query': 0, }, })) + expected.add(Record.new(zone, '1.2.3.4', { + 'ttl': 42, + 'type': 'PTR', + 'values': ['one.one.one.one.', 'two.two.two.two.'], + })) ns1_records = [{ 'type': 'A', @@ -180,6 +185,11 @@ class TestNs1Provider(TestCase): 'ttl': 41, 'short_answers': ['/ http://foo.unit.tests 301 2 0'], 'domain': 'urlfwd.unit.tests.', + }, { + 'type': 'PTR', + 'ttl': 42, + 'short_answers': ['one.one.one.one.', 'two.two.two.two.'], + 'domain': '1.2.3.4.unit.tests.', }] @patch('ns1.rest.records.Records.retrieve') @@ -358,10 +368,10 @@ class TestNs1Provider(TestCase): ResourceException('server error: zone not found') zone_create_mock.side_effect = ['foo'] - # Test out the create rate-limit handling, then 9 successes + # Test out the create rate-limit handling, then successes for the rest record_create_mock.side_effect = [ RateLimitException('boo', period=0), - ] + ([None] * 10) + ] + ([None] * len(self.expected)) got_n = provider.apply(plan) self.assertEquals(expected_n, got_n) @@ -379,6 +389,9 @@ class TestNs1Provider(TestCase): call('unit.tests', 'unit.tests', 'MX', answers=[ (10, 'mx1.unit.tests.'), (20, 'mx2.unit.tests.') ], ttl=35), + call('unit.tests', '1.2.3.4.unit.tests', 'PTR', answers=[ + 'one.one.one.one.', 'two.two.two.two.', + ], ttl=42), ]) # Update & delete diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 3bd48e5..2d8e000 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -2733,7 +2733,7 @@ class TestRecordValidation(TestCase): 'type': 'PTR', 'ttl': 600, }) - self.assertEquals(['missing value'], ctx.exception.reasons) + self.assertEquals(['missing values'], ctx.exception.reasons) # not a valid FQDN with self.assertRaises(ValidationError) as ctx: @@ -2755,6 +2755,16 @@ class TestRecordValidation(TestCase): self.assertEquals(['PTR value "foo.bar" missing trailing .'], ctx.exception.reasons) + # multi-value requesting single-value + with self.assertRaises(AttributeError) as ctx: + Record.new(self.zone, '', { + 'type': 'PTR', + 'ttl': 600, + 'values': ['foo.com.', 'bar.net.'], + }).value + self.assertEquals("Multi-value PTR record has no attribute 'value'", + text_type(ctx.exception)) + def test_SSHFP(self): # doesn't blow up Record.new(self.zone, '', {