Implement dynamic rule validation for targeting a specific location that would have already matched a more generic one

octodns/record/dynamic.py

@@ -215,6 +215,8 @@ class _DynamicMixin(object):
         reasons = []
         pools_seen = set()
 
+        geos_seen = {}
+
         if not isinstance(rules, (list, tuple)):
             reasons.append('rules must be a list')
         elif not rules:
@@ -257,11 +259,25 @@ class _DynamicMixin(object):
                 if not isinstance(geos, (list, tuple)):
                     reasons.append(f'rule {rule_num} geos must be a list')
                 else:
-                    for geo in geos:
+                    # sorted so that NA would come before NA-US so that the code
+                    # below can detect rules that have needlessly targeted a
+                    # more specific location along with it's parent/ancestor
+                    for geo in sorted(geos):
                         reasons.extend(
                             GeoCodes.validate(geo, f'rule {rule_num} ')
                         )
 
+                        # have we ever seen a broader version of the geo we're
+                        # currently looking at, e.g. geo=NA-US and there was a
+                        # previous rule with NA
+                        for seen, where in geos_seen.items():
+                            if geo.startswith(seen):
+                                reasons.append(
+                                    f'rule {rule_num} targets geo {geo} which is more specific than the previously seen {seen} in rule {where}'
+                                )
+
+                        geos_seen[geo] = rule_num
+
         return reasons, pools_seen
 
     @classmethod

tests/test_octodns_record_dynamic.py

@@ -11,7 +11,12 @@ from octodns.record import Record
 from octodns.record.a import ARecord, Ipv4Value
 from octodns.record.aaaa import AaaaRecord
 from octodns.record.cname import CnameRecord
-from octodns.record.dynamic import _Dynamic, _DynamicPool, _DynamicRule
+from octodns.record.dynamic import (
+    _Dynamic,
+    _DynamicMixin,
+    _DynamicPool,
+    _DynamicRule,
+)
 from octodns.record.exception import ValidationError
 from octodns.zone import Zone
 
@@ -1282,3 +1287,37 @@ class TestRecordDynamic(TestCase):
             },
             cname.dynamic.pools['two'].data,
         )
+
+    def test_dynamic_mixin_validate_rules(self):
+        # this one is fine we get more generic with subsequent rules
+        pools = {'iad', 'sfo'}
+        rules = [
+            {'geos': ('AS', 'NA-CA', 'NA-US-OR'), 'pool': 'sfo'},
+            {'geos': ('EU', 'NA'), 'pool': 'iad'},
+        ]
+        reasons, pools_seen = _DynamicMixin._validate_rules(pools, rules)
+        self.assertFalse(reasons)
+        self.assertEqual({'sfo', 'iad'}, pools_seen)
+
+        pools = {'iad', 'sfo'}
+        rules = [
+            {'geos': ('AS', 'NA'), 'pool': 'sfo'},
+            {'geos': ('EU', 'NA-CA'), 'pool': 'iad'},
+        ]
+        reasons, pools_seen = _DynamicMixin._validate_rules(pools, rules)
+        self.assertEqual(
+            [
+                'rule 2 targets geo NA-CA which is more specific than the previously seen NA in rule 1'
+            ],
+            reasons,
+        )
+
+        pools = {'sfo'}
+        rules = [{'geos': ('AS', 'NA-US', 'NA'), 'pool': 'sfo'}]
+        reasons, pools_seen = _DynamicMixin._validate_rules(pools, rules)
+        self.assertEqual(
+            [
+                'rule 1 targets geo NA-US which is more specific than the previously seen NA in rule 1'
+            ],
+            reasons,
+        )