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 <aragwitz+lp@pobox.com>
Change-Id: Id4a2f73cbcfcf36aea750375fd92b1c2d3cd824e

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'
+
+        # 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, {}, {

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/')

novaclient/v2/versions.py

@@ -39,13 +39,34 @@ 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]
+            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
+                # 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