openstack/python-swiftclient
Code Issues Proposed changes

Merge "Stop lazy importing keystoneclient"

Browse Source
This commit is contained in:
Zuul 2018-10-18 18:58:15 +00:00 committed by Gerrit Code Review
commit 5b6e3822c5
4 changed files with 68 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
swiftclient/client.py
View File

@ -60,6 +60,19 @@ except ImportError:
def createLock(self):
self.lock = None
ksexceptions = ksclient_v2 = ksclient_v3 = None
try:
from keystoneclient import exceptions as ksexceptions
# prevent keystoneclient warning us that it has no log handlers
logging.getLogger('keystoneclient').addHandler(NullHandler())
from keystoneclient.v2_0 import client as ksclient_v2
except ImportError:
pass
try:
from keystoneclient.v3 import client as ksclient_v3
except ImportError:
pass
# requests version 1.2.3 try to encode headers in ascii, preventing
# utf-8 encoded header to be 'prepared'
if StrictVersion(requests.__version__) < StrictVersion('2.0.0'):
@ -540,25 +553,6 @@ def get_keystoneclient_2_0(auth_url, user, key, os_options, **kwargs):
return get_auth_keystone(auth_url, user, key, os_options, **kwargs)
def _import_keystone_client(auth_version):
# the attempted imports are encapsulated in this function to allow
# mocking for tests
try:
if auth_version in AUTH_VERSIONS_V3:
from keystoneclient.v3 import client as ksclient
else:
from keystoneclient.v2_0 import client as ksclient
from keystoneclient import exceptions
# prevent keystoneclient warning us that it has no log handlers
logging.getLogger('keystoneclient').addHandler(NullHandler())
return ksclient, exceptions
except ImportError:
raise ClientException('''
Auth versions 2.0 and 3 require python-keystoneclient, install it or use Auth
version 1.0 which requires ST_AUTH, ST_USER, and ST_KEY environment
variables to be set or overridden with -A, -U, or -K.''')
def get_auth_keystone(auth_url, user, key, os_options, **kwargs):
"""
Authenticate against a keystone server.
@ -587,7 +581,20 @@ def get_auth_keystone(auth_url, user, key, os_options, **kwargs):
# Legacy default if not set
if auth_version is None:
auth_version = '2'
ksclient, exceptions = _import_keystone_client(auth_version)
ksclient = None
if auth_version in AUTH_VERSIONS_V3:
if ksclient_v3 is not None:
ksclient = ksclient_v3
else:
if ksclient_v2 is not None:
ksclient = ksclient_v2
if ksclient is None:
raise ClientException('''
Auth versions 2.0 and 3 require python-keystoneclient, install it or use Auth
version 1.0 which requires ST_AUTH, ST_USER, and ST_KEY environment
variables to be set or overridden with -A, -U, or -K.''')
try:
_ksclient = ksclient.Client(
@ -608,13 +615,13 @@ def get_auth_keystone(auth_url, user, key, os_options, **kwargs):
cert=kwargs.get('cert'),
key=kwargs.get('cert_key'),
auth_url=auth_url, insecure=insecure, timeout=timeout)
except exceptions.Unauthorized:
except ksexceptions.Unauthorized:
msg = 'Unauthorized. Check username, password and tenant name/id.'
if auth_version in AUTH_VERSIONS_V3:
msg = ('Unauthorized. Check username/id, password, '
'tenant name/id and user/tenant domain name/id.')
raise ClientException(msg)
except exceptions.AuthorizationFailure as err:
except ksexceptions.AuthorizationFailure as err:
raise ClientException('Authorization Failure. %s' % err)
service_type = os_options.get('service_type') or 'object-store'
endpoint_type = os_options.get('endpoint_type') or 'publicURL'
@ -627,7 +634,7 @@ def get_auth_keystone(auth_url, user, key, os_options, **kwargs):
service_type=service_type,
endpoint_type=endpoint_type,
**filter_kwargs)
except exceptions.EndpointNotFound:
except ksexceptions.EndpointNotFound:
raise ClientException('Endpoint for %s not found - '
'have you specified a region?' % service_type)
return endpoint, _ksclient.auth_token

63
tests/unit/test_shell.py
View File

@ -37,7 +37,7 @@ import swiftclient.utils
from os.path import basename, dirname
from .utils import (
CaptureOutput, fake_get_auth_keystone, _make_fake_import_keystone_client,
CaptureOutput, fake_get_auth_keystone,
FakeKeystone, StubResponse, MockHttpTest)
from swiftclient.utils import (
EMPTY_ETAG, EXPIRES_ISO8601_FORMAT,
@ -2534,7 +2534,17 @@ class TestKeystoneOptions(MockHttpTest):
cmd_args=cmd_args)
ks_endpoint = 'http://example.com:8080/v1/AUTH_acc'
ks_token = 'fake_auth_token'
# check correct auth version gets used
key = 'auth-version'
fake_ks = FakeKeystone(endpoint=ks_endpoint, token=ks_token)
if no_auth:
fake_ks2 = fake_ks3 = None
elif opts.get(key, self.defaults.get(key)) == '2.0':
fake_ks2 = fake_ks
fake_ks3 = None
else:
fake_ks2 = None
fake_ks3 = fake_ks
# fake_conn will check that storage_url and auth_token are as expected
endpoint = os_opts.get('storage-url', ks_endpoint)
token = os_opts.get('auth-token', ks_token)
@ -2542,8 +2552,8 @@ class TestKeystoneOptions(MockHttpTest):
storage_url=endpoint,
auth_token=token)
with mock.patch('swiftclient.client._import_keystone_client',
_make_fake_import_keystone_client(fake_ks)), \
with mock.patch('swiftclient.client.ksclient_v2', fake_ks2), \
mock.patch('swiftclient.client.ksclient_v3', fake_ks3), \
mock.patch('swiftclient.client.http_connection', fake_conn), \
mock.patch.dict(os.environ, env, clear=True), \
patch_disable_warnings() as mock_disable_warnings:
@ -2562,16 +2572,11 @@ class TestKeystoneOptions(MockHttpTest):
self.assertEqual([], mock_disable_warnings.mock_calls)
if no_auth:
# check that keystone client was not used and terminate tests
self.assertIsNone(getattr(fake_ks, 'auth_version'))
self.assertEqual(len(fake_ks.calls), 0)
# We patched out both keystoneclient versions to be None;
# they *can't* have been used and if we tried to, we would
# have raised ClientExceptions
return
# check correct auth version was passed to _import_keystone_client
key = 'auth-version'
expected = opts.get(key, self.defaults.get(key))
self.assertEqual(expected, fake_ks.auth_version)
# check args passed to keystone Client __init__
self.assertEqual(len(fake_ks.calls), 1)
actual_args = fake_ks.calls[0]
@ -2942,9 +2947,9 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
self.account = 'AUTH_alice'
# keystone returns endpoint for another account
fake_ks = FakeKeystone(endpoint='http://example.com:8080/v1/AUTH_bob',
token='bob_token')
self.fake_ks_import = _make_fake_import_keystone_client(fake_ks)
self.fake_ks = FakeKeystone(
endpoint='http://example.com:8080/v1/AUTH_bob',
token='bob_token')
self.cont = 'c1'
self.cont_path = '/v1/%s/%s' % (self.account, self.cont)
@ -3023,8 +3028,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
args, env = self._make_cmd('upload', cmd_args=[self.cont, self.obj,
'--leave-segments'])
with mock.patch('swiftclient.client._import_keystone_client',
self.fake_ks_import):
with mock.patch('swiftclient.client.ksclient_v3', self.fake_ks):
with mock.patch('swiftclient.client.http_connection', fake_conn):
with mock.patch.dict(os.environ, env):
with CaptureOutput() as out:
@ -3046,8 +3050,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
on_request=req_handler)
args, env = self._make_cmd('upload', cmd_args=[self.cont, self.obj,
'--leave-segments'])
with mock.patch('swiftclient.client._import_keystone_client',
self.fake_ks_import):
with mock.patch('swiftclient.client.ksclient_v3', self.fake_ks):
with mock.patch('swiftclient.client.http_connection', fake_conn):
with mock.patch.dict(os.environ, env):
with CaptureOutput() as out:
@ -3073,8 +3076,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
'--segment-size=10',
'--segment-container=%s'
% self.cont])
with mock.patch('swiftclient.client._import_keystone_client',
self.fake_ks_import):
with mock.patch('swiftclient.client.ksclient_v3', self.fake_ks):
with mock.patch('swiftclient.client.http_connection', fake_conn):
with mock.patch.dict(os.environ, env):
with CaptureOutput() as out:
@ -3112,8 +3114,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
cmd_args=[self.cont, self.obj,
'--leave-segments',
'--segment-size=10'])
with mock.patch('swiftclient.client._import_keystone_client',
self.fake_ks_import):
with mock.patch('swiftclient.client.ksclient_v3', self.fake_ks):
with mock.patch('swiftclient.client.http_connection', fake_conn):
with mock.patch.dict(os.environ, env):
with CaptureOutput() as out:
@ -3149,8 +3150,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
args, env = self._make_cmd('upload', cmd_args=[self.cont, self.obj,
'--leave-segments'])
with mock.patch('swiftclient.client._import_keystone_client',
self.fake_ks_import):
with mock.patch('swiftclient.client.ksclient_v3', self.fake_ks):
with mock.patch('swiftclient.client.http_connection', fake_conn):
with mock.patch.dict(os.environ, env):
with CaptureOutput() as out:
@ -3207,8 +3207,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
args, env = self._make_cmd('download', cmd_args=[self.cont,
self.obj.lstrip('/'),
'--no-download'])
with mock.patch('swiftclient.client._import_keystone_client',
self.fake_ks_import):
with mock.patch('swiftclient.client.ksclient_v3', self.fake_ks):
with mock.patch('swiftclient.client.http_connection', fake_conn):
with mock.patch.dict(os.environ, env):
with CaptureOutput() as out:
@ -3229,8 +3228,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
args, env = self._make_cmd('download', cmd_args=[self.cont,
self.obj.lstrip('/'),
'--no-download'])
with mock.patch('swiftclient.client._import_keystone_client',
self.fake_ks_import):
with mock.patch('swiftclient.client.ksclient_v3', self.fake_ks):
with mock.patch('swiftclient.client.http_connection', fake_conn):
with mock.patch.dict(os.environ, env):
with CaptureOutput() as out:
@ -3248,8 +3246,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
args, env = self._make_cmd('download', cmd_args=[self.cont,
self.obj.lstrip('/'),
'--no-download'])
with mock.patch('swiftclient.client._import_keystone_client',
self.fake_ks_import):
with mock.patch('swiftclient.client.ksclient_v3', self.fake_ks):
with mock.patch('swiftclient.client.http_connection', fake_conn):
with mock.patch.dict(os.environ, env):
with CaptureOutput() as out:
@ -3273,8 +3270,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
fake_conn = self.fake_http_connection(resp, on_request=req_handler)
args, env = self._make_cmd('download', cmd_args=[self.cont])
with mock.patch('swiftclient.client._import_keystone_client',
self.fake_ks_import):
with mock.patch('swiftclient.client.ksclient_v3', self.fake_ks):
with mock.patch('swiftclient.client.http_connection', fake_conn):
with mock.patch.dict(os.environ, env):
with CaptureOutput() as out:
@ -3291,8 +3287,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
fake_conn = self.fake_http_connection(403)
args, env = self._make_cmd('download', cmd_args=[self.cont])
with mock.patch('swiftclient.client._import_keystone_client',
self.fake_ks_import):
with mock.patch('swiftclient.client.ksclient_v3', self.fake_ks):
with mock.patch('swiftclient.client.http_connection', fake_conn):
with mock.patch.dict(os.environ, env):
with CaptureOutput() as out:

26
tests/unit/test_swiftclient.py
View File

@ -29,7 +29,7 @@ from six.moves.urllib.parse import urlparse
from requests.exceptions import RequestException
from .utils import (MockHttpTest, fake_get_auth_keystone, StubResponse,
FakeKeystone, _make_fake_import_keystone_client)
FakeKeystone)
from swiftclient.utils import EMPTY_ETAG
from swiftclient.exceptions import ClientException
@ -322,8 +322,7 @@ class TestGetAuth(MockHttpTest):
# TestConnection.test_timeout_passed_down but is required to check that
# get_auth does the right thing when it is not passed a timeout arg
fake_ks = FakeKeystone(endpoint='http://some_url', token='secret')
with mock.patch('swiftclient.client._import_keystone_client',
_make_fake_import_keystone_client(fake_ks)):
with mock.patch('swiftclient.client.ksclient_v2', fake_ks):
c.get_auth('http://www.test.com', 'asdf', 'asdf',
os_options=dict(tenant_name='tenant'),
auth_version="2.0", timeout=42.0)
@ -580,8 +579,7 @@ class TestGetAuth(MockHttpTest):
def test_get_auth_keystone_versionless(self):
fake_ks = FakeKeystone(endpoint='http://some_url', token='secret')
with mock.patch('swiftclient.client._import_keystone_client',
_make_fake_import_keystone_client(fake_ks)):
with mock.patch('swiftclient.client.ksclient_v3', fake_ks):
c.get_auth_keystone('http://authurl', 'user', 'key', {})
self.assertEqual(1, len(fake_ks.calls))
self.assertEqual('http://authurl/v3', fake_ks.calls[0].get('auth_url'))
@ -589,8 +587,7 @@ class TestGetAuth(MockHttpTest):
def test_get_auth_keystone_versionless_auth_version_set(self):
fake_ks = FakeKeystone(endpoint='http://some_url', token='secret')
with mock.patch('swiftclient.client._import_keystone_client',
_make_fake_import_keystone_client(fake_ks)):
with mock.patch('swiftclient.client.ksclient_v2', fake_ks):
c.get_auth_keystone('http://auth_url', 'user', 'key',
{}, auth_version='2.0')
self.assertEqual(1, len(fake_ks.calls))
@ -600,8 +597,7 @@ class TestGetAuth(MockHttpTest):
def test_get_auth_keystone_versionful(self):
fake_ks = FakeKeystone(endpoint='http://some_url', token='secret')
with mock.patch('swiftclient.client._import_keystone_client',
_make_fake_import_keystone_client(fake_ks)):
with mock.patch('swiftclient.client.ksclient_v3', fake_ks):
c.get_auth_keystone('http://auth_url/v3', 'user', 'key',
{}, auth_version='3')
self.assertEqual(1, len(fake_ks.calls))
@ -611,8 +607,7 @@ class TestGetAuth(MockHttpTest):
def test_get_auth_keystone_devstack_versionful(self):
fake_ks = FakeKeystone(
endpoint='http://storage.example.com/v1/AUTH_user', token='secret')
with mock.patch('swiftclient.client._import_keystone_client',
_make_fake_import_keystone_client(fake_ks)):
with mock.patch('swiftclient.client.ksclient_v3', fake_ks):
c.get_auth_keystone('https://192.168.8.8/identity/v3',
'user', 'key', {}, auth_version='3')
self.assertEqual(1, len(fake_ks.calls))
@ -622,8 +617,7 @@ class TestGetAuth(MockHttpTest):
def test_get_auth_keystone_devstack_versionless(self):
fake_ks = FakeKeystone(
endpoint='http://storage.example.com/v1/AUTH_user', token='secret')
with mock.patch('swiftclient.client._import_keystone_client',
_make_fake_import_keystone_client(fake_ks)):
with mock.patch('swiftclient.client.ksclient_v3', fake_ks):
c.get_auth_keystone('https://192.168.8.8/identity',
'user', 'key', {}, auth_version='3')
self.assertEqual(1, len(fake_ks.calls))
@ -634,8 +628,7 @@ class TestGetAuth(MockHttpTest):
fake_ks = FakeKeystone(
endpoint='http://storage.example.com/v1/AUTH_user',
token='secret')
with mock.patch('swiftclient.client._import_keystone_client',
_make_fake_import_keystone_client(fake_ks)):
with mock.patch('swiftclient.client.ksclient_v3', fake_ks):
c.get_auth_keystone('http://blah.example.com/v2moo',
'user', 'key', {}, auth_version='3')
self.assertEqual(1, len(fake_ks.calls))
@ -2456,8 +2449,7 @@ class TestConnection(MockHttpTest):
'http://auth.example.com', 'user', 'password', timeout=33.0,
os_options=os_options, auth_version=2.0)
fake_ks = FakeKeystone(endpoint='http://some_url', token='secret')
with mock.patch('swiftclient.client._import_keystone_client',
_make_fake_import_keystone_client(fake_ks)):
with mock.patch('swiftclient.client.ksclient_v2', fake_ks):
with mock.patch.multiple('swiftclient.client',
http_connection=shim_connection,
sleep=mock.DEFAULT):

8
tests/unit/utils.py
View File

@ -542,14 +542,6 @@ class FakeKeystone(object):
pass
def _make_fake_import_keystone_client(fake_import):
def _fake_import_keystone_client(auth_version):
fake_import.auth_version = auth_version
return fake_import, fake_import
return _fake_import_keystone_client
class FakeStream(object):
def __init__(self, size):
self.bytes_read = 0