Merge pull request #621 from github/alias-root-only

ALIAS records only allowed at the root of zones

docs/records.md

@@ -6,15 +6,17 @@ OctoDNS supports the following record types:
 
 * `A`
 * `AAAA`
+* `ALIAS`
+* `CAA`
 * `CNAME`
 * `DNAME`
 * `MX`
 * `NAPTR`
 * `NS`
 * `PTR`
-* `SSHFP`
 * `SPF`
 * `SRV`
+* `SSHFP`
 * `TXT`
 
 Underlying provider support for each of these varies and some providers have extra requirements or limitations. In cases where a record type is not supported by a provider OctoDNS will ignore it there and continue to manage the record elsewhere. For example `SSHFP` is supported by Dyn, but not Route53. If your source data includes an SSHFP record OctoDNS will keep it in sync on Dyn, but not consider it when evaluating the state of Route53. The best way to find out what types are supported by a provider is to look for its `supports` method. If that method exists the logic will drive which records are supported and which are ignored. If the provider does not implement the method it will fall back to `BaseProvider.supports` which indicates full support.
@@ -82,3 +84,39 @@ In the above example each name had a single record, but there are cases where a
 Each record type has a corresponding set of required data. The easiest way to determine what's required is probably to look at the record object in [`octodns/record/__init__.py`](/octodns/record/__init__.py). You may also utilize `octodns-validate` which will throw errors about what's missing when run.
 
 `type` is required for all records. `ttl` is optional. When TTL is not specified the `YamlProvider`'s default will be used. In any situation where an array of `values` can be used you can opt to go with `value` as a single item if there's only one.
+
+### Lenience
+
+octoDNS is fairly strict in terms of standards compliance and is opinionated in terms of best practices. Examples of former include SRV record naming requirements and the latter that ALIAS records are constrained to the root of zones. The strictness and support of providers varies so you may encounter existing records that fail validation when you try to dump them or you may even have use cases for which you need to create or preserve records that don't validate. octoDNS's solution to this is the `lenient` flag.
+
+It's best to think of the `lenient` flag as "I know what I'm doing and accept any problems I run across." The main reason being is that some providers may allow the non-compliant setup and others may not. The behavior of the non-compliant records may even vary from one provider to another. Caveat emptor.
+
+#### octodns-dump
+
+If you're trying to import a zone into octoDNS config file using `octodns-dump` which fails due to validation errors you can supply the `--lenient` argument to tell octoDNS that you acknowledge that things aren't lining up with its expectations, but you'd like it to go ahead anyway. This will do its best to populate the zone and dump the results out into an octoDNS zone file and include the non-compliant bits. If you go to use that config file octoDNS will again complain about the validation problems. You can correct them in cases where that makes sense, but if you need to preserve the non-compliant records read on for options.
+
+#### Record level lenience
+
+When there are non-compliant records configured in Yaml you can add the following to tell octoDNS to do it's best to proceed with them anyway. If you use `--lenient` above to dump a zone and you'd like to sync it as-is you can mark the problematic records this way.
+
+```yaml
+'not-root':
+  octodns:
+    lenient: true
+  type: ALIAS
+  values: something.else.com.
+```
+
+#### Zone level lenience
+
+If you'd like to enable lenience for a whole zone you can do so with the following, thought it's strongly encouraged to mark things at record level when possible. The most common case where things may need to be done at the zone level is when using something other than `YamlProvider` as a source, e.g. syncing from `Route53Provider` to `Ns1Provider` when there are non-compliant records in the zone in Route53.
+
+```yaml
+  non-compliant-zone.com.:
+    octodns:
+      lenient: true
+    sources:
+    - route53
+    targets:
+    - ns1
+```

octodns/record/__init__.py

@@ -794,6 +794,14 @@ class AliasRecord(_ValueMixin, Record):
     _type = 'ALIAS'
     _value_type = AliasValue
 
+    @classmethod
+    def validate(cls, name, fqdn, data):
+        reasons = []
+        if name != '':
+            reasons.append('non-root ALIAS not allowed')
+        reasons.extend(super(AliasRecord, cls).validate(name, fqdn, data))
+        return reasons
+
 
 class CaaValue(EqualityTupleMixin):
     # https://tools.ietf.org/html/rfc6844#page-5

script/coverage

@@ -27,7 +27,7 @@ export DYN_USERNAME=
 export GOOGLE_APPLICATION_CREDENTIALS=
 
 # Don't allow disabling coverage
-grep -r -I --line-number "# pragma: nocover" octodns && {
+grep -r -I --line-number "# pragma: +no.*cover" octodns && {
   echo "Code coverage should not be disabled"
   exit 1
 }

tests/fixtures/constellix-records.json

@@ -523,43 +523,6 @@
 	"roundRobinFailover": [],
 	"pools": [],
 	"poolsDetail": []
-}, {
-	"id": 1808603,
-	"type": "ANAME",
-	"recordType": "aname",
-	"name": "sub",
-	"recordOption": "roundRobin",
-	"noAnswer": false,
-	"note": "",
-	"ttl": 1800,
-	"gtdRegion": 1,
-	"parentId": 123123,
-	"parent": "domain",
-	"source": "Domain",
-	"modifiedTs": 1565153387855,
-	"value": [{
-		"value": "aname.unit.tests.",
-		"disableFlag": false
-	}],
-	"roundRobin": [{
-		"value": "aname.unit.tests.",
-		"disableFlag": false
-	}],
-	"geolocation": null,
-	"recordFailover": {
-		"disabled": false,
-		"failoverType": 1,
-		"failoverTypeStr": "Normal (always lowest level)",
-		"values": []
-	},
-	"failover": {
-		"disabled": false,
-		"failoverType": 1,
-		"failoverTypeStr": "Normal (always lowest level)",
-		"values": []
-	},
-	"pools": [],
-	"poolsDetail": []
 }, {
 	"id": 1808520,
 	"type": "A",

tests/fixtures/dnsmadeeasy-records.json

@@ -320,20 +320,6 @@
     "name": "",
     "value": "aname.unit.tests.",
     "id": 11189895,
-    "type": "ANAME"
-	 }, {
-    "failover": false,
-    "monitor": false,
-    "sourceId": 123123,
-    "dynamicDns": false,
-    "failed": false,
-    "gtdLocation": "DEFAULT",
-    "hardLink": false,
-    "ttl": 1800,
-    "source": 1,
-    "name": "sub",
-    "value": "aname",
-    "id": 11189896,
     "type": "ANAME"
 	}, {
     "failover": false,

tests/test_octodns_provider_constellix.py

@@ -42,12 +42,6 @@ class TestConstellixProvider(TestCase):
         'value': 'aname.unit.tests.'
     }))
 
