Merge pull request #993 from octodns/dynamic-subnets

Add support for subnet targeting in rules of dynamic records

CHANGELOG.md

@@ -21,6 +21,7 @@
    * Geos should not be repeated in multiple rules
    * Geos in rules subsequent rules should be ordered most to least specific,
      e.g. NA-US-TN must come before NA-US, which must occur before NA
+* Support for subnet targeting in dynamic records added.
 
 #### Stuff
 

docs/dynamic_records.md

@@ -125,6 +125,36 @@ The first portion is the continent:
 
 The second is the two-letter ISO Country Code https://en.wikipedia.org/wiki/ISO_3166-2 and the third is the ISO Country Code Subdivision as per https://en.wikipedia.org/wiki/ISO_3166-2:US. Change the code at the end for the country you are subdividing. Note that these may not always be supported depending on the providers in use.
 
+#### Subnets
+
+Dynamic record rules also support subnet targeting in some providers:
+
+```
+...
+    rules:
+    - geos:
+      - AS
+      - OC
+      subnets:
+      # Subnets used in matching queries
+      - 5.149.176.0/24
+      pool: apac
+...
+```
+
+### Rule ordering
+
+octoDNS has validations in place to ensure that sources have the rules ordered from the most specific match to the least specific match per the following categories:
+
+1. Subnet-only rules
+2. Subnet+Geo rules
+3. Geo-only rules
+4. Catch-all rule (with no subnet or geo matching)
+
+The first 3 categories are optional, while the last one is mandatory.
+
+Subnet targeting is considered more specific than geo targeting. This means that if there is a subnet rule match as well as a geo rule match, subnet match must take precedence. Provider implementations must ensure this behavior of targeting precedence.
+
 ### Health Checks
 
 octoDNS will automatically configure the provider to monitor each IP and check for a 200 response for **https://<ip_address>/_dns**.

octodns/provider/base.py

@@ -59,28 +59,76 @@ class BaseProvider(BaseSource):
                 desired.remove_record(record)
             elif getattr(record, 'dynamic', False):
                 if self.SUPPORTS_DYNAMIC:
-                    if self.SUPPORTS_POOL_VALUE_STATUS:
-                        continue
-                    # drop unsupported up flag
-                    unsupported_pools = []
-                    for _id, pool in record.dynamic.pools.items():
-                        for value in pool.data['values']:
-                            if value['status'] != 'obey':
-                                unsupported_pools.append(_id)
-                    if not unsupported_pools:
-                        continue
-                    unsupported_pools = ','.join(unsupported_pools)
-                    msg = (
-                        f'"status" flag used in pools {unsupported_pools}'
-                        f' in {record.fqdn} is not supported'
-                    )
-                    fallback = 'will ignore it and respect the healthcheck'
-                    self.supports_warn_or_except(msg, fallback)
-                    record = record.copy()
-                    for pool in record.dynamic.pools.values():
-                        for value in pool.data['values']:
-                            value['status'] = 'obey'
-                    desired.add_record(record, replace=True)
+                    if not self.SUPPORTS_POOL_VALUE_STATUS:
+                        # drop unsupported status flag
+                        unsupported_pools = []
+                        for _id, pool in record.dynamic.pools.items():
+                            for value in pool.data['values']:
+                                if value['status'] != 'obey':
+                                    unsupported_pools.append(_id)
+                        if unsupported_pools:
+                            unsupported_pools = ','.join(unsupported_pools)
+                            msg = (
+                                f'"status" flag used in pools {unsupported_pools}'
+                                f' in {record.fqdn} is not supported'
+                            )
+                            fallback = (
+                                'will ignore it and respect the healthcheck'
+                            )
+                            self.supports_warn_or_except(msg, fallback)
+                            record = record.copy()
+                            for pool in record.dynamic.pools.values():
+                                for value in pool.data['values']:
+                                    value['status'] = 'obey'
+                            desired.add_record(record, replace=True)
+
+                    if not self.SUPPORTS_DYNAMIC_SUBNETS:
+                        subnet_rules = []
+                        for i, rule in enumerate(record.dynamic.rules):
+                            rule = rule.data
+                            if not rule.get('subnets'):
+                                continue
+
+                            msg = f'rule {i + 1} contains unsupported subnet matching in {record.fqdn}'
+                            if rule.get('geos'):
+                                fallback = 'using geos only'
+                                self.supports_warn_or_except(msg, fallback)
+                            else:
+                                fallback = 'skipping the rule'
+                                self.supports_warn_or_except(msg, fallback)
+
+                            subnet_rules.append(i)
+
+                        if subnet_rules:
+                            record = record.copy()
+                            rules = record.dynamic.rules
+
+                            # drop subnet rules in reverse order so indices don't shift during rule deletion
+                            for i in sorted(subnet_rules, reverse=True):
+                                rule = rules[i].data
+                                if rule.get('geos'):
+                                    del rule['subnets']
+                                else:
+                                    del rules[i]
+
+                            # drop any pools rendered unused
+                            pools = record.dynamic.pools
+                            pools_seen = set()
+                            for rule in record.dynamic.rules:
+                                pool = rule.data['pool']
+                                while pool:
+                                    pools_seen.add(pool)
+                                    pool = pools[pool].data.get('fallback')
+                            pools_unseen = set(pools.keys()) - pools_seen
+                            for pool in pools_unseen:
+                                self.log.warning(
+                                    '%s: skipping pool %s which is rendered unused due to lack of support for subnet targeting',
+                                    record.fqdn,
+                                    pool,
+                                )
+                                del pools[pool]
+
+                            desired.add_record(record, replace=True)
                 else:
                     msg = f'dynamic records not supported for {record.fqdn}'
                     fallback = 'falling back to simple record'

