diff --git a/mistralclient/auth/keystone.py b/mistralclient/auth/keystone.py index aa633a64..ab9f8a09 100644 --- a/mistralclient/auth/keystone.py +++ b/mistralclient/auth/keystone.py @@ -12,8 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import logging - import keystoneauth1.identity.generic as auth_plugin from keystoneauth1 import session as ks_session import mistralclient.api.httpclient as api @@ -21,9 +19,6 @@ from mistralclient import auth as mistral_auth from oslo_serialization import jsonutils -LOG = logging.getLogger(__name__) - - class KeystoneAuthHandler(mistral_auth.AuthHandler): def authenticate(self, req, session=None): @@ -82,7 +77,6 @@ class KeystoneAuthHandler(mistral_auth.AuthHandler): auth_response = {} if not session: - auth = None if auth_token: auth = auth_plugin.Token( auth_url=auth_url, @@ -105,14 +99,10 @@ class KeystoneAuthHandler(mistral_auth.AuthHandler): project_name=project_name, project_domain_name=project_domain_name, project_domain_id=project_domain_id) - else: - # NOTE(jaosorior): We don't crash here cause it's needed for - # bash-completion to work. However, we do issue a warning to - # the user so if the request doesn't work. It's because of - # this. - LOG.warning("You must either provide a valid token or " - "a password (api_key) and a user.") + raise RuntimeError("You must either provide a valid token or " + "a password (api_key) and a user.") + if auth: session = ks_session.Session(auth=auth) diff --git a/mistralclient/shell.py b/mistralclient/shell.py index 57b8f7b6..b5ce7f65 100644 --- a/mistralclient/shell.py +++ b/mistralclient/shell.py @@ -560,7 +560,13 @@ class MistralShell(app.App): self._set_shell_commands(self._get_commands(ver)) - do_help = ('help' in argv) or ('-h' in argv) or not argv + # bash-completion and help messages should not require client creation + need_client = not ( + ('bash-completion' in argv) or + ('help' in argv) or + ('-h' in argv) or + ('--help' in argv) or + not argv) # Set default for auth_url if not supplied. The default is not # set at the parser to support use cases where auth is not enabled. @@ -586,10 +592,6 @@ class MistralShell(app.App): not self.options.target_user_domain_name): self.options.target_user_domain_id = "default" - # bash-completion should not require authentification. - if do_help or ('bash-completion' in argv): - self.options.auth_url = None - if self.options.auth_url and not self.options.token: if not self.options.username: raise exe.IllegalArgumentException( @@ -602,7 +604,19 @@ class MistralShell(app.App): ("You must provide a password " "via --os-password env[OS_PASSWORD]") ) + self.client = self._create_client() if need_client else None + # Adding client_manager variable to make mistral client work with + # unified OpenStack client. + ClientManager = type( + 'ClientManager', + (object,), + dict(workflow_engine=self.client) + ) + + self.client_manager = ClientManager() + + def _create_client(self): kwargs = { 'cert': self.options.os_cert, 'key': self.options.os_key, @@ -612,13 +626,12 @@ class MistralShell(app.App): 'project_domain_id': self.options.project_domain_id, 'target_project_domain_name': self.options.target_project_domain_name, - 'target_project_domain_id': - self.options.target_project_domain_id, + 'target_project_domain_id': self.options.target_project_domain_id, 'target_user_domain_name': self.options.target_user_domain_name, 'target_user_domain_id': self.options.target_user_domain_id } - self.client = client.client( + return client.client( mistral_url=self.options.mistral_url, username=self.options.username, api_key=self.options.password, @@ -647,16 +660,6 @@ class MistralShell(app.App): **kwargs ) - # Adding client_manager variable to make mistral client work with - # unified OpenStack client. - ClientManager = type( - 'ClientManager', - (object,), - dict(workflow_engine=self.client) - ) - - self.client_manager = ClientManager() - def _set_shell_commands(self, cmds_dict): for k, v in cmds_dict.items(): self.command_manager.add_command(k, v) diff --git a/mistralclient/tests/unit/test_shell.py b/mistralclient/tests/unit/test_shell.py index 7221540f..edc21b90 100644 --- a/mistralclient/tests/unit/test_shell.py +++ b/mistralclient/tests/unit/test_shell.py @@ -19,6 +19,17 @@ import mistralclient.tests.unit.base_shell_test as base class TestShell(base.BaseShellTests): + def test_help(self): + """Test that client is not created for help and bash complete""" + for command in ('-h', + '--help', + 'help', + 'help workbook-list', + 'bash-completion'): + with mock.patch('mistralclient.api.client.client') as client_mock: + self.shell(command) + self.assertFalse(client_mock.called) + @mock.patch('mistralclient.api.client.client') def test_command_no_mistral_url(self, client_mock): self.shell( diff --git a/mistralclient/tests/unit/v2/base.py b/mistralclient/tests/unit/v2/base.py index ef4f0607..5e961a4e 100644 --- a/mistralclient/tests/unit/v2/base.py +++ b/mistralclient/tests/unit/v2/base.py @@ -13,6 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import mock + from mistralclient.api.v2 import client from mistralclient.tests.unit import base @@ -24,14 +26,17 @@ class BaseClientV2Test(base.BaseClientTest): def setUp(self): super(BaseClientV2Test, self).setUp() - self._client = client.Client(project_name="test", - mistral_url=self.TEST_URL) - self.workbooks = self._client.workbooks - self.executions = self._client.executions - self.tasks = self._client.tasks - self.workflows = self._client.workflows - self.environments = self._client.environments - self.action_executions = self._client.action_executions - self.actions = self._client.actions - self.services = self._client.services - self.members = self._client.members + with mock.patch( + 'mistralclient.auth.keystone.KeystoneAuthHandler.authenticate', + return_value={'session': None}): + self._client = client.Client(project_name="test", + mistral_url=self.TEST_URL) + self.workbooks = self._client.workbooks + self.executions = self._client.executions + self.tasks = self._client.tasks + self.workflows = self._client.workflows + self.environments = self._client.environments + self.action_executions = self._client.action_executions + self.actions = self._client.actions + self.services = self._client.services + self.members = self._client.members