diff --git a/cinderclient/client.py b/cinderclient/client.py index 142bd9587..426d0a40d 100644 --- a/cinderclient/client.py +++ b/cinderclient/client.py @@ -30,7 +30,6 @@ from keystoneclient import discover import requests from cinderclient import exceptions -from cinderclient.openstack.common.gettextutils import _ from cinderclient.openstack.common import importutils from cinderclient.openstack.common import strutils @@ -80,25 +79,12 @@ def get_volume_api_from_url(url): class SessionClient(adapter.LegacyJsonAdapter): - def request(self, url, method, **kwargs): + def request(self, *args, **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(url, method, + resp, body = super(SessionClient, self).request(*args, raise_exc=False, **kwargs) if raise_exc and resp.status_code >= 400: @@ -124,14 +110,7 @@ class SessionClient(adapter.LegacyJsonAdapter): return self._cs_request(url, 'DELETE', **kwargs) def get_volume_api_version_from_endpoint(self): - 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) + return get_volume_api_from_url(self.get_endpoint()) def authenticate(self, auth=None): self.invalidate(auth) @@ -569,4 +548,4 @@ def get_client_class(version): def Client(version, *args, **kwargs): client_class = get_client_class(version) - return client_class(*args, version=version, **kwargs) + return client_class(*args, **kwargs) diff --git a/cinderclient/shell.py b/cinderclient/shell.py index 03647b6eb..857b0381c 100644 --- a/cinderclient/shell.py +++ b/cinderclient/shell.py @@ -43,12 +43,11 @@ 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 import exceptions as keystoneclient_exc +from keystoneclient.exceptions import DiscoveryFailure import six.moves.urllib.parse as urlparse osprofiler_profiler = importutils.try_import("osprofiler.profiler") @@ -554,7 +553,6 @@ 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: @@ -564,8 +562,6 @@ 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) @@ -611,7 +607,6 @@ 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. @@ -692,68 +687,8 @@ class OpenStackCinderShell(object): if not auth_plugin: auth_session = self._get_keystone_session() - if auth_session and (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, - region_name=os_region_name, - interface=endpoint_type) - - # 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, - region_name=os_region_name, - interface=endpoint_type) - - 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, - region_name=os_region_name, - service_type=service_type, interface=endpoint_type) - - # 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, - region_name=os_region_name, - interface=endpoint_type) - - except keystoneclient_exc.EndpointNotFound: - raise e - - self.cs = client.Client(version[0], os_username, os_password, - os_tenant_name, os_auth_url, + self.cs = client.Client(options.os_volume_api_version, os_username, + os_password, os_tenant_name, os_auth_url, region_name=os_region_name, tenant_id=os_tenant_id, endpoint_type=endpoint_type, @@ -761,10 +696,12 @@ class OpenStackCinderShell(object): 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): @@ -782,8 +719,7 @@ class OpenStackCinderShell(object): try: endpoint_api_version = \ self.cs.get_volume_api_version_from_endpoint() - if (endpoint_api_version != options.os_volume_api_version - and api_version_input): + if endpoint_api_version != options.os_volume_api_version: 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 " @@ -901,7 +837,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 keystoneclient_exc.DiscoveryFailure: + except 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 e30b78181..9406daf9e 100644 --- a/cinderclient/tests/unit/fixture_data/base.py +++ b/cinderclient/tests/unit/fixture_data/base.py @@ -14,42 +14,6 @@ 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 b9dfaf527..2ce2bc60d 100644 --- a/cinderclient/tests/unit/test_client.py +++ b/cinderclient/tests/unit/test_client.py @@ -21,7 +21,6 @@ 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 @@ -63,12 +62,14 @@ class ClientTest(utils.TestCase): output = self.logger.output.split('\n') + print("JSBRYANT: output is", output) + self.assertNotIn("fakePassword", output[1]) self.assertIn("fakeUser", output[1]) def test_versions(self): - v1_url = fixture_base.VOLUME_V1_URL - v2_url = fixture_base.VOLUME_V2_URL + v1_url = 'http://fakeurl/v1/tenants' + v2_url = 'http://fakeurl/v2/tenants' unknown_url = 'http://fakeurl/v9/tenants' self.assertEqual('1', @@ -112,11 +113,8 @@ class ClientTest(utils.TestCase): # 'request' method of Adaptor will return 202 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_session) - response, body = session_client.request(fixture_base.VOLUME_V1_URL, + session_client = cinderclient.client.SessionClient(session=mock.Mock()) + response, body = session_client.request(mock.sentinel.url, 'POST', **kwargs) # In this case, from_response method will not get called @@ -153,15 +151,13 @@ 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_session) + session=mock.Mock()) # 'from_response' method will raise BadRequest because # resp.status_code is 400 self.assertRaises(exceptions.BadRequest, session_client.request, - fixture_base.VOLUME_V1_URL, 'POST', **kwargs) + mock.sentinel.url, 'POST', **kwargs) @mock.patch.object(exceptions, 'from_response') def test_keystone_request_raises_auth_failure_exception( @@ -181,13 +177,11 @@ 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_session) + session=mock.Mock()) self.assertRaises(keystone_exception.AuthorizationFailure, session_client.request, - fixture_base.VOLUME_V1_URL, 'POST', **kwargs) + mock.sentinel.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 ac1367be3..17542919a 100644 --- a/cinderclient/tests/unit/test_shell.py +++ b/cinderclient/tests/unit/test_shell.py @@ -16,7 +16,6 @@ import re import sys import fixtures -from keystoneclient import fixture as keystone_client_fixture import mock import pkg_resources import requests_mock @@ -29,7 +28,6 @@ from cinderclient import auth_plugin from cinderclient import shell from cinderclient.tests.unit.test_auth_plugins import mock_http_request from cinderclient.tests.unit.test_auth_plugins import requested_headers -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 @@ -42,7 +40,7 @@ class ShellTest(utils.TestCase): 'OS_USERNAME': 'username', 'OS_PASSWORD': 'password', 'OS_TENANT_NAME': 'tenant_name', - 'OS_AUTH_URL': '%s/v2.0' % keystone_client.BASE_HOST, + 'OS_AUTH_URL': 'http://no.where/v2.0', } # Patch os.environ to avoid required auth info. @@ -124,203 +122,6 @@ 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/' - - 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/' - - 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/' - - 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 67523ebf6..936f5a5b6 100644 --- a/cinderclient/tests/unit/v1/test_shell.py +++ b/cinderclient/tests/unit/v1/test_shell.py @@ -16,17 +16,15 @@ # 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.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 +from cinderclient.tests.unit.v1 import fakes +from cinderclient.tests.unit import utils +from cinderclient.tests.unit.fixture_data import keystone_client class ShellTest(utils.TestCase): @@ -56,18 +54,7 @@ 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 - ) - 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() - ) + text=keystone_client.keystone_request_callback) 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 a0f460f48..a69fd4022 100644 --- a/cinderclient/tests/unit/v2/test_shell.py +++ b/cinderclient/tests/unit/v2/test_shell.py @@ -14,7 +14,6 @@ # 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 @@ -22,10 +21,9 @@ 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): @@ -55,18 +53,7 @@ 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 - ) - 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() - ) + text=keystone_client.keystone_request_callback) def tearDown(self): # For some methods like test_image_meta_bad_action we are