From 16f83c4a531853e1432a47135a8af566b9bb890d Mon Sep 17 00:00:00 2001 From: Steve Martinelli <stevemar@ca.ibm.com> Date: Wed, 9 Mar 2016 07:57:04 -0500 Subject: [PATCH] Switch to keystoneauth move cinderclient to keystoneauth as keystoneclient's auth session, plugins and adapter code has been deprecated. Co-Authored-By: Paulo Ewerton <pauloewerton@lsd.ufcg.edu.br> Co-Authored-By: Sean McGinnis <sean.mcginnis@gmail.com> Co-Authored-By: Jamie Lennox <jamielennox@gmail.com> Change-Id: Id4bf0e2088e8ad99e83cd4f9b8549c2aca1f65a2 --- cinderclient/client.py | 19 ++++++---- cinderclient/shell.py | 20 +++++----- .../tests/unit/fixture_data/client.py | 2 +- cinderclient/tests/unit/test_client.py | 13 ++++--- cinderclient/tests/unit/test_http.py | 30 +++++---------- cinderclient/tests/unit/test_shell.py | 37 +++++++++++-------- cinderclient/tests/unit/utils.py | 19 +++++++--- requirements.txt | 2 +- 8 files changed, 74 insertions(+), 68 deletions(-) diff --git a/cinderclient/client.py b/cinderclient/client.py index 8d43176fa..12ef43469 100644 --- a/cinderclient/client.py +++ b/cinderclient/client.py @@ -30,10 +30,10 @@ import pkgutil import re import six -from keystoneclient import access -from keystoneclient import adapter -from keystoneclient.auth.identity import base -from keystoneclient import discover +from keystoneauth1 import access +from keystoneauth1 import adapter +from keystoneauth1.identity import base +from keystoneauth1 import discover import requests from cinderclient import api_versions @@ -109,7 +109,7 @@ class SessionClient(adapter.LegacyJsonAdapter): api_versions.update_headers(kwargs["headers"], self.api_version) kwargs.setdefault('authenticated', False) # Note(tpatil): The standard call raises errors from - # keystoneclient, here we need to raise the cinderclient errors. + # keystoneauth, here we need to raise the cinderclient errors. raise_exc = kwargs.pop('raise_exc', True) resp, body = super(SessionClient, self).request(*args, raise_exc=False, @@ -427,7 +427,7 @@ class HTTPClient(object): if resp.status_code == 200: # content must always present try: self.auth_url = url - self.auth_ref = access.AccessInfo.factory(resp, body) + self.auth_ref = access.create(resp=resp, body=body) self.service_catalog = self.auth_ref.service_catalog if extract_token: @@ -435,7 +435,7 @@ class HTTPClient(object): management_url = self.service_catalog.url_for( region_name=self.region_name, - endpoint_type=self.endpoint_type, + interface=self.endpoint_type, service_type=self.service_type, service_name=self.service_name) self.management_url = management_url.rstrip('/') @@ -444,7 +444,10 @@ class HTTPClient(object): print("Found more than one valid endpoint. Use a more " "restrictive filter") raise - except KeyError: + except ValueError: + # ValueError is raised when you pass an invalid response to + # access.create. This should never happen in reality if the + # status code is 200. raise exceptions.AuthorizationFailure() except exceptions.EndpointNotFound: print("Could not find any suitable endpoint. Correct region?") diff --git a/cinderclient/shell.py b/cinderclient/shell.py index 39ceaf5f4..575370330 100644 --- a/cinderclient/shell.py +++ b/cinderclient/shell.py @@ -34,11 +34,12 @@ from cinderclient import utils import cinderclient.auth_plugin from cinderclient._i18n import _ -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 keystoneauth1 import discover +from keystoneauth1 import loading +from keystoneauth1 import session +from keystoneauth1.identity import v2 as v2_auth +from keystoneauth1.identity import v3 as v3_auth +from keystoneauth1.exceptions import DiscoveryFailure import six.moves.urllib.parse as urlparse from oslo_utils import encodeutils from oslo_utils import importutils @@ -390,7 +391,7 @@ class OpenStackCinderShell(object): help=argparse.SUPPRESS) # Register the CLI arguments that have moved to the session object. - session.Session.register_cli_options(parser) + loading.register_session_argparse_arguments(parser) parser.set_defaults(insecure=utils.env('CINDERCLIENT_INSECURE', default=False)) @@ -468,9 +469,8 @@ class OpenStackCinderShell(object): if hasattr(requests, 'logging'): requests.logging.getLogger(requests.__name__).addHandler(ch) - # required for logging when using a keystone session - self.ks_logger = logging.getLogger("keystoneclient") - self.ks_logger.setLevel(logging.DEBUG) + ks_logger = logging.getLogger("keystoneauth") + ks_logger.setLevel(logging.DEBUG) def _delimit_metadata_args(self, argv): """This function adds -- separator at the appropriate spot @@ -788,7 +788,7 @@ class OpenStackCinderShell(object): v2_auth_url = None v3_auth_url = None try: - ks_discover = discover.Discover(session=session, auth_url=auth_url) + ks_discover = discover.Discover(session=session, url=auth_url) v2_auth_url = ks_discover.url_for('2.0') v3_auth_url = ks_discover.url_for('3.0') except DiscoveryFailure: diff --git a/cinderclient/tests/unit/fixture_data/client.py b/cinderclient/tests/unit/fixture_data/client.py index 310ca4fce..9fd35dc31 100644 --- a/cinderclient/tests/unit/fixture_data/client.py +++ b/cinderclient/tests/unit/fixture_data/client.py @@ -10,7 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. -from keystoneclient import fixture +from keystoneauth1 import fixture from cinderclient.tests.unit.fixture_data import base from cinderclient.v1 import client as v1client diff --git a/cinderclient/tests/unit/test_client.py b/cinderclient/tests/unit/test_client.py index 296281877..4cd1acdd1 100644 --- a/cinderclient/tests/unit/test_client.py +++ b/cinderclient/tests/unit/test_client.py @@ -11,19 +11,20 @@ # See the License for the specific language governing permissions and # limitations under the License. -import logging import json +import logging import fixtures +from keystoneauth1 import adapter +from keystoneauth1 import exceptions as keystone_exception import mock +import six import cinderclient.client import cinderclient.v1.client import cinderclient.v2.client from cinderclient import exceptions from cinderclient.tests.unit import utils -from keystoneclient import adapter -from keystoneclient import exceptions as keystone_exception class ClientTest(utils.TestCase): @@ -115,7 +116,7 @@ class ClientTest(utils.TestCase): mock_response = utils.TestResponse({ "status_code": 202, - "text": json.dumps(resp), + "text": six.b(json.dumps(resp)), }) # 'request' method of Adaptor will return 202 response @@ -155,7 +156,7 @@ class ClientTest(utils.TestCase): mock_response = utils.TestResponse({ "status_code": 400, - "text": json.dumps(resp), + "text": six.b(json.dumps(resp)), }) # 'request' method of Adaptor will return 400 response @@ -182,7 +183,7 @@ class ClientTest(utils.TestCase): mock_response = utils.TestResponse({ "status_code": 413, - "text": json.dumps(resp), + "text": six.b(json.dumps(resp)), }) # 'request' method of Adaptor will return 413 response diff --git a/cinderclient/tests/unit/test_http.py b/cinderclient/tests/unit/test_http.py index 99ed1b21c..2a57e0754 100644 --- a/cinderclient/tests/unit/test_http.py +++ b/cinderclient/tests/unit/test_http.py @@ -12,7 +12,6 @@ # limitations under the License. import mock - import requests from cinderclient import client @@ -24,18 +23,17 @@ fake_response = utils.TestResponse({ "status_code": 200, "text": '{"hi": "there"}', }) - -fake_response_empty = utils.TestResponse({ - "status_code": 200, - "text": '{"access": {}}' -}) - mock_request = mock.Mock(return_value=(fake_response)) -mock_request_empty = mock.Mock(return_value=(fake_response_empty)) + +refused_response = utils.TestResponse({ + "status_code": 400, + "text": '[Errno 111] Connection refused', +}) +refused_mock_request = mock.Mock(return_value=(refused_response)) bad_400_response = utils.TestResponse({ "status_code": 400, - "text": '{"error": {"message": "n/a", "details": "Terrible!"}}', + "text": '', }) bad_400_request = mock.Mock(return_value=(bad_400_response)) @@ -74,6 +72,7 @@ def get_authed_client(retries=0): cl = get_client(retries=retries) cl.management_url = "http://example.com" cl.auth_token = "token" + cl.get_service_url = mock.Mock(return_value="http://example.com") return cl @@ -287,24 +286,13 @@ class ClientTest(utils.TestCase): cl = get_client() # response must not have x-server-management-url header - @mock.patch.object(requests, "request", mock_request_empty) + @mock.patch.object(requests, "request", mock_request) def test_auth_call(): self.assertRaises(exceptions.AuthorizationFailure, cl.authenticate) test_auth_call() - def test_auth_not_implemented(self): - cl = get_client() - - # response must not have x-server-management-url header - # {'hi': 'there'} is neither V2 or V3 - @mock.patch.object(requests, "request", mock_request) - def test_auth_call(): - self.assertRaises(NotImplementedError, cl.authenticate) - - test_auth_call() - def test_get_retry_timeout_error(self): cl = get_authed_client(retries=1) diff --git a/cinderclient/tests/unit/test_shell.py b/cinderclient/tests/unit/test_shell.py index f5566c028..3f0a272e2 100644 --- a/cinderclient/tests/unit/test_shell.py +++ b/cinderclient/tests/unit/test_shell.py @@ -16,6 +16,9 @@ import re import sys import fixtures +import keystoneauth1.exceptions as ks_exc +from keystoneauth1.exceptions import DiscoveryFailure +from keystoneauth1 import session import mock import pkg_resources import requests_mock @@ -31,8 +34,6 @@ 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 keystone_client from cinderclient.tests.unit import utils -import keystoneclient.exceptions as ks_exc -from keystoneclient.exceptions import DiscoveryFailure class ShellTest(utils.TestCase): @@ -103,23 +104,27 @@ class ShellTest(utils.TestCase): @requests_mock.Mocker() def test_version_discovery(self, mocker): _shell = shell.OpenStackCinderShell() + sess = session.Session() os_auth_url = "https://WrongDiscoveryResponse.discovery.com:35357/v2.0" self.register_keystone_auth_fixture(mocker, os_auth_url) - self.assertRaises(DiscoveryFailure, _shell._discover_auth_versions, - None, auth_url=os_auth_url) + + self.assertRaises(DiscoveryFailure, + _shell._discover_auth_versions, + sess, + auth_url=os_auth_url) os_auth_url = "https://DiscoveryNotSupported.discovery.com:35357/v2.0" self.register_keystone_auth_fixture(mocker, os_auth_url) - v2_url, v3_url = _shell._discover_auth_versions( - None, auth_url=os_auth_url) + v2_url, v3_url = _shell._discover_auth_versions(sess, + auth_url=os_auth_url) self.assertEqual(os_auth_url, v2_url, "Expected v2 url") self.assertIsNone(v3_url, "Expected no v3 url") os_auth_url = "https://DiscoveryNotSupported.discovery.com:35357/v3.0" self.register_keystone_auth_fixture(mocker, os_auth_url) - v2_url, v3_url = _shell._discover_auth_versions( - None, auth_url=os_auth_url) + v2_url, v3_url = _shell._discover_auth_versions(sess, + auth_url=os_auth_url) self.assertEqual(os_auth_url, v3_url, "Expected v3 url") self.assertIsNone(v2_url, "Expected no v2 url") @@ -142,18 +147,18 @@ class ShellTest(utils.TestCase): for count in range(1, 4): self.list_volumes_on_service(count) - @mock.patch('keystoneclient.auth.identity.v2.Password') - @mock.patch('keystoneclient.adapter.Adapter.get_token', - side_effect=ks_exc.ConnectionRefused()) - @mock.patch('keystoneclient.discover.Discover', - side_effect=ks_exc.ConnectionRefused()) + @mock.patch('keystoneauth1.identity.v2.Password') + @mock.patch('keystoneauth1.adapter.Adapter.get_token', + side_effect=ks_exc.ConnectFailure()) + @mock.patch('keystoneauth1.discover.Discover', + side_effect=ks_exc.ConnectFailure()) @mock.patch('sys.stdin', side_effect=mock.MagicMock) @mock.patch('getpass.getpass', return_value='password') def test_password_prompted(self, mock_getpass, mock_stdin, mock_discover, mock_token, mock_password): self.make_env(exclude='OS_PASSWORD') _shell = shell.OpenStackCinderShell() - self.assertRaises(ks_exc.ConnectionRefused, _shell.main, ['list']) + self.assertRaises(ks_exc.ConnectFailure, _shell.main, ['list']) mock_getpass.assert_called_with('OS Password: ') # Verify that Password() is called with value of param 'password' # equal to mock_getpass.return_value. @@ -223,7 +228,7 @@ class ShellTest(utils.TestCase): self.assertEqual(False, _shell.cs.client.verify_cert) - @mock.patch('keystoneclient.session.Session.__init__', + @mock.patch('keystoneauth1.session.Session.__init__', side_effect=RuntimeError()) def test_http_client_with_cert(self, mock_session): _shell = shell.OpenStackCinderShell() @@ -234,7 +239,7 @@ class ShellTest(utils.TestCase): self.assertRaises(RuntimeError, _shell.main, args) mock_session.assert_called_once_with(cert='minnie', verify=mock.ANY) - @mock.patch('keystoneclient.session.Session.__init__', + @mock.patch('keystoneauth1.session.Session.__init__', side_effect=RuntimeError()) def test_http_client_with_cert_and_key(self, mock_session): _shell = shell.OpenStackCinderShell() diff --git a/cinderclient/tests/unit/utils.py b/cinderclient/tests/unit/utils.py index db697a075..a19f71047 100644 --- a/cinderclient/tests/unit/utils.py +++ b/cinderclient/tests/unit/utils.py @@ -87,25 +87,34 @@ class FixturedTestCase(TestCase): class TestResponse(requests.Response): - """Class used to wrap requests.Response and provide some - convenience to initialize with a dict. + """Class used to wrap requests.Response. + + Provides some convenience to initialize with a dict. """ def __init__(self, data): + super(TestResponse, self).__init__() + self._content = None self._text = None - super(TestResponse, self) + if isinstance(data, dict): self.status_code = data.get('status_code', None) self.headers = data.get('headers', None) self.reason = data.get('reason', '') - # Fake the text attribute to streamline Response creation - self._text = data.get('text', None) + # Fake text and content attributes to streamline Response creation + text = data.get('text', None) + self._content = text + self._text = text else: self.status_code = data def __eq__(self, other): return self.__dict__ == other.__dict__ + @property + def content(self): + return self._content + @property def text(self): return self._text diff --git a/requirements.txt b/requirements.txt index db693e678..196d870f1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ # process, which may cause wedges in the gate later. pbr>=1.6 # Apache-2.0 PrettyTable<0.8,>=0.7 # BSD -python-keystoneclient!=1.8.0,!=2.1.0,>=1.7.0 # Apache-2.0 +keystoneauth1>=2.7.0 # Apache-2.0 requests>=2.10.0 # Apache-2.0 simplejson>=2.2.0 # MIT Babel>=2.3.4 # BSD