From 085b57c84e078061c32b5f6238ce08903a1f37e6 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Mon, 15 Nov 2021 13:48:53 -0800 Subject: [PATCH] Skip explicit countries when converting continent to countries in NS1 --- octodns/provider/ns1.py | 32 ++++++++++++++----- tests/test_octodns_provider_ns1.py | 50 ++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index c30c9e2..7514ee2 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -613,7 +613,7 @@ class Ns1Provider(BaseProvider): return default, pools - def _parse_rule_geos(self, meta): + def _parse_rule_geos(self, meta, notes): geos = set() for georegion in meta.get('georegion', []): @@ -629,6 +629,11 @@ class Ns1Provider(BaseProvider): # in _CONTINENT_TO_LIST_OF_COUNTRIES[] list are found, # set the continent as the region and remove individual countries + # continents that don't have all countries here because a subset of + # them were used in another rule, but we still need this rule to use + # continent instead of the remaining subset of its countries + continents_from_notes = set(notes.get('continents', '').split(',')) + special_continents = dict() for country in meta.get('country', []): # country_alpha2_to_continent_code fails for Pitcairn ('PN'), @@ -648,8 +653,8 @@ class Ns1Provider(BaseProvider): for continent, countries in special_continents.items(): if countries == self._CONTINENT_TO_LIST_OF_COUNTRIES[ - continent]: - # All countries found, so add it to geos + continent] or continent in continents_from_notes: + # All countries found or continent in notes, so add it to geos geos.add(continent) else: # Partial countries found, so just add them as-is to geos @@ -695,7 +700,7 @@ class Ns1Provider(BaseProvider): } rules[rule_order] = rule - geos = self._parse_rule_geos(meta) + geos = self._parse_rule_geos(meta, notes) if geos: # There are geos, combine them with any existing geos for this # pool and recorded the sorted unique set of them @@ -1248,6 +1253,13 @@ class Ns1Provider(BaseProvider): has_region = False regions = {} + explicit_countries = dict() + for rule in record.dynamic.rules: + for geo in rule.data.get('geos', []): + if len(geo) == 5: + con, country = geo.split('-', 1) + explicit_countries.setdefault(con, set()).add(country) + for i, rule in enumerate(record.dynamic.rules): pool_name = rule.data['pool'] @@ -1288,9 +1300,15 @@ class Ns1Provider(BaseProvider): # Use the country list self.log.debug('Converting geo {} to country list'. format(geo)) - for c in self._CONTINENT_TO_LIST_OF_COUNTRIES[geo]: - country.add(c) - has_country = True + continent_countries = \ + self._CONTINENT_TO_LIST_OF_COUNTRIES[geo] + exclude = explicit_countries.get(geo, set()) + country.update(continent_countries - exclude) + notes.setdefault('continents', set()).add(geo) + has_country = True + + if 'continents' in notes: + notes['continents'] = ','.join(sorted(notes['continents'])) meta = { 'note': self._encode_notes(notes), diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index abc741b..bfaee97 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -2056,6 +2056,56 @@ class TestNs1ProviderDynamic(TestCase): 'value': None, }, data) + @patch('octodns.provider.ns1.Ns1Provider._monitor_sync') + @patch('octodns.provider.ns1.Ns1Provider._monitors_for') + def test_dynamic_explicit_countries(self, monitors_for_mock, + monitors_sync_mock): + provider = Ns1Provider('test', 'api-key') + record_data = { + 'dynamic': { + 'pools': { + 'iad': { + 'values': [{ + 'value': 'iad.unit.tests.', + 'status': 'up', + }], + }, + 'lhr': { + 'values': [{ + 'value': 'lhr.unit.tests.', + 'status': 'up', + }] + } + }, + 'rules': [ + { + 'geos': ['NA-US'], + 'pool': 'iad', + }, + { + 'geos': ['NA'], + 'pool': 'lhr', + }, + ], + }, + 'ttl': 33, + 'type': 'CNAME', + 'value': 'value.unit.tests.', + } + record = Record.new(self.zone, 'foo', record_data) + + ns1_record, _ = provider._params_for_dynamic(record) + regions = [ + r for r in ns1_record['regions'].values() + if 'US' in r['meta']['country'] + ] + self.assertEquals(len(regions), 1) + + ns1_record['domain'] = record.fqdn[:-1] + data = provider._data_for_dynamic(record._type, ns1_record)['dynamic'] + self.assertEquals(data['rules'][0]['geos'], ['NA-US']) + self.assertEquals(data['rules'][1]['geos'], ['NA']) + @patch('ns1.rest.records.Records.retrieve') @patch('ns1.rest.zones.Zones.retrieve') @patch('octodns.provider.ns1.Ns1Provider._monitors_for')