From 8990aa57ee567e975a9324761444e42511e0be42 Mon Sep 17 00:00:00 2001 From: Kamil Sambor Date: Mon, 27 Oct 2014 11:46:55 +0100 Subject: [PATCH] Add code to move --user and --password in cli * moved --user and --password flags before any action * added test Change-Id: I68ae8d15f519ff220d7121b49387013f189e3aa1 Closes-Bug: #1385613 --- fuelclient/cli/error.py | 2 +- fuelclient/cli/parser.py | 62 ++++++++++++++++++++++++--------- fuelclient/tests/test_client.py | 51 +++++++++++++++++---------- 3 files changed, 80 insertions(+), 35 deletions(-) diff --git a/fuelclient/cli/error.py b/fuelclient/cli/error.py index bc48978..e246ba6 100644 --- a/fuelclient/cli/error.py +++ b/fuelclient/cli/error.py @@ -98,7 +98,7 @@ def exceptions_decorator(func): exit_with_error(""" Unauthorized: need authentication! Please provide user and password via client - --user --password + fuel --user=user --password=pass [action] or modify "KEYSTONE_USER" and "KEYSTONE_PASS" in /etc/fuel/client/config.yaml""") except FuelClientException as exc: diff --git a/fuelclient/cli/parser.py b/fuelclient/cli/parser.py index b0c1702..2ff68b6 100644 --- a/fuelclient/cli/parser.py +++ b/fuelclient/cli/parser.py @@ -44,6 +44,7 @@ class Parser: fuel [optional args] [action] [flags]""" ) self.universal_flags = [] + self.credential_flags = [] self.subparsers = self.parser.add_subparsers( title="Namespaces", metavar="", @@ -115,6 +116,8 @@ class Parser: ) def add_keystone_credentials_args(self): + self.credential_flags.append('--user') + self.credential_flags.append('--password') self.parser.add_argument( "--user", dest="user", @@ -140,29 +143,56 @@ class Parser: lambda x: substitutions.get(x, x), self.args ) - # move --json and --debug flags before any action + + # move general used flags before actions, otherwise they will be used + # as a part of action by action_generator + for flag in self.credential_flags: + self.move_argument_before_action(flag) + for flag in self.universal_flags: - if flag in self.args: - self.args.remove(flag) - self.args.insert(1, flag) + self.move_argument_before_action(flag, has_value=False) - self.move_argument_before_action("--env", ) + self.move_argument_after_action("--env", ) - def move_argument_before_action(self, argument): + def move_argument_before_action(self, flag, has_value=True): + """We need to move general argument before action, we use them + not directly in action but in APIClient. + """ for arg in self.args: - if argument in arg: - # if declaration with '=' sign (e.g. --env-id=1) - if "=" in arg: - index_of_env = self.args.index(arg) - env = self.args.pop(index_of_env) - self.args.append(env) + if flag in arg: + if "=" in arg or not has_value: + index_of_flag = self.args.index(arg) + flag = self.args.pop(index_of_flag) + self.args.insert(1, flag) else: try: - index_of_env = self.args.index(arg) - self.args.pop(index_of_env) - env = self.args.pop(index_of_env) + index_of_flag = self.args.index(arg) + flag = self.args.pop(index_of_flag) + value = self.args.pop(index_of_flag) + self.args.insert(1, value) + self.args.insert(1, flag) + except IndexError: + raise ParserException( + 'Corresponding value must follow "{0}" flag' + .format(arg) + ) + break + + def move_argument_after_action(self, flag): + for arg in self.args: + if flag in arg: + # if declaration with '=' sign (e.g. --env-id=1) + if "=" in arg: + index_of_flag = self.args.index(arg) + flag = self.args.pop(index_of_flag) + self.args.append(flag) + else: + try: + index_of_flag = self.args.index(arg) + self.args.pop(index_of_flag) + flag = self.args.pop(index_of_flag) self.args.append(arg) - self.args.append(env) + self.args.append(flag) except IndexError: raise ParserException( 'Corresponding value must follow "{0}" flag' diff --git a/fuelclient/tests/test_client.py b/fuelclient/tests/test_client.py index 48d8325..d818674 100644 --- a/fuelclient/tests/test_client.py +++ b/fuelclient/tests/test_client.py @@ -17,10 +17,13 @@ import os -from fuelclient.tests.base import BaseTestCase +from mock import Mock +from mock import patch + +from fuelclient.tests import base -class TestHandlers(BaseTestCase): +class TestHandlers(base.BaseTestCase): def test_env_action(self): #check env help @@ -99,17 +102,6 @@ class TestHandlers(BaseTestCase): self.assertEqual(result.stderr, '') del os.environ["SERVER_ADDRESS"] - def test_wrong_credentials(self): - result = self.run_cli_command("--user=a --password=a node", - check_errors=True) - must_be = ('\n Unauthorized: need authentication!\n ' - ' Please provide user and password via client\n ' - ' --user --password\n or modify ' - '"KEYSTONE_USER" and "KEYSTONE_PASS" in\n ' - '/etc/fuel/client/config.yaml\n' - ) - self.assertEqual(result.stderr, must_be) - def test_destroy_node(self): self.load_data_to_nailgun_server() self.run_cli_commands(( @@ -171,7 +163,7 @@ class TestHandlers(BaseTestCase): self.run_cli_command(cmd) -class TestUserActions(BaseTestCase): +class TestUserActions(base.BaseTestCase): def test_change_password_params(self): cmd = "user change-password" @@ -180,7 +172,7 @@ class TestUserActions(BaseTestCase): self.assertTrue(msg, result) -class TestCharset(BaseTestCase): +class TestCharset(base.BaseTestCase): def test_charset_problem(self): self.load_data_to_nailgun_server() @@ -191,7 +183,7 @@ class TestCharset(BaseTestCase): )) -class TestFiles(BaseTestCase): +class TestFiles(base.BaseTestCase): def test_file_creation(self): self.load_data_to_nailgun_server() @@ -282,7 +274,7 @@ class TestFiles(BaseTestCase): )) -class TestDownloadUploadNodeAttributes(BaseTestCase): +class TestDownloadUploadNodeAttributes(base.BaseTestCase): def test_upload_download_interfaces(self): self.load_data_to_nailgun_server() @@ -297,7 +289,7 @@ class TestDownloadUploadNodeAttributes(BaseTestCase): self.upload_command(cmd))) -class TestDeployChanges(BaseTestCase): +class TestDeployChanges(base.BaseTestCase): def test_deploy_changes_no_failure(self): self.load_data_to_nailgun_server() @@ -305,3 +297,26 @@ class TestDeployChanges(BaseTestCase): add_node = "--env-id=1 node set --node 1 --role=controller" deploy_changes = "deploy-changes --env 1" self.run_cli_commands((env_create, add_node, deploy_changes)) + + +class TestAuthentication(base.UnitTestCase): + + @patch('fuelclient.client.requests') + @patch('fuelclient.client.auth_client') + def test_wrong_credentials(self, mkeystone_cli, mrequests): + mkeystone_cli.return_value = Mock(auth_token='') + mrequests.get_request.return_value = Mock(status_code=200) + self.execute( + ['fuel', '--user=a', '--password=a', 'node']) + mkeystone_cli.Client.assert_called_with( + username='a', + tenant_name='admin', + password='a', + auth_url='http://127.0.0.1:8003/keystone/v2.0') + self.execute( + ['fuel', '--user=a', '--password', 'a', 'node']) + mkeystone_cli.Client.assert_called_with( + username='a', + tenant_name='admin', + password='a', + auth_url='http://127.0.0.1:8003/keystone/v2.0')