From ae781f9b4f678152ff5141af098aa6d77388c6f4 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Mon, 10 Apr 2023 16:28:29 -0700 Subject: [PATCH] Validate rule ordering --- docs/dynamic_records.md | 8 ++++---- octodns/record/dynamic.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/docs/dynamic_records.md b/docs/dynamic_records.md index d2fee7c..7a50b51 100644 --- a/docs/dynamic_records.md +++ b/docs/dynamic_records.md @@ -117,16 +117,16 @@ Dynamic record rules also support subnet targeting in some providers: ### Rule ordering -Provider implementations should consider subnet matching more specific than geolocation matching. This means that if there is a subnet rule match as well as a geo rule match, subnet match should take precedence; and implementations should ensure this behavior. - -While octoDNS itself doesn't assert for any particular ordering of the rules, it is a strongly recommended best practice to have them ordered such that matching happens from the most specific to the least. Specifically, this means the rules should be in the following order of categories: +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 should be mandatory as a best practice, even though not currently enforced by octoDNS. +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 diff --git a/octodns/record/dynamic.py b/octodns/record/dynamic.py index 2069c63..ca36c2c 100644 --- a/octodns/record/dynamic.py +++ b/octodns/record/dynamic.py @@ -255,6 +255,22 @@ class _DynamicMixin(object): ) pools_seen.add(pool) + 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')