Merge pull request #77 from github/rfc-1035

Rfc 1035 compliance

octodns/provider/cloudflare.py

@@ -116,8 +116,8 @@ class CloudflareProvider(BaseProvider):
         values = []
         for r in records:
             values.append({
-                'priority': r['priority'],
-                'value': '{}.'.format(r['content']),
+                'preference': r['priority'],
+                'exchange': '{}.'.format(r['content']),
             })
         return {
             'ttl': records[0]['ttl'],
@@ -207,8 +207,8 @@ class CloudflareProvider(BaseProvider):
     def _contents_for_MX(self, record):
         for value in record.values:
             yield {
-                'priority': value.priority,
-                'content': value.value
+                'priority': value.preference,
+                'content': value.exchange
             }
 
     def _apply_Create(self, change):

octodns/provider/dnsimple.py

@@ -128,8 +128,8 @@ class DnsimpleProvider(BaseProvider):
         values = []
         for record in records:
             values.append({
-                'priority': record['priority'],
-                'value': '{}.'.format(record['content'])
+                'preference': record['priority'],
+                'exchange': '{}.'.format(record['content'])
             })
         return {
             'ttl': records[0]['ttl'],
@@ -290,9 +290,9 @@ class DnsimpleProvider(BaseProvider):
     def _params_for_MX(self, record):
         for value in record.values:
             yield {
-                'content': value.value,
+                'content': value.exchange,
                 'name': record.name,
-                'priority': value.priority,
+                'priority': value.preference,
                 'ttl': record.ttl,
                 'type': record._type
             }

octodns/provider/dyn.py

@@ -206,7 +206,7 @@ class DynProvider(BaseProvider):
         return {
             'type': _type,
             'ttl': records[0].ttl,
-            'values': [{'priority': r.preference, 'value': r.exchange}
+            'values': [{'preference': r.preference, 'exchange': r.exchange}
                        for r in records],
         }
 
@@ -400,8 +400,8 @@ class DynProvider(BaseProvider):
 
     def _kwargs_for_MX(self, record):
         return [{
-            'preference': v.priority,
-            'exchange': v.value,
+            'preference': v.preference,
+            'exchange': v.exchange,
             'ttl': record.ttl,
         } for v in record.values]
 

octodns/provider/ns1.py

@@ -57,10 +57,10 @@ class Ns1Provider(BaseProvider):
     def _data_for_MX(self, _type, record):
         values = []
         for answer in record['short_answers']:
-            priority, value = answer.split(' ', 1)
+            preference, exchange = answer.split(' ', 1)
             values.append({
-                'priority': priority,
-                'value': value,
+                'preference': preference,
+                'exchange': exchange,
             })
         return {
             'ttl': record['ttl'],
@@ -150,7 +150,7 @@ class Ns1Provider(BaseProvider):
     _params_for_PTR = _params_for_CNAME
 
     def _params_for_MX(self, record):
-        values = [(v.priority, v.value) for v in record.values]
+        values = [(v.preference, v.exchange) for v in record.values]
         return {'answers': values, 'ttl': record.ttl}
 
     def _params_for_NAPTR(self, record):

octodns/provider/powerdns.py

@@ -83,10 +83,10 @@ class PowerDnsBaseProvider(BaseProvider):
     def _data_for_MX(self, rrset):
         values = []
         for record in rrset['records']:
-            priority, value = record['content'].split(' ', 1)
+            preference, exchange = record['content'].split(' ', 1)
             values.append({
-                'priority': priority,
-                'value': value,
+                'preference': preference,
+                'exchange': exchange,
             })
         return {
             'type': rrset['type'],
@@ -208,7 +208,7 @@ class PowerDnsBaseProvider(BaseProvider):
 
     def _records_for_MX(self, record):
         return [{
-            'content': '{} {}'.format(v.priority, v.value),
+            'content': '{} {}'.format(v.preference, v.exchange),
             'disabled': False
         } for v in record.values]
 

octodns/provider/route53.py

@@ -96,7 +96,8 @@ class _Route53Record(object):
     _values_for_PTR = _values_for_value
 
     def _values_for_MX(self, record):
-        return ['{} {}'.format(v.priority, v.value) for v in record.values]
+        return ['{} {}'.format(v.preference, v.exchange)
+                for v in record.values]
 
     def _values_for_NAPTR(self, record):
         return ['{} {} "{}" "{}" "{}" {}'
@@ -335,10 +336,10 @@ class Route53Provider(BaseProvider):
     def _data_for_MX(self, rrset):
         values = []
         for rr in rrset['ResourceRecords']:
-            priority, value = rr['Value'].split(' ')
+            preference, exchange = rr['Value'].split(' ')
             values.append({
-                'priority': priority,
-                'value': value,
+                'preference': preference,
+                'exchange': exchange,
             })
         return {
             'type': rrset['Type'],

octodns/record.py

@@ -424,32 +424,50 @@ class MxValue(object):
     @classmethod
     def _validate_value(cls, value):
         reasons = []
-        if 'priority' not in value:
-            reasons.append('missing priority')
-        if 'value' not in value:
-            reasons.append('missing value')
+        try:
+            int(value.get('preference', None) or value['priority'])
+        except KeyError:
+            reasons.append('missing preference')
+        except ValueError:
+            reasons.append('invalid preference "{}"'
+                           .format(value['preference']))
+        exchange = None
+        try:
+            exchange = value.get('exchange', None) or value['value']
+            if not exchange.endswith('.'):
+                reasons.append('missing trailing .')
+        except KeyError:
+            reasons.append('missing exchange')
         return reasons
 
     def __init__(self, value):
-        # TODO: rename preference
-        self.priority = int(value['priority'])
-        # TODO: rename to exchange?
-        self.value = value['value'].lower()
+        # RFC1035 says preference, half the providers use priority
+        try:
+            preference = value['preference']
+        except KeyError:
+            preference = value['priority']
+        self.preference = int(preference)
+        # UNTIL 1.0 remove value fallback
+        try:
+            exchange = value['exchange']
+        except KeyError:
+            exchange = value['value']
+        self.exchange = exchange
 
     @property
     def data(self):
         return {
-            'priority': self.priority,
-            'value': self.value,
+            'preference': self.preference,
+            'exchange': self.exchange,
         }
 
     def __cmp__(self, other):
-        if self.priority == other.priority:
-            return cmp(self.value, other.value)
-        return cmp(self.priority, other.priority)
+        if self.preference == other.preference:
+            return cmp(self.exchange, other.exchange)
+        return cmp(self.preference, other.preference)
 
     def __repr__(self):
-        return "'{} {}'".format(self.priority, self.value)
+        return "'{} {}'".format(self.preference, self.exchange)
 
 
 class MxRecord(_ValuesMixin, Record):
@@ -464,6 +482,7 @@ class MxRecord(_ValuesMixin, Record):
 
 
 class NaptrValue(object):
+    VALID_FLAGS = ('S', 'A', 'U', 'P')
 
     @classmethod
     def _validate_value(cls, data):
@@ -481,8 +500,15 @@ class NaptrValue(object):
         except ValueError:
             reasons.append('invalid preference "{}"'
                            .format(data['preference']))
-        # TODO: validate field data
-        for k in ('flags', 'service', 'regexp', 'replacement'):
+        try:
+            flags = data['flags']
+            if flags not in cls.VALID_FLAGS:
+                reasons.append('unrecognized flags "{}"'.format(flags))
+        except KeyError:
+            reasons.append('missing flags')
+
+        # TODO: validate these... they're non-trivial
+        for k in ('service', 'regexp', 'replacement'):
             if k not in data:
                 reasons.append('missing {}'.format(k))
         return reasons
@@ -568,19 +594,25 @@ class PtrRecord(_ValueMixin, Record):
 
 
 class SshfpValue(object):
+    VALID_ALGORITHMS = (1, 2)
+    VALID_FINGERPRINT_TYPES = (1,)
 
     @classmethod
     def _validate_value(cls, value):
         reasons = []
-        # TODO: validate algorithm and fingerprint_type values
         try:
-            int(value['algorithm'])
+            algorithm = int(value['algorithm'])
+            if algorithm not in cls.VALID_ALGORITHMS:
+                reasons.append('unrecognized algorithm "{}"'.format(algorithm))
         except KeyError:
             reasons.append('missing algorithm')
         except ValueError:
             reasons.append('invalid algorithm "{}"'.format(value['algorithm']))
         try:
-            int(value['fingerprint_type'])
+            fingerprint_type = int(value['fingerprint_type'])
+            if fingerprint_type not in cls.VALID_FINGERPRINT_TYPES:
+                reasons.append('unrecognized fingerprint_type "{}"'
+                               .format(fingerprint_type))
         except KeyError:
             reasons.append('missing fingerprint_type')
         except ValueError:

octodns/source/tinydns.py

@@ -65,8 +65,8 @@ class TinyDnsBaseSource(BaseSource):
             'ttl': ttl,
             'type': _type,
             'values': [{
-                'priority': r[1],
-                'value': '{}.'.format(r[0])
+                'preference': r[1],
+                'exchange': '{}.'.format(r[0])
             } for r in records]
         }
 

tests/config/unit.tests.yaml

@@ -60,12 +60,12 @@ mx:
   ttl: 300
   type: MX
   values:
-  - priority: 40
-    value: smtp-1.unit.tests.
-  - priority: 20
-    value: smtp-2.unit.tests.
-  - priority: 30
-    value: smtp-3.unit.tests.
+  - exchange: smtp-1.unit.tests.
+    preference: 40
+  - exchange: smtp-2.unit.tests.
+    preference: 20
+  - exchange: smtp-3.unit.tests.
+    preference: 30
   - priority: 10
     value: smtp-4.unit.tests.
 naptr:

tests/test_octodns_provider_dyn.py

@@ -46,11 +46,11 @@ class TestDynProvider(TestCase):
             'type': 'MX',
             'ttl': 302,
             'values': [{
-                'priority': 10,
-                'value': 'smtp-1.unit.tests.'
+                'preference': 10,
+                'exchange': 'smtp-1.unit.tests.'
             }, {
-                'priority': 20,
-                'value': 'smtp-2.unit.tests.'
+                'preference': 20,
+                'exchange': 'smtp-2.unit.tests.'
             }]
         }),
         ('naptr', {

tests/test_octodns_provider_ns1.py

@@ -44,11 +44,11 @@ class TestNs1Provider(TestCase):
         'ttl': 35,
         'type': 'MX',
         'values': [{
-            'priority': 10,
-            'value': 'mx1.unit.tests.',
+            'preference': 10,
+            'exchange': 'mx1.unit.tests.',
         }, {
-            'priority': 20,
-            'value': 'mx2.unit.tests.',
+            'preference': 20,
+            'exchange': 'mx2.unit.tests.',
         }]
     }))
     expected.add(Record.new(zone, 'naptr', {

tests/test_octodns_provider_route53.py

@@ -52,11 +52,11 @@ class TestRoute53Provider(TestCase):
                                                       'Goodbye World?']}),
         ('', {'ttl': 64, 'type': 'MX',
               'values': [{
-                  'priority': 10,
-                  'value': 'smtp-1.unit.tests.',
+                  'preference': 10,
+                  'exchange': 'smtp-1.unit.tests.',
               }, {
-                  'priority': 20,
-                  'value': 'smtp-2.unit.tests.',
+                  'preference': 20,
+                  'exchange': 'smtp-2.unit.tests.',
               }]}),
         ('naptr', {'ttl': 65, 'type': 'NAPTR',
                    'value': {
@@ -1262,8 +1262,8 @@ class TestRoute53Records(TestCase):
         d = _Route53Record(None, Record.new(existing, '',
                                             {'ttl': 42, 'type': 'MX',
                                              'value': {
-                                                 'priority': 10,
-                                                 'value': 'foo.bar.'}}),
+                                                 'preference': 10,
+                                                 'exchange': 'foo.bar.'}}),
                            False)
         self.assertEquals(d, d)
 

tests/test_octodns_record.py

@@ -212,45 +212,49 @@ class TestRecord(TestCase):
 
     def test_mx(self):
         a_values = [{
-            'priority': 10,
-            'value': 'smtp1'
+            'preference': 10,
+            'exchange': 'smtp1.'
         }, {
             'priority': 20,
-            'value': 'smtp2'
+            'value': 'smtp2.'
         }]
         a_data = {'ttl': 30, 'values': a_values}
         a = MxRecord(self.zone, 'a', a_data)
         self.assertEquals('a', a.name)
         self.assertEquals('a.unit.tests.', a.fqdn)
         self.assertEquals(30, a.ttl)
-        self.assertEquals(a_values[0]['priority'], a.values[0].priority)
-        self.assertEquals(a_values[0]['value'], a.values[0].value)
-        self.assertEquals(a_values[1]['priority'], a.values[1].priority)
-        self.assertEquals(a_values[1]['value'], a.values[1].value)
+        self.assertEquals(a_values[0]['preference'], a.values[0].preference)
+        self.assertEquals(a_values[0]['exchange'], a.values[0].exchange)
+        self.assertEquals(a_values[1]['priority'], a.values[1].preference)
+        self.assertEquals(a_values[1]['value'], a.values[1].exchange)
+        a_data['values'][1] = {
+            'preference': 20,
+            'exchange': 'smtp2.',
+        }
         self.assertEquals(a_data, a.data)
 
         b_value = {
-            'priority': 12,
-            'value': 'smtp3',
+            'preference': 12,
+            'exchange': 'smtp3.',
         }
         b_data = {'ttl': 30, 'value': b_value}
         b = MxRecord(self.zone, 'b', b_data)
-        self.assertEquals(b_value['priority'], b.values[0].priority)
-        self.assertEquals(b_value['value'], b.values[0].value)
+        self.assertEquals(b_value['preference'], b.values[0].preference)
+        self.assertEquals(b_value['exchange'], b.values[0].exchange)
         self.assertEquals(b_data, b.data)
 
         target = SimpleProvider()
         # No changes with self
         self.assertFalse(a.changes(a, target))
-        # Diff in priority causes change
+        # Diff in preference causes change
         other = MxRecord(self.zone, 'a', {'ttl': 30, 'values': a_values})
-        other.values[0].priority = 22
+        other.values[0].preference = 22
         change = a.changes(other, target)
         self.assertEqual(change.existing, a)
         self.assertEqual(change.new, other)
         # Diff in value causes change
-        other.values[0].priority = a.values[0].priority
-        other.values[0].value = 'smtpX'
+        other.values[0].preference = a.values[0].preference
+        other.values[0].exchange = 'smtpX'
         change = a.changes(other, target)
         self.assertEqual(change.existing, a)
         self.assertEqual(change.new, other)
@@ -889,32 +893,56 @@ class TestRecordValidation(TestCase):
             'type': 'MX',
             'ttl': 600,
             'value': {
-                'priority': 10,
-                'value': 'foo.bar.com.'
+                'preference': 10,
+                'exchange': 'foo.bar.com.'
             }
         })
 
-        # missing priority
+        # missing preference
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {
                 'type': 'MX',
                 'ttl': 600,
                 'value': {
-                    'value': 'foo.bar.com.'
+                    'exchange': 'foo.bar.com.'
                 }
             })
