diff --git a/CHANGELOG.md b/CHANGELOG.md index f70d67c..e6dcb48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ to data. See the [octodns/processor/](/octodns/processor) directory for examples. The change has been designed to have no impact on the process unless the `processors` key is present in zone configs. +* Fixes NS1 provider's geotarget limitation of using `NA` continent. Now, when + `NA` is used in geos it considers **all** the countries of `North America` + insted of just `us-east`, `us-west` and `us-central` regions +* `SX' & 'UM` country support added to NS1Provider, not yet in the North + America list for backwards compatibility reasons. They will be added in the + next releaser. ## v0.9.12 - 2021-04-30 - Enough time has passed diff --git a/README.md b/README.md index 0445155..502063a 100644 --- a/README.md +++ b/README.md @@ -208,7 +208,7 @@ The above command pulled the existing data out of Route53 and placed the results | [GoogleCloudProvider](/octodns/provider/googlecloud.py) | google-cloud-dns | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT | No | | | [HetznerProvider](/octodns/provider/hetzner.py) | | A, AAAA, CAA, CNAME, MX, NS, SRV, TXT | No | | | [MythicBeastsProvider](/octodns/provider/mythicbeasts.py) | Mythic Beasts | A, AAAA, ALIAS, CNAME, MX, NS, SRV, SSHFP, CAA, TXT | No | | -| [Ns1Provider](/octodns/provider/ns1.py) | ns1-python | All | Yes | Missing `NA` geo target | +| [Ns1Provider](/octodns/provider/ns1.py) | ns1-python | All | Yes | | | [OVH](/octodns/provider/ovh.py) | ovh | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, SSHFP, TXT, DKIM | No | | | [PowerDnsProvider](/octodns/provider/powerdns.py) | | All | No | | | [Rackspace](/octodns/provider/rackspace.py) | | A, AAAA, ALIAS, CNAME, MX, NS, PTR, SPF, TXT | No | | diff --git a/octodns/manager.py b/octodns/manager.py index dcbf083..104e445 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -261,7 +261,8 @@ class Manager(object): try: source.populate(zone, lenient=lenient) except TypeError as e: - if "keyword argument 'lenient'" not in text_type(e): + if ("unexpected keyword argument 'lenient'" + not in text_type(e)): raise self.log.warn('provider %s does not accept lenient ' 'param', source.__class__.__name__) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 4551c6c..87bbef0 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -837,8 +837,8 @@ class AzureProvider(BaseProvider): pool['fallback'] = pool_name if pool_name in pools: - # we've already populated the pool - continue + # we've already populated this and subsequent pools + break # populate the pool from Weighted profile # these should be leaf node entries with no further nesting @@ -922,6 +922,16 @@ class AzureProvider(BaseProvider): for profile in profiles: name = profile.name + + endpoints = set() + for ep in profile.endpoints: + if not ep.target: + continue + if ep.target in endpoints: + msg = '{} contains duplicate endpoint {}' + raise AzureException(msg.format(name, ep.target)) + endpoints.add(ep.target) + if name in seen_profiles: # exit if a possible collision is detected, even though # we've tried to ensure unique mapping @@ -1053,7 +1063,6 @@ class AzureProvider(BaseProvider): while pool_name: # iterate until we reach end of fallback chain - default_seen = False pool = pools[pool_name].data if len(pool['values']) > 1: # create Weighted profile for multi-value pool diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 9313d0d..bf08358 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -341,6 +341,9 @@ class Ns1Provider(BaseProvider): 'ASIAPAC': 'AS', 'EUROPE': 'EU', 'SOUTH-AMERICA': 'SA', + # continent NA has been handled as part of Geofence Country filter + # starting from v0.9.13. These below US-* just need to continue to + # exist here so it doesn't break the ugrade path 'US-CENTRAL': 'NA', 'US-EAST': 'NA', 'US-WEST': 'NA', @@ -350,8 +353,6 @@ class Ns1Provider(BaseProvider): 'AS': ('ASIAPAC',), 'EU': ('EUROPE',), 'SA': ('SOUTH-AMERICA',), - # TODO: what about CA, MX, and all the other NA countries? - 'NA': ('US-CENTRAL', 'US-EAST', 'US-WEST'), } # Necessary for handling unsupported continents in _CONTINENT_TO_REGIONS @@ -359,6 +360,10 @@ class Ns1Provider(BaseProvider): 'OC': {'FJ', 'NC', 'PG', 'SB', 'VU', 'AU', 'NF', 'NZ', 'FM', 'GU', 'KI', 'MH', 'MP', 'NR', 'PW', 'AS', 'CK', 'NU', 'PF', 'PN', 'TK', 'TO', 'TV', 'WF', 'WS'}, + 'NA': {'DO', 'DM', 'BB', 'BL', 'BM', 'HT', 'KN', 'JM', 'VC', 'HN', + 'BS', 'BZ', 'PR', 'NI', 'LC', 'TT', 'VG', 'PA', 'TC', 'PM', + 'GT', 'AG', 'GP', 'AI', 'VI', 'CA', 'GD', 'AW', 'CR', 'GL', + 'CU', 'MF', 'SV', 'US', 'MQ', 'MS', 'KY', 'MX', 'CW', 'BQ'} } def __init__(self, id, api_key, retry_count=4, monitor_regions=None, @@ -549,42 +554,45 @@ class Ns1Provider(BaseProvider): geos = set() - # continents are mapped (imperfectly) to regions, but what about - # Canada/North America for georegion in meta.get('georegion', []): geos.add(self._REGION_TO_CONTINENT[georegion]) # Countries are easy enough to map, we just have to find their # continent # - # NOTE: Special handling for Oceania - # NS1 doesn't support Oceania as a region. So the Oceania countries - # will be present in meta['country']. If all the countries in the - # Oceania countries list are found, set the region to OC and remove - # individual oceania country entries - - oc_countries = set() + # NOTE: Some continents need special handling since NS1 + # does not supprt them as regions. These are defined under + # _CONTINENT_TO_LIST_OF_COUNTRIES. So the countries for these + # regions will be present in meta['country']. If all the countries + # in _CONTINENT_TO_LIST_OF_COUNTRIES[] list are found, + # set the continent as the region and remove individual countries + + special_continents = dict() for country in meta.get('country', []): - # country_alpha2_to_continent_code fails for Pitcairn ('PN') + # country_alpha2_to_continent_code fails for Pitcairn ('PN'), + # United States Minor Outlying Islands ('UM') and + # Sint Maarten ('SX') if country == 'PN': con = 'OC' + elif country in ['SX', 'UM']: + con = 'NA' else: con = country_alpha2_to_continent_code(country) - if con == 'OC': - oc_countries.add(country) + if con in self._CONTINENT_TO_LIST_OF_COUNTRIES: + special_continents.setdefault(con, set()).add(country) else: - # Adding only non-OC countries here to geos geos.add('{}-{}'.format(con, country)) - if oc_countries: - if oc_countries == self._CONTINENT_TO_LIST_OF_COUNTRIES['OC']: - # All OC countries found, so add 'OC' to geos - geos.add('OC') + for continent, countries in special_continents.items(): + if countries == self._CONTINENT_TO_LIST_OF_COUNTRIES[ + continent]: + # All countries found, so add it to geos + geos.add(continent) else: - # Partial OC countries found, just add them as-is to geos - for c in oc_countries: - geos.add('{}-{}'.format('OC', c)) + # Partial countries found, so just add them as-is to geos + for c in countries: + geos.add('{}-{}'.format(continent, c)) # States are easy too, just assume NA-US (CA providences aren't # supported by octoDNS currently) diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index eb74007..e09de28 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -999,6 +999,51 @@ class TestAzureDnsProvider(TestCase): 'Collision in Traffic Manager' )) + @patch( + 'octodns.provider.azuredns.AzureProvider._generate_traffic_managers') + def test_extra_changes_non_last_fallback_contains_default(self, mock_gtm): + provider = self._get_provider() + + desired = Zone(zone.name, sub_zones=[]) + record = Record.new(desired, 'foo', { + 'type': 'CNAME', + 'ttl': 60, + 'value': 'default.unit.tests.', + 'dynamic': { + 'pools': { + 'one': { + 'values': [{'value': 'one.unit.tests.'}], + 'fallback': 'def', + }, + 'def': { + 'values': [{'value': 'default.unit.tests.'}], + 'fallback': 'two', + }, + 'two': { + 'values': [{'value': 'two.unit.tests.'}], + }, + }, + 'rules': [ + {'pool': 'one'}, + ] + } + }) + desired.add_record(record) + changes = [Create(record)] + + # assert that no exception is raised + provider._extra_changes(zone, desired, changes) + + # simulate duplicate endpoint and assert exception + endpoint = Endpoint(target='dup.unit.tests.') + mock_gtm.return_value = [Profile( + name='test-profile', + endpoints=[endpoint, endpoint], + )] + with self.assertRaises(AzureException) as ctx: + provider._extra_changes(zone, desired, changes) + self.assertTrue('duplicate endpoint' in text_type(ctx)) + def test_extra_changes_invalid_dynamic_A(self): provider = self._get_provider() @@ -1642,6 +1687,12 @@ class TestAzureDnsProvider(TestCase): 'value': 'default.unit.tests.', 'dynamic': { 'pools': { + 'sto': { + 'values': [ + {'value': 'sto.unit.tests.'}, + ], + 'fallback': 'iad', + }, 'iad': { 'values': [ {'value': 'iad.unit.tests.'}, @@ -1657,13 +1708,14 @@ class TestAzureDnsProvider(TestCase): 'rules': [ {'geos': ['EU'], 'pool': 'iad'}, {'geos': ['EU-GB'], 'pool': 'lhr'}, + {'geos': ['EU-SE'], 'pool': 'sto'}, {'pool': 'lhr'}, ], } }) profiles = provider._generate_traffic_managers(record) - self.assertEqual(len(profiles), 3) + self.assertEqual(len(profiles), 4) self.assertTrue(_profile_is_match(profiles[-1], Profile( name='foo--unit--tests', traffic_routing_method='Geographic', @@ -1683,6 +1735,12 @@ class TestAzureDnsProvider(TestCase): target_resource_id=profiles[1].id, geo_mapping=['GB', 'WORLD'], ), + Endpoint( + name='rule-sto', + type=nested, + target_resource_id=profiles[2].id, + geo_mapping=['SE'], + ), ], ))) diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 4ae4757..df517a2 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -1034,7 +1034,7 @@ class TestNs1ProviderDynamic(TestCase): rule0 = record.data['dynamic']['rules'][0] rule1 = record.data['dynamic']['rules'][1] rule0['geos'] = ['AF', 'EU'] - rule1['geos'] = ['NA'] + rule1['geos'] = ['AS'] ret, monitor_ids = provider._params_for_A(record) self.assertEquals(10, len(ret['answers'])) self.assertEquals(ret['filters'], @@ -1048,7 +1048,7 @@ class TestNs1ProviderDynamic(TestCase): }, 'iad__georegion': { 'meta': { - 'georegion': ['US-CENTRAL', 'US-EAST', 'US-WEST'], + 'georegion': ['ASIAPAC'], 'note': 'rule-order:1' } }, @@ -1150,7 +1150,7 @@ class TestNs1ProviderDynamic(TestCase): rule0 = record.data['dynamic']['rules'][0] rule1 = record.data['dynamic']['rules'][1] rule0['geos'] = ['AF', 'EU', 'NA-US-CA'] - rule1['geos'] = ['NA', 'NA-US'] + rule1['geos'] = ['AS', 'AS-IN'] ret, _ = provider._params_for_A(record) self.assertEquals(17, len(ret['answers'])) @@ -1210,13 +1210,13 @@ class TestNs1ProviderDynamic(TestCase): }, 'iad__country': { 'meta': { - 'country': ['US'], + 'country': ['IN'], 'note': 'rule-order:1' } }, 'iad__georegion': { 'meta': { - 'georegion': ['US-CENTRAL', 'US-EAST', 'US-WEST'], + 'georegion': ['ASIAPAC'], 'note': 'rule-order:1' } }, @@ -1562,6 +1562,24 @@ class TestNs1ProviderDynamic(TestCase): self.assertTrue( 'OC-{}'.format(c) in data4['dynamic']['rules'][0]['geos']) + # NA test cases + # 1. Full list of countries should return 'NA' in geos + na_countries = Ns1Provider._CONTINENT_TO_LIST_OF_COUNTRIES['NA'] + del ns1_record['regions']['lhr__country']['meta']['us_state'] + ns1_record['regions']['lhr__country']['meta']['country'] = \ + list(na_countries) + data5 = provider._data_for_A('A', ns1_record) + self.assertTrue('NA' in data5['dynamic']['rules'][0]['geos']) + + # 2. Partial list of countries should return just those + partial_na_cntry_list = list(na_countries)[:5] + ['SX', 'UM'] + ns1_record['regions']['lhr__country']['meta']['country'] = \ + partial_na_cntry_list + data6 = provider._data_for_A('A', ns1_record) + for c in partial_na_cntry_list: + self.assertTrue( + 'NA-{}'.format(c) in data6['dynamic']['rules'][0]['geos']) + # Test out fallback only pools and new-style notes ns1_record = { 'answers': [{ diff --git a/tests/test_octodns_provider_ultra.py b/tests/test_octodns_provider_ultra.py index 52e0307..a22a489 100644 --- a/tests/test_octodns_provider_ultra.py +++ b/tests/test_octodns_provider_ultra.py @@ -1,8 +1,11 @@ +from __future__ import unicode_literals + from mock import Mock, call from os.path import dirname, join from requests import HTTPError from requests_mock import ANY, mock as requests_mock from six import text_type +from six.moves.urllib import parse from unittest import TestCase from json import load as json_load @@ -55,7 +58,8 @@ class TestUltraProvider(TestCase): self.assertEquals(1, mock.call_count) expected_payload = "grant_type=password&username=user&"\ "password=rightpass" - self.assertEquals(mock.last_request.text, expected_payload) + self.assertEquals(parse.parse_qs(mock.last_request.text), + parse.parse_qs(expected_payload)) def test_get_zones(self): provider = _get_provider() diff --git a/tests/test_octodns_source_envvar.py b/tests/test_octodns_source_envvar.py index 0714883..ac66a22 100644 --- a/tests/test_octodns_source_envvar.py +++ b/tests/test_octodns_source_envvar.py @@ -1,6 +1,6 @@ +from mock import patch from six import text_type from unittest import TestCase -from unittest.mock import patch from octodns.source.envvar import EnvVarSource from octodns.source.envvar import EnvironmentVariableNotFoundException