mirror of
https://github.com/github/octodns.git
synced 2024-05-11 05:55:00 +00:00
Update record when NS1 filter chain is not as expected
This commit is contained in:
@@ -312,102 +312,111 @@ class Ns1Provider(BaseProvider):
|
||||
ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found'
|
||||
SHARED_NOTIFYLIST_NAME = 'octoDNS NS1 Notify List'
|
||||
|
||||
def _update_filter(self, filter, with_disabled):
|
||||
if with_disabled:
|
||||
filter['disabled'] = False
|
||||
return (dict(sorted(filter.items(), key=lambda t: t[0])))
|
||||
return filter
|
||||
def _update_filter(self, filter):
|
||||
filter.setdefault('disabled', False)
|
||||
return (dict(sorted(filter.items(), key=lambda t: t[0])))
|
||||
|
||||
def _UP_FILTER(self, with_disabled):
|
||||
@property
|
||||
def _UP_FILTER(self):
|
||||
return self._update_filter({
|
||||
'config': {},
|
||||
'filter': 'up'
|
||||
}, with_disabled)
|
||||
})
|
||||
|
||||
def _REGION_FILTER(self, with_disabled):
|
||||
@property
|
||||
def _REGION_FILTER(self):
|
||||
return self._update_filter({
|
||||
'config': {
|
||||
'remove_no_georegion': True
|
||||
},
|
||||
'filter': u'geofence_regional'
|
||||
}, with_disabled)
|
||||
})
|
||||
|
||||
def _COUNTRY_FILTER(self, with_disabled):
|
||||
@property
|
||||
def _COUNTRY_FILTER(self):
|
||||
return self._update_filter({
|
||||
'config': {
|
||||
'remove_no_location': True
|
||||
},
|
||||
'filter': u'geofence_country'
|
||||
}, with_disabled)
|
||||
})
|
||||
|
||||
# In the NS1 UI/portal, this filter is called "SELECT FIRST GROUP" though
|
||||
# the filter name in the NS1 api is 'select_first_region'
|
||||
def _SELECT_FIRST_REGION_FILTER(self, with_disabled):
|
||||
@property
|
||||
def _SELECT_FIRST_REGION_FILTER(self):
|
||||
return self._update_filter({
|
||||
'config': {},
|
||||
'filter': u'select_first_region'
|
||||
}, with_disabled)
|
||||
})
|
||||
|
||||
def _PRIORITY_FILTER(self, with_disabled):
|
||||
@property
|
||||
def _PRIORITY_FILTER(self):
|
||||
return self._update_filter({
|
||||
'config': {
|
||||
'eliminate': u'1'
|
||||
},
|
||||
'filter': 'priority'
|
||||
}, with_disabled)
|
||||
})
|
||||
|
||||
def _WEIGHTED_SHUFFLE_FILTER(self, with_disabled):
|
||||
@property
|
||||
def _WEIGHTED_SHUFFLE_FILTER(self):
|
||||
return self._update_filter({
|
||||
'config': {},
|
||||
'filter': u'weighted_shuffle'
|
||||
}, with_disabled)
|
||||
})
|
||||
|
||||
def _SELECT_FIRST_N_FILTER(self, with_disabled):
|
||||
@property
|
||||
def _SELECT_FIRST_N_FILTER(self):
|
||||
return self._update_filter({
|
||||
'config': {
|
||||
'N': u'1'
|
||||
},
|
||||
'filter': u'select_first_n'
|
||||
}, with_disabled)
|
||||
})
|
||||
|
||||
def _BASIC_FILTER_CHAIN(self, with_disabled):
|
||||
@property
|
||||
def _BASIC_FILTER_CHAIN(self):
|
||||
return [
|
||||
self._UP_FILTER(with_disabled),
|
||||
self._SELECT_FIRST_REGION_FILTER(with_disabled),
|
||||
self._PRIORITY_FILTER(with_disabled),
|
||||
self._WEIGHTED_SHUFFLE_FILTER(with_disabled),
|
||||
self._SELECT_FIRST_N_FILTER(with_disabled)
|
||||
self._UP_FILTER,
|
||||
self._SELECT_FIRST_REGION_FILTER,
|
||||
self._PRIORITY_FILTER,
|
||||
self._WEIGHTED_SHUFFLE_FILTER,
|
||||
self._SELECT_FIRST_N_FILTER
|
||||
]
|
||||
|
||||
def _FILTER_CHAIN_WITH_REGION(self, with_disabled):
|
||||
@property
|
||||
def _FILTER_CHAIN_WITH_REGION(self):
|
||||
return [
|
||||
self._UP_FILTER(with_disabled),
|
||||
self._REGION_FILTER(with_disabled),
|
||||
self._SELECT_FIRST_REGION_FILTER(with_disabled),
|
||||
self._PRIORITY_FILTER(with_disabled),
|
||||
self._WEIGHTED_SHUFFLE_FILTER(with_disabled),
|
||||
self._SELECT_FIRST_N_FILTER(with_disabled)
|
||||
self._UP_FILTER,
|
||||
self._REGION_FILTER,
|
||||
self._SELECT_FIRST_REGION_FILTER,
|
||||
self._PRIORITY_FILTER,
|
||||
self._WEIGHTED_SHUFFLE_FILTER,
|
||||
self._SELECT_FIRST_N_FILTER
|
||||
]
|
||||
|
||||
def _FILTER_CHAIN_WITH_COUNTRY(self, with_disabled):
|
||||
@property
|
||||
def _FILTER_CHAIN_WITH_COUNTRY(self):
|
||||
return [
|
||||
self._UP_FILTER(with_disabled),
|
||||
self._COUNTRY_FILTER(with_disabled),
|
||||
self._SELECT_FIRST_REGION_FILTER(with_disabled),
|
||||
self._PRIORITY_FILTER(with_disabled),
|
||||
self._WEIGHTED_SHUFFLE_FILTER(with_disabled),
|
||||
self._SELECT_FIRST_N_FILTER(with_disabled)
|
||||
self._UP_FILTER,
|
||||
self._COUNTRY_FILTER,
|
||||
self._SELECT_FIRST_REGION_FILTER,
|
||||
self._PRIORITY_FILTER,
|
||||
self._WEIGHTED_SHUFFLE_FILTER,
|
||||
self._SELECT_FIRST_N_FILTER
|
||||
]
|
||||
|
||||
def _FILTER_CHAIN_WITH_REGION_AND_COUNTRY(self, with_disabled):
|
||||
@property
|
||||
def _FILTER_CHAIN_WITH_REGION_AND_COUNTRY(self):
|
||||
return [
|
||||
self._UP_FILTER(with_disabled),
|
||||
self._REGION_FILTER(with_disabled),
|
||||
self._COUNTRY_FILTER(with_disabled),
|
||||
self._SELECT_FIRST_REGION_FILTER(with_disabled),
|
||||
self._PRIORITY_FILTER(with_disabled),
|
||||
self._WEIGHTED_SHUFFLE_FILTER(with_disabled),
|
||||
self._SELECT_FIRST_N_FILTER(with_disabled)
|
||||
self._UP_FILTER,
|
||||
self._REGION_FILTER,
|
||||
self._COUNTRY_FILTER,
|
||||
self._SELECT_FIRST_REGION_FILTER,
|
||||
self._PRIORITY_FILTER,
|
||||
self._WEIGHTED_SHUFFLE_FILTER,
|
||||
self._SELECT_FIRST_N_FILTER
|
||||
]
|
||||
|
||||
_REGION_TO_CONTINENT = {
|
||||
@@ -452,29 +461,27 @@ class Ns1Provider(BaseProvider):
|
||||
super(Ns1Provider, self).__init__(id, *args, **kwargs)
|
||||
self.monitor_regions = monitor_regions
|
||||
self.shared_notifylist = shared_notifylist
|
||||
self.record_filters = dict()
|
||||
self._client = Ns1Client(api_key, parallelism, retry_count,
|
||||
client_config)
|
||||
|
||||
def _valid_filter_config(self, filter_cfg, domain):
|
||||
with_disabled = self._disabled_flag_in_filters(filter_cfg, domain)
|
||||
has_region = self._REGION_FILTER(with_disabled) in filter_cfg
|
||||
has_country = self._COUNTRY_FILTER(with_disabled) in filter_cfg
|
||||
def _valid_filter_config(self, filter_cfg):
|
||||
self._disabled_flag_in_filters(filter_cfg)
|
||||
has_region = self._REGION_FILTER in filter_cfg
|
||||
has_country = self._COUNTRY_FILTER in filter_cfg
|
||||
expected_filter_cfg = self._get_updated_filter_chain(has_region,
|
||||
has_country,
|
||||
with_disabled)
|
||||
has_country)
|
||||
return filter_cfg == expected_filter_cfg
|
||||
|
||||
def _get_updated_filter_chain(self, has_region, has_country,
|
||||
with_disabled=True):
|
||||
def _get_updated_filter_chain(self, has_region, has_country):
|
||||
if has_region and has_country:
|
||||
filter_chain = self._FILTER_CHAIN_WITH_REGION_AND_COUNTRY(
|
||||
with_disabled)
|
||||
filter_chain = self._FILTER_CHAIN_WITH_REGION_AND_COUNTRY
|
||||
elif has_region:
|
||||
filter_chain = self._FILTER_CHAIN_WITH_REGION(with_disabled)
|
||||
filter_chain = self._FILTER_CHAIN_WITH_REGION
|
||||
elif has_country:
|
||||
filter_chain = self._FILTER_CHAIN_WITH_COUNTRY(with_disabled)
|
||||
filter_chain = self._FILTER_CHAIN_WITH_COUNTRY
|
||||
else:
|
||||
filter_chain = self._BASIC_FILTER_CHAIN(with_disabled)
|
||||
filter_chain = self._BASIC_FILTER_CHAIN
|
||||
|
||||
return filter_chain
|
||||
|
||||
@@ -546,11 +553,9 @@ class Ns1Provider(BaseProvider):
|
||||
return pool_name
|
||||
|
||||
def _data_for_dynamic(self, _type, record):
|
||||
# First make sure we have the expected filters config
|
||||
if not self._valid_filter_config(record['filters'], record['domain']):
|
||||
self.log.error('_data_for_dynamic: %s %s has unsupported '
|
||||
'filters', record['domain'], _type)
|
||||
raise Ns1Exception('Unrecognized advanced record')
|
||||
# Cache record filters for later use
|
||||
record_filters = self.record_filters.setdefault(record['domain'], {})
|
||||
record_filters[_type] = record['filters']
|
||||
|
||||
# All regions (pools) will include the list of default values
|
||||
# (eventually) at higher priorities, we'll just add them to this set to
|
||||
@@ -1394,41 +1399,12 @@ class Ns1Provider(BaseProvider):
|
||||
for v in record.values]
|
||||
return {'answers': values, 'ttl': record.ttl}, None
|
||||
|
||||
def _get_ns1_filters(self, ns1_zone_name):
|
||||
ns1_filters = {}
|
||||
ns1_zone = {}
|
||||
|
||||
try:
|
||||
ns1_zone = self._client.zones_retrieve(ns1_zone_name)
|
||||
except ResourceException as e:
|
||||
if e.message != self.ZONE_NOT_FOUND_MESSAGE:
|
||||
raise
|
||||
|
||||
if 'records' in ns1_zone:
|
||||
for ns1_record in ns1_zone['records']:
|
||||
if ns1_record.get('tier', 1) > 1:
|
||||
# Need to get the full record data for geo records
|
||||
full_rec = self._client.records_retrieve(
|
||||
ns1_zone_name,
|
||||
ns1_record['domain'],
|
||||
ns1_record['type'])
|
||||
if 'filters' in full_rec:
|
||||
filter_key = f'{ns1_record["domain"]}.'
|
||||
ns1_filters[filter_key] = full_rec['filters']
|
||||
|
||||
return ns1_filters
|
||||
|
||||
def _disabled_flag_in_filters(self, filters, domain):
|
||||
disabled_count = ['disabled' in f for f in filters].count(True)
|
||||
if disabled_count and disabled_count != len(filters):
|
||||
# Some filters have the disabled flag, and some don't. Disallow
|
||||
exception_msg = f'Mixed disabled flag in filters for {domain}'
|
||||
raise Ns1Exception(exception_msg)
|
||||
return disabled_count == len(filters)
|
||||
def _disabled_flag_in_filters(self, filters):
|
||||
# fill up filters with disabled=False flag whenever absent
|
||||
return [self._update_filter(f) for f in filters]
|
||||
|
||||
def _extra_changes(self, desired, changes, **kwargs):
|
||||
self.log.debug('_extra_changes: desired=%s', desired.name)
|
||||
ns1_filters = self._get_ns1_filters(desired.name[:-1])
|
||||
changed = set([c.record for c in changes])
|
||||
extra = []
|
||||
for record in desired.records:
|
||||
@@ -1440,16 +1416,16 @@ class Ns1Provider(BaseProvider):
|
||||
# Check if filters for existing domains need an update
|
||||
# Needs an explicit check since there might be no change in the
|
||||
# config at all. Filters however might still need an update
|
||||
domain = f'{record.name}.{record.zone.name}'
|
||||
if domain in ns1_filters:
|
||||
domain_filters = ns1_filters[domain]
|
||||
if not self._disabled_flag_in_filters(domain_filters, domain):
|
||||
# 'disabled' entry absent in filter config. Need to update
|
||||
# filters. Update record
|
||||
self.log.info('_extra_changes: change in filters for %s',
|
||||
domain)
|
||||
extra.append(Update(record, record))
|
||||
continue
|
||||
domain = record.fqdn[:-1]
|
||||
_type = record._type
|
||||
record_filters = self.record_filters.get(domain, {}).get(_type, [])
|
||||
if not self._valid_filter_config(record_filters):
|
||||
# unrecognized set of filters, overwrite them by updating the
|
||||
# record
|
||||
self.log.info('_extra_changes: unrecognized filters in %s, '
|
||||
'will update record', domain)
|
||||
extra.append(Update(record, record))
|
||||
continue
|
||||
|
||||
for value, have in self._monitors_for(record).items():
|
||||
expected = self._monitor_gen(record, value)
|
||||
|
||||
@@ -1278,8 +1278,7 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
ret, monitor_ids = provider._params_for_A(record)
|
||||
self.assertEquals(10, len(ret['answers']))
|
||||
self.assertEquals(ret['filters'],
|
||||
Ns1Provider._FILTER_CHAIN_WITH_REGION(provider,
|
||||
True))
|
||||
provider._FILTER_CHAIN_WITH_REGION)
|
||||
self.assertEquals({
|
||||
'iad__catchall': {
|
||||
'meta': {
|
||||
@@ -1335,8 +1334,7 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
rule1['geos'] = ['NA-US-CA', 'NA-CA-NL']
|
||||
ret, _ = provider._params_for_A(record)
|
||||
self.assertEquals(10, len(ret['answers']))
|
||||
exp = Ns1Provider._FILTER_CHAIN_WITH_REGION_AND_COUNTRY(provider,
|
||||
True)
|
||||
exp = provider._FILTER_CHAIN_WITH_REGION_AND_COUNTRY
|
||||
self.assertEquals(ret['filters'], exp)
|
||||
self.assertEquals({
|
||||
'iad__catchall': {
|
||||
@@ -1439,8 +1437,7 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
notes['from:lhr__country'])
|
||||
|
||||
# We have both country and region filter chain entries
|
||||
exp = Ns1Provider._FILTER_CHAIN_WITH_REGION_AND_COUNTRY(provider,
|
||||
True)
|
||||
exp = provider._FILTER_CHAIN_WITH_REGION_AND_COUNTRY
|
||||
self.assertEquals(ret['filters'], exp)
|
||||
|
||||
# and our region details match the expected behaviors/targeting
|
||||
@@ -1518,8 +1515,7 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
# When rules has 'OC', it is converted to list of countries in the
|
||||
# params. Look if the returned filters is the filter chain with country
|
||||
self.assertEquals(ret['filters'],
|
||||
Ns1Provider._FILTER_CHAIN_WITH_COUNTRY(provider,
|
||||
True))
|
||||
provider._FILTER_CHAIN_WITH_COUNTRY)
|
||||
|
||||
@patch('octodns.provider.ns1.Ns1Provider._monitor_sync')
|
||||
@patch('octodns.provider.ns1.Ns1Provider._monitors_for')
|
||||
@@ -1555,8 +1551,7 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
# Given that record has both country and region in the rules,
|
||||
# the returned filter chain should be one with region and country
|
||||
self.assertEquals(ret['filters'],
|
||||
Ns1Provider._FILTER_CHAIN_WITH_REGION_AND_COUNTRY(
|
||||
provider, True))
|
||||
provider._FILTER_CHAIN_WITH_REGION_AND_COUNTRY)
|
||||
|
||||
monitors_for_mock.assert_has_calls([call(record)])
|
||||
monitors_sync_mock.assert_has_calls([
|
||||
@@ -1610,20 +1605,11 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
def test_data_for_dynamic(self):
|
||||
provider = Ns1Provider('test', 'api-key')
|
||||
|
||||
# Unexpected filters throws an error
|
||||
ns1_record = {
|
||||
'domain': 'unit.tests',
|
||||
'filters': [],
|
||||
}
|
||||
with self.assertRaises(Ns1Exception) as ctx:
|
||||
provider._data_for_dynamic('A', ns1_record)
|
||||
self.assertEquals('Unrecognized advanced record', str(ctx.exception))
|
||||
|
||||
# empty record turns into empty data
|
||||
ns1_record = {
|
||||
'answers': [],
|
||||
'domain': 'unit.tests',
|
||||
'filters': Ns1Provider._BASIC_FILTER_CHAIN(provider, True),
|
||||
'filters': provider._BASIC_FILTER_CHAIN,
|
||||
'regions': {},
|
||||
'ttl': 42,
|
||||
}
|
||||
@@ -2038,13 +2024,6 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
extra = provider._extra_changes(desired, [])
|
||||
self.assertFalse(extra)
|
||||
|
||||
# Unexpected exception message
|
||||
reset()
|
||||
zones_retrieve_mock.side_effect = ResourceException('boom')
|
||||
with self.assertRaises(ResourceException) as ctx:
|
||||
extra = provider._extra_changes(desired, [])
|
||||
self.assertEquals(zones_retrieve_mock.side_effect, ctx.exception)
|
||||
|
||||
# Simple record, ignored, filter update lookups ignored
|
||||
reset()
|
||||
zones_retrieve_mock.side_effect = \
|
||||
@@ -2093,6 +2072,9 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
# untouched, but everything in sync so no change needed
|
||||
reset()
|
||||
# Generate what we expect to have
|
||||
provider.record_filters[dynamic.fqdn[:-1]] = {
|
||||
dynamic._type: provider._get_updated_filter_chain(False, False)
|
||||
}
|
||||
gend = provider._monitor_gen(dynamic, '1.2.3.4')
|
||||
gend.update({
|
||||
'id': 'mid', # need to add an id
|
||||
@@ -2153,7 +2135,7 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
"zone": "unit.tests",
|
||||
"type": "A",
|
||||
"tier": 3,
|
||||
"filters": Ns1Provider._BASIC_FILTER_CHAIN(provider, True)
|
||||
"filters": provider._BASIC_FILTER_CHAIN
|
||||
}],
|
||||
}
|
||||
monitors_for_mock.side_effect = [{}]
|
||||
@@ -2170,16 +2152,20 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
"zone": "unit.tests",
|
||||
"type": "A",
|
||||
"tier": 3,
|
||||
"filters": Ns1Provider._BASIC_FILTER_CHAIN(provider, False)
|
||||
"filters": provider._BASIC_FILTER_CHAIN[:-1]
|
||||
}],
|
||||
}
|
||||
monitors_for_mock.side_effect = [{}]
|
||||
zones_retrieve_mock.side_effect = [ns1_zone]
|
||||
records_retrieve_mock.side_effect = ns1_zone['records']
|
||||
ns1_record = ns1_zone['records'][0]
|
||||
provider.record_filters[ns1_record['domain']] = {
|
||||
ns1_record['type']: ns1_record['filters']
|
||||
}
|
||||
extra = provider._extra_changes(desired, [])
|
||||
self.assertTrue(extra)
|
||||
|
||||
# Mixed disabled in filters. Raise Ns1Exception
|
||||
# Mixed disabled in filters doesn't trigger an update
|
||||
reset()
|
||||
ns1_zone = {
|
||||
'records': [{
|
||||
@@ -2187,16 +2173,19 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
"zone": "unit.tests",
|
||||
"type": "A",
|
||||
"tier": 3,
|
||||
"filters": Ns1Provider._BASIC_FILTER_CHAIN(provider, True)
|
||||
"filters": provider._BASIC_FILTER_CHAIN
|
||||
}],
|
||||
}
|
||||
del ns1_zone['records'][0]['filters'][0]['disabled']
|
||||
monitors_for_mock.side_effect = [{}]
|
||||
zones_retrieve_mock.side_effect = [ns1_zone]
|
||||
records_retrieve_mock.side_effect = ns1_zone['records']
|
||||
with self.assertRaises(Ns1Exception) as ctx:
|
||||
extra = provider._extra_changes(desired, [])
|
||||
self.assertTrue('Mixed disabled flag in filters' in str(ctx.exception))
|
||||
ns1_record = ns1_zone['records'][0]
|
||||
provider.record_filters[ns1_record['domain']] = {
|
||||
ns1_record['type']: ns1_record['filters']
|
||||
}
|
||||
extra = provider._extra_changes(desired, [])
|
||||
self.assertFalse(extra)
|
||||
|
||||
DESIRED = Zone('unit.tests.', [])
|
||||
|
||||
|
||||
Reference in New Issue
Block a user