From 323f4e4bc4710d42e493eb56e40ba139a84d67b3 Mon Sep 17 00:00:00 2001
From: wangxiyuan <wangxiyuan@huawei.com>
Date: Tue, 17 Jul 2018 19:43:21 +0800
Subject: [PATCH] Add netloc and version check for version discovery

If the url netloc in the catalog and service's response
are not the same, we should choose the catalog's and
add the version info to it if needed.

Change-Id: If78d368bd505156a5416bb9cbfaf988204925c79
Closes-bug: #1733052
---
 keystoneauth1/discover.py                     | 20 ++++++++++-
 .../unit/identity/test_identity_common.py     |  2 +-
 keystoneauth1/tests/unit/test_discovery.py    | 34 +++++++++++++++++++
 .../notes/bug-1733052-1b4af3b3fe1b05bb.yaml   |  7 ++++
 4 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/bug-1733052-1b4af3b3fe1b05bb.yaml

diff --git a/keystoneauth1/discover.py b/keystoneauth1/discover.py
index 8fe84816..b8c768c6 100644
--- a/keystoneauth1/discover.py
+++ b/keystoneauth1/discover.py
@@ -417,10 +417,28 @@ def _combine_relative_url(discovery_url, version_url):
     parsed_version_url = urllib.parse.urlparse(url)
     parsed_discovery_url = urllib.parse.urlparse(discovery_url)
 
+    # The services can override the version_url with some config options.(for
+    # example, In Keystone, Cinder and Glance, the option is "public_endpoint",
+    # and "compute_link_prefix", "network_link_prefix" in Nova and Neutron.
+    # In this case, it's hard to distinguish which part in version_url is
+    # useful for discovery_url , so here we just get the version from
+    # version_url and then add it into the discovery_url if needed.
+    path = parsed_version_url.path
+    if parsed_discovery_url.netloc != parsed_version_url.netloc:
+        version = version_url.rstrip('/').split('/')[-1]
+        url_path = parsed_discovery_url.path.rstrip('/')
+        if not url_path.endswith(version):
+            path = url_path + '/' + version
+            if version_url.endswith('/'):
+                # add '/' back to keep backward compatibility.
+                path = path + '/'
+        else:
+            path = parsed_discovery_url.path
+
     return urllib.parse.ParseResult(
         parsed_discovery_url.scheme,
         parsed_discovery_url.netloc,
-        parsed_version_url.path,
+        path,
         parsed_version_url.params,
         parsed_version_url.query,
         parsed_version_url.fragment).geturl()
diff --git a/keystoneauth1/tests/unit/identity/test_identity_common.py b/keystoneauth1/tests/unit/identity/test_identity_common.py
index 9a7c5fb9..8aba6ece 100644
--- a/keystoneauth1/tests/unit/identity/test_identity_common.py
+++ b/keystoneauth1/tests/unit/identity/test_identity_common.py
@@ -329,7 +329,7 @@ class CommonIdentityTests(object):
         sb = session.Session()
         discovery_cache = {}
 
-        expected_url = urllib.parse.urljoin(self.TEST_COMPUTE_ADMIN, '/v2.0')
+        expected_url = self.TEST_COMPUTE_ADMIN + '/v2.0'
         for sess in (sa, sb):
 
             disc = discover.get_discovery(
diff --git a/keystoneauth1/tests/unit/test_discovery.py b/keystoneauth1/tests/unit/test_discovery.py
index 15490cf3..cb035139 100644
--- a/keystoneauth1/tests/unit/test_discovery.py
+++ b/keystoneauth1/tests/unit/test_discovery.py
@@ -491,6 +491,40 @@ class VersionDataTests(utils.TestCase):
 
             self.assertTrue(mock.called_once)
 
+    def test_version_data_override_version_url(self):
+        # if the request url is versioned already, just return it.
+        self.requests_mock.get(
+            V3_URL, status_code=200,
+            json={'version': fixture.V3Discovery('http://override/identity/v3')
+                  }
+        )
+
+        disc = discover.Discover(self.session, V3_URL)
+        version_data = disc.version_data()
+
+        for v in version_data:
+            self.assertEqual(v['version'], (3, 0))
+            self.assertEqual(v['status'], discover.Status.CURRENT)
+            self.assertEqual(v['raw_status'], 'stable')
+            self.assertEqual(v['url'], V3_URL)
+
+        # if the request url is not versioned, just add version info to it.(
+        # do not changed the url's netloc or path)
+        self.requests_mock.get(
+            BASE_URL, status_code=200,
+            json={'version': fixture.V3Discovery('http://override/identity/v3')
+                  }
+        )
+
+        disc = discover.Discover(self.session, BASE_URL)
+        version_data = disc.version_data()
+
+        for v in version_data:
+            self.assertEqual(v['version'], (3, 0))
+            self.assertEqual(v['status'], discover.Status.CURRENT)
+            self.assertEqual(v['raw_status'], 'stable')
+            self.assertEqual(v['url'], V3_URL)
+
     def test_version_data_unknown(self):
         discovery_fixture = fixture.V3Discovery(V3_URL)
         discovery_fixture.status = 'hungry'
diff --git a/releasenotes/notes/bug-1733052-1b4af3b3fe1b05bb.yaml b/releasenotes/notes/bug-1733052-1b4af3b3fe1b05bb.yaml
new file mode 100644
index 00000000..56a11f85
--- /dev/null
+++ b/releasenotes/notes/bug-1733052-1b4af3b3fe1b05bb.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - >
+    [`bug 1733052 <https://bugs.launchpad.net/keystoneauth/+bug/1733052>`_]
+    Now the version discovery mechanism only fetches the version info from
+    server side if the versioned url has been overrode. So that the request
+    url's path won't be changed completely.