diff --git a/ironicclient/common/base.py b/ironicclient/common/base.py index 6411d5ad3..0bd4a35d3 100644 --- a/ironicclient/common/base.py +++ b/ironicclient/common/base.py @@ -155,6 +155,20 @@ class Manager(object): kwargs['headers'] = {'X-OpenStack-Ironic-API-Version': os_ironic_api_version} + # NOTE(jroll) + # endpoint_trimmed is what is prepended if we only pass a path + # to json_request. This might be something like + # https://example.com:4443/baremetal + # The code below which parses the 'next' URL keeps any path prefix + # we're using, so it ends up calling json_request() with e.g. + # /baremetal/v1/nodes. When this is appended to endpoint_trimmed, + # we end up with a full URL of e.g. + # https://example.com:4443/baremetal/baremetal/v1/nodes. + # Parse out the prefix from the base endpoint here, so we can strip + # it below and make sure we're passing a correct path. + endpoint_parts = urlparse.urlparse(self.api.endpoint_trimmed) + url_path_prefix = endpoint_parts[2] + object_list = [] object_count = 0 limit_reached = False @@ -179,6 +193,9 @@ class Manager(object): # the scheme and netloc url_parts = list(urlparse.urlparse(url)) url_parts[0] = url_parts[1] = '' + if url_path_prefix: + url_parts[2] = url_parts[2].replace( + url_path_prefix, '', 1) url = urlparse.urlunparse(url_parts) return object_list diff --git a/ironicclient/common/http.py b/ironicclient/common/http.py index fbd713a46..c9ecaa766 100644 --- a/ironicclient/common/http.py +++ b/ironicclient/common/http.py @@ -614,20 +614,25 @@ class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter): super(SessionClient, self).__init__(**kwargs) + endpoint_filter = self._get_endpoint_filter() + endpoint = self.session.get_endpoint(**endpoint_filter) + self.endpoint_trimmed = _trim_endpoint_api_version(endpoint) + def _parse_version_headers(self, resp): return self._generic_parse_version_headers(resp.headers.get) - def _make_simple_request(self, conn, method, url): - endpoint_filter = { + def _get_endpoint_filter(self): + return { 'interface': self.interface, 'service_type': self.service_type, 'region_name': self.region_name } + def _make_simple_request(self, conn, method, url): # NOTE: conn is self.session for this class return conn.request(url, method, raise_exc=False, user_agent=USER_AGENT, - endpoint_filter=endpoint_filter, + endpoint_filter=self._get_endpoint_filter(), endpoint_override=self.endpoint_override) @with_retries diff --git a/ironicclient/tests/unit/common/test_http.py b/ironicclient/tests/unit/common/test_http.py index af1ad4545..7828bfc08 100644 --- a/ironicclient/tests/unit/common/test_http.py +++ b/ironicclient/tests/unit/common/test_http.py @@ -642,7 +642,8 @@ class SessionClientTest(utils.BaseTestCase): @mock.patch.object(http.LOG, 'warning', autospec=True) def test_session_client_endpoint_deprecation(self, log_mock): - http.SessionClient(os_ironic_api_version=1, session=mock.Mock(), + http.SessionClient(os_ironic_api_version=1, + session=utils.mockSession({}), api_version_select_state='user', max_retries=5, retry_interval=5, endpoint='abc') self.assertIn('deprecated', log_mock.call_args[0][0]) @@ -728,7 +729,7 @@ class SessionClientTest(utils.BaseTestCase): self._test_endpoint_override(True) def test_make_simple_request(self): - session = mock.Mock(spec=['request']) + session = utils.mockSession({}) client = _session_client(session=session, endpoint_override='http://127.0.0.1') @@ -890,7 +891,7 @@ class RetriesTestCase(utils.BaseTestCase): {'Content-Type': 'application/json'}, b"OK", http_client.OK) - fake_session = mock.Mock(spec=requests.Session) + fake_session = utils.mockSession({}) fake_session.request.side_effect = iter((fake_resp, ok_resp)) client = _session_client(session=fake_session) @@ -908,7 +909,7 @@ class RetriesTestCase(utils.BaseTestCase): {'Content-Type': 'application/json'}, b"OK", http_client.OK) - fake_session = mock.Mock(spec=requests.Session) + fake_session = utils.mockSession({}) fake_session.request.side_effect = iter((fake_resp, ok_resp)) client = _session_client(session=fake_session) @@ -920,7 +921,7 @@ class RetriesTestCase(utils.BaseTestCase): {'Content-Type': 'application/json'}, b"OK", http_client.OK) - fake_session = mock.Mock(spec=requests.Session) + fake_session = utils.mockSession({}) fake_session.request.side_effect = iter((exc.ConnectionRefused(), ok_resp)) @@ -933,7 +934,7 @@ class RetriesTestCase(utils.BaseTestCase): {'Content-Type': 'application/json'}, b"OK", http_client.OK) - fake_session = mock.Mock(spec=requests.Session) + fake_session = utils.mockSession({}) fake_session.request.side_effect = iter( (kexc.RetriableConnectionFailure(), ok_resp)) @@ -948,7 +949,7 @@ class RetriesTestCase(utils.BaseTestCase): {'Content-Type': 'application/json'}, error_body, http_client.CONFLICT) - fake_session = mock.Mock(spec=requests.Session) + fake_session = utils.mockSession({}) fake_session.request.return_value = fake_resp client = _session_client(session=fake_session) @@ -965,7 +966,7 @@ class RetriesTestCase(utils.BaseTestCase): {'Content-Type': 'application/json'}, error_body, http_client.CONFLICT) - fake_session = mock.Mock(spec=requests.Session) + fake_session = utils.mockSession({}) fake_session.request.return_value = fake_resp client = _session_client(session=fake_session) @@ -983,7 +984,7 @@ class RetriesTestCase(utils.BaseTestCase): {'Content-Type': 'application/json'}, error_body, http_client.CONFLICT) - fake_session = mock.Mock(spec=requests.Session) + fake_session = utils.mockSession({}) fake_session.request.return_value = fake_resp client = _session_client(session=fake_session) diff --git a/ironicclient/tests/unit/test_client.py b/ironicclient/tests/unit/test_client.py index 5238fb6b3..ff10f197a 100644 --- a/ironicclient/tests/unit/test_client.py +++ b/ironicclient/tests/unit/test_client.py @@ -58,11 +58,19 @@ class ClientTest(utils.BaseTestCase): [o.dest for o in session_loader_options]} mock_ks_session.return_value.load_from_options.assert_called_once_with( auth='auth', **session_opts) + get_endpoint_call = mock.call( + service_type=kwargs.get('service_type') or 'baremetal', + interface=expected_interface, + region_name=kwargs.get('region_name')) if not {'endpoint', 'ironic_url'}.intersection(kwargs): - session.get_endpoint.assert_called_once_with( - service_type=kwargs.get('service_type') or 'baremetal', - interface=expected_interface, - region_name=kwargs.get('region_name')) + calls = [get_endpoint_call, + mock.call(interface=client.http_client.interface, + service_type=client.http_client.service_type, + region_name=client.http_client.region_name)] + self.assertEqual(calls, session.get_endpoint.call_args_list) + else: + self.assertEqual([get_endpoint_call], + session.get_endpoint.call_args_list) if 'os_ironic_api_version' in kwargs: # NOTE(TheJulia): This does not test the negotiation logic # as a request must be triggered in order for any version @@ -279,9 +287,10 @@ class ClientTest(utils.BaseTestCase): 'session': session, } iroclient.get_client('1', **kwargs) - session.get_endpoint.assert_called_once_with(service_type='baremetal', - interface=None, - region_name=None) + endpoint_calls = 2 * [mock.call(service_type='baremetal', + interface=None, + region_name=None)] + self.assertEqual(endpoint_calls, session.get_endpoint.call_args_list) def test_get_client_incorrect_session_passed(self): session = mock.Mock() @@ -302,7 +311,7 @@ class ClientTest(utils.BaseTestCase): with mock.patch.object(loader_class, '__init__', autospec=True) as init_mock: init_mock.return_value = None - iroclient.get_client('1', **passed_kwargs) + client = iroclient.get_client('1', **passed_kwargs) iroclient.convert_keystoneauth_opts(passed_kwargs) init_mock.assert_called_once_with(mock.ANY, **expected_kwargs) @@ -312,9 +321,16 @@ class ClientTest(utils.BaseTestCase): auth=mock.ANY, **session_opts) if 'ironic_url' not in passed_kwargs: service_type = passed_kwargs.get('service_type') or 'baremetal' - session.get_endpoint.assert_called_once_with( - service_type=service_type, interface=expected_interface, - region_name=passed_kwargs.get('region_name')) + endpoint_calls = [ + mock.call(service_type=service_type, + interface=expected_interface, + region_name=passed_kwargs.get('region_name')), + mock.call(service_type=client.http_client.service_type, + interface=client.http_client.interface, + region_name=client.http_client.region_name) + ] + self.assertEqual(endpoint_calls, + session.get_endpoint.call_args_list) def test_loader_arguments_admin_token(self): passed_kwargs = { diff --git a/ironicclient/tests/unit/utils.py b/ironicclient/tests/unit/utils.py index 81fe67396..ea3308e63 100644 --- a/ironicclient/tests/unit/utils.py +++ b/ironicclient/tests/unit/utils.py @@ -40,11 +40,16 @@ class BaseTestCase(testtools.TestCase): class FakeAPI(object): - def __init__(self, responses): + def __init__(self, responses, path_prefix=None): self.responses = responses self.calls = [] + self.path_prefix = path_prefix or '' + self.endpoint_trimmed = 'http://127.0.0.1:6385' + self.path_prefix def _request(self, method, url, headers=None, body=None, params=None): + # url should always just be a path here, e.g. /v1/nodes + url = self.path_prefix + url + call = (method, url, headers or {}, body) if params: call += (params,) @@ -62,7 +67,7 @@ class FakeAPI(object): class FakeConnection(object): - def __init__(self, response=None): + def __init__(self, response=None, path_prefix=None): self._response = response self._last_request = None @@ -135,6 +140,7 @@ def mockSession(headers, content=None, status_code=None, version=None): session = mock.Mock(spec=requests.Session, verify=False, cert=('test_cert', 'test_key')) + session.get_endpoint = mock.Mock(return_value='https://test') response = mockSessionResponse(headers, content, status_code, version) session.request = mock.Mock(return_value=response) diff --git a/ironicclient/tests/unit/v1/test_node.py b/ironicclient/tests/unit/v1/test_node.py index baba5eac4..dd6b9361c 100644 --- a/ironicclient/tests/unit/v1/test_node.py +++ b/ironicclient/tests/unit/v1/test_node.py @@ -575,6 +575,24 @@ fake_responses_pagination = { }, } +fake_responses_pagination_path_prefix = { + '/baremetal/v1/nodes': + { + 'GET': ( + {}, + {"nodes": [NODE1], + "next": "http://127.0.0.1:6385/baremetal/v1/nodes/?limit=1"} + ), + }, + '/baremetal/v1/nodes/?limit=1': + { + 'GET': ( + {}, + {"nodes": [NODE2]} + ), + }, +} + fake_responses_sorting = { '/v1/nodes/?sort_key=updated_at': { @@ -699,6 +717,18 @@ class NodeManagerTest(testtools.TestCase): self.assertEqual(expect, self.api.calls) self.assertEqual(2, len(nodes)) + def test_node_list_pagination_no_limit_path_prefix(self): + self.api = utils.FakeAPI(fake_responses_pagination_path_prefix, + path_prefix='/baremetal') + self.mgr = node.NodeManager(self.api) + nodes = self.mgr.list(limit=0) + expect = [ + ('GET', '/baremetal/v1/nodes', {}, None), + ('GET', '/baremetal/v1/nodes/?limit=1', {}, None) + ] + self.assertEqual(expect, self.api.calls) + self.assertEqual(2, len(nodes)) + def test_node_list_sort_key(self): self.api = utils.FakeAPI(fake_responses_sorting) self.mgr = node.NodeManager(self.api)