diff --git a/swiftclient/client.py b/swiftclient/client.py index def40bf4..8cbedc74 100644 --- a/swiftclient/client.py +++ b/swiftclient/client.py @@ -396,7 +396,9 @@ def get_auth(auth_url, user, key, **kwargs): if not (os_options.get('tenant_name') or os_options.get('tenant_id') or os_options.get('project_name') or os_options.get('project_id')): - raise ClientException('No tenant specified') + if auth_version in AUTH_VERSIONS_V2: + raise ClientException('No tenant specified') + raise ClientException('No project name or project id specified.') cacert = kwargs.get('cacert', None) storage_url, token = get_auth_keystone(auth_url, user, diff --git a/swiftclient/service.py b/swiftclient/service.py index a91c6050..15decf7f 100644 --- a/swiftclient/service.py +++ b/swiftclient/service.py @@ -434,7 +434,7 @@ class SwiftService(object): }) return res except ClientException as err: - if err.http_status == 404: + if err.http_status != 404: res.update({ 'success': False, 'error': err diff --git a/swiftclient/shell.py b/swiftclient/shell.py index 619ec5c6..015c9d9f 100755 --- a/swiftclient/shell.py +++ b/swiftclient/shell.py @@ -488,13 +488,15 @@ def st_stat(parser, args, output_manager): _opts = vars(options) with SwiftService(options=_opts) as swift: - if not args: - stat_result = swift.stat() - items = stat_result['items'] - headers = stat_result['headers'] - print_account_stats(items, headers, output_manager) - else: - try: + try: + if not args: + stat_result = swift.stat() + if not stat_result['success']: + raise stat_result['error'] + items = stat_result['items'] + headers = stat_result['headers'] + print_account_stats(items, headers, output_manager) + else: container = args[0] if '/' in container: output_manager.error( @@ -505,6 +507,8 @@ def st_stat(parser, args, output_manager): args = args[1:] if not args: stat_result = swift.stat(container=container) + if not stat_result['success']: + raise stat_result['error'] items = stat_result['items'] headers = stat_result['headers'] print_container_stats(items, headers, output_manager) @@ -527,8 +531,8 @@ def st_stat(parser, args, output_manager): 'Usage: %s stat %s\n%s', BASENAME, st_stat_options, st_stat_help) - except SwiftError as e: - output_manager.error(e.value) + except SwiftError as e: + output_manager.error(e.value) st_post_options = '''[--read-acl <acl>] [--write-acl <acl>] [--sync-to] @@ -961,8 +965,8 @@ def parse_args(parser, args, enforce_requires=True): if (not (options.auth and options.user and options.key) and options.auth_version != '3'): - # Use keystone auth if any of the old-style args are missing - options.auth_version = '2.0' + # Use keystone auth if any of the old-style args are missing + options.auth_version = '2.0' # Use new-style args if old ones not present if not options.auth and options.os_auth_url: diff --git a/tests/unit/test_shell.py b/tests/unit/test_shell.py index 8e61cb42..29d74729 100644 --- a/tests/unit/test_shell.py +++ b/tests/unit/test_shell.py @@ -24,6 +24,7 @@ import swiftclient from swiftclient.service import SwiftError import swiftclient.shell import swiftclient.utils +from swiftclient.multithreading import OutputManager from os.path import basename, dirname from tests.unit.test_swiftclient import MockHttpTest @@ -551,13 +552,13 @@ class TestParsing(unittest.TestCase): # check the expected opts are set for key, v in opts.items(): actual = getattr(actual_opts, key) - self.assertEqual(v, actual, 'Expected %s for key %s, found %s' - % (v, key, actual)) + self.assertEqual(v, actual, 'Expected %s for key %s, found %s' % + (v, key, actual)) for key, v in os_opts.items(): actual = getattr(actual_opts, "os_" + key) - self.assertEqual(v, actual, 'Expected %s for key %s, found %s' - % (v, key, actual)) + self.assertEqual(v, actual, 'Expected %s for key %s, found %s' % + (v, key, actual)) # check the os_options dict values are set self.assertTrue(hasattr(actual_opts, 'os_options')) @@ -725,6 +726,55 @@ class TestParsing(unittest.TestCase): args = _make_args("stat", opts, os_opts) self.assertRaises(SystemExit, swiftclient.shell.main, args) + def test_no_tenant_name_or_id_v2(self): + os_opts = {"password": "secret", + "username": "user", + "auth_url": "http://example.com:5000/v3", + "tenant_name": "", + "tenant_id": ""} + + out = six.StringIO() + err = six.StringIO() + mock_output = _make_output_manager(out, err) + with mock.patch('swiftclient.shell.OutputManager', mock_output): + args = _make_args("stat", {}, os_opts) + self.assertRaises(SystemExit, swiftclient.shell.main, args) + self.assertEqual(err.getvalue().strip(), 'No tenant specified') + + out = six.StringIO() + err = six.StringIO() + mock_output = _make_output_manager(out, err) + with mock.patch('swiftclient.shell.OutputManager', mock_output): + args = _make_args("stat", {}, os_opts, cmd_args=["testcontainer"]) + self.assertRaises(SystemExit, swiftclient.shell.main, args) + self.assertEqual(err.getvalue().strip(), 'No tenant specified') + + def test_no_tenant_name_or_id_v3(self): + os_opts = {"password": "secret", + "username": "user", + "auth_url": "http://example.com:5000/v3", + "tenant_name": "", + "tenant_id": ""} + + out = six.StringIO() + err = six.StringIO() + mock_output = _make_output_manager(out, err) + with mock.patch('swiftclient.shell.OutputManager', mock_output): + args = _make_args("stat", {"auth_version": "3"}, os_opts) + self.assertRaises(SystemExit, swiftclient.shell.main, args) + self.assertEqual(err.getvalue().strip(), + 'No project name or project id specified.') + + out = six.StringIO() + err = six.StringIO() + mock_output = _make_output_manager(out, err) + with mock.patch('swiftclient.shell.OutputManager', mock_output): + args = _make_args("stat", {"auth_version": "3"}, + os_opts, cmd_args=["testcontainer"]) + self.assertRaises(SystemExit, swiftclient.shell.main, args) + self.assertEqual(err.getvalue().strip(), + 'No project name or project id specified.') + def test_insufficient_env_vars_v3(self): args = _make_args("stat", {}, {}) opts = {"auth_version": "3"} @@ -1043,3 +1093,18 @@ class TestKeystoneOptions(MockHttpTest): opts = {'auth-version': '2.0'} self._test_options(opts, os_opts) + + +def _make_output_manager(stdout, stderr): + class MockOutputManager(OutputManager): + # This class is used to mock OutputManager so that we can + # override stdout and stderr. Mocking sys.stdout & sys.stdout + # doesn't work because they are argument defaults in the + # OutputManager constructor and those defaults are pinned to + # the value of sys.stdout/stderr before we get chance to mock them. + def __init__(self, print_stream=None, error_stream=None): + super(MockOutputManager, self).__init__() + self.print_stream = stdout + self.error_stream = stderr + + return MockOutputManager