-        self.assertEquals(['missing priority'], ctx.exception.reasons)
+        self.assertEquals(['missing preference'], ctx.exception.reasons)
 
-        # missing value
+        # invalid preference
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {
                 'type': 'MX',
                 'ttl': 600,
                 'value': {
-                    'priority': 10,
+                    'preference': 'nope',
+                    'exchange': 'foo.bar.com.'
                 }
             })
-        self.assertEquals(['missing value'], ctx.exception.reasons)
+        self.assertEquals(['invalid preference "nope"'], ctx.exception.reasons)
+
+        # missing exchange
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, '', {
+                'type': 'MX',
+                'ttl': 600,
+                'value': {
+                    'preference': 10,
+                }
+            })
+        self.assertEquals(['missing exchange'], ctx.exception.reasons)
+
+        # missing trailing .
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, '', {
+                'type': 'MX',
+                'ttl': 600,
+                'value': {
+                    'preference': 10,
+                    'exchange': 'foo.bar.com'
+                }
+            })
+        self.assertEquals(['missing trailing .'], ctx.exception.reasons)
 
     def test_NXPTR(self):
         # doesn't blow up
@@ -924,7 +952,7 @@ class TestRecordValidation(TestCase):
             'value': {
                 'order': 10,
                 'preference': 20,
-                'flags': 'f',
+                'flags': 'S',
                 'service': 'srv',
                 'regexp': '.*',
                 'replacement': '.'
@@ -935,7 +963,7 @@ class TestRecordValidation(TestCase):
         value = {
             'order': 10,
             'preference': 20,
-            'flags': 'f',
+            'flags': 'S',
             'service': 'srv',
             'regexp': '.*',
             'replacement': '.'
@@ -974,6 +1002,17 @@ class TestRecordValidation(TestCase):
             })
         self.assertEquals(['invalid preference "who"'], ctx.exception.reasons)
 