-    expected.add_record(Record.new(expected, 'sub', {
-        'ttl': 1800,
-        'type': 'ALIAS',
-        'value': 'aname.unit.tests.'
-    }))
-
     for record in list(expected.records):
         if record.name == 'sub' and record._type == 'NS':
             expected._remove_record(record)
@@ -107,14 +101,14 @@ class TestConstellixProvider(TestCase):
 
                 zone = Zone('unit.tests.', [])
                 provider.populate(zone)
-                self.assertEquals(15, len(zone.records))
+                self.assertEquals(14, len(zone.records))
                 changes = self.expected.changes(zone, provider)
                 self.assertEquals(0, len(changes))
 
         # 2nd populate makes no network calls/all from cache
         again = Zone('unit.tests.', [])
         provider.populate(again)
-        self.assertEquals(15, len(again.records))
+        self.assertEquals(14, len(again.records))
 
         # bust the cache
         del provider._zone_records[zone.name]
@@ -169,7 +163,7 @@ class TestConstellixProvider(TestCase):
             }),
         ])
 
-        self.assertEquals(18, provider._client._request.call_count)
+        self.assertEquals(17, provider._client._request.call_count)
 
         provider._client._request.reset_mock()
 

tests/test_octodns_provider_dnsmadeeasy.py

@@ -44,12 +44,6 @@ class TestDnsMadeEasyProvider(TestCase):
         'value': 'aname.unit.tests.'
     }))
 
-    expected.add_record(Record.new(expected, 'sub', {
-        'ttl': 1800,
-        'type': 'ALIAS',
-        'value': 'aname.unit.tests.'
-    }))
-
     for record in list(expected.records):
         if record.name == 'sub' and record._type == 'NS':
             expected._remove_record(record)
@@ -108,14 +102,14 @@ class TestDnsMadeEasyProvider(TestCase):
 
                 zone = Zone('unit.tests.', [])
                 provider.populate(zone)
-                self.assertEquals(15, len(zone.records))
+                self.assertEquals(14, len(zone.records))
                 changes = self.expected.changes(zone, provider)
                 self.assertEquals(0, len(changes))
 
         # 2nd populate makes no network calls/all from cache
         again = Zone('unit.tests.', [])
         provider.populate(again)
-        self.assertEquals(15, len(again.records))
+        self.assertEquals(14, len(again.records))
 
         # bust the cache
         del provider._zone_records[zone.name]
@@ -180,7 +174,7 @@ class TestDnsMadeEasyProvider(TestCase):
                 'port': 30
             }),
         ])
-        self.assertEquals(27, provider._client._request.call_count)
+        self.assertEquals(26, provider._client._request.call_count)
 
         provider._client._request.reset_mock()
 

tests/test_octodns_provider_mythicbeasts.py

@@ -171,7 +171,7 @@ class TestMythicBeastsProvider(TestCase):
 
     def test_command_generation(self):
         zone = Zone('unit.tests.', [])
-        zone.add_record(Record.new(zone, 'prawf-alias', {
+        zone.add_record(Record.new(zone, '', {
             'ttl': 60,
             'type': 'ALIAS',
             'value': 'alias.unit.tests.',
@@ -228,7 +228,7 @@ class TestMythicBeastsProvider(TestCase):
                 )
 
             expected_commands = [
-                'ADD prawf-alias.unit.tests 60 ANAME alias.unit.tests.',
+                'ADD unit.tests 60 ANAME alias.unit.tests.',
                 'ADD prawf-ns.unit.tests 300 NS alias.unit.tests.',
                 'ADD prawf-ns.unit.tests 300 NS alias2.unit.tests.',
                 'ADD prawf-a.unit.tests 60 A 1.2.3.4',

tests/test_octodns_record.py

@@ -1743,6 +1743,16 @@ class TestRecordValidation(TestCase):
             'value': 'foo.bar.com.',
         })
 
+        # root only
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'nope', {
+                'type': 'ALIAS',
+                'ttl': 600,
+                'value': 'foo.bar.com.',
+            })
+        self.assertEquals(['non-root ALIAS not allowed'],
+                          ctx.exception.reasons)
+
         # missing value
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {
@@ -1753,7 +1763,7 @@ class TestRecordValidation(TestCase):
 
         # missing value
         with self.assertRaises(ValidationError) as ctx:
-            Record.new(self.zone, 'www', {
+            Record.new(self.zone, '', {
                 'type': 'ALIAS',
                 'ttl': 600,
                 'value': None
@@ -1762,7 +1772,7 @@ class TestRecordValidation(TestCase):
 
         # empty value
         with self.assertRaises(ValidationError) as ctx:
-            Record.new(self.zone, 'www', {
+            Record.new(self.zone, '', {
                 'type': 'ALIAS',
                 'ttl': 600,
                 'value': ''