diff --git a/cinderclient/client.py b/cinderclient/client.py index 2e892c82e..4687fe821 100644 --- a/cinderclient/client.py +++ b/cinderclient/client.py @@ -28,6 +28,7 @@ from keystoneclient.auth.identity import base import requests from cinderclient import exceptions +from cinderclient.openstack.common.gettextutils import _ from cinderclient.openstack.common import importutils from cinderclient.openstack.common import strutils @@ -71,12 +72,25 @@ def get_volume_api_from_url(url): class SessionClient(adapter.LegacyJsonAdapter): - def request(self, *args, **kwargs): + def request(self, url, method, **kwargs): kwargs.setdefault('authenticated', False) + + # NOTE(thingee): v1 and v2 require the project id in the url. Prepend + # it if we're doing discovery. We figure out if we're doing discovery + # if there is no project id already specified in the path. parts is + # a list where index 1 is the version discovered and index 2 might be + # an empty string or a project id. + endpoint = self.get_endpoint() + parts = urlparse.urlsplit(endpoint).path.split('/') + project_id = self.get_project_id() + if (parts[1] in ['v1', 'v2'] and parts[2] == '' + and project_id is not None): + url = '{0}{1}{2}'.format(endpoint, project_id, url) + # Note(tpatil): The standard call raises errors from # keystoneclient, here we need to raise the cinderclient errors. raise_exc = kwargs.pop('raise_exc', True) - resp, body = super(SessionClient, self).request(*args, + resp, body = super(SessionClient, self).request(url, method, raise_exc=False, **kwargs) if raise_exc and resp.status_code >= 400: @@ -102,7 +116,14 @@ class SessionClient(adapter.LegacyJsonAdapter): return self._cs_request(url, 'DELETE', **kwargs) def get_volume_api_version_from_endpoint(self): - return get_volume_api_from_url(self.get_endpoint()) + endpoint = self.get_endpoint() + if not endpoint: + msg = _('The Cinder server does not support %s. Check your ' + 'providers supported versions and try again with ' + 'setting --os-volume-api-version or the environment ' + 'variable OS_VOLUME_API_VERSION.') % self.version + raise exceptions.InvalidAPIVersion(msg) + return get_volume_api_from_url(endpoint) def authenticate(self, auth=None): self.invalidate(auth) @@ -540,4 +561,4 @@ def get_client_class(version): def Client(version, *args, **kwargs): client_class = get_client_class(version) - return client_class(*args, **kwargs) + return client_class(*args, version=version, **kwargs) diff --git a/cinderclient/shell.py b/cinderclient/shell.py index c82877395..da562e10f 100644 --- a/cinderclient/shell.py +++ b/cinderclient/shell.py @@ -43,11 +43,12 @@ from cinderclient.openstack.common.gettextutils import _ from cinderclient.v1 import shell as shell_v1 from cinderclient.v2 import shell as shell_v2 +from keystoneclient import adapter 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 +from keystoneclient import exceptions as keystoneclient_exc import six.moves.urllib.parse as urlparse osprofiler_profiler = importutils.try_import("osprofiler.profiler") @@ -542,6 +543,7 @@ class OpenStackCinderShell(object): (options, args) = parser.parse_known_args(argv) self.setup_debugging(options.debug) api_version_input = True + service_type_input = True self.options = options if not options.os_volume_api_version: @@ -551,6 +553,8 @@ class OpenStackCinderShell(object): options.os_volume_api_version = DEFAULT_OS_VOLUME_API_VERSION api_version_input = False + version = (options.os_volume_api_version,) + # build available subcommands based on version self.extensions = self._discover_extensions( options.os_volume_api_version) @@ -599,6 +603,7 @@ class OpenStackCinderShell(object): if not service_type: service_type = DEFAULT_CINDER_SERVICE_TYPE service_type = utils.get_service_type(args.func) or service_type + service_type_input = False # FIXME(usrleon): Here should be restrict for project id same as # for os_username or os_password but for compatibility it is not. @@ -676,22 +681,75 @@ class OpenStackCinderShell(object): "through --os-auth-url or env[OS_AUTH_URL].") auth_session = self._get_keystone_session() + if not service_type_input or not api_version_input: + # NOTE(thingee): Unfortunately the v2 shell is tied to volumev2 + # service_type. If the service_catalog just contains service_type + # volume with x.x.x.x:8776 for discovery, and the user sets version + # 2 for the client, it'll default to volumev2 and raise + # EndpointNotFound. This is a workaround until we feel comfortable + # with removing the volumev2 assumption. + keystone_adapter = adapter.Adapter(auth_session) + try: + # Try just the client's defaults + endpoint = keystone_adapter.get_endpoint( + service_type=service_type, + version=version, + interface='public') - self.cs = client.Client(options.os_volume_api_version, os_username, - os_password, os_tenant_name, os_auth_url, - insecure, region_name=os_region_name, + # Service was found, but wrong version. Lets try a different + # version, if the user did not specify one. + if not endpoint and not api_version_input: + if version == ('1',): + version = ('2',) + else: + version = ('1',) + + endpoint = keystone_adapter.get_endpoint( + service_type=service_type, version=version, + interface='public') + + except keystoneclient_exc.EndpointNotFound as e: + # No endpoint found with that service_type, lets fall back to + # other service_types if the user did not specify one. + if not service_type_input: + if service_type == 'volume': + service_type = 'volumev2' + else: + service_type = 'volume' + + try: + endpoint = keystone_adapter.get_endpoint( + version=version, + service_type=service_type, interface='public') + + # Service was found, but wrong version. Lets try + # a different version, if the user did not specify one. + if not endpoint and not api_version_input: + if version == ('1',): + version = ('2',) + else: + version = ('1',) + + endpoint = keystone_adapter.get_endpoint( + service_type=service_type, version=version, + interface='public') + + except keystoneclient_exc.EndpointNotFound: + raise e + + self.cs = client.Client(version[0], os_username, os_password, + os_tenant_name, os_auth_url, + region_name=os_region_name, tenant_id=os_tenant_id, endpoint_type=endpoint_type, extensions=self.extensions, service_type=service_type, service_name=service_name, volume_service_name=volume_service_name, - bypass_url=bypass_url, - retries=options.retries, - http_log_debug=args.debug, - cacert=cacert, auth_system=os_auth_system, - auth_plugin=auth_plugin, - session=auth_session) + bypass_url=bypass_url, retries=options.retries, + http_log_debug=args.debug, cacert=cacert, + auth_system=os_auth_system, + auth_plugin=auth_plugin, session=auth_session) try: if not utils.isunauthenticated(args.func): @@ -709,7 +767,8 @@ class OpenStackCinderShell(object): try: endpoint_api_version = \ self.cs.get_volume_api_version_from_endpoint() - if endpoint_api_version != options.os_volume_api_version: + if (endpoint_api_version != options.os_volume_api_version + and api_version_input): msg = (("OpenStack Block Storage API version is set to %s " "but you are accessing a %s endpoint. " "Change its value through --os-volume-api-version " @@ -827,7 +886,7 @@ class OpenStackCinderShell(object): 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: + except keystoneclient_exc.DiscoveryFailure: # Discovery response mismatch. Raise the error raise except Exception: diff --git a/cinderclient/tests/unit/fixture_data/base.py b/cinderclient/tests/unit/fixture_data/base.py index 9406daf9e..e30b78181 100644 --- a/cinderclient/tests/unit/fixture_data/base.py +++ b/cinderclient/tests/unit/fixture_data/base.py @@ -14,6 +14,42 @@ import fixtures IDENTITY_URL = 'http://identityserver:5000/v2.0' VOLUME_URL = 'http://volume.host' +TENANT_ID = 'b363706f891f48019483f8bd6503c54b' + +VOLUME_V1_URL = '%(volume_url)s/v1/%(tenant_id)s' % {'volume_url': VOLUME_URL, + 'tenant_id': TENANT_ID} +VOLUME_V2_URL = '%(volume_url)s/v2/%(tenant_id)s' % {'volume_url': VOLUME_URL, + 'tenant_id': TENANT_ID} + + +def generate_version_output(v1=True, v2=True): + v1_dict = { + "status": "SUPPORTED", + "updated": "2014-06-28T12:20:21Z", + "id": "v1.0", + "links": [{ + "href": "http://127.0.0.1:8776/v1/", + "rel": "self" + }] + } + + v2_dict = { + "status": "CURRENT", + "updated": "2012-11-21T11:33:21Z", + "id": "v2.0", "links": [{ + "href": "http://127.0.0.1:8776/v2/", + "rel": "self" + }] + } + + versions = [] + if v1: + versions.append(v1_dict) + + if v2: + versions.append(v2_dict) + + return {"versions": versions} class Fixture(fixtures.Fixture): diff --git a/cinderclient/tests/unit/test_client.py b/cinderclient/tests/unit/test_client.py index f54097957..b9dfaf527 100644 --- a/cinderclient/tests/unit/test_client.py +++ b/cinderclient/tests/unit/test_client.py @@ -21,6 +21,7 @@ import cinderclient.client import cinderclient.v1.client import cinderclient.v2.client from cinderclient import exceptions +from cinderclient.tests.unit.fixture_data import base as fixture_base from cinderclient.tests.unit import utils from keystoneclient import adapter from keystoneclient import exceptions as keystone_exception @@ -66,8 +67,8 @@ class ClientTest(utils.TestCase): self.assertIn("fakeUser", output[1]) def test_versions(self): - v1_url = 'http://fakeurl/v1/tenants' - v2_url = 'http://fakeurl/v2/tenants' + v1_url = fixture_base.VOLUME_V1_URL + v2_url = fixture_base.VOLUME_V2_URL unknown_url = 'http://fakeurl/v9/tenants' self.assertEqual('1', @@ -111,8 +112,11 @@ class ClientTest(utils.TestCase): # 'request' method of Adaptor will return 202 response mock_request.return_value = mock_response - session_client = cinderclient.client.SessionClient(session=mock.Mock()) - response, body = session_client.request(mock.sentinel.url, + mock_session = mock.Mock() + mock_session.get_endpoint.return_value = fixture_base.VOLUME_V1_URL + session_client = cinderclient.client.SessionClient( + session=mock_session) + response, body = session_client.request(fixture_base.VOLUME_V1_URL, 'POST', **kwargs) # In this case, from_response method will not get called @@ -149,13 +153,15 @@ class ClientTest(utils.TestCase): # 'request' method of Adaptor will return 400 response mock_request.return_value = mock_response + mock_session = mock.Mock() + mock_session.get_endpoint.return_value = fixture_base.VOLUME_V1_URL session_client = cinderclient.client.SessionClient( - session=mock.Mock()) + session=mock_session) # 'from_response' method will raise BadRequest because # resp.status_code is 400 self.assertRaises(exceptions.BadRequest, session_client.request, - mock.sentinel.url, 'POST', **kwargs) + fixture_base.VOLUME_V1_URL, 'POST', **kwargs) @mock.patch.object(exceptions, 'from_response') def test_keystone_request_raises_auth_failure_exception( @@ -175,11 +181,13 @@ class ClientTest(utils.TestCase): with mock.patch.object(adapter.Adapter, 'request', side_effect= keystone_exception.AuthorizationFailure()): + mock_session = mock.Mock() + mock_session.get_endpoint.return_value = fixture_base.VOLUME_V1_URL session_client = cinderclient.client.SessionClient( - session=mock.Mock()) + session=mock_session) self.assertRaises(keystone_exception.AuthorizationFailure, session_client.request, - mock.sentinel.url, 'POST', **kwargs) + fixture_base.VOLUME_V1_URL, 'POST', **kwargs) # As keystonesession.request method will raise # AuthorizationFailure exception, check exceptions.from_response diff --git a/cinderclient/tests/unit/test_shell.py b/cinderclient/tests/unit/test_shell.py index 98ee55a77..d279756bf 100644 --- a/cinderclient/tests/unit/test_shell.py +++ b/cinderclient/tests/unit/test_shell.py @@ -16,6 +16,7 @@ import re import sys import fixtures +from keystoneclient import fixture as keystone_client_fixture import mock import requests_mock from six import moves @@ -23,8 +24,9 @@ from testtools import matchers from cinderclient import exceptions from cinderclient import shell -from cinderclient.tests.unit import utils +from cinderclient.tests.unit.fixture_data import base as fixture_base from cinderclient.tests.unit.fixture_data import keystone_client +from cinderclient.tests.unit import utils import keystoneclient.exceptions as ks_exc from keystoneclient.exceptions import DiscoveryFailure @@ -35,7 +37,7 @@ class ShellTest(utils.TestCase): 'OS_USERNAME': 'username', 'OS_PASSWORD': 'password', 'OS_TENANT_NAME': 'tenant_name', - 'OS_AUTH_URL': 'http://no.where/v2.0', + 'OS_AUTH_URL': '%s/v2.0' % keystone_client.BASE_HOST, } # Patch os.environ to avoid required auth info. @@ -116,6 +118,203 @@ class ShellTest(utils.TestCase): self.assertEqual(v3_url, os_auth_url, "Expected v3 url") self.assertEqual(v2_url, None, "Expected no v2 url") + @requests_mock.Mocker() + def test_cinder_version_legacy_endpoint_v1_and_v2(self, mocker): + """Verify that legacy endpoint settings still work. + + Legacy endpoints that are not using version discovery is + ://(tenant_id)s. For this unit test, we fill + in the tenant_id for mocking purposes. + """ + token = keystone_client_fixture.V2Token() + cinder_url = 'http://127.0.0.1:8776/v1/%s' % fixture_base.TENANT_ID + + volume_service = token.add_service('volume', 'Cinder v1') + volume_service.add_endpoint(public=cinder_url, region='RegionOne') + + volumev2_service = token.add_service('volumev2', 'Cinder v2') + volumev2_service.add_endpoint(public=cinder_url, region='RegionOne') + + mocker.post(keystone_client.BASE_HOST + '/v2.0/tokens', + json=token) + mocker.get(cinder_url, json=fixture_base.generate_version_output()) + volume_request = mocker.get('http://127.0.0.1:8776/v1/volumes/detail', + json={'volumes': {}}) + + self.shell('list') + self.assertTrue(volume_request.called) + + @requests_mock.Mocker() + def test_cinder_version_legacy_endpoint_only_v1(self, mocker): + """Verify that v1 legacy endpoint settings still work. + + Legacy endpoints that are not using version discovery is + ://(tenant_id)s. For this unit test, we fill + in the tenant_id for mocking purposes. + """ + token = keystone_client_fixture.V2Token() + cinder_url = 'http://127.0.0.1:8776/v1/%s' % fixture_base.TENANT_ID + + volume_service = token.add_service('volume', 'Cinder v1') + volume_service.add_endpoint( + public=cinder_url, + region='RegionOne' + ) + + mocker.get( + cinder_url, + json=fixture_base.generate_version_output(v1=True, v2=False) + ) + mocker.post(keystone_client.BASE_HOST + '/v2.0/tokens', + json=token) + volume_request = mocker.get('http://127.0.0.1:8776/v1/volumes/detail', + json={'volumes': {}}) + + self.shell('list') + self.assertTrue(volume_request.called) + + @requests_mock.Mocker() + def test_cinder_version_legacy_endpoint_only_v2(self, mocker): + """Verify that v2 legacy endpoint settings still work. + + Legacy endpoints that are not using version discovery is + ://(tenant_id)s. For this unit test, we fill + in the tenant_id for mocking purposes. + """ + token = keystone_client_fixture.V2Token() + cinder_url = 'http://127.0.0.1:8776/v2/%s' % fixture_base.TENANT_ID + + volumev2_service = token.add_service('volumev2', 'Cinder v2') + volumev2_service.add_endpoint( + public=cinder_url, + region='RegionOne' + ) + + mocker.post(keystone_client.BASE_HOST + '/v2.0/tokens', + json=token) + + mocker.get( + cinder_url, + json=fixture_base.generate_version_output(v1=False, v2=True) + ) + volume_request = mocker.get('http://127.0.0.1:8776/v2/volumes/detail', + json={'volumes': {}}) + + self.shell('list') + self.assertTrue(volume_request.called) + + @requests_mock.Mocker() + def test_cinder_version_discovery(self, mocker): + """Verify client works two endpoints enabled under one service.""" + token = keystone_client_fixture.V2Token() + + volume_service = token.add_service('volume', 'Cinder') + volume_service.add_endpoint(public='http://127.0.0.1:8776', + region='RegionOne') + + mocker.post(keystone_client.BASE_HOST + '/v2.0/tokens', + json=token) + mocker.get( + 'http://127.0.0.1:8776/', + json=fixture_base.generate_version_output(v1=True, v2=True) + ) + + v1_request = mocker.get('http://127.0.0.1:8776/v1/volumes/detail', + json={'volumes': {}}) + v2_request = mocker.get('http://127.0.0.1:8776/v2/volumes/detail', + json={'volumes': {}}) + + self.shell('list') + self.assertTrue(v1_request.called) + + self.shell('--os-volume-api-version 2 list') + self.assertTrue(v2_request.called) + + @requests_mock.Mocker() + def test_cinder_version_discovery_only_v1(self, mocker): + """Verify when v1 is only enabled, the client discovers it.""" + token = keystone_client_fixture.V2Token() + + volume_service = token.add_service('volume', 'Cinder') + volume_service.add_endpoint(public='http://127.0.0.1:8776', + region='RegionOne') + + mocker.post(keystone_client.BASE_HOST + '/v2.0/tokens', + json=token) + mocker.get( + 'http://127.0.0.1:8776/', + json=fixture_base.generate_version_output(v1=True, v2=True) + ) + volume_request = mocker.get('http://127.0.0.1:8776/v1/volumes/detail', + json={'volumes': {}}) + + self.shell('list') + self.assertTrue(volume_request.called) + + @requests_mock.Mocker() + def test_cinder_version_discovery_only_v2(self, mocker): + """Verify when v2 is enabled, the client discovers it.""" + token = keystone_client_fixture.V2Token() + + volumev2_service = token.add_service('volume', 'Cinder') + volumev2_service.add_endpoint(public='http://127.0.0.1:8776', + region='RegionOne') + + mocker.post(keystone_client.BASE_HOST + '/v2.0/tokens', + json=token) + + mocker.get( + 'http://127.0.0.1:8776/', + json=fixture_base.generate_version_output(v1=False, v2=True) + ) + volume_request = mocker.get('http://127.0.0.1:8776/v2/volumes/detail', + json={'volumes': {}}) + + self.shell('list') + self.assertTrue(volume_request.called) + + @requests_mock.Mocker() + def test_cinder_version_discovery_fallback(self, mocker): + """Client defaults to v1, but v2 is only available, fallback to v2.""" + token = keystone_client_fixture.V2Token() + + volumev2_service = token.add_service('volumev2', 'Cinder v2') + volumev2_service.add_endpoint(public='http://127.0.0.1:8776', + region='RegionOne') + + mocker.post(keystone_client.BASE_HOST + '/v2.0/tokens', + json=token) + + mocker.get( + 'http://127.0.0.1:8776/', + json=fixture_base.generate_version_output(v1=False, v2=True) + ) + volume_request = mocker.get('http://127.0.0.1:8776/v2/volumes/detail', + json={'volumes': {}}) + + self.shell('list') + self.assertTrue(volume_request.called) + + @requests_mock.Mocker() + def test_cinder_version_discovery_unsupported_version(self, mocker): + """Try a version from the client that's not enabled in Cinder.""" + token = keystone_client_fixture.V2Token() + + volume_service = token.add_service('volume', 'Cinder') + volume_service.add_endpoint(public='http://127.0.0.1:8776', + region='RegionOne') + + mocker.post(keystone_client.BASE_HOST + '/v2.0/tokens', + json=token) + + mocker.get( + 'http://127.0.0.1:8776/', + json=fixture_base.generate_version_output(v1=False, v2=True) + ) + + self.assertRaises(exceptions.InvalidAPIVersion, + self.shell, '--os-volume-api-version 1 list') + @mock.patch('sys.stdin', side_effect=mock.MagicMock) @mock.patch('getpass.getpass', return_value='password') def test_password_prompted(self, mock_getpass, mock_stdin): diff --git a/cinderclient/tests/unit/v1/test_shell.py b/cinderclient/tests/unit/v1/test_shell.py index fc87d5b59..cd02527a1 100644 --- a/cinderclient/tests/unit/v1/test_shell.py +++ b/cinderclient/tests/unit/v1/test_shell.py @@ -16,15 +16,17 @@ # under the License. import fixtures +from keystoneclient import fixture as keystone_client_fixture from requests_mock.contrib import fixture as requests_mock_fixture from cinderclient import client from cinderclient import exceptions from cinderclient import shell -from cinderclient.v1 import shell as shell_v1 -from cinderclient.tests.unit.v1 import fakes -from cinderclient.tests.unit import utils +from cinderclient.tests.unit.fixture_data import base as fixture_base from cinderclient.tests.unit.fixture_data import keystone_client +from cinderclient.tests.unit import utils +from cinderclient.tests.unit.v1 import fakes +from cinderclient.v1 import shell as shell_v1 class ShellTest(utils.TestCase): @@ -54,7 +56,18 @@ class ShellTest(utils.TestCase): self.requests = self.useFixture(requests_mock_fixture.Fixture()) self.requests.register_uri( 'GET', keystone_client.BASE_URL, - text=keystone_client.keystone_request_callback) + text=keystone_client.keystone_request_callback + ) + token = keystone_client_fixture.V2Token() + s = token.add_service('volume', 'cinder') + s.add_endpoint(public='http://127.0.0.1:8776') + + self.requests.post(keystone_client.BASE_URL + 'v2.0/tokens', + json=token) + self.requests.get( + 'http://127.0.0.1:8776', + json=fixture_base.generate_version_output() + ) def tearDown(self): # For some method like test_image_meta_bad_action we are diff --git a/cinderclient/tests/unit/v2/test_shell.py b/cinderclient/tests/unit/v2/test_shell.py index 10a8f5339..e80ff6426 100644 --- a/cinderclient/tests/unit/v2/test_shell.py +++ b/cinderclient/tests/unit/v2/test_shell.py @@ -14,6 +14,7 @@ # under the License. import fixtures +from keystoneclient import fixture as keystone_client_fixture import mock from requests_mock.contrib import fixture as requests_mock_fixture from six.moves.urllib import parse @@ -21,9 +22,10 @@ from six.moves.urllib import parse from cinderclient import client from cinderclient import exceptions from cinderclient import shell +from cinderclient.tests.unit.fixture_data import base as fixture_base +from cinderclient.tests.unit.fixture_data import keystone_client from cinderclient.tests.unit import utils from cinderclient.tests.unit.v2 import fakes -from cinderclient.tests.unit.fixture_data import keystone_client class ShellTest(utils.TestCase): @@ -53,7 +55,18 @@ class ShellTest(utils.TestCase): self.requests = self.useFixture(requests_mock_fixture.Fixture()) self.requests.register_uri( 'GET', keystone_client.BASE_URL, - text=keystone_client.keystone_request_callback) + text=keystone_client.keystone_request_callback + ) + token = keystone_client_fixture.V2Token() + s = token.add_service('volume', 'cinder') + s.add_endpoint(public='http://127.0.0.1:8776') + + self.requests.post(keystone_client.BASE_URL + 'v2.0/tokens', + json=token) + self.requests.get( + 'http://127.0.0.1:8776', + json=fixture_base.generate_version_output() + ) def tearDown(self): # For some methods like test_image_meta_bad_action we are