Merge pull request #754 from octodns/multi-value-PTR

Multi-value PTR records

octodns/provider/azuredns.py

@@ -456,6 +456,7 @@ class AzureProvider(BaseProvider):
     '''
     SUPPORTS_GEO = False
     SUPPORTS_DYNAMIC = True
+    SUPPORTS_MUTLIVALUE_PTR = True
     SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'PTR', 'SRV',
                     'TXT'))
 
@@ -707,8 +708,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,

octodns/provider/base.py

@@ -44,6 +44,29 @@ class BaseProvider(BaseSource):
         '''
         return []
 
+    def _process_desired_zone(self, desired):
+        '''
+        Providers can use this method to make any custom changes to the
+        desired zone.
+        '''
+        if self.SUPPORTS_MUTLIVALUE_PTR:
+            # nothing do here
+            return desired
+
+        new_desired = Zone(desired.name, desired.sub_zones)
+        for record in desired.records:
+            if record._type == 'PTR' and len(record.values) > 1:
+                # replace with a single-value copy
+                self.log.warn('does not support multi-value PTR records; '
+                              'will use only %s for %s', record.value,
+                              record.fqdn)
+                record = record.copy()
+                record.values = [record.value]
+
+            new_desired.add_record(record)
+
+        return new_desired
+
     def plan(self, desired, processors=[]):
         self.log.info('plan: desired=%s', desired.name)
 
@@ -58,6 +81,9 @@ class BaseProvider(BaseSource):
         for processor in processors:
             existing = processor.process_target_zone(existing, target=self)
 
+        # process desired zone for any custom zone/record modification
+        desired = self._process_desired_zone(desired)
+
         # compute the changes at the zone/record level
         changes = existing.changes(desired, self)
 

octodns/provider/ns1.py

@@ -20,6 +20,10 @@ from ..record import Record, Update
 from .base import BaseProvider
 
 
+def _ensure_endswith_dot(string):
+    return string if string.endswith('.') else '{}.'.format(string)
+
+
 class Ns1Exception(Exception):
     pass
 
@@ -257,6 +261,7 @@ class Ns1Provider(BaseProvider):
     '''
     SUPPORTS_GEO = True
     SUPPORTS_DYNAMIC = True
+    SUPPORTS_MUTLIVALUE_PTR = True
     SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR',
                     'NS', 'PTR', 'SPF', 'SRV', 'TXT', 'URLFWD'))
 
@@ -720,7 +725,6 @@ class Ns1Provider(BaseProvider):
         }
 
     _data_for_ALIAS = _data_for_CNAME
-    _data_for_PTR = _data_for_CNAME
 
     def _data_for_MX(self, _type, record):
         values = []
@@ -759,10 +763,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']:
@@ -812,9 +817,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'] = [
+                        _ensure_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
@@ -1304,7 +1310,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]
@@ -1315,6 +1320,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]

octodns/provider/yaml.py

@@ -104,6 +104,7 @@ class YamlProvider(BaseProvider):
     '''
     SUPPORTS_GEO = True
     SUPPORTS_DYNAMIC = True
+    SUPPORTS_MUTLIVALUE_PTR = True
     SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'DNAME', 'LOC', 'MX',
                     'NAPTR', 'NS', 'PTR', 'SSHFP', 'SPF', 'SRV', 'TXT',
                     'URLFWD'))

octodns/record/__init__.py

@@ -1256,13 +1256,37 @@ 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
+
+    @classmethod
+    def process(cls, values):
+        return [super(PtrValue, cls).process(v) for v in values]
 
-class PtrRecord(_ValueMixin, Record):
+
+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):
+        return self.values[0]
+
 
 class SshfpValue(EqualityTupleMixin):
     VALID_ALGORITHMS = (1, 2, 3, 4)

octodns/source/base.py

@@ -8,6 +8,8 @@ from __future__ import absolute_import, division, print_function, \
 
 class BaseSource(object):
 