octodns/provider/yaml.py

@@ -104,6 +104,7 @@ class YamlProvider(BaseProvider):
     SUPPORTS_GEO = True
     SUPPORTS_DYNAMIC = True
     SUPPORTS_POOL_VALUE_STATUS = True
+    SUPPORTS_DYNAMIC_SUBNETS = True
     SUPPORTS_MULTIVALUE_PTR = True
 
     def __init__(

octodns/record/dynamic.py

@@ -7,6 +7,7 @@ from logging import getLogger
 
 from .change import Update
 from .geo import GeoCodes
+from .subnet import Subnets
 
 
 class _DynamicPool(object):
@@ -70,6 +71,10 @@ class _DynamicRule(object):
             self.data['geos'] = sorted(data['geos'])
         except KeyError:
             pass
+        try:
+            self.data['subnets'] = sorted(data['subnets'])
+        except KeyError:
+            pass
 
     def _data(self):
         return self.data
@@ -215,6 +220,7 @@ class _DynamicMixin(object):
         reasons = []
         pools_seen = set()
 
+        subnets_seen = {}
         geos_seen = {}
 
         if not isinstance(rules, (list, tuple)):
@@ -232,10 +238,8 @@ class _DynamicMixin(object):
                     reasons.append(f'rule {rule_num} missing pool')
                     continue
 
-                try:
-                    geos = rule['geos']
-                except KeyError:
-                    geos = []
+                subnets = rule.get('subnets', [])
+                geos = rule.get('geos', [])
 
                 if not isinstance(pool, str):
                     reasons.append(f'rule {rule_num} invalid pool "{pool}"')
@@ -244,18 +248,65 @@ class _DynamicMixin(object):
                         reasons.append(
                             f'rule {rule_num} undefined pool ' f'"{pool}"'
                         )
-                    elif pool in pools_seen and geos:
+                    elif pool in pools_seen and (subnets or geos):
                         reasons.append(
                             f'rule {rule_num} invalid, target '
                             f'pool "{pool}" reused'
                         )
                     pools_seen.add(pool)
 
-                if not geos:
+                if i > 0:
+                    # validate that rules are ordered as:
+                    # subnets-only > subnets + geos > geos-only
+                    previous_subnets = rules[i - 1].get('subnets', [])
+                    previous_geos = rules[i - 1].get('geos', [])
+                    if subnets and geos:
+                        if not previous_subnets and previous_geos:
+                            reasons.append(
+                                f'rule {rule_num} with subnet(s) and geo(s) should appear before all geo-only rules'
+                            )
+                    elif subnets:
+                        if previous_geos:
+                            reasons.append(
+                                f'rule {rule_num} with only subnet targeting should appear before all geo targeting rules'
+                            )
+
+                if not (subnets or geos):
                     if seen_default:
                         reasons.append(f'rule {rule_num} duplicate default')
                     seen_default = True
 
+                if not isinstance(subnets, (list, tuple)):
+                    reasons.append(f'rule {rule_num} subnets must be a list')
+                else:
+                    for subnet in subnets:
+                        reasons.extend(
+                            Subnets.validate(subnet, f'rule {rule_num} ')
+                        )
+                    networks = []
+                    for subnet in subnets:
+                        try:
+                            networks.append(Subnets.parse(subnet))
+                        except:
+                            # previous loop will log any invalid subnets, here we
+                            # process only valid ones and skip invalid ones
+                            pass
+                    # sort subnets from largest to smallest so that we can
+                    # detect rule that have needlessly targeted a more specific
+                    # subnet along with a larger subnet that already contains it
+                    for subnet in sorted(networks):
+                        for seen, where in subnets_seen.items():
+                            if subnet == seen:
+                                reasons.append(
+                                    f'rule {rule_num} targets subnet {subnet} which has previously been seen in rule {where}'
+                                )
+                            elif subnet.subnet_of(seen):
+                                reasons.append(
+                                    f'rule {rule_num} targets subnet {subnet} which is more specific than the previously seen {seen} in rule {where}'
+                                )
+
+                        subnets_seen[subnet] = rule_num
+
                 if not isinstance(geos, (list, tuple)):
                     reasons.append(f'rule {rule_num} geos must be a list')
                 else:
@@ -282,8 +333,10 @@ class _DynamicMixin(object):
 
                         geos_seen[geo] = rule_num
 
-            if 'geos' in rules[-1]:
-                reasons.append('final rule has "geos" and is not catchall')
+            if rules[-1].get('subnets') or rules[-1].get('geos'):
+                reasons.append(
+                    'final rule has "subnets" and/or "geos" and is not catchall'
+                )
 
         return reasons, pools_seen
 

octodns/record/subnet.py

@@ -0,0 +1,25 @@
+#
+#
+#
+
+import ipaddress
+
+
+class Subnets(object):
+    @classmethod
+    def validate(cls, subnet, prefix):
+        '''
+        Validates an octoDNS subnet making sure that it is valid
+        '''
+        reasons = []
+
+        try:
+            cls.parse(subnet)
+        except ValueError:
+            reasons.append(f'{prefix}invalid subnet "{subnet}"')
+
+        return reasons
+
+    @classmethod
+    def parse(cls, subnet):
+        return ipaddress.ip_network(subnet)

octodns/source/base.py

@@ -8,6 +8,7 @@ class BaseSource(object):
     SUPPORTS_MULTIVALUE_PTR = False
     SUPPORTS_POOL_VALUE_STATUS = False
     SUPPORTS_ROOT_NS = False
+    SUPPORTS_DYNAMIC_SUBNETS = False
 
     def __init__(self, id):
         self.id = id

tests/test_octodns_provider_base.py

@@ -394,6 +394,47 @@ class TestBaseProvider(TestCase):
             record2.dynamic.pools['one'].data['values'][0]['status'], 'obey'
         )
 
