Improve ALIAS, CNAME, DNAME & PTR record FQDN validation

Use fqdn package to help verify if the record value is really valid.

The original behavior will treat value like `_.` or `.` be a valid
record, which is strange, and the real world may not have those use
cases at all.

The RFC documents are pretty long, as I didn't read them all or enough
to tell should it be valid or not by the spec, so I opened issue #612 to
discuss this case and got a positive response from the main maintainer
to have the change.

Close #628

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_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, '', {