From 4f16fe65c41ac1158f0493fcbd5c8109bd053872 Mon Sep 17 00:00:00 2001 From: Sean Dague Date: Wed, 14 Oct 2015 09:09:57 -0400 Subject: [PATCH] make project_id optional in urls for version discovery Currently there is a baked in assumption throughout the nova ecosystem that endpoint urls will end in {project_id}. This means that the version document for a specific major API version is "endpoint - last_chunk_of_url". This is not a discoverable URL, it's one that we heuristically generate. GET /v2.1/{project_id} is a 404. If we make Nova able to handle endpoint ids without {project_id} we run into a problem with novaclient, which is that end_point is now /v2.1. The heuristic gives us a version url of /. Which returns a very different doc than /v2.1. This causes all novaclient commands to explode. In order to support this transition we need to make both Nova and Novaclient handle endpoints with and without {project_id}. The proposal is thus to try the raw value of the endpoint first. If it works, great. If it fails with a 404, fall back to the heuristics. While this change and the nova change are technically independent, we should land this one as soon as possible, because clients before this change will not work with Nova deploys that get rid of project_id in their service catalog in the future. Part of bp:service-catalog-tng Co-Authored-By: Augustina Ragwitz Change-Id: Id4a2f73cbcfcf36aea750375fd92b1c2d3cd824e --- novaclient/tests/unit/v2/fakes.py | 29 +++++++++++++++++-- novaclient/tests/unit/v2/test_versions.py | 18 ++++++++++++ novaclient/v2/versions.py | 35 ++++++++++++++++++----- 3 files changed, 73 insertions(+), 9 deletions(-) diff --git a/novaclient/tests/unit/v2/fakes.py b/novaclient/tests/unit/v2/fakes.py index e50cddd0f..66ce6b207 100644 --- a/novaclient/tests/unit/v2/fakes.py +++ b/novaclient/tests/unit/v2/fakes.py @@ -18,6 +18,7 @@ import datetime import mock from oslo_utils import strutils +import re import six from six.moves.urllib import parse @@ -27,6 +28,12 @@ from novaclient.tests.unit import fakes from novaclient.tests.unit import utils from novaclient.v2 import client +# regex to compare callback to result of get_endpoint() +ENDPOINT_RE = re.compile( + r"^(get_http:__nova_api:8774_)(v\d(_\d)?)_(\w{32})$") + +ENDPOINT_TYPE_RE = re.compile(r"^(v\d(\.\d)?)$") + class FakeClient(fakes.FakeClient, client.Client): @@ -49,7 +56,13 @@ class FakeHTTPClient(base_client.HTTPClient): self.projectid = 'projectid' self.user = 'user' self.region_name = 'region_name' - self.endpoint_type = 'endpoint_type' + + # determines which endpoint to return in get_endpoint() + if 'endpoint_type' in kwargs: + self.endpoint_type = kwargs['endpoint_type'] + else: + self.endpoint_type = 'endpoint_type' + self.service_type = 'service_type' self.service_name = 'service_name' self.volume_service_name = 'volume_service_name' @@ -86,6 +99,12 @@ class FakeHTTPClient(base_client.HTTPClient): callback = "get_versions" elif callback == "get_http:__nova_api:8774_v2_1": callback = "get_current_version" + elif ENDPOINT_RE.search(callback): + # compare callback to result of get_endpoint() + # NOTE(sdague): if we try to call a thing that doesn't + # exist, just return a 404. This allows the stack to act + # more like we'd expect when making REST calls. + raise exceptions.NotFound('404') if not hasattr(self, callback): raise AssertionError('Called unknown API method: %s %s, ' @@ -104,7 +123,13 @@ class FakeHTTPClient(base_client.HTTPClient): return r, body def get_endpoint(self): - return "http://nova-api:8774/v2.1/190a755eef2e4aac9f06aa6be9786385" + # check if endpoint matches expected format (eg, v2.1) + if (hasattr(self, 'endpoint_type') + and ENDPOINT_TYPE_RE.search(self.endpoint_type)): + return "http://nova-api:8774/%s/" % self.endpoint_type + else: + return ( + "http://nova-api:8774/v2.1/190a755eef2e4aac9f06aa6be9786385") def get_versions(self): return (200, {}, { diff --git a/novaclient/tests/unit/v2/test_versions.py b/novaclient/tests/unit/v2/test_versions.py index aa8e84a6e..2504b104d 100644 --- a/novaclient/tests/unit/v2/test_versions.py +++ b/novaclient/tests/unit/v2/test_versions.py @@ -81,3 +81,21 @@ class VersionsTest(utils.TestCase): def test_get_current_with_rax_auth_plugin_workaround(self, session, _list): self.cs.callback = [] self.assertIsNone(self.cs.versions.get_current()) + + @mock.patch.object(versions.VersionManager, '_is_session_client', + return_value=True) + def test_get_endpoint_without_project_id(self, mock_is_session_client): + # create a fake client such that get_endpoint() + # doesn't return uuid in url + endpoint_type = 'v2.1' + expected_endpoint = 'http://nova-api:8774/v2.1/' + cs_2_1 = fakes.FakeClient(endpoint_type=endpoint_type) + + result = cs_2_1.versions.get_current() + self.assertEqual(result.manager.api.client.endpoint_type, + endpoint_type, "Check endpoint_type was set") + self.assertEqual(result.manager.api.client.management_url, + expected_endpoint, "Check endpoint without uuid") + + # check that the full request works as expected + cs_2_1.assert_called('GET', 'http://nova-api:8774/v2.1/') diff --git a/novaclient/v2/versions.py b/novaclient/v2/versions.py index 796ea7df5..ee33bd93a 100644 --- a/novaclient/v2/versions.py +++ b/novaclient/v2/versions.py @@ -39,14 +39,35 @@ class VersionManager(base.ManagerWithFind): def _get_current(self): """Returns info about current version.""" + # TODO(sdague): we've now got to make up to 3 HTTP requests to + # determine what version we are running, due to differences in + # deployments and versions. We really need to cache the + # results of this per endpoint and keep the results of it for + # some reasonable TTL (like 24 hours) to reduce our round trip + # traffic. if self._is_session_client(): - url = self.api.client.get_endpoint().rsplit("/", 1)[0] - # NOTE(sdague): many service providers don't really - # implement GET / in the expected way, if we do a GET /v2 - # that's actually a 300 redirect to /v2/... because of how - # paste works. So adding the end slash is really important. - url = "%s/" % url - return self._get(url, "version") + try: + # Assume that the value of get_endpoint() is something + # we can get the version of. This is a 404 for Nova < + # Mitaka if the service catalog contains project_id. + # + # TODO(sdague): add microversion for when this will + # change + url = "%s" % self.api.client.get_endpoint() + return self._get(url, "version") + except exc.NotFound: + # If that's a 404, we can instead try hacking together + # an endpoint root url by chopping off the last 2 /s. + # This is kind of gross, but we've had this baked in + # so long people got used to this hard coding. + # + # NOTE(sdague): many service providers don't really + # implement GET / in the expected way, if we do a GET + # /v2 that's actually a 300 redirect to + # /v2/... because of how paste works. So adding the + # end slash is really important. + url = "%s/" % url.rsplit("/", 1)[0] + return self._get(url, "version") else: # NOTE(andreykurilin): HTTPClient doesn't have ability to send get # request without token in the url, so `self._get` doesn't work.