+        # SUPPORTS_DYNAMIC_SUBNETS
+        provider.SUPPORTS_POOL_VALUE_STATUS = False
+        zone1 = Zone('unit.tests.', [])
+        record1 = Record.new(
+            zone1,
+            'a',
+            {
+                'dynamic': {
+                    'pools': {
+                        'one': {'values': [{'value': '1.1.1.1'}]},
+                        'two': {'values': [{'value': '2.2.2.2'}]},
+                        'three': {'values': [{'value': '3.3.3.3'}]},
+                    },
+                    'rules': [
+                        {'subnets': ['10.1.0.0/16'], 'pool': 'two'},
+                        {
+                            'subnets': ['11.1.0.0/16'],
+                            'geos': ['NA'],
+                            'pool': 'three',
+                        },
+                        {'pool': 'one'},
+                    ],
+                },
+                'type': 'A',
+                'ttl': 3600,
+                'values': ['2.2.2.2'],
+            },
+        )
+        zone1.add_record(record1)
+
+        zone2 = provider._process_desired_zone(zone1.copy())
+        record2 = list(zone2.records)[0]
+        dynamic = record2.dynamic
+        # subnet-only rule is dropped
+        self.assertNotEqual('two', dynamic.rules[0].data['pool'])
+        self.assertEqual(2, len(dynamic.rules))
+        # subnets are dropped from subnet+geo rule
+        self.assertFalse('subnets' in dynamic.rules[0].data)
+        # unused pool is dropped
+        self.assertFalse('two' in record2.dynamic.pools)
+
         # SUPPORTS_ROOT_NS
         provider.SUPPORTS_ROOT_NS = False
         zone1 = Zone('unit.tests.', [])

