From 939862e55e42c5fafee9c2fec42b5f5fde8fc205 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Mon, 21 Dec 2015 11:35:56 -0600 Subject: [PATCH] Fix glance endpoints with endpoint_override 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 --- os_client_config/cloud_config.py | 15 +++- os_client_config/tests/test_cloud_config.py | 92 ++++++++++++++++++++- 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,