1
0
mirror of https://github.com/github/octodns.git synced 2024-05-11 05:55:00 +00:00

Simplify logic, remove unnecessary coverage tests

This commit is contained in:
Pavan Chandrashekar
2020-04-11 02:28:28 -07:00
parent 4c21cfd85b
commit d93ddddaf2
2 changed files with 25 additions and 71 deletions

View File

@@ -237,7 +237,7 @@ class Ns1Provider(BaseProvider):
'NS', 'PTR', 'SPF', 'SRV', 'TXT')) 'NS', 'PTR', 'SPF', 'SRV', 'TXT'))
ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found' ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found'
CATCHALL_PREFIX = 'catchall_' CATCHALL_PREFIX = 'catchall__'
def _update_filter(self, filter, with_disabled): def _update_filter(self, filter, with_disabled):
if with_disabled: if with_disabled:
@@ -472,8 +472,7 @@ class Ns1Provider(BaseProvider):
pool_name = answer['region'] pool_name = answer['region']
# Get the actual pool name from the constructed pool name in case # Get the actual pool name from the constructed pool name in case
# of the catchall # of the catchall
if pool_name.startswith(self.CATCHALL_PREFIX): pool_name = pool_name.replace(self.CATCHALL_PREFIX, '')
pool_name = pool_name[len(self.CATCHALL_PREFIX):]
pool = pools[pool_name] pool = pools[pool_name]
meta = answer['meta'] meta = answer['meta']
@@ -505,8 +504,7 @@ class Ns1Provider(BaseProvider):
# Rules that refer to the catchall pool would have the # Rules that refer to the catchall pool would have the
# CATCHALL_PREFIX in the pool name. Strip the prefix to get back # CATCHALL_PREFIX in the pool name. Strip the prefix to get back
# the pool name as in the config # the pool name as in the config
if pool_name.startswith(self.CATCHALL_PREFIX): pool_name = pool_name.replace(self.CATCHALL_PREFIX, '')
pool_name = pool_name[len(self.CATCHALL_PREFIX):]
meta = region['meta'] meta = region['meta']
notes = self._parse_notes(meta.get('note', '')) notes = self._parse_notes(meta.get('note', ''))
@@ -1000,12 +998,9 @@ class Ns1Provider(BaseProvider):
has_country = False has_country = False
has_region = False has_region = False
regions = {} regions = {}
catchall_pool_name = None
pool_usage = defaultdict(lambda: 0)
for i, rule in enumerate(record.dynamic.rules): for i, rule in enumerate(record.dynamic.rules):
pool_name = rule.data['pool'] pool_name = rule.data['pool']
pool_usage[pool_name] += 1
notes = { notes = {
'rule-order': i, 'rule-order': i,
@@ -1066,14 +1061,11 @@ class Ns1Provider(BaseProvider):
# reuse. # reuse.
# (We expect only one catchall per record. Any associated # (We expect only one catchall per record. Any associated
# validation is expected to covered under record validation) # validation is expected to covered under record validation)
catchall_pool_name = pool_name pool_name = '{}{}'.format(self.CATCHALL_PREFIX, pool_name)
regions['{}{}'.format(self.CATCHALL_PREFIX, pool_name)] = {
'meta': meta, regions[pool_name] = {
} 'meta': meta,
else: }
regions[pool_name] = {
'meta': meta,
}
existing_monitors = self._monitors_for(record) existing_monitors = self._monitors_for(record)
active_monitors = set() active_monitors = set()
@@ -1101,30 +1093,20 @@ class Ns1Provider(BaseProvider):
} for v in record.values] } for v in record.values]
# Build our list of answers # Build our list of answers
# The regions dictionary built above already has the required pool
# names. Iterate over them and add answers.
# In the case of the catchall, original pool name can be obtained
# by stripping the CATCHALL_PREFIX from the pool name
answers = [] answers = []
for pool_name in sorted(pools.keys()): for pool_name in sorted(regions.keys()):
priority = 1 priority = 1
answers_added = False
# Dynamic/health checked # Dynamic/health checked
if pool_name == catchall_pool_name: pool_label = pool_name
# For the catchall, use the internal pool name with the prefix pool_name = pool_name.replace(self.CATCHALL_PREFIX, '')
pool_label = '{}{}'.format(self.CATCHALL_PREFIX, pool_name) self._add_answers_for_pool(answers, default_answers, pool_name,
self._add_answers_for_pool(answers, default_answers, pool_name, pool_label, pool_answers, pools,
pool_label, pool_answers, pools, priority)
priority)
answers_added = True
# If the catchall pool has been reused, we need the original
# pool name too, the creation of which is controlled by the
# answers_added flag
if pool_usage[pool_name] > 1:
answers_added = False
if not answers_added:
self._add_answers_for_pool(answers, default_answers, pool_name,
pool_name, pool_answers, pools,
priority)
# Update filters as necessary # Update filters as necessary
filters = self._get_updated_filter_chain(has_region, has_country) filters = self._get_updated_filter_chain(has_region, has_country)

