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)