Merge pull request #631 from PeterDaveHello/ImproveValueValidation

Improve ALIAS, CNAME, DNAME & PTR record FQDN validation

octodns/provider/azuredns.py

@@ -416,14 +416,8 @@ class AzureProvider(BaseProvider):
             :type  azrecord: azure.mgmt.dns.models.RecordSet
 
             :type  return: dict
-
-            CNAME and PTR both use the catch block to catch possible empty
-            records. Refer to population comment.
         '''
-        try:
-            return {'value': _check_endswith_dot(azrecord.cname_record.cname)}
-        except:
-            return {'value': '.'}
+        return {'value': _check_endswith_dot(azrecord.cname_record.cname)}
 
     def _data_for_MX(self, azrecord):
         return {'values': [{'preference': ar.preference,
@@ -435,11 +429,8 @@ class AzureProvider(BaseProvider):
         return {'values': [_check_endswith_dot(val) for val in vals]}
 
     def _data_for_PTR(self, azrecord):
-        try:
-            ptrdname = azrecord.ptr_records[0].ptrdname
-            return {'value': _check_endswith_dot(ptrdname)}
-        except:
-            return {'value': '.'}
+        ptrdname = azrecord.ptr_records[0].ptrdname
+        return {'value': _check_endswith_dot(ptrdname)}
 
     def _data_for_SRV(self, azrecord):
         return {'values': [{'priority': ar.priority, 'weight': ar.weight,

octodns/record/__init__.py

@@ -10,6 +10,7 @@ from logging import getLogger
 import re
 
 from six import string_types, text_type
+from fqdn import FQDN
 
 from ..equality import EqualityTupleMixin
 from .geo import GeoCodes
@@ -757,6 +758,9 @@ class _TargetValue(object):
             reasons.append('empty value')
         elif not data:
             reasons.append('missing value')
+        elif not FQDN(data, allow_underscores=True).is_valid:
+            reasons.append('{} value "{}" is not a valid FQDN'
+                           .format(_type, data))
         elif not data.endswith('.'):
             reasons.append('{} value "{}" missing trailing .'
                            .format(_type, data))

requirements.txt

@@ -7,6 +7,7 @@ dnspython==1.16.0
 docutils==0.16
 dyn==1.8.1
 edgegrid-python==1.1.1
+fqdn==1.5.0
 futures==3.2.0; python_version < '3.2'
 google-cloud-core==1.4.1
 google-cloud-dns==0.32.0

tests/test_octodns_provider_azuredns.py

@@ -389,9 +389,6 @@ class TestAzureDnsProvider(TestCase):
         recordSet = RecordSet(cname_record=cname1)
         recordSet.name, recordSet.ttl, recordSet.type = 'cname1', 5, 'CNAME'
         rs.append(recordSet)
-        recordSet = RecordSet(cname_record=None)
-        recordSet.name, recordSet.ttl, recordSet.type = 'cname2', 6, 'CNAME'
-        rs.append(recordSet)
         recordSet = RecordSet(mx_records=[MxRecord(preference=10,
                                                    exchange='mx1.unit.test.')])
         recordSet.name, recordSet.ttl, recordSet.type = 'mx1', 7, 'MX'
@@ -413,9 +410,6 @@ class TestAzureDnsProvider(TestCase):
         recordSet = RecordSet(ptr_records=[ptr1])
         recordSet.name, recordSet.ttl, recordSet.type = 'ptr1', 11, 'PTR'
         rs.append(recordSet)
-        recordSet = RecordSet(ptr_records=[PtrRecord(ptrdname=None)])
-        recordSet.name, recordSet.ttl, recordSet.type = 'ptr2', 12, 'PTR'
-        rs.append(recordSet)
         recordSet = RecordSet(srv_records=[SrvRecord(priority=1,
                                                      weight=2,
                                                      port=3,
@@ -449,7 +443,7 @@ class TestAzureDnsProvider(TestCase):
         exists = provider.populate(zone)
         self.assertTrue(exists)
 
-        self.assertEquals(len(zone.records), 18)
+        self.assertEquals(len(zone.records), 16)
 
     def test_populate_zone(self):
         provider = self._get_provider()

tests/test_octodns_record.py

@@ -1799,6 +1799,16 @@ class TestRecordValidation(TestCase):
             })
         self.assertEquals(['empty value'], ctx.exception.reasons)
 
+        # not a valid FQDN
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, '', {
+                'type': 'ALIAS',
+                'ttl': 600,
+                'value': '__.',
+            })
+        self.assertEquals(['ALIAS value "__." is not a valid FQDN'],
+                          ctx.exception.reasons)
+
         # missing trailing .
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {
@@ -1895,6 +1905,16 @@ class TestRecordValidation(TestCase):
             })
         self.assertEquals(['root CNAME not allowed'], ctx.exception.reasons)
 
+        # not a valid FQDN
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'www', {
+                'type': 'CNAME',
+                'ttl': 600,
+                'value': '___.',
+            })
+        self.assertEquals(['CNAME value "___." is not a valid FQDN'],
+                          ctx.exception.reasons)
+
         # missing trailing .
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, 'www', {
@@ -1920,6 +1940,16 @@ class TestRecordValidation(TestCase):
             'value': 'foo.bar.com.',
         })
 
+        # not a valid FQDN
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'www', {
+                'type': 'DNAME',
+                'ttl': 600,
+                'value': '.',
+            })
+        self.assertEquals(['DNAME value "." is not a valid FQDN'],
+                          ctx.exception.reasons)
+
         # missing trailing .
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, 'www', {
@@ -2103,6 +2133,16 @@ class TestRecordValidation(TestCase):
             })
         self.assertEquals(['missing value'], ctx.exception.reasons)
 
+        # not a valid FQDN
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, '', {
+                'type': 'PTR',
+                'ttl': 600,
+                'value': '_.',
+            })
+        self.assertEquals(['PTR value "_." is not a valid FQDN'],
+                          ctx.exception.reasons)
+
         # no trailing .
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {