Merge pull request #1300 from octodns/cname-record-lenience

Allow CNAME to coexist if all records have lenient=True

.changelog/b15794db1fe142b18e5a7da21abc76cc.md

@@ -0,0 +1,4 @@
+---
+type: minor
+---
+Allow CNAME to coexist if all records have lenient=True

octodns/zone.py

@@ -184,6 +184,8 @@ class Zone(object):
             self._records[name].discard(record)
 
         node = self._records[name]
+        new_lenient = record.lenient
+        existing_lenient = all(r.lenient for r in node)
         if record in node:
             # We already have a record at this node of this type
             existing = [c for c in node if c == record][0]
@@ -192,9 +194,16 @@ class Zone(object):
                 existing,
                 record,
             )
-        elif not lenient and (
-            (record._type == 'CNAME' and len(node) > 0)
-            or ('CNAME' in [r._type for r in node])
+        elif (
+            # add was not called with lenience
+            not lenient
+            # existing and new records aren't lenient
+            and not (existing_lenient and new_lenient)
+            # and there'll be a CNAME co-existing with other records
+            and (
+                (record._type == 'CNAME' and len(node) > 0)
+                or ('CNAME' in [r._type for r in node])
+            )
         ):
             # We're adding a CNAME to existing records or adding to an existing
             # CNAME

tests/test_octodns_zone.py

@@ -514,6 +514,81 @@ class TestZone(TestCase):
         zone.add_record(a, lenient=True)
         self.assertEqual(set([a, cname]), zone.records)
 
+        # add cname to lenient a
+        a = Record.new(
+            zone,
+            'www',
+            {
+                'ttl': 60,
+                'type': 'A',
+                'value': '9.9.9.9',
+                'octodns': {'lenient': True},
+            },
+        )
+        zone = Zone('unit.tests.', [])
+        zone.add_record(a)
+        with self.assertRaises(InvalidNodeException) as ctx:
+            zone.add_record(cname)
+        self.assertTrue(', has some context' in str(ctx.exception))
+        self.assertEqual(set([a]), zone.records)
+
+        # add lenient a to cname
+        zone = Zone('unit.tests.', [])
+        zone.add_record(cname)
+        with self.assertRaises(InvalidNodeException):
+            zone.add_record(a)
+        self.assertEqual(set([cname]), zone.records)
+
+        # add lenient cname to lenient a
+        cname = Record.new(
+            zone,
+            'www',
+            {
+                'ttl': 60,
+                'type': 'CNAME',
+                'value': 'foo.bar.com.',
+                'octodns': {'lenient': True},
+            },
+        )
+        zone = Zone('unit.tests.', [])
+        zone.add_record(a)
+        zone.add_record(cname)
+        self.assertEqual(set([a, cname]), zone.records)
+
+        # add lenient a to lenient cname
+        zone = Zone('unit.tests.', [])
+        zone.add_record(cname)
+        zone.add_record(a)
+        self.assertEqual(set([a, cname]), zone.records)
+
+        # adding something else w/o lenient still errors
+        zone = Zone('unit.tests.', [])
+        zone.add_record(cname)
+        zone.add_record(a)
+        txt = Record.new(
+            zone, 'www', {'ttl': 60, 'type': 'TXT', 'value': 'Hello World'}
+        )
+        with self.assertRaises(InvalidNodeException):
+            zone.add_record(txt)
+        self.assertEqual(set([a, cname]), zone.records)
+
+        # if the 3rd record is lenient it can be added
+        zone = Zone('unit.tests.', [])
+        zone.add_record(cname)
+        zone.add_record(a)
+        txt = Record.new(
+            zone,
+            'www',
+            {
+                'ttl': 60,
+                'type': 'TXT',
+                'value': 'Hello World',
+                'octodns': {'lenient': True},
+            },
+        )
+        zone.add_record(txt)
+        self.assertEqual(set([a, cname, txt]), zone.records)
+
     def test_excluded_records(self):
         zone_normal = Zone('unit.tests.', [])
         zone_excluded = Zone('unit.tests.', [])