From b4906c855fe86af6ac8f469ae6e76adddffe6586 Mon Sep 17 00:00:00 2001 From: Haneef Ali Date: Wed, 25 Jun 2014 10:04:57 -0700 Subject: [PATCH] Fix version discovery and auth_plugins V3Client support added version discovery and session supports. Most of the external auth system doesn't support this. This fix bypasses version discovery if the idenity service doesn't support that. Session is used only if no external auth plugin is used Change-Id: Ia84a2ad45940d35c5ef740727f5f7477c72ea9d4 Closes-Bug: #1333961 --- cinderclient/client.py | 50 ++++++++++--------- cinderclient/shell.py | 47 +++++++++++++---- .../tests/fixture_data/keystone_client.py | 12 ++++- cinderclient/tests/test_shell.py | 33 +++++++++++- 4 files changed, 106 insertions(+), 36 deletions(-) diff --git a/cinderclient/client.py b/cinderclient/client.py index a6788b464..71d2a42b0 100644 --- a/cinderclient/client.py +++ b/cinderclient/client.py @@ -506,7 +506,9 @@ def _construct_http_client(username=None, password=None, project_id=None, cacert=None, tenant_id=None, session=None, auth=None): - if session: + + # Don't use sessions if third party plugin is used + if session and not auth_plugin: # If auth pluggin is specified use that pluggin session.auth = auth or session.auth @@ -525,29 +527,29 @@ def _construct_http_client(username=None, password=None, project_id=None, service_name=service_name, region_name=region_name, http_log_debug=http_log_debug) - else: - # FIXME(jamielennox): username and password are now optional. Need - # to test that they were provided in this mode. - return HTTPClient(username, - password, - projectid=project_id, - auth_url=auth_url, - insecure=insecure, - timeout=timeout, - tenant_id=tenant_id, - proxy_token=proxy_token, - proxy_tenant_id=proxy_tenant_id, - region_name=region_name, - endpoint_type=endpoint_type, - service_type=service_type, - service_name=service_name, - volume_service_name=volume_service_name, - retries=retries, - http_log_debug=http_log_debug, - cacert=cacert, - auth_system=auth_system, - auth_plugin=auth_plugin, - ) + + # FIXME(jamielennox): username and password are now optional. Need + # to test that they were provided in this mode. + return HTTPClient(username, + password, + projectid=project_id, + auth_url=auth_url, + insecure=insecure, + timeout=timeout, + tenant_id=tenant_id, + proxy_token=proxy_token, + proxy_tenant_id=proxy_tenant_id, + region_name=region_name, + endpoint_type=endpoint_type, + service_type=service_type, + service_name=service_name, + volume_service_name=volume_service_name, + retries=retries, + http_log_debug=http_log_debug, + cacert=cacert, + auth_system=auth_system, + auth_plugin=auth_plugin, + ) def get_client_class(version): diff --git a/cinderclient/shell.py b/cinderclient/shell.py index ecba0309d..af166a02a 100644 --- a/cinderclient/shell.py +++ b/cinderclient/shell.py @@ -43,6 +43,8 @@ from keystoneclient import discover from keystoneclient import session from keystoneclient.auth.identity import v2 as v2_auth from keystoneclient.auth.identity import v3 as v3_auth +from keystoneclient.exceptions import DiscoveryFailure +import six.moves.urllib.parse as urlparse DEFAULT_OS_VOLUME_API_VERSION = "1" @@ -157,8 +159,7 @@ class OpenStackCinderShell(object): parser.add_argument('--endpoint-type', metavar='', default=utils.env('CINDER_ENDPOINT_TYPE', - default= - DEFAULT_CINDER_ENDPOINT_TYPE), + default=DEFAULT_CINDER_ENDPOINT_TYPE), help='Endpoint type, which is publicURL or ' 'internalURL. ' 'Default=nova env[CINDER_ENDPOINT_TYPE] or ' @@ -737,6 +738,36 @@ class OpenStackCinderShell(object): project_domain_id=project_domain_id, ) + def _discover_auth_versions(self, session, auth_url): + # discover the API versions the server is supporting based on the + # given URL + v2_auth_url = None + v3_auth_url = None + try: + ks_discover = discover.Discover(session=session, auth_url=auth_url) + v2_auth_url = ks_discover.url_for('2.0') + v3_auth_url = ks_discover.url_for('3.0') + except DiscoveryFailure: + # Discovery response mismatch. Raise the error + raise + except Exception: + # Some public clouds throw some other exception or doesn't support + # discovery. In that case try to determine version from auth_url + # API version from the original URL + url_parts = urlparse.urlparse(auth_url) + (scheme, netloc, path, params, query, fragment) = url_parts + path = path.lower() + if path.startswith('/v3'): + v3_auth_url = auth_url + elif path.startswith('/v2'): + v2_auth_url = auth_url + else: + raise exc.CommandError('Unable to determine the Keystone' + ' version to authenticate with ' + 'using the given auth_url.') + + return (v2_auth_url, v3_auth_url) + def _get_keystone_session(self, **kwargs): # first create a Keystone session cacert = self.options.os_cacert or None @@ -747,16 +778,12 @@ class OpenStackCinderShell(object): verify = False else: verify = cacert or True + ks_session = session.Session(verify=verify, cert=cert) # discover the supported keystone versions using the given url - ks_discover = discover.Discover(session=ks_session, - auth_url=self.options.os_auth_url) - - # Determine which authentication plugin to use. First inspect the - # auth_url to see the supported version. If both v3 and v2 are - # supported, then use the highest version if possible. - v2_auth_url = ks_discover.url_for('v2.0') - v3_auth_url = ks_discover.url_for('v3.0') + (v2_auth_url, v3_auth_url) = self._discover_auth_versions( + session=ks_session, + auth_url=self.options.os_auth_url) username = self.options.os_username or None user_domain_name = self.options.os_user_domain_name or None diff --git a/cinderclient/tests/fixture_data/keystone_client.py b/cinderclient/tests/fixture_data/keystone_client.py index a7eb17721..814323c4a 100644 --- a/cinderclient/tests/fixture_data/keystone_client.py +++ b/cinderclient/tests/fixture_data/keystone_client.py @@ -49,6 +49,11 @@ V3_VERSION = {'id': 'v3.0', 'status': 'stable', 'updated': UPDATED} +WRONG_VERSION_RESPONSE = {'id': 'v2.0', + 'links': [V2_DESCRIBED_BY_HTML, V2_DESCRIBED_BY_PDF], + 'status': 'stable', + 'updated': UPDATED} + def _create_version_list(versions): return json.dumps({'versions': {'values': versions}}) @@ -206,7 +211,8 @@ def generate_v3_project_scoped_token(**kwargs): def keystone_request_callback(request, uri, headers): - response_headers = {"content-type": "application/json"} + response_headers = {"content-type": "application/json", + 'server': 'Python/HTTPretty', } if uri == BASE_URL: return (200, headers, V3_VERSION_LIST) elif uri == BASE_URL + "/v2.0": @@ -216,3 +222,7 @@ def keystone_request_callback(request, uri, headers): token_id, token_data = generate_v3_project_scoped_token() response_headers["X-Subject-Token"] = token_id return (201, response_headers, token_data) + elif "WrongDiscoveryResponse.discovery.com" in uri: + return (200, response_headers, str(WRONG_VERSION_RESPONSE)) + else: + return (500, response_headers, str(WRONG_VERSION_RESPONSE)) diff --git a/cinderclient/tests/test_shell.py b/cinderclient/tests/test_shell.py index a4cc1c52e..693d3778c 100644 --- a/cinderclient/tests/test_shell.py +++ b/cinderclient/tests/test_shell.py @@ -22,6 +22,9 @@ from testtools import matchers from cinderclient import exceptions from cinderclient import shell from cinderclient.tests import utils +from cinderclient.tests.fixture_data import keystone_client +from keystoneclient.exceptions import DiscoveryFailure +import httpretty class ShellTest(utils.TestCase): @@ -80,8 +83,36 @@ class ShellTest(utils.TestCase): self.assertThat(help_text, matchers.MatchesRegex(r, re.DOTALL | re.MULTILINE)) + def register_keystone_auth_fixture(self, url): + httpretty.register_uri(httpretty.GET, url, + body=keystone_client.keystone_request_callback) + + @httpretty.activate + def test_version_discovery(self): + _shell = shell.OpenStackCinderShell() + + os_auth_url = "https://WrongDiscoveryResponse.discovery.com:35357/v2.0" + self.register_keystone_auth_fixture(os_auth_url) + self.assertRaises(DiscoveryFailure, _shell._discover_auth_versions, + None, auth_url=os_auth_url) + + os_auth_url = "https://DiscoveryNotSupported.discovery.com:35357/v2.0" + self.register_keystone_auth_fixture(os_auth_url) + v2_url, v3_url = _shell._discover_auth_versions( + None, auth_url=os_auth_url) + self.assertEqual(v2_url, os_auth_url, "Expected v2 url") + self.assertEqual(v3_url, None, "Expected no v3 url") + + os_auth_url = "https://DiscoveryNotSupported.discovery.com:35357/v3.0" + self.register_keystone_auth_fixture(os_auth_url) + v2_url, v3_url = _shell._discover_auth_versions( + None, auth_url=os_auth_url) + self.assertEqual(v3_url, os_auth_url, "Expected v3 url") + self.assertEqual(v2_url, None, "Expected no v2 url") + class CinderClientArgumentParserTest(utils.TestCase): + def test_ambiguity_solved_for_one_visible_argument(self): parser = shell.CinderClientArgumentParser(add_help=False) parser.add_argument('--test-parameter', @@ -94,7 +125,7 @@ class CinderClientArgumentParserTest(utils.TestCase): opts = parser.parse_args(['--test']) - #visible argument must be set + # visible argument must be set self.assertTrue(opts.visible_param) self.assertFalse(opts.hidden_param)