From 816bfc82e7a1835cdbfccf0e023beb4c11eb2a30 Mon Sep 17 00:00:00 2001 From: Victoria Martinez de la Cruz Date: Thu, 31 Jan 2019 14:00:32 +0000 Subject: [PATCH] Fix get_base_url get_base_url method makes assumptions on how the base URL should look like and trims it up to the host IP. This change modifies how the base endpoint is processed so we get the correct values. Change-Id: Ie01b02f2e731e25fd4032436d3fa106da03af242 Closes-Bug: #1814094 (cherry picked from commit cb1ac1ad910efcd716c2e785699c44179fb1a8cf) --- manilaclient/common/httpclient.py | 13 ++++-- .../tests/unit/common/test_httpclient.py | 45 +++++++++++++++---- ..._get_base_url-method-168e12292aeec8f1.yaml | 6 +++ 3 files changed, 53 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/bug-1814094-fix-_get_base_url-method-168e12292aeec8f1.yaml diff --git a/manilaclient/common/httpclient.py b/manilaclient/common/httpclient.py index f309bf816..56067db61 100644 --- a/manilaclient/common/httpclient.py +++ b/manilaclient/common/httpclient.py @@ -21,8 +21,10 @@ import logging from oslo_serialization import jsonutils from oslo_utils import strutils +import re import requests import six +from six.moves.urllib import parse from manilaclient import exceptions @@ -78,9 +80,14 @@ class HTTPClient(object): rql.addHandler(ch) def _get_base_url(self, url): - """Truncates url and returns transport, address, and port number.""" - base_url = '/'.join(url.split('/')[:3]) + '/' - return base_url + """Truncates url and returns base endpoint""" + service_endpoint = parse.urlparse(url) + service_endpoint_base_path = re.search( + '(.+?)/v([0-9]+|[0-9]+\.[0-9]+)(/.*|$)', service_endpoint.path) + base_path = (service_endpoint_base_path.group(1) + if service_endpoint_base_path else '') + base_url = service_endpoint._replace(path=base_path) + return parse.urlunparse(base_url) + '/' def _set_request_options(self, insecure, cacert, timeout=None): options = {'verify': True} diff --git a/manilaclient/tests/unit/common/test_httpclient.py b/manilaclient/tests/unit/common/test_httpclient.py index 4d48c769d..a71bb8260 100644 --- a/manilaclient/tests/unit/common/test_httpclient.py +++ b/manilaclient/tests/unit/common/test_httpclient.py @@ -10,7 +10,9 @@ # License for the specific language governing permissions and limitations # under the License. +import ddt import mock +import re import requests import manilaclient @@ -71,13 +73,14 @@ retry_after_non_supporting_mock_request = mock.Mock( return_value=retry_after_non_supporting_response) -def get_authed_client(retries=0): - cl = httpclient.HTTPClient("http://example.com", "token", fake_user_agent, +def get_authed_client(endpoint_url="http://example.com", retries=0): + cl = httpclient.HTTPClient(endpoint_url, "token", fake_user_agent, retries=retries, http_log_debug=True, api_version=manilaclient.API_MAX_VERSION) return cl +@ddt.ddt class ClientTest(utils.TestCase): def setUp(self): @@ -85,8 +88,19 @@ class ClientTest(utils.TestCase): self.max_version = manilaclient.API_MAX_VERSION self.max_version_str = self.max_version.get_string() - def test_get(self): - cl = get_authed_client() + @ddt.data( + "http://manila.example.com/v2/b2d18606-2673-4965-885a-4f5a8b955b9b", + "http://manila.example.com/v1", + "http://manila.example.com/share/v2.22/", + "http://manila.example.com/share/v1/" + "b2d18606-2673-4965-885a-4f5a8b955b9b", + "http://10.10.10.10:3366/v1", + "http://10.10.10.10:3366/v2/b2d18606-2673-4965-885a-4f5a8b955b9b", + "http://manila.example.com:3366/v1.1/", + "http://manila.example.com:3366/v2/" + "b2d18606-2673-4965-885a-4f5a8b955b9b") + def test_get(self, endpoint_url): + cl = get_authed_client(endpoint_url) @mock.patch.object(requests, "request", mock_request) @mock.patch('time.time', mock.Mock(return_value=1234)) @@ -100,11 +114,13 @@ class ClientTest(utils.TestCase): } mock_request.assert_called_with( "GET", - "http://example.com/hi", + endpoint_url + "/hi", headers=headers, **self.TEST_REQUEST_BASE) # Automatic JSON parsing self.assertEqual(body, {"hi": "there"}) + self.assertEqual(re.split('/v[0-9]+[\.0-9]*', + endpoint_url)[0] + "/", cl.base_url) test_get_call() @@ -185,8 +201,19 @@ class ClientTest(utils.TestCase): self.assertRaises(exceptions.Unauthorized, test_get_call) - def test_post(self): - cl = get_authed_client() + @ddt.data( + "http://manila.example.com/v1/b2d18606-2673-4965-885a-4f5a8b955b9b", + "http://manila.example.com/v1", + "http://manila.example.com/share/v2.1/", + "http://manila.example.com/share/v1/" + "b2d18606-2673-4965-885a-4f5a8b955b9b", + "http://10.10.10.10:3366/v1.1", + "http://10.10.10.10:3366/v2/b2d18606-2673-4965-885a-4f5a8b955b9b", + "http://manila.example.com:3366/v2.22/", + "http://manila.example.com:3366/v1/" + "b2d18606-2673-4965-885a-4f5a8b955b9b") + def test_post(self, endpoint_url): + cl = get_authed_client(endpoint_url) @mock.patch.object(requests, "request", mock_request) def test_post_call(): @@ -200,9 +227,11 @@ class ClientTest(utils.TestCase): } mock_request.assert_called_with( "POST", - "http://example.com/hi", + endpoint_url + "/hi", headers=headers, data='[1, 2, 3]', **self.TEST_REQUEST_BASE) + self.assertEqual(re.split('/v[0-9]+[\.0-9]*', + endpoint_url)[0] + "/", cl.base_url) test_post_call() diff --git a/releasenotes/notes/bug-1814094-fix-_get_base_url-method-168e12292aeec8f1.yaml b/releasenotes/notes/bug-1814094-fix-_get_base_url-method-168e12292aeec8f1.yaml new file mode 100644 index 000000000..f9fd3ee03 --- /dev/null +++ b/releasenotes/notes/bug-1814094-fix-_get_base_url-method-168e12292aeec8f1.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + `Launchpad bug 1814094 `_ + has been fixed and the client now correctly parses the base URL from manila's + endpoint url, accounting for proxy URLs.