View File

@@ -528,7 +528,7 @@ class TestNs1Provider(TestCase):
class TestNs1ProviderDynamic(TestCase): class TestNs1ProviderDynamic(TestCase):
zone = Zone('unit.tests.', []) zone = Zone('unit.tests.', [])
record_data = { record = Record.new(zone, '', {
'dynamic': { 'dynamic': {
'pools': { 'pools': {
'lhr': { 'lhr': {
@@ -573,8 +573,7 @@ class TestNs1ProviderDynamic(TestCase):
'type': 'A', 'type': 'A',
'value': '1.2.3.4', 'value': '1.2.3.4',
'meta': {}, 'meta': {},
} })
record = Record.new(zone, '', record_data)
def test_notes(self): def test_notes(self):
provider = Ns1Provider('test', 'api-key') provider = Ns1Provider('test', 'api-key')
@@ -979,34 +978,6 @@ class TestNs1ProviderDynamic(TestCase):
rule0['geos'] = rule0_saved_geos rule0['geos'] = rule0_saved_geos
rule1['geos'] = rule1_saved_geos rule1['geos'] = rule1_saved_geos
# Test record with no reuse of the catchall
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'),
]
# Modify the record data before creating the Record object
rule0 = self.record_data['dynamic']['rules'][0]
rule0_saved_geos = rule0['geos']
rule0['geos'] = ['AF', 'EU']
rule1 = self.record_data['dynamic']['rules'].pop(1)
# Create a local record object without reuse of catchall
zone = Zone('unit.tests.', [])
record = Record.new(zone, '', self.record_data)
ret, _ = provider._params_for_A(record)
self.assertEquals(ret['filters'],
Ns1Provider._FILTER_CHAIN_WITH_REGION(provider,
True))
# Restore record_data
rule0['geos'] = rule0_saved_geos
self.record_data['dynamic']['rules'].insert(1, rule1)
@patch('octodns.provider.ns1.Ns1Provider._monitor_sync') @patch('octodns.provider.ns1.Ns1Provider._monitor_sync')
@patch('octodns.provider.ns1.Ns1Provider._monitors_for') @patch('octodns.provider.ns1.Ns1Provider._monitors_for')
def test_params_for_dynamic_oceania(self, monitors_for_mock, def test_params_for_dynamic_oceania(self, monitors_for_mock,
@@ -1123,6 +1094,7 @@ class TestNs1ProviderDynamic(TestCase):
# Test out a small, but realistic setup that covers all the options # Test out a small, but realistic setup that covers all the options
# We have country and region in the test config # We have country and region in the test config
filters = provider._get_updated_filter_chain(True, True) filters = provider._get_updated_filter_chain(True, True)
catchall_pool_name = '{}{}'.format(provider.CATCHALL_PREFIX, 'iad')
ns1_record = { ns1_record = {
'answers': [{ 'answers': [{
'answer': ['3.4.5.6'], 'answer': ['3.4.5.6'],
@@ -1166,16 +1138,16 @@ class TestNs1ProviderDynamic(TestCase):
'meta': { 'meta': {
'priority': 1, 'priority': 1,
'weight': 12, 'weight': 12,
'note': 'from:catchall_iad', 'note': 'from:{}'.format(catchall_pool_name),
}, },
'region': 'catchall_iad', 'region': catchall_pool_name,
}, { }, {
'answer': ['1.2.3.4'], 'answer': ['1.2.3.4'],
'meta': { 'meta': {
'priority': 2, 'priority': 2,
'note': 'from:--default--', 'note': 'from:--default--',
}, },
'region': 'catchall_iad', 'region': catchall_pool_name,
}], }],
'domain': 'unit.tests', 'domain': 'unit.tests',
'filters': filters, 'filters': filters,
@@ -1194,7 +1166,7 @@ class TestNs1ProviderDynamic(TestCase):
'country': ['ZW'], 'country': ['ZW'],
}, },
}, },
'catchall_iad': { catchall_pool_name: {
'meta': { 'meta': {
'note': 'rule-order:3', 'note': 'rule-order:3',
}, },