Merge branch 'master' into gcore-provider

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
 

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 |  |

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__)

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

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[<region>] 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)

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'],
+                ),
             ],
         )))
 

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': [{

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()

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