Skip to content

Commit

Permalink
Merge "Stop lazy importing keystoneclient"
Browse files Browse the repository at this point in the history
  • Loading branch information
@openstack-gerrit
Zuul authored and openstack-gerrit committed Oct 18, 2018
2 parents a6baf00 + d1e1f8d commit 5b6e382
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 82 deletions.
53 changes: 30 additions & 23 deletions swiftclient/client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,19 @@ def emit(self, record):
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'):
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand All @@ -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'
Expand All @@ -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
Expand Down
63 changes: 29 additions & 34 deletions tests/unit/test_shell.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

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,
Expand Down Expand Up @@ -2534,16 +2534,26 @@ def _test_options_passed_to_keystone(self, cmd, opts, os_opts,
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)
fake_conn = self.fake_http_connection(204, headers={},
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:
Expand All @@ -2562,16 +2572,11 @@ def _test_options_passed_to_keystone(self, cmd, opts, os_opts,
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]
Expand Down Expand Up @@ -2942,9 +2947,9 @@ def setUp(self):
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)
Expand Down Expand Up @@ -3023,8 +3028,7 @@ def test_upload_with_read_write_access(self):

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:
Expand All @@ -3046,8 +3050,7 @@ def test_upload_with_write_only_access(self):
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:
Expand All @@ -3073,8 +3076,7 @@ def test_segment_upload_with_write_only_access(self):
'--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:
Expand Down Expand Up @@ -3112,8 +3114,7 @@ def test_segment_upload_with_write_only_access_segments_container(self):
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:
Expand Down Expand Up @@ -3149,8 +3150,7 @@ def test_upload_with_no_access(self):

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:
Expand Down Expand Up @@ -3207,8 +3207,7 @@ def test_download_with_read_write_access(self):
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:
Expand All @@ -3229,8 +3228,7 @@ def test_download_with_read_only_access(self):
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:
Expand All @@ -3248,8 +3246,7 @@ def test_download_with_no_access(self):
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:
Expand All @@ -3273,8 +3270,7 @@ def test_list_with_read_access(self):
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:
Expand All @@ -3291,8 +3287,7 @@ def test_list_with_no_access(self):
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:
Expand Down
26 changes: 9 additions & 17 deletions tests/unit/test_swiftclient.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
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
Expand Down Expand Up @@ -322,8 +322,7 @@ def test_auth_v2_timeout(self):
# 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)
Expand Down Expand Up @@ -580,17 +579,15 @@ def test_get_keystone_client_2_0(self):
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'))

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))
Expand All @@ -600,8 +597,7 @@ def test_get_auth_keystone_versionless_auth_version_set(self):
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))
Expand All @@ -611,8 +607,7 @@ def test_get_auth_keystone_versionful(self):
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))
Expand All @@ -622,8 +617,7 @@ def test_get_auth_keystone_devstack_versionful(self):
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))
Expand All @@ -634,8 +628,7 @@ def test_auth_keystone_url_some_junk_nonsense(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('http://blah.example.com/v2moo',
'user', 'key', {}, auth_version='3')
self.assertEqual(1, len(fake_ks.calls))
Expand Down Expand Up @@ -2456,8 +2449,7 @@ def shim_connection(*a, **kw):
'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):
Expand Down
8 changes: 0 additions & 8 deletions tests/unit/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,14 +542,6 @@ class EndpointNotFound(Exception):
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
Expand Down

0 comments on commit 5b6e382

Please sign in to comment.