From 6a15f90daed6514e30b4af0ebb52c7864acaafcc Mon Sep 17 00:00:00 2001 From: Dean Troyer Date: Thu, 23 Jun 2016 17:51:54 -0500 Subject: [PATCH] osc-lib: shell Convert to using ClientManager and OpenStackShell from osc-lib. * Change all internal uses of ClientManager private attributes that are now public in osc-lib's ClientManager. Leave back-compat copies in place in OSC's clientManager so we don't break plugins. * Put some work-arounds in place for changes in osc-lib that we need until a new release makes it through the g-r and u-c change process. * Add a test for Unicode decoding of argv in shell.main() to parallel the one in osc-lib. Change-Id: I85289740d4ca081f2aca8c9b40ec422ad25d302c --- openstackclient/common/clientmanager.py | 2 +- openstackclient/compute/client.py | 4 +- openstackclient/identity/client.py | 7 +- openstackclient/image/client.py | 12 +- openstackclient/network/client.py | 4 +- openstackclient/object/client.py | 4 +- openstackclient/shell.py | 389 +--------------- openstackclient/tests/test_shell.py | 592 +++++------------------- openstackclient/volume/client.py | 4 +- test-requirements.txt | 1 + 10 files changed, 149 insertions(+), 870 deletions(-) diff --git a/openstackclient/common/clientmanager.py b/openstackclient/common/clientmanager.py index 2105a49711..ccfde2d0d3 100644 --- a/openstackclient/common/clientmanager.py +++ b/openstackclient/common/clientmanager.py @@ -86,7 +86,7 @@ def get_plugin_modules(group): # Add the plugin to the ClientManager setattr( - ClientManager, + clientmanager.ClientManager, module.API_NAME, clientmanager.ClientCache( getattr(sys.modules[ep.module_name], 'make_client', None) diff --git a/openstackclient/compute/client.py b/openstackclient/compute/client.py index 1583676a8c..4cc3be9855 100644 --- a/openstackclient/compute/client.py +++ b/openstackclient/compute/client.py @@ -64,7 +64,7 @@ def make_client(instance): if ext.name == "list_extensions"] # Remember interface only if it is set - kwargs = utils.build_kwargs_dict('endpoint_type', instance._interface) + kwargs = utils.build_kwargs_dict('endpoint_type', instance.interface) client = nova_client.Client( version, @@ -72,7 +72,7 @@ def make_client(instance): extensions=extensions, http_log_debug=http_log_debug, timings=instance.timing, - region_name=instance._region_name, + region_name=instance.region_name, **kwargs ) diff --git a/openstackclient/identity/client.py b/openstackclient/identity/client.py index 0c609313f9..0292aac216 100644 --- a/openstackclient/identity/client.py +++ b/openstackclient/identity/client.py @@ -16,7 +16,6 @@ import logging from keystoneclient.v2_0 import client as identity_client_v2 -from osc_lib.api import auth from osc_lib import utils from openstackclient.i18n import _ @@ -50,11 +49,11 @@ def make_client(instance): LOG.debug('Instantiating identity client: %s', identity_client) # Remember interface only if interface is set - kwargs = utils.build_kwargs_dict('interface', instance._interface) + kwargs = utils.build_kwargs_dict('interface', instance.interface) client = identity_client( session=instance.session, - region_name=instance._region_name, + region_name=instance.region_name, **kwargs ) @@ -70,7 +69,7 @@ def build_option_parser(parser): help=_('Identity API version, default=%s ' '(Env: OS_IDENTITY_API_VERSION)') % DEFAULT_API_VERSION, ) - return auth.build_auth_plugins_option_parser(parser) + return parser class IdentityClientv2(identity_client_v2.Client): diff --git a/openstackclient/image/client.py b/openstackclient/image/client.py index 0ef694f46d..1be6c7654d 100644 --- a/openstackclient/image/client.py +++ b/openstackclient/image/client.py @@ -47,15 +47,15 @@ def make_client(instance): endpoint = instance.get_endpoint_for_service_type( API_NAME, - region_name=instance._region_name, - interface=instance._interface, + region_name=instance.region_name, + interface=instance.interface, ) client = image_client( endpoint, token=instance.auth.get_token(instance.session), - cacert=instance._cacert, - insecure=instance._insecure, + cacert=instance.cacert, + insecure=not instance.verify, ) # Create the low-level API @@ -70,8 +70,8 @@ def make_client(instance): session=instance.session, endpoint=instance.get_endpoint_for_service_type( IMAGE_API_TYPE, - region_name=instance._region_name, - interface=instance._interface, + region_name=instance.region_name, + interface=instance.interface, ) ) diff --git a/openstackclient/network/client.py b/openstackclient/network/client.py index d12987dd5d..c562058da5 100644 --- a/openstackclient/network/client.py +++ b/openstackclient/network/client.py @@ -34,9 +34,9 @@ API_VERSIONS = { def make_client(instance): """Returns a network proxy""" prof = profile.Profile() - prof.set_region(API_NAME, instance._region_name) + prof.set_region(API_NAME, instance.region_name) prof.set_version(API_NAME, instance._api_version[API_NAME]) - prof.set_interface(API_NAME, instance._interface) + prof.set_interface(API_NAME, instance.interface) conn = connection.Connection(authenticator=instance.session.auth, verify=instance.session.verify, cert=instance.session.cert, diff --git a/openstackclient/object/client.py b/openstackclient/object/client.py index cb3ddc288f..865f18f6de 100644 --- a/openstackclient/object/client.py +++ b/openstackclient/object/client.py @@ -32,8 +32,8 @@ def make_client(instance): endpoint = instance.get_endpoint_for_service_type( 'object-store', - region_name=instance._region_name, - interface=instance._interface, + region_name=instance.region_name, + interface=instance.interface, ) client = object_store_v1.APIv1( diff --git a/openstackclient/shell.py b/openstackclient/shell.py index d6ce4ef661..7a042e1ec7 100644 --- a/openstackclient/shell.py +++ b/openstackclient/shell.py @@ -16,30 +16,17 @@ """Command-line interface to the OpenStack APIs""" -import argparse -import getpass import locale -import logging -import six import sys -import traceback -from cliff import app -from cliff import command -from cliff import complete -from cliff import help -from osc_lib.cli import client_config as cloud_config -from osc_lib.command import timing -from osc_lib import exceptions as exc -from osc_lib import logs -from osc_lib import utils +from osc_lib.api import auth +from osc_lib import shell from oslo_utils import importutils -from oslo_utils import strutils +import six import openstackclient from openstackclient.common import clientmanager from openstackclient.common import commandmanager -from openstackclient.i18n import _ osprofiler_profiler = importutils.try_import("osprofiler.profiler") @@ -47,47 +34,9 @@ osprofiler_profiler = importutils.try_import("osprofiler.profiler") DEFAULT_DOMAIN = 'default' -def prompt_for_password(prompt=None): - """Prompt user for a password - - Prompt for a password if stdin is a tty. - """ - - if not prompt: - prompt = 'Password: ' - pw = None - # If stdin is a tty, try prompting for the password - if hasattr(sys.stdin, 'isatty') and sys.stdin.isatty(): - # Check for Ctl-D - try: - pw = getpass.getpass(prompt) - except EOFError: - pass - # No password because we did't have a tty or nothing was entered - if not pw: - raise exc.CommandError(_("No password entered, or found via" - " --os-password or OS_PASSWORD"),) - return pw - - -class OpenStackShell(app.App): - - CONSOLE_MESSAGE_FORMAT = '%(levelname)s: %(name)s %(message)s' - - log = logging.getLogger(__name__) - timing_data = [] +class OpenStackShell(shell.OpenStackShell): def __init__(self): - # Patch command.Command to add a default auth_required = True - command.Command.auth_required = True - - # Some commands do not need authentication - help.HelpCommand.auth_required = False - complete.CompleteCommand.auth_required = False - - # Slight change to the meaning of --debug - self.DEFAULT_DEBUG_VALUE = None - self.DEFAULT_DEBUG_HELP = 'Set debug logging and traceback on errors.' super(OpenStackShell, self).__init__( description=__doc__.strip(), @@ -97,281 +46,28 @@ class OpenStackShell(app.App): self.api_version = {} - # Until we have command line arguments parsed, dump any stack traces - self.dump_stack_trace = True - # Assume TLS host certificate verification is enabled self.verify = True - self.client_manager = None - self.command_options = None - - self.do_profile = False - - def configure_logging(self): - """Configure logging for the app.""" - self.log_configurator = logs.LogConfigurator(self.options) - self.dump_stack_trace = self.log_configurator.dump_trace - - def run(self, argv): - ret_val = 1 - self.command_options = argv - try: - ret_val = super(OpenStackShell, self).run(argv) - return ret_val - except Exception as e: - if not logging.getLogger('').handlers: - logging.basicConfig() - if self.dump_stack_trace: - self.log.error(traceback.format_exc()) - else: - self.log.error('Exception raised: ' + str(e)) - - return ret_val - - finally: - self.log.info("END return value: %s", ret_val) - - def init_profile(self): - # NOTE(dtroyer): Remove this 'if' block when the --profile global - # option is removed - if osprofiler_profiler and self.options.old_profile: - self.log.warning( - 'The --profile option is deprecated, ' - 'please use --os-profile instead' - ) - if not self.options.profile: - self.options.profile = self.options.old_profile - - self.do_profile = osprofiler_profiler and self.options.profile - if self.do_profile: - osprofiler_profiler.init(self.options.profile) - - def close_profile(self): - if self.do_profile: - trace_id = osprofiler_profiler.get().get_base_id() - - # NOTE(dbelova): let's use warning log level to see these messages - # printed. In fact we can define custom log level here with value - # bigger than most big default one (CRITICAL) or something like - # that (PROFILE = 60 for instance), but not sure we need it here. - self.log.warning("Trace ID: %s" % trace_id) - self.log.warning("Display trace with command:\n" - "osprofiler trace show --html %s " % trace_id) - - def run_subcommand(self, argv): - self.init_profile() - try: - ret_value = super(OpenStackShell, self).run_subcommand(argv) - finally: - self.close_profile() - return ret_value - - def interact(self): - self.init_profile() - try: - ret_value = super(OpenStackShell, self).interact() - finally: - self.close_profile() - return ret_value - def build_option_parser(self, description, version): parser = super(OpenStackShell, self).build_option_parser( description, version) + parser = clientmanager.build_plugin_option_parser(parser) + parser = auth.build_auth_plugins_option_parser(parser) + return parser - # service token auth argument - parser.add_argument( - '--os-cloud', - metavar='', - dest='cloud', - default=utils.env('OS_CLOUD'), - help=_('Cloud name in clouds.yaml (Env: OS_CLOUD)'), - ) - # Global arguments - parser.add_argument( - '--os-region-name', - metavar='', - dest='region_name', - default=utils.env('OS_REGION_NAME'), - help=_('Authentication region name (Env: OS_REGION_NAME)'), - ) - parser.add_argument( - '--os-cacert', - metavar='', - dest='cacert', - default=utils.env('OS_CACERT'), - help=_('CA certificate bundle file (Env: OS_CACERT)'), - ) - parser.add_argument( - '--os-cert', - metavar='', - dest='cert', - default=utils.env('OS_CERT'), - help=_('Client certificate bundle file (Env: OS_CERT)'), - ) - parser.add_argument( - '--os-key', - metavar='', - dest='key', - default=utils.env('OS_KEY'), - help=_('Client certificate key file (Env: OS_KEY)'), - ) - verify_group = parser.add_mutually_exclusive_group() - verify_group.add_argument( - '--verify', - action='store_true', - default=None, - help=_('Verify server certificate (default)'), - ) - verify_group.add_argument( - '--insecure', - action='store_true', - default=None, - help=_('Disable server certificate verification'), - ) - parser.add_argument( - '--os-default-domain', - metavar='', - dest='default_domain', - default=utils.env( - 'OS_DEFAULT_DOMAIN', - default=DEFAULT_DOMAIN), - help=_('Default domain ID, default=%s. ' - '(Env: OS_DEFAULT_DOMAIN)') % DEFAULT_DOMAIN, - ) - parser.add_argument( - '--os-interface', - metavar='', - dest='interface', - choices=['admin', 'public', 'internal'], - default=utils.env('OS_INTERFACE'), - help=_('Select an interface type.' - ' Valid interface types: [admin, public, internal].' - ' (Env: OS_INTERFACE)'), - ) - parser.add_argument( - '--timing', - default=False, - action='store_true', - help=_("Print API call timing info"), - ) - parser.add_argument( - '--os-beta-command', - action='store_true', - help=_("Enable beta commands which are subject to change"), - ) + def _final_defaults(self): + super(OpenStackShell, self)._final_defaults() - # osprofiler HMAC key argument - if osprofiler_profiler: - parser.add_argument( - '--os-profile', - metavar='hmac-key', - dest='profile', - help=_('HMAC key for encrypting profiling context data'), - ) - # NOTE(dtroyer): This global option should have been named - # --os-profile as --profile interferes with at - # least one existing command option. Deprecate - # --profile and remove after Apr 2017. - parser.add_argument( - '--profile', - metavar='hmac-key', - dest='old_profile', - help=argparse.SUPPRESS, - ) + # Set default auth type to password + self._auth_type = 'password' - return clientmanager.build_plugin_option_parser(parser) + def _load_plugins(self): + """Load plugins via stevedore - def initialize_app(self, argv): - """Global app init bits: - - * set up API versions - * validate authentication info - * authenticate against Identity if requested + osc-lib has no opinion on what plugins should be loaded """ - - # Parent __init__ parses argv into self.options - super(OpenStackShell, self).initialize_app(argv) - self.log.info("START with options: %s", - strutils.mask_password(self.command_options)) - self.log.debug("options: %s", - strutils.mask_password(self.options)) - - # Set the default plugin to token_endpoint if url and token are given - if (self.options.url and self.options.token): - # Use service token authentication - auth_type = 'token_endpoint' - else: - auth_type = 'password' - - project_id = getattr(self.options, 'project_id', None) - project_name = getattr(self.options, 'project_name', None) - tenant_id = getattr(self.options, 'tenant_id', None) - tenant_name = getattr(self.options, 'tenant_name', None) - - # Save default domain - self.default_domain = self.options.default_domain - - # handle some v2/v3 authentication inconsistencies by just acting like - # both the project and tenant information are both present. This can - # go away if we stop registering all the argparse options together. - if project_id and not tenant_id: - self.options.tenant_id = project_id - if project_name and not tenant_name: - self.options.tenant_name = project_name - if tenant_id and not project_id: - self.options.project_id = tenant_id - if tenant_name and not project_name: - self.options.project_name = tenant_name - - # Do configuration file handling - # Ignore the default value of interface. Only if it is set later - # will it be used. - try: - cc = cloud_config.OSC_Config( - override_defaults={ - 'interface': None, - 'auth_type': auth_type, - }, - ) - except (IOError, OSError): - self.log.critical("Could not read clouds.yaml configuration file") - self.print_help_if_requested() - raise - - # TODO(thowe): Change cliff so the default value for debug - # can be set to None. - if not self.options.debug: - self.options.debug = None - self.cloud = cc.get_one_cloud( - cloud=self.options.cloud, - argparse=self.options, - ) - - self.log_configurator.configure(self.cloud) - self.dump_stack_trace = self.log_configurator.dump_trace - self.log.debug("defaults: %s", cc.defaults) - self.log.debug("cloud cfg: %s", - strutils.mask_password(self.cloud.config)) - - # Set up client TLS - # NOTE(dtroyer): --insecure is the non-default condition that - # overrides any verify setting in clouds.yaml - # so check it first, then fall back to any verify - # setting provided. - self.verify = not self.cloud.config.get( - 'insecure', - not self.cloud.config.get('verify', True), - ) - - # NOTE(dtroyer): Per bug https://bugs.launchpad.net/bugs/1447784 - # --insecure now overrides any --os-cacert setting, - # where before --insecure was ignored if --os-cacert - # was set. - if self.verify and self.cloud.cacert: - self.verify = self.cloud.cacert - # Loop through extensions to get API versions for mod in clientmanager.PLUGIN_MODULES: default_version = getattr(mod, 'DEFAULT_API_VERSION', None) @@ -406,6 +102,11 @@ class OpenStackShell(app.App): {'name': api, 'version': version_opt, 'group': cmd_group} ) + def _load_commands(self): + """Load commands via cliff/stevedore + + osc-lib has no opinion on what commands should be loaded + """ # Commands that span multiple APIs self.command_manager.add_command_group( 'openstack.common') @@ -422,59 +123,19 @@ class OpenStackShell(app.App): # } self.command_manager.add_command_group( 'openstack.extension') - # call InitializeXxx() here - # set up additional clients to stuff in to client_manager?? - # Handle deferred help and exit - self.print_help_if_requested() + def initialize_app(self, argv): + super(OpenStackShell, self).initialize_app(argv) + # For now we need to build our own ClientManager so re-do what + # has already been done :( + # TODO(dtroyer): remove when osc-lib is fixed self.client_manager = clientmanager.ClientManager( cli_options=self.cloud, api_version=self.api_version, - pw_func=prompt_for_password, + pw_func=shell.prompt_for_password, ) - def prepare_to_run_command(self, cmd): - """Set up auth and API versions""" - self.log.info( - 'command: %s -> %s.%s', - getattr(cmd, 'cmd_name', ''), - cmd.__class__.__module__, - cmd.__class__.__name__, - ) - if cmd.auth_required: - self.client_manager.setup_auth() - if hasattr(cmd, 'required_scope') and cmd.required_scope: - # let the command decide whether we need a scoped token - self.client_manager.validate_scope() - # Trigger the Identity client to initialize - self.client_manager.auth_ref - - def clean_up(self, cmd, result, err): - self.log.debug('clean_up %s: %s', cmd.__class__.__name__, err or '') - - # Process collected timing data - if self.options.timing: - # Get session data - self.timing_data.extend( - self.client_manager.session.get_timings(), - ) - - # Use the Timing pseudo-command to generate the output - tcmd = timing.Timing(self, self.options) - tparser = tcmd.get_parser('Timing') - - # If anything other than prettytable is specified, force csv - format = 'table' - # Check the formatter used in the actual command - if hasattr(cmd, 'formatter') \ - and cmd.formatter != cmd._formatter_plugins['table'].obj: - format = 'csv' - - sys.stdout.write('\n') - targs = tparser.parse_args(['-f', format]) - tcmd.run(targs) - def main(argv=None): if argv is None: diff --git a/openstackclient/tests/test_shell.py b/openstackclient/tests/test_shell.py index 7d0bbd12e4..87cd7f5185 100644 --- a/openstackclient/tests/test_shell.py +++ b/openstackclient/tests/test_shell.py @@ -13,14 +13,15 @@ # under the License. # -import copy -import fixtures import mock import os -import testtools +import sys + +from osc_lib.tests import utils as osc_lib_test_utils +from oslo_utils import importutils +import wrapt from openstackclient import shell -from openstackclient.tests import utils DEFAULT_AUTH_URL = "http://127.0.0.1:5000/v2.0/" @@ -116,155 +117,50 @@ global_options = { '--os-interface': (DEFAULT_INTERFACE, True, True) } -auth_options = { - '--os-auth-url': (DEFAULT_AUTH_URL, True, True), - '--os-project-id': (DEFAULT_PROJECT_ID, True, True), - '--os-project-name': (DEFAULT_PROJECT_NAME, True, True), - '--os-domain-id': (DEFAULT_DOMAIN_ID, True, True), - '--os-domain-name': (DEFAULT_DOMAIN_NAME, True, True), - '--os-user-domain-id': (DEFAULT_USER_DOMAIN_ID, True, True), - '--os-user-domain-name': (DEFAULT_USER_DOMAIN_NAME, True, True), - '--os-project-domain-id': (DEFAULT_PROJECT_DOMAIN_ID, True, True), - '--os-project-domain-name': (DEFAULT_PROJECT_DOMAIN_NAME, True, True), - '--os-username': (DEFAULT_USERNAME, True, True), - '--os-password': (DEFAULT_PASSWORD, True, True), - '--os-region-name': (DEFAULT_REGION_NAME, True, True), - '--os-trust-id': ("1234", True, True), - '--os-auth-type': ("v2password", True, True), - '--os-token': (DEFAULT_TOKEN, True, True), - '--os-url': (DEFAULT_SERVICE_URL, True, True), - '--os-interface': (DEFAULT_INTERFACE, True, True), -} + +# Wrap the osc_lib make_shell() function to set the shell class since +# osc-lib's TestShell class doesn't allow us to specify it yet. +# TODO(dtroyer): remove this once the shell_class_patch patch is released +# in osc-lib +def make_shell_wrapper(func, inst, args, kwargs): + if 'shell_class' not in kwargs: + kwargs['shell_class'] = shell.OpenStackShell + return func(*args, **kwargs) -def opt2attr(opt): - if opt.startswith('--os-'): - attr = opt[5:] - elif opt.startswith('--'): - attr = opt[2:] - else: - attr = opt - return attr.lower().replace('-', '_') +wrapt.wrap_function_wrapper( + osc_lib_test_utils, + 'make_shell', + make_shell_wrapper, +) -def opt2env(opt): - return opt[2:].upper().replace('-', '_') +class TestShell(osc_lib_test_utils.TestShell): + # Full name of the OpenStackShell class to test (cliff.app.App subclass) + shell_class_name = "openstackclient.shell.OpenStackShell" -def make_shell(): - """Create a new command shell and mock out some bits.""" - _shell = shell.OpenStackShell() - _shell.command_manager = mock.Mock() - - return _shell - - -def fake_execute(shell, cmd): - """Pretend to execute shell commands.""" - return shell.run(cmd.split()) - - -class EnvFixture(fixtures.Fixture): - """Environment Fixture. - - This fixture replaces os.environ with provided env or an empty env. - """ - - def __init__(self, env=None): - self.new_env = env or {} - - def _setUp(self): - self.orig_env, os.environ = os.environ, self.new_env - self.addCleanup(self.revert) - - def revert(self): - os.environ = self.orig_env - - -class TestShell(utils.TestCase): + # TODO(dtroyer): remove this once the shell_class_patch patch is released + # in osc-lib + app_patch = shell_class_name def setUp(self): super(TestShell, self).setUp() - patch = "openstackclient.shell.OpenStackShell.run_subcommand" - self.cmd_patch = mock.patch(patch) - self.cmd_save = self.cmd_patch.start() - self.addCleanup(self.cmd_patch.stop) - self.app = mock.Mock("Test Shell") - - def _assert_initialize_app_arg(self, cmd_options, default_args): - """Check the args passed to initialize_app() - - The argv argument to initialize_app() is the remainder from parsing - global options declared in both cliff.app and - openstackclient.OpenStackShell build_option_parser(). Any global - options passed on the commmad line should not be in argv but in - _shell.options. - """ - - with mock.patch( - "openstackclient.shell.OpenStackShell.initialize_app", - self.app, - ): - _shell, _cmd = make_shell(), cmd_options + " list project" - fake_execute(_shell, _cmd) - - self.app.assert_called_with(["list", "project"]) - for k in default_args.keys(): - self.assertEqual( - default_args[k], - vars(_shell.options)[k], - "%s does not match" % k, - ) - - def _assert_cloud_config_arg(self, cmd_options, default_args): - """Check the args passed to cloud_config.get_one_cloud() - - The argparse argument to get_one_cloud() is an argparse.Namespace - object that contains all of the options processed to this point in - initialize_app(). - """ - - cloud = mock.Mock(name="cloudy") - cloud.config = {} - self.occ_get_one = mock.Mock(return_value=cloud) - with mock.patch( - "os_client_config.config.OpenStackConfig.get_one_cloud", - self.occ_get_one, - ): - _shell, _cmd = make_shell(), cmd_options + " list project" - fake_execute(_shell, _cmd) - - opts = self.occ_get_one.call_args[1]['argparse'] - for k in default_args.keys(): - self.assertEqual( - default_args[k], - vars(opts)[k], - "%s does not match" % k, - ) - - def _assert_token_auth(self, cmd_options, default_args): - with mock.patch("openstackclient.shell.OpenStackShell.initialize_app", - self.app): - _shell, _cmd = make_shell(), cmd_options + " list role" - fake_execute(_shell, _cmd) - - self.app.assert_called_with(["list", "role"]) - self.assertEqual( - default_args.get("token", ''), - _shell.options.token, - "token" - ) - self.assertEqual( - default_args.get("auth_url", ''), - _shell.options.auth_url, - "auth_url" - ) + # TODO(dtroyer): remove this once the shell_class_patch patch is + # released in osc-lib + self.shell_class = importutils.import_class(self.shell_class_name) def _assert_token_endpoint_auth(self, cmd_options, default_args): - with mock.patch("openstackclient.shell.OpenStackShell.initialize_app", - self.app): - _shell, _cmd = make_shell(), cmd_options + " list role" - fake_execute(_shell, _cmd) + with mock.patch( + self.shell_class_name + ".initialize_app", + self.app, + ): + _shell = osc_lib_test_utils.make_shell( + shell_class=self.shell_class, + ) + _cmd = cmd_options + " list role" + osc_lib_test_utils.fake_execute(_shell, _cmd) + print("_shell: %s" % _shell) self.app.assert_called_with(["list", "role"]) self.assertEqual( @@ -278,11 +174,40 @@ class TestShell(utils.TestCase): "url", ) + def _assert_token_auth(self, cmd_options, default_args): + with mock.patch( + self.app_patch + ".initialize_app", + self.app, + ): + _shell = osc_lib_test_utils.make_shell( + shell_class=self.shell_class, + ) + _cmd = cmd_options + " list role" + osc_lib_test_utils.fake_execute(_shell, _cmd) + print("_shell: %s" % _shell) + + self.app.assert_called_with(["list", "role"]) + self.assertEqual( + default_args.get("token", ''), + _shell.options.token, + "token" + ) + self.assertEqual( + default_args.get("auth_url", ''), + _shell.options.auth_url, + "auth_url" + ) + def _assert_cli(self, cmd_options, default_args): - with mock.patch("openstackclient.shell.OpenStackShell.initialize_app", - self.app): - _shell, _cmd = make_shell(), cmd_options + " list server" - fake_execute(_shell, _cmd) + with mock.patch( + self.shell_class_name + ".initialize_app", + self.app, + ): + _shell = osc_lib_test_utils.make_shell( + shell_class=self.shell_class, + ) + _cmd = cmd_options + " list server" + osc_lib_test_utils.fake_execute(_shell, _cmd) self.app.assert_called_with(["list", "server"]) self.assertEqual(default_args["compute_api_version"], @@ -297,39 +222,17 @@ class TestShell(utils.TestCase): _shell.options.os_network_api_version) -class TestShellHelp(TestShell): - """Test the deferred help flag""" - - def setUp(self): - super(TestShellHelp, self).setUp() - self.useFixture(EnvFixture()) - - @testtools.skip("skip until bug 1444983 is resolved") - def test_help_options(self): - flag = "-h list server" - kwargs = { - "deferred_help": True, - } - with mock.patch("openstackclient.shell.OpenStackShell.initialize_app", - self.app): - _shell, _cmd = make_shell(), flag - fake_execute(_shell, _cmd) - - self.assertEqual(kwargs["deferred_help"], - _shell.options.deferred_help) - - class TestShellOptions(TestShell): def setUp(self): super(TestShellOptions, self).setUp() - self.useFixture(EnvFixture()) + self.useFixture(osc_lib_test_utils.EnvFixture()) def _test_options_init_app(self, test_opts): for opt in test_opts.keys(): if not test_opts[opt][1]: continue - key = opt2attr(opt) + key = osc_lib_test_utils.opt2attr(opt) if isinstance(test_opts[opt][0], str): cmd = opt + " " + test_opts[opt][0] else: @@ -343,7 +246,7 @@ class TestShellOptions(TestShell): for opt in test_opts.keys(): if not test_opts[opt][1]: continue - key = opt2attr(opt) + key = osc_lib_test_utils.opt2attr(opt) if isinstance(test_opts[opt][0], str): cmd = opt + " " + test_opts[opt][0] else: @@ -357,12 +260,12 @@ class TestShellOptions(TestShell): for opt in test_opts.keys(): if not test_opts[opt][2]: continue - key = opt2attr(opt) + key = osc_lib_test_utils.opt2attr(opt) kwargs = { key: test_opts[opt][0], } env = { - opt2env(opt): test_opts[opt][0], + osc_lib_test_utils.opt2env(opt): test_opts[opt][0], } os.environ = env.copy() self._assert_initialize_app_arg("", kwargs) @@ -371,37 +274,16 @@ class TestShellOptions(TestShell): for opt in test_opts.keys(): if not test_opts[opt][2]: continue - key = opt2attr(opt) + key = osc_lib_test_utils.opt2attr(opt) kwargs = { key: test_opts[opt][0], } env = { - opt2env(opt): test_opts[opt][0], + osc_lib_test_utils.opt2env(opt): test_opts[opt][0], } os.environ = env.copy() self._assert_cloud_config_arg("", kwargs) - def test_empty_auth(self): - os.environ = {} - self._assert_initialize_app_arg("", {}) - self._assert_cloud_config_arg("", {}) - - def test_global_options(self): - self._test_options_init_app(global_options) - self._test_options_get_one_cloud(global_options) - - def test_auth_options(self): - self._test_options_init_app(auth_options) - self._test_options_get_one_cloud(auth_options) - - def test_global_env(self): - self._test_env_init_app(global_options) - self._test_env_get_one_cloud(global_options) - - def test_auth_env(self): - self._test_env_init_app(auth_options) - self._test_env_get_one_cloud(auth_options) - class TestShellTokenAuthEnv(TestShell): @@ -411,7 +293,7 @@ class TestShellTokenAuthEnv(TestShell): "OS_TOKEN": DEFAULT_TOKEN, "OS_AUTH_URL": DEFAULT_AUTH_URL, } - self.useFixture(EnvFixture(env.copy())) + self.useFixture(osc_lib_test_utils.EnvFixture(env.copy())) def test_env(self): flag = "" @@ -455,7 +337,7 @@ class TestShellTokenEndpointAuthEnv(TestShell): "OS_TOKEN": DEFAULT_TOKEN, "OS_URL": DEFAULT_SERVICE_URL, } - self.useFixture(EnvFixture(env.copy())) + self.useFixture(osc_lib_test_utils.EnvFixture(env.copy())) def test_env(self): flag = "" @@ -463,7 +345,7 @@ class TestShellTokenEndpointAuthEnv(TestShell): "token": DEFAULT_TOKEN, "url": DEFAULT_SERVICE_URL, } - self._assert_token_auth(flag, kwargs) + self._assert_token_endpoint_auth(flag, kwargs) def test_only_token(self): flag = "--os-token xyzpdq" @@ -502,85 +384,7 @@ class TestShellCli(TestShell): "OS_VOLUME_API_VERSION": DEFAULT_VOLUME_API_VERSION, "OS_NETWORK_API_VERSION": DEFAULT_NETWORK_API_VERSION, } - self.useFixture(EnvFixture(env.copy())) - - def test_shell_args_no_options(self): - _shell = make_shell() - with mock.patch("openstackclient.shell.OpenStackShell.initialize_app", - self.app): - fake_execute(_shell, "list user") - self.app.assert_called_with(["list", "user"]) - - def test_shell_args_ca_options(self): - _shell = make_shell() - - # NOTE(dtroyer): The commented out asserts below are the desired - # behaviour and will be uncommented when the - # handling for --verify and --insecure is fixed. - - # Default - fake_execute(_shell, "list user") - self.assertIsNone(_shell.options.verify) - self.assertIsNone(_shell.options.insecure) - self.assertEqual('', _shell.options.cacert) - self.assertTrue(_shell.verify) - - # --verify - fake_execute(_shell, "--verify list user") - self.assertTrue(_shell.options.verify) - self.assertIsNone(_shell.options.insecure) - self.assertEqual('', _shell.options.cacert) - self.assertTrue(_shell.verify) - - # --insecure - fake_execute(_shell, "--insecure list user") - self.assertIsNone(_shell.options.verify) - self.assertTrue(_shell.options.insecure) - self.assertEqual('', _shell.options.cacert) - self.assertFalse(_shell.verify) - - # --os-cacert - fake_execute(_shell, "--os-cacert foo list user") - self.assertIsNone(_shell.options.verify) - self.assertIsNone(_shell.options.insecure) - self.assertEqual('foo', _shell.options.cacert) - self.assertTrue(_shell.verify) - - # --os-cacert and --verify - fake_execute(_shell, "--os-cacert foo --verify list user") - self.assertTrue(_shell.options.verify) - self.assertIsNone(_shell.options.insecure) - self.assertEqual('foo', _shell.options.cacert) - self.assertTrue(_shell.verify) - - # --os-cacert and --insecure - # NOTE(dtroyer): Per bug https://bugs.launchpad.net/bugs/1447784 - # in this combination --insecure now overrides any - # --os-cacert setting, where before --insecure - # was ignored if --os-cacert was set. - fake_execute(_shell, "--os-cacert foo --insecure list user") - self.assertIsNone(_shell.options.verify) - self.assertTrue(_shell.options.insecure) - self.assertEqual('foo', _shell.options.cacert) - self.assertFalse(_shell.verify) - - def test_shell_args_cert_options(self): - _shell = make_shell() - - # Default - fake_execute(_shell, "list user") - self.assertEqual('', _shell.options.cert) - self.assertEqual('', _shell.options.key) - - # --os-cert - fake_execute(_shell, "--os-cert mycert list user") - self.assertEqual('mycert', _shell.options.cert) - self.assertEqual('', _shell.options.key) - - # --os-key - fake_execute(_shell, "--os-key mickey list user") - self.assertEqual('', _shell.options.cert) - self.assertEqual('mickey', _shell.options.key) + self.useFixture(osc_lib_test_utils.EnvFixture(env.copy())) def test_default_env(self): flag = "" @@ -605,220 +409,34 @@ class TestShellCli(TestShell): } self._assert_cli(flag, kwargs) - @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - def test_shell_args_cloud_no_vendor(self, config_mock): - config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_1)) - _shell = make_shell() - fake_execute( - _shell, - "--os-cloud scc list user", - ) - self.assertEqual( - 'scc', - _shell.cloud.name, - ) - - # These come from clouds.yaml - self.assertEqual( - DEFAULT_AUTH_URL, - _shell.cloud.config['auth']['auth_url'], - ) - self.assertEqual( - DEFAULT_PROJECT_NAME, - _shell.cloud.config['auth']['project_name'], - ) - self.assertEqual( - 'zaphod', - _shell.cloud.config['auth']['username'], - ) - self.assertEqual( - 'occ-cloud', - _shell.cloud.config['region_name'], - ) - self.assertEqual( - 'glazed', - _shell.cloud.config['donut'], - ) - self.assertEqual( - 'public', - _shell.cloud.config['interface'], - ) - - @mock.patch("os_client_config.config.OpenStackConfig._load_vendor_file") - @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - def test_shell_args_cloud_public(self, config_mock, public_mock): - config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2)) - public_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1)) - _shell = make_shell() - - fake_execute( - _shell, - "--os-cloud megacloud list user", - ) - self.assertEqual( - 'megacloud', - _shell.cloud.name, - ) - - # These come from clouds-public.yaml - self.assertEqual( - DEFAULT_AUTH_URL, - _shell.cloud.config['auth']['auth_url'], - ) - self.assertEqual( - 'cake', - _shell.cloud.config['donut'], - ) - - # These come from clouds.yaml - self.assertEqual( - 'heart-o-gold', - _shell.cloud.config['auth']['project_name'], - ) - self.assertEqual( - 'zaphod', - _shell.cloud.config['auth']['username'], - ) - self.assertEqual( - 'occ-cloud', - _shell.cloud.config['region_name'], - ) - - self.assertEqual('mycert', _shell.cloud.config['cert']) - self.assertEqual('mickey', _shell.cloud.config['key']) - - @mock.patch("os_client_config.config.OpenStackConfig._load_vendor_file") - @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - def test_shell_args_precedence(self, config_mock, vendor_mock): - config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2)) - vendor_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1)) - _shell = make_shell() - - # Test command option overriding config file value - fake_execute( - _shell, - "--os-cloud megacloud --os-region-name krikkit list user", - ) - self.assertEqual( - 'megacloud', - _shell.cloud.name, - ) - - # These come from clouds-public.yaml - self.assertEqual( - DEFAULT_AUTH_URL, - _shell.cloud.config['auth']['auth_url'], - ) - self.assertEqual( - 'cake', - _shell.cloud.config['donut'], - ) - - # These come from clouds.yaml - self.assertEqual( - 'heart-o-gold', - _shell.cloud.config['auth']['project_name'], - ) - self.assertEqual( - 'zaphod', - _shell.cloud.config['auth']['username'], - ) - self.assertEqual( - 'krikkit', - _shell.cloud.config['region_name'], - ) - - -class TestShellCliEnv(TestShell): +class TestShellArgV(TestShell): + """Test the deferred help flag""" def setUp(self): - super(TestShellCliEnv, self).setUp() - env = { - 'OS_REGION_NAME': 'occ-env', - } - self.useFixture(EnvFixture(env.copy())) + super(TestShellArgV, self).setUp() - @mock.patch("os_client_config.config.OpenStackConfig._load_vendor_file") - @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - def test_shell_args_precedence_1(self, config_mock, vendor_mock): - config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2)) - vendor_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1)) - _shell = make_shell() + def test_shell_argv(self): + """Test argv decoding - # Test env var - fake_execute( - _shell, - "--os-cloud megacloud list user", - ) - self.assertEqual( - 'megacloud', - _shell.cloud.name, - ) + Python 2 does nothing with argv while Python 3 decodes it into + Unicode before we ever see it. We manually decode when running + under Python 2 so verify that we get the right argv types. - # These come from clouds-public.yaml - self.assertEqual( - DEFAULT_AUTH_URL, - _shell.cloud.config['auth']['auth_url'], - ) - self.assertEqual( - 'cake', - _shell.cloud.config['donut'], - ) + Use the argv supplied by the test runner so we get actual Python + runtime behaviour; we only need to check the type of argv[0] + which will alwyas be present. + """ - # These come from clouds.yaml - self.assertEqual( - 'heart-o-gold', - _shell.cloud.config['auth']['project_name'], - ) - self.assertEqual( - 'zaphod', - _shell.cloud.config['auth']['username'], - ) - self.assertEqual( - 'occ-env', - _shell.cloud.config['region_name'], - ) + with mock.patch( + self.shell_class_name + ".run", + self.app, + ): + # Ensure type gets through unmolested through shell.main() + argv = sys.argv + shell.main(sys.argv) + self.assertEqual(type(argv[0]), type(self.app.call_args[0][0][0])) - @mock.patch("os_client_config.config.OpenStackConfig._load_vendor_file") - @mock.patch("os_client_config.config.OpenStackConfig._load_config_file") - def test_shell_args_precedence_2(self, config_mock, vendor_mock): - config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2)) - vendor_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1)) - _shell = make_shell() - - # Test command option overriding config file value - fake_execute( - _shell, - "--os-cloud megacloud --os-region-name krikkit list user", - ) - self.assertEqual( - 'megacloud', - _shell.cloud.name, - ) - - # These come from clouds-public.yaml - self.assertEqual( - DEFAULT_AUTH_URL, - _shell.cloud.config['auth']['auth_url'], - ) - self.assertEqual( - 'cake', - _shell.cloud.config['donut'], - ) - - # These come from clouds.yaml - self.assertEqual( - 'heart-o-gold', - _shell.cloud.config['auth']['project_name'], - ) - self.assertEqual( - 'zaphod', - _shell.cloud.config['auth']['username'], - ) - - # These come from the command line - self.assertEqual( - 'krikkit', - _shell.cloud.config['region_name'], - ) + # When shell.main() gets sys.argv itself it should be decoded + shell.main() + self.assertEqual(type(u'x'), type(self.app.call_args[0][0][0])) diff --git a/openstackclient/volume/client.py b/openstackclient/volume/client.py index 11f20673da..ade5a95f55 100644 --- a/openstackclient/volume/client.py +++ b/openstackclient/volume/client.py @@ -57,13 +57,13 @@ def make_client(instance): extensions = [extension.Extension('list_extensions', list_extensions)] # Remember interface only if it is set - kwargs = utils.build_kwargs_dict('endpoint_type', instance._interface) + kwargs = utils.build_kwargs_dict('endpoint_type', instance.interface) client = volume_client( session=instance.session, extensions=extensions, http_log_debug=http_log_debug, - region_name=instance._region_name, + region_name=instance.region_name, **kwargs ) diff --git a/test-requirements.txt b/test-requirements.txt index 6a98478f09..271554d1d5 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -17,6 +17,7 @@ testtools>=1.4.0 # MIT tempest>=12.1.0 # Apache-2.0 osprofiler>=1.3.0 # Apache-2.0 bandit>=1.0.1 # Apache-2.0 +wrapt>=1.7.0 # BSD License # Install these to generate sphinx autodocs aodhclient>=0.5.0 # Apache-2.0