Merge "Add code to move --user and --password in cli"

fuelclient/cli/error.py

@@ -106,7 +106,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:

fuelclient/cli/parser.py

@@ -45,6 +45,7 @@ class Parser:
             fuel [optional args] <namespace> [action] [flags]"""
         )
         self.universal_flags = []
+        self.credential_flags = []
         self.subparsers = self.parser.add_subparsers(
             title="Namespaces",
             metavar="",
@@ -126,6 +127,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",
@@ -151,29 +154,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'

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')