mirror of
https://github.com/github/octodns.git
synced 2024-05-11 05:55:00 +00:00
Address review comments, add explicit filter chains
This commit is contained in:
@@ -13,7 +13,6 @@ from ns1.rest.errors import RateLimitException, ResourceException
|
||||
from pycountry_convert import country_alpha2_to_continent_code
|
||||
from time import sleep
|
||||
from uuid import uuid4
|
||||
from copy import deepcopy
|
||||
|
||||
from six import text_type
|
||||
|
||||
@@ -239,27 +238,11 @@ class Ns1Provider(BaseProvider):
|
||||
|
||||
ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found'
|
||||
|
||||
# See comment before _valid_filter_config() for details on filter indices
|
||||
_BASIC_DYNAMIC_FILTERS = [{
|
||||
_UP_FILTER = {
|
||||
'config': {},
|
||||
'filter': 'up'
|
||||
}, {
|
||||
'config': {},
|
||||
'filter': u'select_first_region'
|
||||
}, {
|
||||
'config': {
|
||||
'eliminate': u'1'
|
||||
},
|
||||
'filter': 'priority'
|
||||
}, {
|
||||
'config': {},
|
||||
'filter': u'weighted_shuffle'
|
||||
}, {
|
||||
'config': {
|
||||
'N': u'1'
|
||||
},
|
||||
'filter': u'select_first_n'
|
||||
}]
|
||||
}
|
||||
|
||||
_REGION_FILTER = {
|
||||
'config': {},
|
||||
'filter': u'geofence_regional'
|
||||
@@ -268,9 +251,66 @@ class Ns1Provider(BaseProvider):
|
||||
'config': {},
|
||||
'filter': u'geofence_country'
|
||||
}
|
||||
_REGION_FILTER_INDEX = 1
|
||||
_COUNTRY_FILTER_INDEX = 1
|
||||
_COUNTRY_FILTER_INDEX_WITH_REGION = 2
|
||||
|
||||
_SELECT_FIRST_REGION_FILTER = {
|
||||
'config': {},
|
||||
'filter': u'select_first_region'
|
||||
}
|
||||
|
||||
_PRIORITY_FILTER = {
|
||||
'config': {
|
||||
'eliminate': u'1'
|
||||
},
|
||||
'filter': 'priority'
|
||||
}
|
||||
|
||||
_WEIGHTED_SHUFFLE_FILTER = {
|
||||
'config': {},
|
||||
'filter': u'weighted_shuffle'
|
||||
}
|
||||
|
||||
_SELECT_FIRST_N_FILTER = {
|
||||
'config': {
|
||||
'N': u'1'
|
||||
},
|
||||
'filter': u'select_first_n'
|
||||
}
|
||||
|
||||
_BASIC_FILTER_CHAIN = [
|
||||
_UP_FILTER,
|
||||
_SELECT_FIRST_REGION_FILTER,
|
||||
_PRIORITY_FILTER,
|
||||
_WEIGHTED_SHUFFLE_FILTER,
|
||||
_SELECT_FIRST_N_FILTER
|
||||
]
|
||||
|
||||
_FILTER_CHAIN_WITH_REGION = [
|
||||
_UP_FILTER,
|
||||
_REGION_FILTER,
|
||||
_SELECT_FIRST_REGION_FILTER,
|
||||
_PRIORITY_FILTER,
|
||||
_WEIGHTED_SHUFFLE_FILTER,
|
||||
_SELECT_FIRST_N_FILTER
|
||||
]
|
||||
|
||||
_FILTER_CHAIN_WITH_COUNTRY = [
|
||||
_UP_FILTER,
|
||||
_COUNTRY_FILTER,
|
||||
_SELECT_FIRST_REGION_FILTER,
|
||||
_PRIORITY_FILTER,
|
||||
_WEIGHTED_SHUFFLE_FILTER,
|
||||
_SELECT_FIRST_N_FILTER
|
||||
]
|
||||
|
||||
_FILTER_CHAIN_WITH_REGION_AND_COUNTRY = [
|
||||
_UP_FILTER,
|
||||
_REGION_FILTER,
|
||||
_COUNTRY_FILTER,
|
||||
_SELECT_FIRST_REGION_FILTER,
|
||||
_PRIORITY_FILTER,
|
||||
_WEIGHTED_SHUFFLE_FILTER,
|
||||
_SELECT_FIRST_N_FILTER
|
||||
]
|
||||
|
||||
_REGION_TO_CONTINENT = {
|
||||
'AFRICA': 'AF',
|
||||
@@ -309,33 +349,22 @@ class Ns1Provider(BaseProvider):
|
||||
self._client = Ns1Client(api_key, parallelism, retry_count,
|
||||
client_config)
|
||||
|
||||
# Allowed filter configurations:
|
||||
# 1. Filter chain is the basic filter chain
|
||||
# 2. In addition, it could have the 'geofence_country' filter and or
|
||||
# 'geofence_regional' filter only at index 1 in the filter chain
|
||||
# 3. If both country and regional filters are present, they are required
|
||||
# to be as below:
|
||||
# - 'geofence_regional' at index 1
|
||||
# - 'geofence_country' at index 2
|
||||
# Any other deviation is returned as an unsupported filter configuration
|
||||
def _valid_filter_config(self, filter_cfg):
|
||||
has_region = 'geofence_regional' in [f['filter'] for f in filter_cfg]
|
||||
has_country = 'geofence_country' in [f['filter'] for f in filter_cfg]
|
||||
has_region = self._REGION_FILTER in filter_cfg
|
||||
has_country = self._COUNTRY_FILTER in filter_cfg
|
||||
return filter_cfg == self._get_updated_filter_chain(has_region,
|
||||
has_country)
|
||||
|
||||
def _get_updated_filter_chain(self, has_region, has_country):
|
||||
filters = deepcopy(self._BASIC_DYNAMIC_FILTERS)
|
||||
if has_region:
|
||||
filters.insert(self._REGION_FILTER_INDEX, self._REGION_FILTER)
|
||||
if has_country:
|
||||
if has_region:
|
||||
filters.insert(self._COUNTRY_FILTER_INDEX_WITH_REGION,
|
||||
self._COUNTRY_FILTER)
|
||||
else:
|
||||
filters.insert(self._COUNTRY_FILTER_INDEX,
|
||||
self._COUNTRY_FILTER)
|
||||
return filters
|
||||
if has_region and has_country:
|
||||
filter_chain = self._FILTER_CHAIN_WITH_REGION_AND_COUNTRY
|
||||
elif has_region:
|
||||
filter_chain = self._FILTER_CHAIN_WITH_REGION
|
||||
elif has_country:
|
||||
filter_chain = self._FILTER_CHAIN_WITH_COUNTRY
|
||||
else:
|
||||
filter_chain = self._BASIC_FILTER_CHAIN
|
||||
return filter_chain
|
||||
|
||||
def _encode_notes(self, data):
|
||||
return ' '.join(['{}:{}'.format(k, v)
|
||||
|
||||
@@ -926,6 +926,41 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
monitors_delete_mock.assert_has_calls([call('mon-id2')])
|
||||
notifylists_delete_mock.assert_has_calls([call('nl-id2')])
|
||||
|
||||
@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,
|
||||
monitor_sync_mock):
|
||||
provider = Ns1Provider('test', 'api-key')
|
||||
|
||||
# pre-fill caches to avoid extranious calls (things we're testing
|
||||
# elsewhere)
|
||||
provider._client._datasource_id = 'foo'
|
||||
provider._client._feeds_for_monitors = {
|
||||
'mon-id': 'feed-id',
|
||||
}
|
||||
|
||||
# provider._params_for_A() calls provider._monitors_for() and
|
||||
# provider._monitor_sync(). Mock their return values so that we don't
|
||||
# make NS1 API calls during tests
|
||||
monitors_for_mock.reset_mock()
|
||||
monitor_sync_mock.reset_mock()
|
||||
monitors_for_mock.side_effect = [{
|
||||
'3.4.5.6': 'mid-3',
|
||||
}]
|
||||
monitor_sync_mock.side_effect = [
|
||||
('mid-1', 'fid-1'),
|
||||
('mid-2', 'fid-2'),
|
||||
('mid-3', 'fid-3'),
|
||||
]
|
||||
|
||||
rule0 = self.record.data['dynamic']['rules'][0]
|
||||
saved_geos = rule0['geos']
|
||||
rule0['geos'] = ['AF', 'EU']
|
||||
ret, _ = provider._params_for_A(self.record)
|
||||
self.assertEquals(ret['filters'],
|
||||
Ns1Provider._FILTER_CHAIN_WITH_REGION)
|
||||
rule0['geos'] = saved_geos
|
||||
|
||||
@patch('octodns.provider.ns1.Ns1Provider._monitor_sync')
|
||||
@patch('octodns.provider.ns1.Ns1Provider._monitors_for')
|
||||
def test_params_for_dynamic_oceania(self, monitors_for_mock,
|
||||
@@ -962,9 +997,9 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
self.assertEquals(set(ret['regions']['lhr']['meta']['country']),
|
||||
Ns1Provider._CONTINENT_TO_LIST_OF_COUNTRIES['OC'])
|
||||
# When rules has 'OC', it is converted to list of countries in the
|
||||
# params. Look for filter 'geofence_country' in the returned filters at
|
||||
# index 2
|
||||
self.assertEquals(ret['filters'][1]['filter'], 'geofence_country')
|
||||
# params. Look if the returned filters is the filter chain with country
|
||||
self.assertEquals(ret['filters'],
|
||||
Ns1Provider._FILTER_CHAIN_WITH_COUNTRY)
|
||||
rule0['geos'] = saved_geos
|
||||
|
||||
@patch('octodns.provider.ns1.Ns1Provider._monitor_sync')
|
||||
@@ -993,9 +1028,10 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
# handling to get there
|
||||
ret, _ = provider._params_for_A(self.record)
|
||||
|
||||
# Given that self.record has a country in the rules, we should find
|
||||
# the filter 'geofence_country' in the returned filters at index 2
|
||||
self.assertEquals(ret['filters'][2]['filter'], 'geofence_country')
|
||||
# Given that self.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)
|
||||
|
||||
monitors_for_mock.assert_has_calls([call(self.record)])
|
||||
monitors_sync_mock.assert_has_calls([
|
||||
@@ -1021,7 +1057,7 @@ class TestNs1ProviderDynamic(TestCase):
|
||||
ns1_record = {
|
||||
'answers': [],
|
||||
'domain': 'unit.tests',
|
||||
'filters': Ns1Provider._BASIC_DYNAMIC_FILTERS,
|
||||
'filters': Ns1Provider._BASIC_FILTER_CHAIN,
|
||||
'regions': {},
|
||||
'ttl': 42,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user