diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bccc0f..aab426b 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/docs/assets/dynamic-rules-and-pools.jpg b/docs/assets/dynamic-rules-and-pools.jpg deleted file mode 100644 index 005383c..0000000 Binary files a/docs/assets/dynamic-rules-and-pools.jpg and /dev/null differ diff --git a/docs/dynamic_records.md b/docs/dynamic_records.md index a44aec0..f2881d7 100644 --- a/docs/dynamic_records.md +++ b/docs/dynamic_records.md @@ -76,7 +76,34 @@ If you encounter validation errors in dynamic records suggesting best practices #### Visual Representation of the Rules and Pools -![Diagram of the example records rules and pools](assets/dynamic-rules-and-pools.jpg) +```mermaid +--- +title: Visual Representation of the Rules and Pools +--- +flowchart LR + query((Query)) --> rule_0[Rule 0
AF-ZA
AS
OC] + rule_0 --no match--> rule_1[Rule 1
AF
EU] + rule_1 --no match--> rule_2["Rule 2
(catch all)"] + + rule_0 --match--> pool_apac[Pool apac
1.1.1.1
2.2.2.2] + pool_apac --fallback--> pool_na + rule_1 --match--> pool_eu["Pool eu
3.3.3.3 (2/5)
4.4.4.4 (3/5)"] + pool_eu --fallback--> pool_na + rule_2 --> pool_na[Pool na
5.5.5.5
6.6.6.6
7.7.7.7] + pool_na --fallback--> values[values
3.3.3.3
4.4.4.4
5.5.5.5
6.6.6.6
7.7.7.7] + + classDef queryColor fill:#3B67A8,color:#ffffff + classDef ruleColor fill:#D8F57A,color:#000000 + classDef poolColor fill:#F57261,color:#000000 + classDef valueColor fill:#498FF5,color:#000000 + + class query queryColor + class rule_0,rule_1,rule_2 ruleColor + class pool_apac,pool_eu,pool_na poolColor + class values valueColor +``` + + #### Geo Codes @@ -98,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:///_dns**. diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 54fa58f..e1e2234 100644 --- a/octodns/provider/base.py +++ b/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' diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index e536a97..9fd8f04 100644 --- a/octodns/provider/yaml.py +++ b/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__( diff --git a/octodns/record/dynamic.py b/octodns/record/dynamic.py index fbb3ca8..8423937 100644 --- a/octodns/record/dynamic.py +++ b/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 diff --git a/octodns/record/subnet.py b/octodns/record/subnet.py new file mode 100644 index 0000000..b2e8280 --- /dev/null +++ b/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) diff --git a/octodns/source/base.py b/octodns/source/base.py index 30d3190..1c3ff15 100644 --- a/octodns/source/base.py +++ b/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 diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index 1ff5999..3b69b4a 100644 --- a/tests/test_octodns_provider_base.py +++ b/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.', []) diff --git a/tests/test_octodns_record_dynamic.py b/tests/test_octodns_record_dynamic.py index ceb65bf..589c7d0 100644 --- a/tests/test_octodns_record_dynamic.py +++ b/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'}, + ] + ), + )