Merge "Make sure auth_url exists and is not versionless"

ironic/common/keystone.py

@@ -25,7 +25,10 @@ acl.register_opts(CONF)
 
 def get_service_url(service_type='baremetal', endpoint_type='internal'):
     """Wrapper for get service url from keystone service catalog."""
-    auth_url = CONF.keystone_authtoken.auth_uri or ''
+    auth_url = CONF.keystone_authtoken.auth_uri
+    if not auth_url:
+        raise exception.CatalogFailure(_('Keystone API endpoint is missing'))
+
     api_v3 = CONF.keystone_authtoken.auth_version == 'v3.0' or \
             'v3' in parse.urlparse(auth_url).path
 
@@ -34,6 +37,10 @@ def get_service_url(service_type='baremetal', endpoint_type='internal'):
     else:
         from keystoneclient.v2_0 import client
 
+    api_version = 'v3' if api_v3 else 'v2.0'
+    # NOTE(lucasagomes): Get rid of the trailing '/' otherwise urljoin()
+    #   fails to override the version in the URL
+    auth_url = parse.urljoin(auth_url.rstrip('/'), api_version)
     try:
         ksclient = client.Client(username=CONF.keystone_authtoken.admin_user,
                         password=CONF.keystone_authtoken.admin_password,

ironic/tests/test_keystone.py

@@ -14,12 +14,28 @@
 
 import fixtures
 from keystoneclient import exceptions as ksexception
+import mock
 
 from ironic.common import exception
 from ironic.common import keystone
 from ironic.tests import base
 
 
+class FakeCatalog:
+    def url_for(self, **kwargs):
+        return 'fake-url'
+
+
+class FakeClient:
+    def __init__(self, **kwargs):
+        self.service_catalog = FakeCatalog()
+
+    def has_service_catalog(self):
+        return True
+
+
+# TODO(lucasagomes): Replace fixtures with mock for some of the tests in
+#                    the KeystoneTestCase class
 class KeystoneTestCase(base.TestCase):
 
     def setUp(self):
@@ -99,3 +115,40 @@ class KeystoneTestCase(base.TestCase):
 
         self.assertRaises(exception.CatalogUnauthorized,
                           keystone.get_service_url)
+
+    def test_get_service_url_fail_missing_auth_uri(self):
+        self.config(group='keystone_authtoken', auth_uri=None)
+        self.assertRaises(exception.CatalogFailure,
+                          keystone.get_service_url)
+
+    @mock.patch('keystoneclient.v2_0.client.Client')
+    def test_get_service_url_versionless_v2(self, mock_ks):
+        mock_ks.return_value = FakeClient()
+        self.config(group='keystone_authtoken', auth_uri='http://127.0.0.1')
+        expected_url = 'http://127.0.0.1/v2.0'
+        keystone.get_service_url()
+        mock_ks.assert_called_once_with(username='fake', password='fake',
+                                        tenant_name='fake',
+                                        auth_url=expected_url)
+
+    @mock.patch('keystoneclient.v3.client.Client')
+    def test_get_service_url_versionless_v3(self, mock_ks):
+        mock_ks.return_value = FakeClient()
+        self.config(group='keystone_authtoken', auth_version='v3.0',
+                    auth_uri='http://127.0.0.1')
+        expected_url = 'http://127.0.0.1/v3'
+        keystone.get_service_url()
+        mock_ks.assert_called_once_with(username='fake', password='fake',
+                                        tenant_name='fake',
+                                        auth_url=expected_url)
+
+    @mock.patch('keystoneclient.v2_0.client.Client')
+    def test_get_service_url_version_override(self, mock_ks):
+        mock_ks.return_value = FakeClient()
+        self.config(group='keystone_authtoken',
+                    auth_uri='http://127.0.0.1/v2.0/')
+        expected_url = 'http://127.0.0.1/v2.0'
+        keystone.get_service_url()
+        mock_ks.assert_called_once_with(username='fake', password='fake',
+                                        tenant_name='fake',
+                                        auth_url=expected_url)