From 1c531a92815a1009ea17a38da5764c3a38e6de22 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Fri, 17 Sep 2021 11:20:42 -0700 Subject: [PATCH 01/20] Force enable/disable pool values --- octodns/provider/azuredns.py | 28 ++++++++++++++++++++++++++++ octodns/provider/ns1.py | 32 +++++++++++++++++++++----------- octodns/provider/route53.py | 10 ++++++++++ octodns/record/__init__.py | 1 + 4 files changed, 60 insertions(+), 11 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 3290ae5..18762d9 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -807,9 +807,16 @@ class AzureProvider(BaseProvider): defaults.add(val) ep_name = ep_name[:-len('--default--')] + up = None + if pool_ep.endpoint_status == 'Disabled': + up = False + elif ep_name.endswith('--UP'): + up = True + values.append({ 'value': val, 'weight': pool_ep.weight or 1, + 'up': up, }) return values @@ -898,6 +905,22 @@ class AzureProvider(BaseProvider): return data + def _process_desired_zone(self, desired): + # check for support of force up/down values + for record in desired.records: + if not getattr(record, 'dynamic', False): + continue + for name, pool in record.dynamic.pools.items(): + for value in pool.data['values']: + if value['up']: + # Azure only supports up=None & up=False, not up=True + msg = 'up=True is not supported for "{}" pool in {}' \ + .format(name, record.fqdn) + fallback = 'ignoring it' + self.supports_warn_or_except(msg, fallback) + + return super()._process_desired_zone(desired) + def _extra_changes(self, existing, desired, changes): changed = set(c.record for c in changes) @@ -1033,16 +1056,21 @@ class AzureProvider(BaseProvider): if record._type == 'CNAME': target = target[:-1] ep_name = f'{pool_name}--{target}' + if val['up']: + ep_name += '--UP' # Endpoint names cannot have colons, drop them from IPv6 addresses ep_name = ep_name.replace(':', '-') if target in defaults: # mark default ep_name += '--default--' default_seen = True + ep_status = 'Disabled' if val['up'] == False else \ + 'Enabled' endpoints.append(Endpoint( name=ep_name, target=target, weight=val.get('weight', 1), + endpoint_status=ep_status, )) pool_profile = self._generate_tm_profile( diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index e6fab6e..6e47c65 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -591,6 +591,9 @@ class Ns1Provider(BaseProvider): 'value': value, 'weight': int(meta.get('weight', 1)), } + if isinstance(meta['up'], bool): + value_dict['up'] = meta['up'] + if value_dict not in pool['values']: # If we haven't seen this value before add it to the pool pool['values'].append(value_dict) @@ -1141,6 +1144,10 @@ class Ns1Provider(BaseProvider): pool = pools[current_pool_name] for answer in pool_answers[current_pool_name]: fallback = pool.data['fallback'] + if answer['feed_id']: + up = {'feed': answer['feed_id']} + else: + up = answer['up'] answer = { 'answer': answer['answer'], 'meta': { @@ -1150,9 +1157,7 @@ class Ns1Provider(BaseProvider): 'pool': current_pool_name, 'fallback': fallback or '', }), - 'up': { - 'feed': answer['feed_id'], - }, + 'up': up, 'weight': answer['weight'], }, 'region': pool_label, # the one we're answering @@ -1273,20 +1278,25 @@ class Ns1Provider(BaseProvider): for pool_name, pool in sorted(pools.items()): for value in pool.data['values']: weight = value['weight'] + up = value['up'] value = value['value'] - feed_id = value_feed.get(value) - # check for identical monitor and skip creating one if found - if not feed_id: - existing = existing_monitors.get(value) - monitor_id, feed_id = self._monitor_sync(record, value, - existing) - value_feed[value] = feed_id - active_monitors.add(monitor_id) + feed_id = None + if up is None: + # state is not forced, let's find a monitor + feed_id = value_feed.get(value) + # check for identical monitor and skip creating one if found + if not feed_id: + existing = existing_monitors.get(value) + monitor_id, feed_id = self._monitor_sync(record, value, + existing) + value_feed[value] = feed_id + active_monitors.add(monitor_id) pool_answers[pool_name].append({ 'answer': [value], 'weight': weight, 'feed_id': feed_id, + 'up': up, }) if record._type == 'CNAME': diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 0ecce72..a145190 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -941,6 +941,16 @@ class Route53Provider(BaseProvider): rule.data['geos'] = filtered_geos rules.append(rule) + # check for use of 'up' flag and warn/except if used + for name, pool in dynamic.pools.items(): + for value in pool.data['values']: + if value['up'] is not None: + msg = '"up" flag is not supported for "{}" pool' \ + ' in {}'.format(name, record.fqdn) + fallback = 'ignoring it, octodns-sync command ' \ + 'will keep showing changes' + self.supports_warn_or_except(msg, fallback) + if rules != dynamic.rules: record = record.copy() record.dynamic.rules = rules diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index d6fc1d4..fc2cff2 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -418,6 +418,7 @@ class _DynamicPool(object): { 'value': d['value'], 'weight': d.get('weight', 1), + 'up': d.get('up'), # TODO: add validation on this field } for d in data['values'] ] values.sort(key=lambda d: d['value']) From f2e83a6a2501692a5835272ccec9d02a8c358295 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Fri, 17 Sep 2021 11:39:37 -0700 Subject: [PATCH 02/20] f-strings and lint fixes --- octodns/provider/azuredns.py | 6 +++--- octodns/provider/ns1.py | 5 +++-- octodns/provider/route53.py | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 18762d9..1701066 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -914,8 +914,8 @@ class AzureProvider(BaseProvider): for value in pool.data['values']: if value['up']: # Azure only supports up=None & up=False, not up=True - msg = 'up=True is not supported for "{}" pool in {}' \ - .format(name, record.fqdn) + msg = f'up=True is not supported for "{name}" pool ' \ + f'in {record.fqdn}' fallback = 'ignoring it' self.supports_warn_or_except(msg, fallback) @@ -1064,7 +1064,7 @@ class AzureProvider(BaseProvider): # mark default ep_name += '--default--' default_seen = True - ep_status = 'Disabled' if val['up'] == False else \ + ep_status = 'Disabled' if val['up'] is False else \ 'Enabled' endpoints.append(Endpoint( name=ep_name, diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 6e47c65..e88ea09 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -1284,11 +1284,12 @@ class Ns1Provider(BaseProvider): if up is None: # state is not forced, let's find a monitor feed_id = value_feed.get(value) - # check for identical monitor and skip creating one if found + # check for identical monitor and skip creating one if + # found if not feed_id: existing = existing_monitors.get(value) monitor_id, feed_id = self._monitor_sync(record, value, - existing) + existing) value_feed[value] = feed_id active_monitors.add(monitor_id) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index a145190..bde7c11 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -945,8 +945,8 @@ class Route53Provider(BaseProvider): for name, pool in dynamic.pools.items(): for value in pool.data['values']: if value['up'] is not None: - msg = '"up" flag is not supported for "{}" pool' \ - ' in {}'.format(name, record.fqdn) + msg = f'"up" flag is not supported for "{name}"' \ + f' pool in {record.fqdn}' fallback = 'ignoring it, octodns-sync command ' \ 'will keep showing changes' self.supports_warn_or_except(msg, fallback) From 201c57298a44aef7549d2109d255ef162ab93d9e Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Fri, 17 Sep 2021 15:39:16 -0700 Subject: [PATCH 03/20] Deny up-flag support in BaseProvider --- octodns/provider/azuredns.py | 1 + octodns/provider/base.py | 30 ++++++++++++++++++++++-------- octodns/provider/ns1.py | 1 + octodns/provider/route53.py | 10 ---------- octodns/source/base.py | 1 + 5 files changed, 25 insertions(+), 18 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 1701066..8cc716e 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -457,6 +457,7 @@ class AzureProvider(BaseProvider): ''' SUPPORTS_GEO = False SUPPORTS_DYNAMIC = True + SUPPORTS_POOL_VALUE_UP = True SUPPORTS_MUTLIVALUE_PTR = True SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'PTR', 'SRV', 'TXT')) diff --git a/octodns/provider/base.py b/octodns/provider/base.py index d45312f..708d38d 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -57,14 +57,28 @@ class BaseProvider(BaseSource): fallback = 'omitting record' self.supports_warn_or_except(msg, fallback) desired.remove_record(record) - elif getattr(record, 'dynamic', False) and \ - not self.SUPPORTS_DYNAMIC: - msg = f'dynamic records not supported for {record.fqdn}' - fallback = 'falling back to simple record' - self.supports_warn_or_except(msg, fallback) - record = record.copy() - record.dynamic = None - desired.add_record(record, replace=True) + elif getattr(record, 'dynamic', False): + if self.SUPPORTS_DYNAMIC: + pools = record.dynamic.pools.values() + values = [p.data['values'] for p in pools] + if any(isinstance(v['up'], bool) for v in values) and not \ + self.SUPPORTS_POOL_VALUE_UP: + msg = f'"up" flag used in {record.fqdn} is not ' \ + f'supported' + fallback = 'falling back to using healthchecks' + self.supports_warn_or_except(msg, fallback) + record = record.copy() + for pool in record.dynamic.pools.values(): + for value in pool.data['values']: + value['up'] = None + desired.add_record(record, replace=True) + else: + msg = f'dynamic records not supported for {record.fqdn}' + fallback = 'falling back to simple record' + self.supports_warn_or_except(msg, fallback) + record = record.copy() + record.dynamic = None + desired.add_record(record, replace=True) elif record._type == 'PTR' and len(record.values) > 1 and \ not self.SUPPORTS_MUTLIVALUE_PTR: # replace with a single-value copy diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index e88ea09..2bf6787 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -306,6 +306,7 @@ class Ns1Provider(BaseProvider): ''' SUPPORTS_GEO = True SUPPORTS_DYNAMIC = True + SUPPORTS_POOL_VALUE_UP = True SUPPORTS_MUTLIVALUE_PTR = True SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', 'SRV', 'TXT', 'URLFWD')) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index bde7c11..0ecce72 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -941,16 +941,6 @@ class Route53Provider(BaseProvider): rule.data['geos'] = filtered_geos rules.append(rule) - # check for use of 'up' flag and warn/except if used - for name, pool in dynamic.pools.items(): - for value in pool.data['values']: - if value['up'] is not None: - msg = f'"up" flag is not supported for "{name}"' \ - f' pool in {record.fqdn}' - fallback = 'ignoring it, octodns-sync command ' \ - 'will keep showing changes' - self.supports_warn_or_except(msg, fallback) - if rules != dynamic.rules: record = record.copy() record.dynamic.rules = rules diff --git a/octodns/source/base.py b/octodns/source/base.py index 6094726..1011dd1 100644 --- a/octodns/source/base.py +++ b/octodns/source/base.py @@ -9,6 +9,7 @@ from __future__ import absolute_import, division, print_function, \ class BaseSource(object): SUPPORTS_MUTLIVALUE_PTR = False + SUPPORTS_POOL_VALUE_UP = False def __init__(self, id): self.id = id From 1745a5c239c540ce3606dbd9638e4838a86785a8 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Fri, 17 Sep 2021 15:42:50 -0700 Subject: [PATCH 04/20] Better fallback message --- octodns/provider/azuredns.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 8cc716e..d918a3a 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -917,7 +917,8 @@ class AzureProvider(BaseProvider): # Azure only supports up=None & up=False, not up=True msg = f'up=True is not supported for "{name}" pool ' \ f'in {record.fqdn}' - fallback = 'ignoring it' + fallback = \ + 'will ignore the flag and respect healthcheck' self.supports_warn_or_except(msg, fallback) return super()._process_desired_zone(desired) From dae3dd55d7958c0237b9ab7f31e74556848a3eb4 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Fri, 17 Sep 2021 21:56:05 -0700 Subject: [PATCH 05/20] YamlProvider supports everything --- octodns/provider/yaml.py | 1 + 1 file changed, 1 insertion(+) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index aaf84c6..c9b377e 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_UP = True SUPPORTS_MUTLIVALUE_PTR = True SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'DNAME', 'LOC', 'MX', 'NAPTR', 'NS', 'PTR', 'SSHFP', 'SPF', 'SRV', 'TXT', From aff4ea5411f4cb84d54e09274b0e97203491b1b9 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Tue, 21 Sep 2021 01:19:50 -0700 Subject: [PATCH 06/20] Reset up flag in azure when unsupported --- octodns/provider/azuredns.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index fede211..23da755 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -811,8 +811,6 @@ class AzureProvider(BaseProvider): up = None if pool_ep.endpoint_status == 'Disabled': up = False - elif ep_name.endswith('--UP'): - up = True values.append({ 'value': val, @@ -911,15 +909,28 @@ class AzureProvider(BaseProvider): for record in desired.records: if not getattr(record, 'dynamic', False): continue + + up_pools = [] for name, pool in record.dynamic.pools.items(): for value in pool.data['values']: if value['up']: # Azure only supports up=None & up=False, not up=True - msg = f'up=True is not supported for "{name}" pool ' \ - f'in {record.fqdn}' - fallback = \ - 'will ignore the flag and respect healthcheck' - self.supports_warn_or_except(msg, fallback) + up_pools.append(name) + if not up_pools: + continue + + up_pools = ','.join(up_pools) + msg = f'up=True is not supported for pools {up_pools} in ' \ + f'{record.fqdn}' + 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']: + if value['up']: + value['up'] = None + desired.add_record(record, replace=True) return super()._process_desired_zone(desired) @@ -1058,8 +1069,6 @@ class AzureProvider(BaseProvider): if record._type == 'CNAME': target = target[:-1] ep_name = f'{pool_name}--{target}' - if val['up']: - ep_name += '--UP' # Endpoint names cannot have colons, drop them from IPv6 addresses ep_name = ep_name.replace(':', '-') if target in defaults: From e737161c76467d87e4c7a314d3e3667413c0f09b Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Tue, 21 Sep 2021 01:34:51 -0700 Subject: [PATCH 07/20] Show pool names that don't support the flag --- octodns/provider/base.py | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 416bf9b..49ba692 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -59,19 +59,26 @@ class BaseProvider(BaseSource): desired.remove_record(record) elif getattr(record, 'dynamic', False): if self.SUPPORTS_DYNAMIC: - pools = record.dynamic.pools.values() - values = [p.data['values'] for p in pools] - if any(isinstance(v['up'], bool) for v in values) and not \ - self.SUPPORTS_POOL_VALUE_UP: - msg = f'"up" flag used in {record.fqdn} is not ' \ - f'supported' - fallback = 'falling back to using healthchecks' - self.supports_warn_or_except(msg, fallback) - record = record.copy() - for pool in record.dynamic.pools.values(): - for value in pool.data['values']: - value['up'] = None - desired.add_record(record, replace=True) + if self.SUPPORTS_POOL_VALUE_UP: + continue + # drop unsupported up flag + unsupported_pools = [] + for _id, pool in record.dynamic.pools.items(): + for value in pool.data['values']: + if isinstance(value['up'], bool): + unsupported_pools.append(_id) + if not unsupported_pools: + continue + unsupported_pools = ','.join(unsupported_pools) + msg = f'"up" flag used in pools {unsupported_pools} in ' \ + f'{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['up'] = None + desired.add_record(record, replace=True) else: msg = f'dynamic records not supported for {record.fqdn}' fallback = 'falling back to simple record' From 43e02c916c0b3b0ae137375fb16672a3bfd84d4c Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Tue, 21 Sep 2021 01:54:55 -0700 Subject: [PATCH 08/20] enum for status flag of pool values --- octodns/provider/azuredns.py | 22 +++++++++++----------- octodns/provider/base.py | 6 +++--- octodns/provider/ns1.py | 13 +++++++------ octodns/provider/yaml.py | 2 +- octodns/record/__init__.py | 2 +- octodns/source/base.py | 2 +- 6 files changed, 24 insertions(+), 23 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 23da755..ef9affd 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -457,7 +457,7 @@ class AzureProvider(BaseProvider): ''' SUPPORTS_GEO = False SUPPORTS_DYNAMIC = True - SUPPORTS_POOL_VALUE_UP = True + SUPPORTS_POOL_VALUE_STATUS = True SUPPORTS_MULTIVALUE_PTR = True SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'PTR', 'SRV', 'TXT')) @@ -808,14 +808,14 @@ class AzureProvider(BaseProvider): defaults.add(val) ep_name = ep_name[:-len('--default--')] - up = None + status = 'obey' if pool_ep.endpoint_status == 'Disabled': - up = False + status = 'down' values.append({ 'value': val, 'weight': pool_ep.weight or 1, - 'up': up, + 'status': status, }) return values @@ -905,7 +905,7 @@ class AzureProvider(BaseProvider): return data def _process_desired_zone(self, desired): - # check for support of force up/down values + # check for status=up values for record in desired.records: if not getattr(record, 'dynamic', False): continue @@ -913,14 +913,14 @@ class AzureProvider(BaseProvider): up_pools = [] for name, pool in record.dynamic.pools.items(): for value in pool.data['values']: - if value['up']: - # Azure only supports up=None & up=False, not up=True + if value['status'] == 'up': + # Azure only supports obey and down, not up up_pools.append(name) if not up_pools: continue up_pools = ','.join(up_pools) - msg = f'up=True is not supported for pools {up_pools} in ' \ + msg = f'status=up is not supported for pools {up_pools} in ' \ f'{record.fqdn}' fallback = 'will ignore it and respect the healthcheck' self.supports_warn_or_except(msg, fallback) @@ -928,8 +928,8 @@ class AzureProvider(BaseProvider): record = record.copy() for pool in record.dynamic.pools.values(): for value in pool.data['values']: - if value['up']: - value['up'] = None + if value['status'] == 'up': + value['status'] = 'obey' desired.add_record(record, replace=True) return super()._process_desired_zone(desired) @@ -1075,7 +1075,7 @@ class AzureProvider(BaseProvider): # mark default ep_name += '--default--' default_seen = True - ep_status = 'Disabled' if val['up'] is False else \ + ep_status = 'Disabled' if val['status'] == 'down' else \ 'Enabled' endpoints.append(Endpoint( name=ep_name, diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 49ba692..459437d 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -59,13 +59,13 @@ class BaseProvider(BaseSource): desired.remove_record(record) elif getattr(record, 'dynamic', False): if self.SUPPORTS_DYNAMIC: - if self.SUPPORTS_POOL_VALUE_UP: + 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 isinstance(value['up'], bool): + if value['status'] != 'obey': unsupported_pools.append(_id) if not unsupported_pools: continue @@ -77,7 +77,7 @@ class BaseProvider(BaseSource): record = record.copy() for pool in record.dynamic.pools.values(): for value in pool.data['values']: - value['up'] = None + value['status'] = 'obey' desired.add_record(record, replace=True) else: msg = f'dynamic records not supported for {record.fqdn}' diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 5afc3fd..02d879e 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -306,7 +306,7 @@ class Ns1Provider(BaseProvider): ''' SUPPORTS_GEO = True SUPPORTS_DYNAMIC = True - SUPPORTS_POOL_VALUE_UP = True + SUPPORTS_POOL_VALUE_STATUS = True SUPPORTS_MULTIVALUE_PTR = True SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', 'SRV', 'TXT', 'URLFWD')) @@ -593,7 +593,7 @@ class Ns1Provider(BaseProvider): 'weight': int(meta.get('weight', 1)), } if isinstance(meta['up'], bool): - value_dict['up'] = meta['up'] + value_dict['status'] = 'up' if meta['up'] else 'down' if value_dict not in pool['values']: # If we haven't seen this value before add it to the pool @@ -1148,7 +1148,7 @@ class Ns1Provider(BaseProvider): if answer['feed_id']: up = {'feed': answer['feed_id']} else: - up = answer['up'] + up = answer['status'] == 'up' answer = { 'answer': answer['answer'], 'meta': { @@ -1279,10 +1279,11 @@ class Ns1Provider(BaseProvider): for pool_name, pool in sorted(pools.items()): for value in pool.data['values']: weight = value['weight'] - up = value['up'] + status = value['status'] value = value['value'] + feed_id = None - if up is None: + if status == 'obey': # state is not forced, let's find a monitor feed_id = value_feed.get(value) # check for identical monitor and skip creating one if @@ -1298,7 +1299,7 @@ class Ns1Provider(BaseProvider): 'answer': [value], 'weight': weight, 'feed_id': feed_id, - 'up': up, + 'status': status, }) if record._type == 'CNAME': diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 1aefe3a..57fae3b 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -104,7 +104,7 @@ class YamlProvider(BaseProvider): ''' SUPPORTS_GEO = True SUPPORTS_DYNAMIC = True - SUPPORTS_POOL_VALUE_UP = True + SUPPORTS_POOL_VALUE_STATUS = True SUPPORTS_MULTIVALUE_PTR = True SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'DNAME', 'LOC', 'MX', 'NAPTR', 'NS', 'PTR', 'SSHFP', 'SPF', 'SRV', 'TXT', diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index fc2cff2..ba8e3c6 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -418,7 +418,7 @@ class _DynamicPool(object): { 'value': d['value'], 'weight': d.get('weight', 1), - 'up': d.get('up'), # TODO: add validation on this field + 'status': d.get('status', 'obey'), # TODO: add validation on this field } for d in data['values'] ] values.sort(key=lambda d: d['value']) diff --git a/octodns/source/base.py b/octodns/source/base.py index 6214e96..569a1b4 100644 --- a/octodns/source/base.py +++ b/octodns/source/base.py @@ -9,7 +9,7 @@ from __future__ import absolute_import, division, print_function, \ class BaseSource(object): SUPPORTS_MULTIVALUE_PTR = False - SUPPORTS_POOL_VALUE_UP = False + SUPPORTS_POOL_VALUE_STATUS = False def __init__(self, id): self.id = id From c521f06de2fb1a66b7dfaf3131d5843250be0a51 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Tue, 21 Sep 2021 01:58:07 -0700 Subject: [PATCH 09/20] fix flag name in message --- octodns/provider/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 459437d..74c8c2b 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -70,8 +70,8 @@ class BaseProvider(BaseSource): if not unsupported_pools: continue unsupported_pools = ','.join(unsupported_pools) - msg = f'"up" flag used in pools {unsupported_pools} in ' \ - f'{record.fqdn} is not supported' + 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() From ee517587a9045848b0cfc7b3f41fe64857a5cdfd Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Tue, 21 Sep 2021 02:04:52 -0700 Subject: [PATCH 10/20] add validation for status flag --- octodns/record/__init__.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index ba8e3c6..f24a89e 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -418,7 +418,7 @@ class _DynamicPool(object): { 'value': d['value'], 'weight': d.get('weight', 1), - 'status': d.get('status', 'obey'), # TODO: add validation on this field + 'status': d.get('status', 'obey'), } for d in data['values'] ] values.sort(key=lambda d: d['value']) @@ -574,6 +574,14 @@ class _DynamicMixin(object): reasons.append(f'missing value in pool "{_id}" ' f'value {value_num}') + try: + status = value['status'] + if status not in ['up', 'down', 'obey']: + reasons.append(f'invalid status "{status}" in ' + f'pool "{_id}" value {value_num}') + except KeyError: + pass + if len(values) == 1 and values[0].get('weight', 1) != 1: reasons.append(f'pool "{_id}" has single value with ' 'weight!=1') From f2164f3a92347a77a3fc57fc95307e0409ec173c Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Tue, 21 Sep 2021 02:14:07 -0700 Subject: [PATCH 11/20] fix status validation --- octodns/record/__init__.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index f24a89e..a498fb7 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -566,14 +566,6 @@ class _DynamicMixin(object): reasons.append(f'invalid weight "{weight}" in ' f'pool "{_id}" value {value_num}') - try: - value = value['value'] - reasons.extend(cls._value_type.validate(value, - cls._type)) - except KeyError: - reasons.append(f'missing value in pool "{_id}" ' - f'value {value_num}') - try: status = value['status'] if status not in ['up', 'down', 'obey']: @@ -582,6 +574,14 @@ class _DynamicMixin(object): except KeyError: pass + try: + value = value['value'] + reasons.extend(cls._value_type.validate(value, + cls._type)) + except KeyError: + reasons.append(f'missing value in pool "{_id}" ' + f'value {value_num}') + if len(values) == 1 and values[0].get('weight', 1) != 1: reasons.append(f'pool "{_id}" has single value with ' 'weight!=1') From 787ce7ccc86f3ad9fbe4b337b394023772f119eb Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Mon, 27 Sep 2021 18:31:38 -0700 Subject: [PATCH 12/20] Add the default status in tests to make them pass --- octodns/provider/constellix.py | 7 ++++++- tests/test_octodns_provider_dyn.py | 3 +++ tests/test_octodns_provider_ns1.py | 10 ++++++++++ tests/test_octodns_provider_route53.py | 12 ++++++------ tests/test_octodns_record.py | 20 ++++++++++++++++++++ 5 files changed, 45 insertions(+), 7 deletions(-) diff --git a/octodns/provider/constellix.py b/octodns/provider/constellix.py index 863ad66..93f809b 100644 --- a/octodns/provider/constellix.py +++ b/octodns/provider/constellix.py @@ -618,7 +618,12 @@ class ConstellixProvider(BaseProvider): for i, rule in enumerate(record.dynamic.rules): pool_name = rule.data.get('pool') pool = record.dynamic.pools.get(pool_name) - values = pool.data.get('values') + values = [ + { + 'value': value['value'], + 'weight': value['weight'], + } for value in pool.data.get('values', []) + ] # Make a pool name based on zone, record, type and name generated_pool_name = \ diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 7c023fd..4072947 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -1921,12 +1921,15 @@ class TestDynProviderDynamic(TestCase): 'values': [{ 'value': '1.2.3.5', 'weight': 1, + 'status': 'obey', }, { 'value': '1.2.3.6', 'weight': 1, + 'status': 'obey', }, { 'value': '1.2.3.7', 'weight': 1, + 'status': 'obey', }] }, record.dynamic.pools['pool1'].data) self.assertEquals(2, len(record.dynamic.rules)) diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 1b58e6b..6cd6a36 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -1614,6 +1614,7 @@ class TestNs1ProviderDynamic(TestCase): 'meta': { 'priority': 1, 'note': 'from:lhr__country', + 'up': {}, }, 'region': 'lhr', }, { @@ -1622,6 +1623,7 @@ class TestNs1ProviderDynamic(TestCase): 'priority': 2, 'weight': 12, 'note': 'from:iad', + 'up': {}, }, 'region': 'lhr', }, { @@ -1637,6 +1639,7 @@ class TestNs1ProviderDynamic(TestCase): 'priority': 1, 'weight': 12, 'note': 'from:iad', + 'up': {}, }, 'region': 'iad', }, { @@ -1652,6 +1655,7 @@ class TestNs1ProviderDynamic(TestCase): 'priority': 1, 'weight': 12, 'note': f'from:{catchall_pool_name}', + 'up': {}, }, 'region': catchall_pool_name, }, { @@ -1798,6 +1802,7 @@ class TestNs1ProviderDynamic(TestCase): 'meta': { 'priority': 1, 'note': 'from:one__country pool:one fallback:two', + 'up': {}, }, 'region': 'one_country', }, { @@ -1805,6 +1810,7 @@ class TestNs1ProviderDynamic(TestCase): 'meta': { 'priority': 2, 'note': 'from:one__country pool:two fallback:three', + 'up': {}, }, 'region': 'one_country', }, { @@ -1812,6 +1818,7 @@ class TestNs1ProviderDynamic(TestCase): 'meta': { 'priority': 3, 'note': 'from:one__country pool:three fallback:', + 'up': {}, }, 'region': 'one_country', }, { @@ -1826,6 +1833,7 @@ class TestNs1ProviderDynamic(TestCase): 'meta': { 'priority': 1, 'note': 'from:four__country pool:four fallback:', + 'up': {}, }, 'region': 'four_country', }, { @@ -1916,6 +1924,7 @@ class TestNs1ProviderDynamic(TestCase): 'priority': 1, 'weight': 12, 'note': f'from:{catchall_pool_name}', + 'up': {}, }, 'region': catchall_pool_name, }, { @@ -1923,6 +1932,7 @@ class TestNs1ProviderDynamic(TestCase): 'meta': { 'priority': 2, 'note': 'from:--default--', + 'up': {}, }, 'region': catchall_pool_name, }], diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 9c9ff28..2759712 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -2591,25 +2591,25 @@ class TestRoute53Provider(TestCase): 'ap-southeast-1': { 'fallback': 'us-east-1', 'values': [{ - 'weight': 2, 'value': '1.4.1.1' + 'weight': 2, 'value': '1.4.1.1', 'status': 'obey', }, { - 'weight': 2, 'value': '1.4.1.2' + 'weight': 2, 'value': '1.4.1.2', 'status': 'obey', }] }, 'eu-central-1': { 'fallback': 'us-east-1', 'values': [{ - 'weight': 1, 'value': '1.3.1.1' + 'weight': 1, 'value': '1.3.1.1', 'status': 'obey', }, { - 'weight': 1, 'value': '1.3.1.2' + 'weight': 1, 'value': '1.3.1.2', 'status': 'obey', }], }, 'us-east-1': { 'fallback': None, 'values': [{ - 'weight': 1, 'value': '1.5.1.1' + 'weight': 1, 'value': '1.5.1.1', 'status': 'obey', }, { - 'weight': 1, 'value': '1.5.1.2' + 'weight': 1, 'value': '1.5.1.2', 'status': 'obey', }], } }, {k: v.data for k, v in record.dynamic.pools.items()}) diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 0edfbcb..60d71ff 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -3423,20 +3423,25 @@ class TestDynamicRecords(TestCase): self.assertEquals({ 'value': '3.3.3.3', 'weight': 1, + 'status': 'obey', }, pools['one'].data['values'][0]) self.assertEquals([{ 'value': '4.4.4.4', 'weight': 1, + 'status': 'obey', }, { 'value': '5.5.5.5', 'weight': 1, + 'status': 'obey', }], pools['two'].data['values']) self.assertEquals([{ 'weight': 10, 'value': '4.4.4.4', + 'status': 'obey', }, { 'weight': 12, 'value': '5.5.5.5', + 'status': 'obey', }], pools['three'].data['values']) rules = dynamic.rules @@ -3526,20 +3531,25 @@ class TestDynamicRecords(TestCase): self.assertEquals({ 'value': '2601:642:500:e210:62f8:1dff:feb8:9473', 'weight': 1, + 'status': 'obey', }, pools['one'].data['values'][0]) self.assertEquals([{ 'value': '2601:642:500:e210:62f8:1dff:feb8:9474', 'weight': 1, + 'status': 'obey', }, { 'value': '2601:642:500:e210:62f8:1dff:feb8:9475', 'weight': 1, + 'status': 'obey', }], pools['two'].data['values']) self.assertEquals([{ 'weight': 10, 'value': '2601:642:500:e210:62f8:1dff:feb8:9476', + 'status': 'obey', }, { 'weight': 12, 'value': '2601:642:500:e210:62f8:1dff:feb8:9477', + 'status': 'obey', }], pools['three'].data['values']) rules = dynamic.rules @@ -3596,17 +3606,21 @@ class TestDynamicRecords(TestCase): self.assertEquals({ 'value': 'one.cname.target.', 'weight': 1, + 'status': 'obey', }, pools['one'].data['values'][0]) self.assertEquals({ 'value': 'two.cname.target.', 'weight': 1, + 'status': 'obey', }, pools['two'].data['values'][0]) self.assertEquals([{ 'value': 'three-1.cname.target.', 'weight': 12, + 'status': 'obey', }, { 'value': 'three-2.cname.target.', 'weight': 32, + 'status': 'obey', }], pools['three'].data['values']) rules = dynamic.rules @@ -4587,6 +4601,7 @@ class TestDynamicRecords(TestCase): 'values': [{ 'value': '3.3.3.3', 'weight': 1, + 'status': 'obey', }] }, 'two': { @@ -4594,9 +4609,11 @@ class TestDynamicRecords(TestCase): 'values': [{ 'value': '4.4.4.4', 'weight': 1, + 'status': 'obey', }, { 'value': '5.5.5.5', 'weight': 2, + 'status': 'obey', }] }, }, @@ -4642,6 +4659,7 @@ class TestDynamicRecords(TestCase): 'values': [{ 'value': '3.3.3.3', 'weight': 1, + 'status': 'obey', }] }, 'two': { @@ -4649,9 +4667,11 @@ class TestDynamicRecords(TestCase): 'values': [{ 'value': '4.4.4.4', 'weight': 1, + 'status': 'obey', }, { 'value': '5.5.5.5', 'weight': 2, + 'status': 'obey', }] }, }, From e3f76e562eb03aad71d7d0f9bad5e797b830508c Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Mon, 27 Sep 2021 19:42:51 -0700 Subject: [PATCH 13/20] Add tests for full coverage --- octodns/provider/azuredns.py | 6 +- tests/test_octodns_provider_azuredns.py | 74 +++++++++++++++++++++++++ tests/test_octodns_provider_base.py | 13 +++++ tests/test_octodns_provider_ns1.py | 34 +++++++++++- tests/test_octodns_record.py | 23 ++++++++ 5 files changed, 147 insertions(+), 3 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index ef9affd..f590776 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -1114,7 +1114,8 @@ class AzureProvider(BaseProvider): else: # Skip Weighted profile hop for single-value pool; append its # value as an external endpoint to fallback rule profile - target = pool_values[0]['value'] + value = pool_values[0] + target = value['value'] if record._type == 'CNAME': target = target[:-1] ep_name = pool_name @@ -1122,10 +1123,13 @@ class AzureProvider(BaseProvider): # mark default ep_name += '--default--' default_seen = True + ep_status = 'Disabled' if value['status'] == 'down' else \ + 'Enabled' return Endpoint( name=ep_name, target=target, priority=priority, + endpoint_status=ep_status, ), default_seen def _make_rule_profile(self, rule_endpoints, rule_name, record, geos, diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 72369a2..b531ad2 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -1717,6 +1717,80 @@ class TestAzureDnsProvider(TestCase): changes = provider._extra_changes(zone, desired, []) self.assertEqual(len(changes), 0) + def test_dynamic_pool_status(self): + # test that traffic managers are generated as expected for pool value + # statuses + provider = self._get_provider() + zone1 = Zone('unit.tests.', []) + record1 = Record.new(zone1, 'foo', data={ + 'type': 'CNAME', + 'ttl': 60, + 'value': 'default.unit.tests.', + 'dynamic': { + 'pools': { + 'one': { + 'values': [ + {'value': 'one1.unit.tests.', 'status': 'up'}, + ], + }, + 'two': { + 'values': [ + {'value': 'two1.unit.tests.', 'status': 'down'}, + {'value': 'two2.unit.tests.'}, + ], + }, + }, + 'rules': [ + {'geos': ['AS'], 'pool': 'one'}, + {'pool': 'two'}, + ], + } + }) + zone1.add_record(record1) + zone2 = provider._process_desired_zone(zone1.copy()) + record2 = list(zone2.records)[0] + self.assertTrue( + record2.dynamic.pools['one'].data['values'][0]['status'], + 'obey' + ) + + record1.dynamic.pools['one'].data['values'][0]['status'] = 'down' + profiles = provider._generate_traffic_managers(record1) + self.assertEqual(len(profiles), 4) + self.assertEqual(profiles[0].endpoints[0].endpoint_status, 'Disabled') + self.assertEqual(profiles[1].endpoints[0].endpoint_status, 'Disabled') + + # # test that same record gets populated back from traffic managers + tm_list = provider._tm_client.profiles.list_by_resource_group + tm_list.return_value = profiles + azrecord = RecordSet( + ttl=60, + target_resource=SubResource(id=profiles[-1].id), + ) + azrecord.name = record1.name or '@' + azrecord.type = f'Microsoft.Network/dnszones/{record1._type}' + record2 = provider._populate_record(zone, azrecord) + self.assertEqual(record1.dynamic._data(), record2.dynamic._data()) + + # _process_desired_zone shouldn't change anything when not needed + zone1 = Zone(zone.name, sub_zones=[]) + zone1.add_record(record1) + zone2 = provider._process_desired_zone(zone1.copy()) + record2 = list(zone2.records)[0] + self.assertTrue(record1.data, record2.data) + + # simple records should not get changed by _process_desired_zone + zone1 = Zone(zone.name, sub_zones=[]) + record1 = Record.new(zone1, 'foo', data={ + 'type': 'CNAME', + 'ttl': 86400, + 'value': 'one.unit.tests.', + }) + zone1.add_record(record1) + zone2 = provider._process_desired_zone(zone1.copy()) + record2 = list(zone2.records)[0] + self.assertTrue(record1.data, record2.data) + def test_dynamic_A(self): provider = self._get_provider() external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints' diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index 7fd60c5..a5b4e10 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -293,6 +293,19 @@ class TestBaseProvider(TestCase): record2 = list(zone2.records)[0] self.assertTrue(record2.dynamic) + # SUPPORTS_POOL_VALUE_STATUS + provider.SUPPORTS_POOL_VALUE_STATUS = False + zone1 = Zone('unit.tests.', []) + record1.dynamic.pools['one'].data['values'][0]['status'] = 'up' + zone1.add_record(record1) + + zone2 = provider._process_desired_zone(zone1.copy()) + record2 = list(zone2.records)[0] + self.assertEqual( + record2.dynamic.pools['one'].data['values'][0]['status'], + 'obey' + ) + def test_safe_none(self): # No changes is safe Plan(None, None, [], True).raise_if_unsafe() diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 6cd6a36..ffa9181 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -1209,6 +1209,34 @@ class TestNs1ProviderDynamic(TestCase): monitors_delete_mock.assert_has_calls([call('mon-id2')]) notifylists_delete_mock.assert_not_called() + @patch('octodns.provider.ns1.Ns1Provider._monitors_for') + def test_params_for_dynamic_with_pool_status(self, monitors_for_mock): + provider = Ns1Provider('test', 'api-key') + monitors_for_mock.reset_mock() + monitors_for_mock.side_effect = [{}] + record = Record.new(self.zone, '', { + 'dynamic': { + 'pools': { + 'iad': { + 'values': [{ + 'value': '1.2.3.4', + 'status': 'up', + }], + }, + }, + 'rules': [{ + 'pool': 'iad', + }], + }, + 'ttl': 32, + 'type': 'A', + 'value': '1.2.3.4', + 'meta': {}, + }) + params, active_monitors = provider._params_for_dynamic(record) + self.assertTrue(params['answers'][0]['meta']['up']) + self.assertEqual(len(active_monitors), 0) + @patch('octodns.provider.ns1.Ns1Provider._monitor_sync') @patch('octodns.provider.ns1.Ns1Provider._monitors_for') def test_params_for_dynamic_region_only(self, monitors_for_mock, @@ -1802,7 +1830,7 @@ class TestNs1ProviderDynamic(TestCase): 'meta': { 'priority': 1, 'note': 'from:one__country pool:one fallback:two', - 'up': {}, + 'up': True, }, 'region': 'one_country', }, { @@ -1880,7 +1908,9 @@ class TestNs1ProviderDynamic(TestCase): }, 'one': { 'fallback': 'two', - 'values': [{'value': '1.1.1.1', 'weight': 1}] + 'values': [ + {'value': '1.1.1.1', 'weight': 1, 'status': 'up'}, + ], }, 'three': { 'fallback': None, diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 60d71ff..68d3d61 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -4543,6 +4543,29 @@ class TestDynamicRecords(TestCase): # This should be valid, no exception Record.new(self.zone, 'bad', a_data) + # invalid status + a_data = { + 'dynamic': { + 'pools': { + 'one': { + 'values': [{ + 'value': '2.2.2.2', + 'status': 'none', + }], + }, + }, + 'rules': [{ + 'pool': 'one', + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': ['1.1.1.1'], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertIn('invalid status', ctx.exception.reasons[0]) + def test_dynamic_lenient(self): # Missing pools a_data = { From 0d543d9b78085b91c6bde38b82f4330ac27a3184 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Mon, 27 Sep 2021 22:24:13 -0700 Subject: [PATCH 14/20] compare endpoint_status in Azure profiles --- octodns/provider/azuredns.py | 5 ++++- tests/test_octodns_provider_azuredns.py | 10 ++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index f590776..aff7a25 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -375,8 +375,11 @@ def _profile_is_match(have, desired): desired_endpoints = desired.endpoints endpoints = zip(have_endpoints, desired_endpoints) for have_endpoint, desired_endpoint in endpoints: + have_status = have_endpoint.endpoint_status or 'Enabled' + desired_status = desired_endpoint.endpoint_status or 'Enabled' if have_endpoint.name != desired_endpoint.name or \ - have_endpoint.type != desired_endpoint.type: + have_endpoint.type != desired_endpoint.type or \ + have_status != desired_status: return false(have_endpoint, desired_endpoint, have.name) target_type = have_endpoint.type.split('/')[-1] if target_type == 'externalEndpoints': diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index b531ad2..708b229 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -473,6 +473,7 @@ class Test_ProfileIsMatch(TestCase): endpoints = 1, endpoint_name = 'name', endpoint_type = 'profile/nestedEndpoints', + endpoint_status = None, target = 'target.unit.tests', target_id = 'resource/id', geos = ['GEO-AF'], @@ -490,6 +491,7 @@ class Test_ProfileIsMatch(TestCase): endpoints=[Endpoint( name=endpoint_name, type=endpoint_type, + endpoint_status=endpoint_status, target=target, target_resource_id=target_id, geo_mapping=geos, @@ -506,6 +508,9 @@ class Test_ProfileIsMatch(TestCase): self.assertFalse(is_match(profile(), profile(monitor_proto='HTTP'))) self.assertFalse(is_match(profile(), profile(endpoint_name='a'))) self.assertFalse(is_match(profile(), profile(endpoint_type='b'))) + self.assertFalse( + is_match(profile(), profile(endpoint_status='Disabled')) + ) self.assertFalse( is_match(profile(endpoint_type='b'), profile(endpoint_type='b')) ) @@ -1760,7 +1765,7 @@ class TestAzureDnsProvider(TestCase): self.assertEqual(profiles[0].endpoints[0].endpoint_status, 'Disabled') self.assertEqual(profiles[1].endpoints[0].endpoint_status, 'Disabled') - # # test that same record gets populated back from traffic managers + # test that same record gets populated back from traffic managers tm_list = provider._tm_client.profiles.list_by_resource_group tm_list.return_value = profiles azrecord = RecordSet( @@ -1772,7 +1777,8 @@ class TestAzureDnsProvider(TestCase): record2 = provider._populate_record(zone, azrecord) self.assertEqual(record1.dynamic._data(), record2.dynamic._data()) - # _process_desired_zone shouldn't change anything when not needed + # _process_desired_zone shouldn't change anything when status value is + # supported zone1 = Zone(zone.name, sub_zones=[]) zone1.add_record(record1) zone2 = provider._process_desired_zone(zone1.copy()) From 9e863be58de191abc541152abbc6aeb282dc1b4f Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Mon, 27 Sep 2021 22:38:44 -0700 Subject: [PATCH 15/20] Add NS1 tests for down status --- tests/test_octodns_provider_ns1.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index ffa9181..2364d92 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -1213,7 +1213,7 @@ class TestNs1ProviderDynamic(TestCase): def test_params_for_dynamic_with_pool_status(self, monitors_for_mock): provider = Ns1Provider('test', 'api-key') monitors_for_mock.reset_mock() - monitors_for_mock.side_effect = [{}] + monitors_for_mock.return_value = {} record = Record.new(self.zone, '', { 'dynamic': { 'pools': { @@ -1234,7 +1234,13 @@ class TestNs1ProviderDynamic(TestCase): 'meta': {}, }) params, active_monitors = provider._params_for_dynamic(record) - self.assertTrue(params['answers'][0]['meta']['up']) + self.assertEqual(params['answers'][0]['meta']['up'], True) + self.assertEqual(len(active_monitors), 0) + + # check for down also + record.dynamic.pools['iad'].data['values'][0]['status'] = 'down' + params, active_monitors = provider._params_for_dynamic(record) + self.assertEqual(params['answers'][0]['meta']['up'], False) self.assertEqual(len(active_monitors), 0) @patch('octodns.provider.ns1.Ns1Provider._monitor_sync') @@ -1846,7 +1852,7 @@ class TestNs1ProviderDynamic(TestCase): 'meta': { 'priority': 3, 'note': 'from:one__country pool:three fallback:', - 'up': {}, + 'up': False, }, 'region': 'one_country', }, { @@ -1914,7 +1920,9 @@ class TestNs1ProviderDynamic(TestCase): }, 'three': { 'fallback': None, - 'values': [{'value': '3.3.3.3', 'weight': 1}] + 'values': [ + {'value': '3.3.3.3', 'weight': 1, 'status': 'down'} + ] }, 'two': { 'fallback': 'three', From 090fc7184836e075fd9181d9640854e4487b3eda Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Tue, 28 Sep 2021 16:06:57 -0700 Subject: [PATCH 16/20] Document the new status flag --- docs/dynamic_records.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/dynamic_records.md b/docs/dynamic_records.md index a992eda..d7ab509 100644 --- a/docs/dynamic_records.md +++ b/docs/dynamic_records.md @@ -105,6 +105,30 @@ test: | port | port to check | 443 | | protocol | HTTP/HTTPS/TCP | HTTPS | +Healthchecks can also be skipped for individual pool values. These values can be forced to always-serve or never-serve using the `status` flag. + +`status` flag is optional and accepts one of three possible values, `up`/`down`/`obey`, with `obey` being the default: + +```yaml +test: + ... + dynamic: + pools: + na: + values: + - value: 1.2.3.4 + status: down + - value: 2.3.4.5 + status: up + - value: 3.4.5.6 + # defaults to status: obey + ... +``` + +* NS1 supports all 3 flags values +* Azure DNS supports only `obey` and `down` +* All other dynamic-capable providers only support the default `obey` + #### Route53 Healtch Check Options | Key | Description | Default | From 9f7b686c53a1db4c5a35598f337ea3940fa42e1c Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Tue, 28 Sep 2021 16:09:26 -0700 Subject: [PATCH 17/20] Typo in dynamic_records.md --- docs/dynamic_records.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/dynamic_records.md b/docs/dynamic_records.md index d7ab509..1d58ae7 100644 --- a/docs/dynamic_records.md +++ b/docs/dynamic_records.md @@ -125,7 +125,8 @@ test: ... ``` -* NS1 supports all 3 flags values +Support matrix: +* NS1 supports all 3 flag values * Azure DNS supports only `obey` and `down` * All other dynamic-capable providers only support the default `obey` From 6959a9a6e197defde9b107d047338500999e148d Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 28 Sep 2021 17:41:24 -0700 Subject: [PATCH 18/20] Remove python version gates on requirements, all py3 now --- requirements-dev.txt | 2 +- requirements.txt | 2 -- setup.py | 2 -- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 27b2967..83c5c5f 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -7,7 +7,7 @@ pycodestyle==2.6.0 pyflakes==2.2.0 readme_renderer[md]==26.0 requests_mock -twine==3.2.0; python_version >= '3.2' +twine==3.4.2 # Profiling tests... # nose-cprof diff --git a/requirements.txt b/requirements.txt index cd3e0f9..84b7eae 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,10 +10,8 @@ docutils==0.16 dyn==1.8.1 edgegrid-python==1.1.1 fqdn==1.5.0 -futures==3.2.0; python_version < '3.2' google-cloud-core==1.4.1 google-cloud-dns==0.32.0 -ipaddress==1.0.23; python_version < '3.3' jmespath==0.10.0 msrestazure==0.6.4 natsort==6.2.1 diff --git a/setup.py b/setup.py index 0b15571..41870f8 100644 --- a/setup.py +++ b/setup.py @@ -68,9 +68,7 @@ setup( install_requires=[ 'PyYaml>=4.2b1', 'dnspython>=1.15.0', - 'futures>=3.2.0; python_version<"3.2"', 'fqdn>=1.5.0', - 'ipaddress>=1.0.22; python_version<"3.3"', 'natsort>=5.5.0', 'pycountry>=19.8.18', 'pycountry-convert>=0.7.2', From 47f94cc50e307f965610d62b50509920ca603353 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 28 Sep 2021 17:42:09 -0700 Subject: [PATCH 19/20] use build module with both sdist and wheel packages --- requirements-dev.txt | 1 + script/release | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 83c5c5f..59041bb 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,3 +1,4 @@ +build==0.7.0 coverage mock nose diff --git a/script/release b/script/release index f2c90bf..975aac2 100755 --- a/script/release +++ b/script/release @@ -21,7 +21,7 @@ VERSION="$(grep __VERSION__ "$ROOT/octodns/__init__.py" | sed -e "s/.* = '//" -e git tag -s "v$VERSION" -m "Release $VERSION" git push origin "v$VERSION" echo "Tagged and pushed v$VERSION" -python setup.py sdist -twine check dist/*$VERSION.tar.gz -twine upload dist/*$VERSION.tar.gz +python -m build --sdist --wheel +twine check dist/*$VERSION.tar.gz dist/*$VERSION*.whl +twine upload dist/*$VERSION.tar.gz dist/*$VERSION*.whl echo "Uploaded $VERSION" From 5774e7490b268662619ab3f2b27b7cb5d370204c Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 28 Sep 2021 17:43:03 -0700 Subject: [PATCH 20/20] Add changelog helper script --- script/changelog | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100755 script/changelog diff --git a/script/changelog b/script/changelog new file mode 100755 index 0000000..4257b67 --- /dev/null +++ b/script/changelog @@ -0,0 +1,7 @@ +#!/bin/bash + +set -e + +VERSION=v$(grep __VERSION__ octodns/__init__.py | sed -e "s/^[^']*'//" -e "s/'$//") +echo $VERSION +git log --pretty="%h - %cr - %s (%an)" "${VERSION}..HEAD"