+        # unrecognized flags
+        v = dict(value)
+        v['flags'] = 'X'
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, '', {
+                'type': 'NAPTR',
+                'ttl': 600,
+                'value': v
+            })
+        self.assertEquals(['unrecognized flags "X"'], ctx.exception.reasons)
+
     def test_NS(self):
         # doesn't blow up
         Record.new(self.zone, '', {
@@ -1065,6 +1104,20 @@ class TestRecordValidation(TestCase):
             })
         self.assertEquals(['invalid algorithm "nope"'], ctx.exception.reasons)
 
+        # unrecognized algorithm
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, '', {
+                'type': 'SSHFP',
+                'ttl': 600,
+                'value': {
+                    'algorithm': 42,
+                    'fingerprint_type': 1,
+                    'fingerprint': 'bf6b6825d2977c511a475bbefb88aad54a92ac73'
+                }
+            })
+        self.assertEquals(['unrecognized algorithm "42"'],
+                          ctx.exception.reasons)
+
         # missing fingerprint_type
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {
@@ -1091,6 +1144,20 @@ class TestRecordValidation(TestCase):
         self.assertEquals(['invalid fingerprint_type "yeeah"'],
                           ctx.exception.reasons)
 
+        # unrecognized fingerprint_type
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, '', {
+                'type': 'SSHFP',
+                'ttl': 600,
+                'value': {
+                    'algorithm': 1,
+                    'fingerprint_type': 42,
+                    'fingerprint': 'bf6b6825d2977c511a475bbefb88aad54a92ac73'
+                }
+            })
+        self.assertEquals(['unrecognized fingerprint_type "42"'],
+                          ctx.exception.reasons)
+
         # missing fingerprint
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {

tests/test_octodns_source_tinydns.py

@@ -68,22 +68,22 @@ class TestTinyDnsFileSource(TestCase):
                 'type': 'MX',
                 'ttl': 3600,
                 'values': [{
-                    'priority': 10,
-                    'value': 'smtp-1-host.example.com.',
+                    'preference': 10,
+                    'exchange': 'smtp-1-host.example.com.',
                 }, {
-                    'priority': 20,
-                    'value': 'smtp-2-host.example.com.',
+                    'preference': 20,
+                    'exchange': 'smtp-2-host.example.com.',
                 }]
             }),
             ('smtp', {
                 'type': 'MX',
                 'ttl': 1800,
                 'values': [{
-                    'priority': 30,
-                    'value': 'smtp-1-host.example.com.',
+                    'preference': 30,
+                    'exchange': 'smtp-1-host.example.com.',
                 }, {
-                    'priority': 40,
-                    'value': 'smtp-2-host.example.com.',
+                    'preference': 40,
+                    'exchange': 'smtp-2-host.example.com.',
                 }]
             }),
         ):