tests/test_octodns_record_dynamic.py

@@ -871,6 +871,54 @@ class TestRecordDynamic(TestCase):
             ctx.exception.reasons,
         )
 
+        # rule with invalid subnets
+        a_data = {
+            'dynamic': {
+                'pools': {
+                    'one': {'values': [{'value': '3.3.3.3'}]},
+                    'two': {
+                        'values': [{'value': '4.4.4.4'}, {'value': '5.5.5.5'}]
+                    },
+                },
+                'rules': [
+                    {'subnets': '10.1.0.0/16', 'pool': 'two'},
+                    {'pool': 'one'},
+                ],
+            },
+            'ttl': 60,
+            'type': 'A',
+            'values': ['1.1.1.1', '2.2.2.2'],
+        }
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'bad', a_data)
+        self.assertEqual(
+            ['rule 1 subnets must be a list'], ctx.exception.reasons
+        )
+
+        # rule with invalid subnet
+        a_data = {
+            'dynamic': {
+                'pools': {
+                    'one': {'values': [{'value': '3.3.3.3'}]},
+                    'two': {
+                        'values': [{'value': '4.4.4.4'}, {'value': '5.5.5.5'}]
+                    },
+                },
+                'rules': [
+                    {'subnets': ['invalid'], 'pool': 'two'},
+                    {'pool': 'one'},
+                ],
+            },
+            'ttl': 60,
+            'type': 'A',
+            'values': ['1.1.1.1', '2.2.2.2'],
+        }
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'bad', a_data)
+        self.assertEqual(
+            ['rule 1 invalid subnet "invalid"'], ctx.exception.reasons
+        )
+
         # rule with invalid geos
         a_data = {
             'dynamic': {
@@ -1350,4 +1398,166 @@ class TestRecordDynamic(TestCase):
             {'geos': ('EU', 'NA'), 'pool': 'iad'},
         ]
         reasons, pools_seen = _DynamicMixin._validate_rules(pools, rules)
-        self.assertEqual(['final rule has "geos" and is not catchall'], reasons)
+        self.assertEqual(
+            ['final rule has "subnets" and/or "geos" and is not catchall'],
+            reasons,
+        )
+
+    def test_dynamic_subnet_rule_ordering(self):
+        # boiler plate
+        a_data = {
+            'dynamic': {
+                'pools': {
+                    'one': {'values': [{'value': '3.3.3.3'}]},
+                    'two': {
+                        'values': [{'value': '4.4.4.4'}, {'value': '5.5.5.5'}]
+                    },
+                    'three': {'values': [{'value': '2.2.2.2'}]},
+                }
+            },
+            'ttl': 60,
+            'type': 'A',
+            'values': ['1.1.1.1', '2.2.2.2'],
+        }
+        dynamic = a_data['dynamic']
+
+        def validate_rules(rules):
+            dynamic['rules'] = rules
+            with self.assertRaises(ValidationError) as ctx:
+                Record.new(self.zone, 'bad', a_data)
+            return ctx.exception.reasons
+
+        # valid subnet-only → subnet+geo
+        dynamic['rules'] = [
+            {'subnets': ['10.1.0.0/16'], 'pool': 'one'},
+            {'subnets': ['11.1.0.0/16'], 'geos': ['NA'], 'pool': 'two'},
+            {'pool': 'three'},
+        ]
+        record = Record.new(self.zone, 'good', a_data)
+        self.assertEqual(
+            '10.1.0.0/16', record.dynamic.rules[0].data['subnets'][0]
+        )
+
+        # geo-only → subnet-only
+        self.assertEqual(
+            [
+                'rule 2 with only subnet targeting should appear before all geo targeting rules'
+            ],
+            validate_rules(
+                [
+                    {'geos': ['NA'], 'pool': 'two'},
+                    {'subnets': ['10.1.0.0/16'], 'pool': 'one'},
+                    {'pool': 'three'},
+                ]
+            ),
+        )
+
+        # geo-only → subnet+geo
+        self.assertEqual(
+            [
+                'rule 2 with subnet(s) and geo(s) should appear before all geo-only rules'
+            ],
+            validate_rules(
+                [
+                    {'geos': ['NA'], 'pool': 'two'},
+                    {'subnets': ['10.1.0.0/16'], 'geos': ['AS'], 'pool': 'one'},
+                    {'pool': 'three'},
+                ]
+            ),
+        )
+
+        # subnet+geo → subnet-only
+        self.assertEqual(
+            [
+                'rule 2 with only subnet targeting should appear before all geo targeting rules'
+            ],
+            validate_rules(
+                [
+                    {'subnets': ['11.1.0.0/16'], 'geos': ['NA'], 'pool': 'two'},
+                    {'subnets': ['10.1.0.0/16'], 'pool': 'one'},
+                    {'pool': 'three'},
+                ]
+            ),
+        )
+
+        # geo-only → subnet+geo → subnet-only
+        self.assertEqual(
+            [
+                'rule 2 with subnet(s) and geo(s) should appear before all geo-only rules',
+                'rule 3 with only subnet targeting should appear before all geo targeting rules',
+            ],
+            validate_rules(
+                [
+                    {'geos': ['NA'], 'pool': 'two'},
+                    {'subnets': ['10.1.0.0/16'], 'geos': ['AS'], 'pool': 'one'},
+                    {'subnets': ['11.1.0.0/16'], 'pool': 'three'},
+                    {'pool': 'one'},
+                ]
+            ),
+        )
+
+    def test_dynanic_subnet_ordering(self):
+        # boiler plate
+        a_data = {
+            'dynamic': {
+                'pools': {
+                    'one': {'values': [{'value': '3.3.3.3'}]},
+                    'two': {
+                        'values': [{'value': '4.4.4.4'}, {'value': '5.5.5.5'}]
+                    },
+                    'three': {'values': [{'value': '2.2.2.2'}]},
+                }
+            },
+            'ttl': 60,
+            'type': 'A',
+            'values': ['1.1.1.1', '2.2.2.2'],
+        }
+        dynamic = a_data['dynamic']
+
+        def validate_rules(rules):
+            dynamic['rules'] = rules
+            with self.assertRaises(ValidationError) as ctx:
+                Record.new(self.zone, 'bad', a_data)
+            return ctx.exception.reasons
+
+        # duplicate subnet
+        self.assertEqual(
+            [
+                'rule 2 targets subnet 10.1.0.0/16 which has previously been seen in rule 1'
+            ],
+            validate_rules(
+                [
+                    {'subnets': ['10.1.0.0/16'], 'pool': 'two'},
+                    {'subnets': ['10.1.0.0/16'], 'pool': 'one'},
+                    {'pool': 'three'},
+                ]
+            ),
+        )
+
+        # more specific subnet than previous
+        self.assertEqual(
+            [
+                'rule 2 targets subnet 10.1.1.0/24 which is more specific than the previously seen 10.1.0.0/16 in rule 1'
+            ],
+            validate_rules(
+                [
+                    {'subnets': ['10.1.0.0/16'], 'pool': 'two'},
+                    {'subnets': ['10.1.1.0/24'], 'pool': 'one'},
+                    {'pool': 'three'},
+                ]
+            ),
+        )
+
+        # sub-subnet in the same rule
+        self.assertEqual(
+            [
+                'rule 1 targets subnet 10.1.1.0/24 which is more specific than the previously seen 10.1.0.0/16 in rule 1'
+            ],
+            validate_rules(
+                [
+                    {'subnets': ['10.1.0.0/16', '10.1.1.0/24'], 'pool': 'two'},
+                    {'subnets': ['11.1.0.0/16'], 'pool': 'one'},
+                    {'pool': 'three'},
+                ]
+            ),
+        )