Fix glance endpoints with endpoint_override1.13.0

Now that we properly pass endpoint_override all the time, we broke
glance. The reason for this is that we calculate the glance url via
glance url stripping in all cases, so the case where we did not have
a configured endpoint override was passing the wrong information to the
constructor, causing double version addition.

Change-Id: I5699b0581d0cb68fed68800c29c8a847e2606ec9

2 files changed, 101 insertions, 6 deletions

diff --git a/os_client_config/cloud_config.py b/os_client_config/cloud_config.py
index 0233eff..f73da04 100644
--- a/os_client_config/cloud_config.py
+++ b/os_client_config/cloud_config.py
@@ -302,6 +302,7 @@ class CloudConfig(object):
         interface = self.get_interface(service_key)
         # trigger exception on lack of service
         endpoint = self.get_session_endpoint(service_key)
+        endpoint_override = self.get_endpoint(service_key)
 
         if not interface_key:
             if service_key == 'image':
@@ -313,7 +314,7 @@ class CloudConfig(object):
             session=self.get_session(),
             service_name=self.get_service_name(service_key),
             service_type=self.get_service_type(service_key),
-            endpoint_override=self.get_endpoint(service_key),
+            endpoint_override=endpoint_override,
             region_name=self.region)
 
         if service_key == 'image':
@@ -322,8 +323,16 @@ class CloudConfig(object):
             # would need to do if they were requesting 'image' - then
             # they necessarily have glanceclient installed
             from glanceclient.common import utils as glance_utils
-            endpoint, _ = glance_utils.strip_version(endpoint)
-            constructor_kwargs['endpoint'] = endpoint
+            endpoint, detected_version = glance_utils.strip_version(endpoint)
+            # If the user has passed in a version, that's explicit, use it
+            if not version:
+                version = detected_version
+            # If the user has passed in or configured an override, use it.
+            # Otherwise, ALWAYS pass in an endpoint_override becuase
+            # we've already done version stripping, so we don't want version
+            # reconstruction to happen twice
+            if not endpoint_override:
+                constructor_kwargs['endpoint_override'] = endpoint
         constructor_kwargs.update(kwargs)
         constructor_kwargs[interface_key] = interface
         constructor_args = []
diff --git a/os_client_config/tests/test_cloud_config.py b/os_client_config/tests/test_cloud_config.py
index 3412254..01581b1 100644
--- a/os_client_config/tests/test_cloud_config.py
+++ b/os_client_config/tests/test_cloud_config.py
@@ -305,10 +305,96 @@ class TestCloudConfig(base.TestCase):
             "test1", "region-al", config_dict, auth_plugin=mock.Mock())
         cc.get_legacy_client('image', mock_client)
         mock_client.assert_called_with(
-            '2',
+            2.0,
             service_name=None,
-            endpoint='http://example.com',
-            endpoint_override=None,
+            endpoint_override='http://example.com',
+            region_name='region-al',
+            interface='public',
+            session=mock.ANY,
+            # Not a typo - the config dict above overrides this
+            service_type='mage'
+        )
+
+    @mock.patch.object(cloud_config.CloudConfig, 'get_session_endpoint')
+    def test_legacy_client_image_override(self, mock_get_session_endpoint):
+        mock_client = mock.Mock()
+        mock_get_session_endpoint.return_value = 'http://example.com/v2'
+        config_dict = defaults.get_defaults()
+        config_dict.update(fake_services_dict)
+        config_dict['image_endpoint_override'] = 'http://example.com/override'
+        cc = cloud_config.CloudConfig(
+            "test1", "region-al", config_dict, auth_plugin=mock.Mock())
+        cc.get_legacy_client('image', mock_client)
+        mock_client.assert_called_with(
+            2.0,
+            service_name=None,
+            endpoint_override='http://example.com/override',
+            region_name='region-al',
+            interface='public',
+            session=mock.ANY,
+            # Not a typo - the config dict above overrides this
+            service_type='mage'
+        )
+
+    @mock.patch.object(cloud_config.CloudConfig, 'get_session_endpoint')
+    def test_legacy_client_image_versioned(self, mock_get_session_endpoint):
+        mock_client = mock.Mock()
+        mock_get_session_endpoint.return_value = 'http://example.com/v2'
+        config_dict = defaults.get_defaults()
+        config_dict.update(fake_services_dict)
+        # v2 endpoint was passed, 1 requested in config, endpoint wins
+        config_dict['image_api_version'] = '1'
+        cc = cloud_config.CloudConfig(
+            "test1", "region-al", config_dict, auth_plugin=mock.Mock())
+        cc.get_legacy_client('image', mock_client)
+        mock_client.assert_called_with(
+            2.0,
+            service_name=None,
+            endpoint_override='http://example.com',
+            region_name='region-al',
+            interface='public',
+            session=mock.ANY,
+            # Not a typo - the config dict above overrides this
+            service_type='mage'
+        )
+
+    @mock.patch.object(cloud_config.CloudConfig, 'get_session_endpoint')
+    def test_legacy_client_image_unversioned(self, mock_get_session_endpoint):
+        mock_client = mock.Mock()
+        mock_get_session_endpoint.return_value = 'http://example.com/'
+        config_dict = defaults.get_defaults()
+        config_dict.update(fake_services_dict)
+        # Versionless endpoint, config wins
+        config_dict['image_api_version'] = '1'
+        cc = cloud_config.CloudConfig(
+            "test1", "region-al", config_dict, auth_plugin=mock.Mock())
+        cc.get_legacy_client('image', mock_client)
+        mock_client.assert_called_with(
+            '1',
+            service_name=None,
+            endpoint_override='http://example.com',
+            region_name='region-al',
+            interface='public',
+            session=mock.ANY,
+            # Not a typo - the config dict above overrides this
+            service_type='mage'
+        )
+
+    @mock.patch.object(cloud_config.CloudConfig, 'get_session_endpoint')
+    def test_legacy_client_image_argument(self, mock_get_session_endpoint):
+        mock_client = mock.Mock()
+        mock_get_session_endpoint.return_value = 'http://example.com/v3'
+        config_dict = defaults.get_defaults()
+        config_dict.update(fake_services_dict)
+        # Versionless endpoint, config wins
+        config_dict['image_api_version'] = '6'
+        cc = cloud_config.CloudConfig(
+            "test1", "region-al", config_dict, auth_plugin=mock.Mock())
+        cc.get_legacy_client('image', mock_client, version='beef')
+        mock_client.assert_called_with(
+            'beef',
+            service_name=None,
+            endpoint_override='http://example.com',
             region_name='region-al',
             interface='public',
             session=mock.ANY,