If cloud doesn't list regions expand passed name

Don't fail on a cloud not having regions when a region name is passed.
Instead just use the name that is given and expand it properly.

This adds test coverage for the paths through the
OpenStackConfig._get_region() method to avoid problems like this in the
future.

In order for this work to be done cleanly a small refactor of
get_regions() is done to split it into two methods, one that gets all
regions with a sane fallback default (for backward compat) and another
that returns only regions that are known in the config and None
otherwise. This allows us to switch on whether or not there are known
regions.

Change-Id: I62736ea82f365badaea5016a23d37a9f1c760927

3 files changed, 76 insertions, 5 deletions

diff --git a/os_client_config/config.py b/os_client_config/config.py
index 077c109..89015cc 100644
--- a/os_client_config/config.py
+++ b/os_client_config/config.py
@@ -366,6 +366,13 @@ class OpenStackConfig(object):
     def _get_regions(self, cloud):
         if cloud not in self.cloud_config['clouds']:
             return [self._expand_region_name('')]
+        regions = self._get_known_regions(cloud)
+        if not regions:
+            # We don't know of any regions use a workable default.
+            regions = [self._expand_region_name('')]
+        return regions
+
+    def _get_known_regions(self, cloud):
         config = self._normalize_keys(self.cloud_config['clouds'][cloud])
         if 'regions' in config:
             return self._expand_regions(config['regions'])
@@ -386,15 +393,15 @@ class OpenStackConfig(object):
                 return self._expand_regions(new_cloud['regions'])
             elif 'region_name' in new_cloud and new_cloud['region_name']:
                 return [self._expand_region_name(new_cloud['region_name'])]
-            else:
-                # Wow. We really tried
-                return [self._expand_region_name('')]
 
     def _get_region(self, cloud=None, region_name=''):
         if not cloud:
             return self._expand_region_name(region_name)
 
-        regions = self._get_regions(cloud)
+        regions = self._get_known_regions(cloud)
+        if not regions:
+            return self._expand_region_name(region_name)
+
         if not region_name:
             return regions[0]
 
diff --git a/os_client_config/tests/base.py b/os_client_config/tests/base.py
index fdc50cd..3f00f6d 100644
--- a/os_client_config/tests/base.py
+++ b/os_client_config/tests/base.py
@@ -119,7 +119,15 @@ USER_CONF = {
                 'auth_url': 'http://example.com/v2',
             },
             'region_name': 'test-region',
-        }
+        },
+        '_test-cloud_no_region': {
+            'profile': '_test_cloud_in_our_cloud',
+            'auth': {
+                'auth_url': 'http://example.com/v2',
+                'username': 'testuser',
+                'password': 'testpass',
+            },
+        },
     },
     'ansible': {
         'expand-hostvars': False,
diff --git a/os_client_config/tests/test_config.py b/os_client_config/tests/test_config.py
index 30ed731..3ea6690 100644
--- a/os_client_config/tests/test_config.py
+++ b/os_client_config/tests/test_config.py
@@ -178,6 +178,7 @@ class TestConfig(base.TestCase):
             ['_test-cloud-domain-id_',
              '_test-cloud-int-project_',
              '_test-cloud_',
+             '_test-cloud_no_region',
              '_test_cloud_hyphenated',
              '_test_cloud_no_vendor',
              '_test_cloud_regions',
@@ -230,6 +231,61 @@ class TestConfig(base.TestCase):
             written_config['cache'].pop('path', None)
             self.assertEqual(written_config, resulting_config)
 
+    def test_get_region_no_region_default(self):
+        c = config.OpenStackConfig(config_files=[self.cloud_yaml],
+                                   vendor_files=[self.vendor_yaml],
+                                   secure_files=[self.no_yaml])
+        region = c._get_region(cloud='_test-cloud_no_region')
+        self.assertEqual(region, {'name': '', 'values': {}})
+
+    def test_get_region_no_region(self):
+        c = config.OpenStackConfig(config_files=[self.cloud_yaml],
+                                   vendor_files=[self.vendor_yaml],
+                                   secure_files=[self.no_yaml])
+        region = c._get_region(cloud='_test-cloud_no_region',
+                               region_name='override-region')
+        self.assertEqual(region, {'name': 'override-region', 'values': {}})
+
+    def test_get_region_region_set(self):
+        c = config.OpenStackConfig(config_files=[self.cloud_yaml],
+                                   vendor_files=[self.vendor_yaml],
+                                   secure_files=[self.no_yaml])
+        region = c._get_region(cloud='_test-cloud_', region_name='test-region')
+        self.assertEqual(region, {'name': 'test-region', 'values': {}})
+
+    def test_get_region_many_regions_default(self):
+        c = config.OpenStackConfig(config_files=[self.cloud_yaml],
+                                   vendor_files=[self.vendor_yaml],
+                                   secure_files=[self.no_yaml])
+        region = c._get_region(cloud='_test_cloud_regions',
+                               region_name='')
+        self.assertEqual(region, {'name': 'region1', 'values':
+                         {'external_network': 'region1-network'}})
+
+    def test_get_region_many_regions(self):
+        c = config.OpenStackConfig(config_files=[self.cloud_yaml],
+                                   vendor_files=[self.vendor_yaml],
+                                   secure_files=[self.no_yaml])
+        region = c._get_region(cloud='_test_cloud_regions',
+                               region_name='region2')
+        self.assertEqual(region, {'name': 'region2', 'values':
+                         {'external_network': 'my-network'}})
+
+    def test_get_region_invalid_region(self):
+        c = config.OpenStackConfig(config_files=[self.cloud_yaml],
+                                   vendor_files=[self.vendor_yaml],
+                                   secure_files=[self.no_yaml])
+        self.assertRaises(
+            exceptions.OpenStackConfigException, c._get_region,
+            cloud='_test_cloud_regions', region_name='invalid-region')
+
+    def test_get_region_no_cloud(self):
+        c = config.OpenStackConfig(config_files=[self.cloud_yaml],
+                                   vendor_files=[self.vendor_yaml],
+                                   secure_files=[self.no_yaml])
+        region = c._get_region(region_name='no-cloud-region')
+        self.assertEqual(region, {'name': 'no-cloud-region', 'values': {}})
+
 
 class TestConfigArgparse(base.TestCase):