+    SUPPORTS_MUTLIVALUE_PTR = False
+
     def __init__(self, id):
         self.id = id
         if not getattr(self, 'log', False):

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.]

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

tests/test_octodns_provider_azuredns.py

@@ -150,6 +150,11 @@ octo_records.append(Record.new(zone, 'txt3', {
     'type': 'TXT',
     'values': ['txt multiple test', long_txt]}))
 
+octo_records.append(Record.new(zone, 'ptr2', {
+    'ttl': 11,
+    'type': 'PTR',
+    'values': ['ptr21.unit.tests.', 'ptr22.unit.tests.']}))
+
 azure_records = []
 _base0 = _AzureRecord('TestAzure', octo_records[0])
 _base0.zone_name = 'unit.tests'
@@ -338,6 +343,15 @@ _base18.params['txt_records'] = [TxtRecord(value=['txt multiple test']),
                                  TxtRecord(value=[long_txt_az1, long_txt_az2])]
 azure_records.append(_base18)
 
+_base19 = _AzureRecord('TestAzure', octo_records[19])
+_base19.zone_name = 'unit.tests'
+_base19.relative_record_set_name = 'ptr2'
+_base19.record_type = 'PTR'
+_base19.params['ttl'] = 11
+_base19.params['ptr_records'] = [PtrRecord(ptrdname='ptr21.unit.tests.'),
+                                 PtrRecord(ptrdname='ptr22.unit.tests.')]
+azure_records.append(_base19)
+
 
 class Test_AzureRecord(TestCase):
     def test_azure_record(self):
@@ -2054,15 +2068,16 @@ class TestAzureDnsProvider(TestCase):
     def test_apply(self):
         provider = self._get_provider()
 
-        half = int(len(octo_records) / 2)
+        expected_n = len(octo_records)
+        half = int(expected_n / 2)
         changes = [Create(r) for r in octo_records[:half]] + \
             [Update(r, r) for r in octo_records[half:]]
         deletes = [Delete(r) for r in octo_records]
 
-        self.assertEquals(19, provider.apply(Plan(None, zone,
-                                                  changes, True)))
-        self.assertEquals(19, provider.apply(Plan(zone, zone,
-                                                  deletes, True)))
+        self.assertEquals(expected_n, provider.apply(Plan(None, zone,
+                                                     changes, True)))
+        self.assertEquals(expected_n, provider.apply(Plan(zone, zone,
+                                                     deletes, True)))
 
     def test_apply_create_dynamic(self):
         provider = self._get_provider()
@@ -2320,8 +2335,9 @@ class TestAzureDnsProvider(TestCase):
         _get = provider._dns_client.zones.get
         _get.side_effect = CloudError(Mock(status=404), err_msg)
 
-        self.assertEquals(19, provider.apply(Plan(None, desired, changes,
-                                                  True)))
+        expected_n = len(octo_records)
+        self.assertEquals(expected_n, provider.apply(Plan(None, desired,
+                                                          changes, True)))
 
     def test_check_zone_no_create(self):
         provider = self._get_provider()

tests/test_octodns_provider_base.py

@@ -230,6 +230,20 @@ class TestBaseProvider(TestCase):
         # We filtered out the only change
         self.assertFalse(plan)
 
+    def test_process_desired_zone(self):
+        zone1 = Zone('unit.tests.', [])
+        record1 = Record.new(zone1, 'ptr', {
+            'type': 'PTR',
+            'ttl': 3600,
+            'values': ['foo.com.', 'bar.com.'],
+        })
+        zone1.add_record(record1)
+
+        zone2 = HelperProvider('hasptr')._process_desired_zone(zone1)
+        record2 = list(zone2.records)[0]
+
+        self.assertEqual(len(record2.values), 1)
+
     def test_safe_none(self):
         # No changes is safe
         Plan(None, None, [], True).raise_if_unsafe()

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

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: