Use OS command line parameters for credentials

In order to comply with OpenStack de-facto standards
user's credentials must be supplied via appropriate
--os parameters.

Closes-bug: #1535417
Change-Id: Ifd186f0d703a840635f6f111c379338e93fde0a3

fuelclient/cli/actions/base.py

@@ -48,12 +48,6 @@ class Action(object):
         """Entry point for all actions subclasses
         """
         APIClient.debug_mode(debug=params.debug)
-        if getattr(params, 'user') and getattr(params, 'password'):
-            APIClient.user = params.user
-            APIClient.password = params.password
-            # tenant is set by default to 'admin' in parser.add_argument
-            APIClient.tenant = params.tenant
-            APIClient.initialize_keystone_client()
 
         self.serializer = Serializer.from_params(params)
         if self.flag_func_map is not None:

fuelclient/cli/parser.py

@@ -23,7 +23,9 @@ from fuelclient.cli.error import exceptions_decorator
 from fuelclient.cli.error import ParserException
 from fuelclient.cli.serializers import Serializer
 from fuelclient import consts
+from fuelclient import fuelclient_settings
 from fuelclient import profiler
+from fuelclient import utils
 
 
 class Parser(object):
@@ -96,8 +98,8 @@ class Parser(object):
         self.generate_actions()
         self.add_version_args()
         self.add_debug_arg()
-        self.add_keystone_credentials_args()
         self.add_serializers_args()
+        utils.add_os_cli_parameters(self.parser)
 
     def generate_actions(self):
         for action, action_object in actions.items():
@@ -129,7 +131,12 @@ class Parser(object):
         if len(self.args) < 2:
             self.parser.print_help()
             sys.exit(0)
+
         parsed_params = self.parser.parse_args(self.args[1:])
+
+        settings = fuelclient_settings.get_settings()
+        settings.update_from_command_line_options(parsed_params)
+
         if parsed_params.action not in actions:
             self.parser.print_help()
             sys.exit(0)
@@ -169,32 +176,6 @@ class Parser(object):
             default=False
         )
 
-    def add_keystone_credentials_args(self):
-        self.credential_flags.append('--user')
-        self.credential_flags.append('--password')
-        self.credential_flags.append('--tenant')
-        self.parser.add_argument(
-            "--user",
-            dest="user",
-            type=str,
-            help="credentials for keystone authentication user",
-            default=None
-        )
-        self.parser.add_argument(
-            "--password",
-            dest="password",
-            type=str,
-            help="credentials for keystone authentication password",
-            default=None
-        )
-        self.parser.add_argument(
-            "--tenant",
-            dest="tenant",
-            type=str,
-            help="credentials for keystone authentication tenant",
-            default="admin"
-        )
-
     def add_version_args(self):
         for args in (get_version_arg(), get_fuel_version_arg()):
             self.parser.add_argument(*args["args"], **args["params"])

fuelclient/client.py

@@ -44,9 +44,6 @@ class Client(object):
         self.keystone_base = urlparse.urljoin(self.root, "/keystone/v2.0")
         self.api_root = urlparse.urljoin(self.root, "/api/v1/")
         self.ostf_root = urlparse.urljoin(self.root, "/ostf/")
-        self.user = conf.KEYSTONE_USER
-        self.password = conf.KEYSTONE_PASS
-        self.tenant = 'admin'
         self._keystone_client = None
         self._auth_required = None
         self._session = None
@@ -122,17 +119,22 @@ class Client(object):
         return self._keystone_client
 
     def update_own_password(self, new_pass):
+        conf = fuelclient_settings.get_settings()
+
         if self.auth_token:
-            self.keystone_client.users.update_own_password(
+            self.keystone_client.users.update_own_password(conf.OS_PASSWORD,
-                self.password, new_pass)
+                                                           new_pass)
 
     def initialize_keystone_client(self):
+        conf = fuelclient_settings.get_settings()
+
         if self.auth_required:
             self._keystone_client = auth_client.Client(
-                username=self.user,
-                password=self.password,
                 auth_url=self.keystone_base,
-                tenant_name=self.tenant)
+                username=conf.OS_USERNAME,
+                password=conf.OS_PASSWORD,
+                tenant_name=conf.OS_TENANT_NAME)
+
             self._keystone_client.session.auth = self._keystone_client
             self._keystone_client.authenticate()
 

fuelclient/fuel_client.yaml

@@ -1,8 +1,12 @@
 # Connection settings
 SERVER_ADDRESS: "127.0.0.1"
 SERVER_PORT: "8000"
-KEYSTONE_USER:
+OS_USERNAME:
-KEYSTONE_PASS:
+OS_PASSWORD:
+# There's a need to provide default value for
+# tenant name until fuel-library is updated.
+# After that, it will be removed.
+OS_TENANT_NAME: "admin"
 HTTP_PROXY: null
 HTTP_TIMEOUT: 10
 

fuelclient/fuelclient_settings.py

@@ -27,10 +27,12 @@ from fuelclient.cli import error
 _SETTINGS = None
 
 # Format: old parameter: new parameter or None
-DEPRECATION_TABLE = {'LISTEN_PORT': 'SERVER_PORT'}
+DEPRECATION_TABLE = {'LISTEN_PORT': 'SERVER_PORT',
+                     'KEYSTONE_USER': 'OS_USERNAME',
+                     'KEYSTONE_PASS': 'OS_PASSWORD'}
 
 # Format: parameter: fallback parameter
-FALLBACK_TABLE = {'SERVER_PORT': 'LISTEN_PORT'}
+FALLBACK_TABLE = {DEPRECATION_TABLE[p]: p for p in DEPRECATION_TABLE}
 
 
 class FuelClientSettings(object):
@@ -101,25 +103,40 @@ class FuelClientSettings(object):
             if k in os.environ:
                 self.config[k] = os.environ[k]
 
-    def _check_deprecated(self):
+    def _print_deprecation_warning(self, old_option, new_option=None):
-        """Looks for deprecated options and prints warnings if finds any."""
+        """Print deprecation warning for an option."""
 
-        dep_msg = ('DEPRECATION WARNING: {old} parameter was deprecated and '
+        deprecation_tpl = ('DEPRECATION WARNING: {} parameter was '
-                   'will not be supported in the next version of '
+                           'deprecated and will not be supported in the next '
-                   'python-fuelclient.{repl}')
+                           'version of python-fuelclient.')
-        repl_msg = ' Please replace this parameter with {0}'
+        replace_tpl = ' Please replace this parameter with {}'
+
+        deprecation = deprecation_tpl.format(old_option)
+        replace = '' if new_option is None else replace_tpl.format(new_option)
+
+        six.print_(deprecation, end='')
+        six.print_(replace)
+
+    def _check_deprecated(self):
+        """Looks for deprecated options in user's configuration."""
 
         dep_opts = [opt for opt in self.config if opt in DEPRECATION_TABLE]
 
         for opt in dep_opts:
-            replace = ''
 
-            if DEPRECATION_TABLE.get(opt) is not None:
+            new_opt = DEPRECATION_TABLE.get(opt)
-                replace = repl_msg.format(DEPRECATION_TABLE.get(opt))
 
-            msg = dep_msg.format(old=opt, repl=replace)
+            # Clean up new option if it was not set by a user
+            # Produce a warning, if both old and new options are set.
+            if self.config.get(new_opt) is None:
+                self.config.pop(new_opt, None)
+            else:
+                six.print_('WARNING: configuration contains both {old} and '
+                           '{new} options set. Since {old} was deprecated, '
+                           'only the value of {new} '
+                           'will be used.'.format(old=opt, new=new_opt))
 
-            six.print_(msg)
+            self._print_deprecation_warning(opt, new_opt)
 
     def populate_default_settings(self, source, destination):
         """Puts default configuration file to a user's home directory."""
@@ -137,6 +154,16 @@ class FuelClientSettings(object):
                    'directory is writable')
             raise error.SettingsException(msg.format(dst_dir))
 
+    def update_from_command_line_options(self, options):
+        """Update parameters from valid command line options."""
+
+        for param in self.config:
+            opt_name = param.lower()
+
+            value = getattr(options, opt_name, None)
+            if value is not None:
+                self.config[param] = value
+
     def dump(self):
         return yaml.dump(self.config)
 

fuelclient/main.py

@@ -19,6 +19,8 @@ from cliff import app
 from cliff.commandmanager import CommandManager
 
 from fuelclient.actions import fuel_version
+from fuelclient import fuelclient_settings
+from fuelclient import utils
 
 
 LOG = logging.getLogger(__name__)
@@ -34,6 +36,7 @@ class FuelClient(app.App):
 
     def build_option_parser(self, description, version, argparse_kwargs=None):
         """Overrides default options for backwards compatibility."""
+
         p_inst = super(FuelClient, self)
         parser = p_inst.build_option_parser(description=description,
                                             version=version,
@@ -46,6 +49,8 @@ class FuelClient(app.App):
                   "WARNING: deprecated since 7.0 release. "
                   "Please use fuel-version command instead"))
 
+        utils.add_os_cli_parameters(parser)
+
         return parser
 
     def configure_logging(self):
@@ -65,6 +70,14 @@ class FuelClient(app.App):
                             'urllib3.connectionpool'):
             logging.getLogger(logger_name).setLevel(logging.WARNING)
 
+    def run(self, argv):
+        options, _ = self.parser.parse_known_args(argv)
+
+        settings = fuelclient_settings.get_settings()
+        settings.update_from_command_line_options(options)
+
+        return super(FuelClient, self).run(argv)
+
 
 def main(argv=sys.argv[1:]):
     fuelclient_app = FuelClient(

fuelclient/tests/unit/common/test_config.py

@@ -75,16 +75,41 @@ class TestSettings(base.UnitTestCase):
 
     @mock.patch('six.print_')
     def test_deprecated_option_produces_warning(self, m_print):
-        expected_waring = ('DEPRECATION WARNING: LISTEN_PORT parameter was '
+        expected_warings = [mock.call('DEPRECATION WARNING: LISTEN_PORT '
-                           'deprecated and will not be supported in the next '
+                                      'parameter was deprecated and will not '
-                           'version of python-fuelclient. Please replace this '
+                                      'be supported in the next version of '
-                           'parameter with SERVER_PORT')
+                                      'python-fuelclient.', end=''),
+                            mock.call(' Please replace this '
+                                      'parameter with SERVER_PORT')]
 
         m = mock.mock_open(read_data='LISTEN_PORT: 9000')
         with mock.patch('fuelclient.fuelclient_settings.open', m):
             fuelclient_settings.get_settings()
 
-        m_print.assert_called_once_with(expected_waring)
+        m_print.assert_has_calls(expected_warings, any_order=False)
+
+    @mock.patch('six.print_')
+    def test_both_deprecated_and_new_options_produce_warning(self, m_print):
+        expected_waring = ('WARNING: configuration contains both '
+                           'LISTEN_PORT and SERVER_PORT options set. Since '
+                           'LISTEN_PORT was deprecated, only the value of '
+                           'SERVER_PORT will be used.')
+
+        m = mock.mock_open(read_data='LISTEN_PORT: 9000\nSERVER_PORT: 9000')
+        with mock.patch('fuelclient.fuelclient_settings.open', m):
+            fuelclient_settings.get_settings()
+
+        m_print.assert_has_calls([mock.call(expected_waring)])
+
+    @mock.patch('six.print_')
+    def test_set_deprecated_option_overwrites_unset_new_option(self, m_print):
+        m = mock.mock_open(read_data='KEYSTONE_PASS: "admin"\n'
+                                     'OS_PASSWORD:\n')
+        with mock.patch('fuelclient.fuelclient_settings.open', m):
+            settings = fuelclient_settings.get_settings()
+
+        self.assertEqual('admin', settings.OS_PASSWORD)
+        self.assertNotIn('OS_PASSWORD', settings.config)
 
     def test_fallback_to_deprecated_option(self):
         m = mock.mock_open(read_data='LISTEN_PORT: 9000')
@@ -92,3 +117,26 @@ class TestSettings(base.UnitTestCase):
             settings = fuelclient_settings.get_settings()
 
         self.assertEqual(9000, settings.LISTEN_PORT)
+
+    def test_update_from_cli_params(self):
+        test_config_text = ('SERVER_ADDRESS: "127.0.0.1"\n'
+                            'SERVER_PORT: "8000"\n'
+                            'OS_USERNAME: "admin"\n'
+                            'OS_PASSWORD:\n'
+                            'OS_TENANT_NAME:\n')
+
+        test_parsed_args = mock.Mock(os_password='test_password',
+                                     server_port="3000",
+                                     os_username=None)
+        del test_parsed_args.server_address
+        del test_parsed_args.os_tenant_name
+
+        m = mock.mock_open(read_data=test_config_text)
+        with mock.patch('fuelclient.fuelclient_settings.open', m):
+            settings = fuelclient_settings.get_settings()
+
+        settings.update_from_command_line_options(test_parsed_args)
+
+        self.assertEqual('3000', settings.SERVER_PORT)
+        self.assertEqual('test_password', settings.OS_PASSWORD)
+        self.assertEqual('admin', settings.OS_USERNAME)

fuelclient/tests/unit/v1/test_authentication.py

@@ -14,87 +14,92 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from mock import Mock
+import fixtures
-from mock import patch
+import mock
-from six.moves import urllib
 
 from fuelclient import fuelclient_settings
 from fuelclient.tests.unit.v1 import base
 
 
+@mock.patch('keystoneclient.v2_0.client.Client',
+            return_value=mock.Mock(auth_token=''))
 class TestAuthentication(base.UnitTestCase):
 
     def setUp(self):
         super(TestAuthentication, self).setUp()
+
         self.auth_required_mock.return_value = True
-
-    def validate_credentials_response(self,
-                                      args,
-                                      username=None,
-                                      password=None,
-                                      tenant_name=None):
-        conf = fuelclient_settings.get_settings()
-
-        self.assertEqual(args['username'], username)
-        self.assertEqual(args['password'], password)
-        self.assertEqual(args['tenant_name'], tenant_name)
-        pr = urllib.parse.urlparse(args['auth_url'])
-        self.assertEqual(conf.SERVER_ADDRESS, pr.hostname)
-        self.assertEqual(int(conf.SERVER_PORT), int(pr.port))
-        self.assertEqual('/keystone/v2.0', pr.path)
-
-    @patch('fuelclient.client.auth_client')
-    def test_credentials(self, mkeystone_cli):
         self.m_request.get('/api/v1/nodes/', json={})
 
-        mkeystone_cli.return_value = Mock(auth_token='')
+        self.useFixture(fixtures.MockPatchObject(fuelclient_settings,
-        self.execute(
+                                                 '_SETTINGS',
-            ['fuel', '--user=a', '--password=b', 'node'])
+                                                 None))
-        self.validate_credentials_response(
+
-            mkeystone_cli.Client.call_args[1],
+    def validate_credentials_response(self, m_client, username=None,
-            username='a',
+                                      password=None, tenant_name=None):
-            password='b',
+        """Checks whether keystone was called properly."""
-            tenant_name='admin'
+
-        )
+        conf = fuelclient_settings.get_settings()
-        self.execute(
+
-            ['fuel', '--user=a', '--password', 'b', 'node'])
+        expected_url = 'http://{}:{}{}'.format(conf.SERVER_ADDRESS,
-        self.validate_credentials_response(
+                                               conf.SERVER_PORT,
-            mkeystone_cli.Client.call_args[1],
+                                               '/keystone/v2.0')
-            username='a',
+        m_client.__init__assert_called_once_with(auth_url=expected_url,
-            password='b',
+                                                 username=username,
-            tenant_name='admin'
+                                                 password=password,
-        )
+                                                 tenant_name=tenant_name)
-        self.execute(
+
-            ['fuel', '--user=a', '--password=b', '--tenant=t', 'node'])
+    def test_credentials_settings(self, mkeystone_cli):
-        self.validate_credentials_response(
+        self.useFixture(fixtures.EnvironmentVariable('OS_USERNAME'))
-            mkeystone_cli.Client.call_args[1],
+        self.useFixture(fixtures.EnvironmentVariable('OS_PASSWORD'))
-            username='a',
+        self.useFixture(fixtures.EnvironmentVariable('OS_TENANT_NAME'))
-            password='b',
+
-            tenant_name='t'
+        conf = fuelclient_settings.get_settings()
-        )
+        conf.config['OS_USERNAME'] = 'test_user'
-        self.execute(
+        conf.config['OS_PASSWORD'] = 'test_password'
-            ['fuel', '--user', 'a', '--password', 'b', '--tenant', 't',
+        conf.config['OS_TENANT_NAME'] = 'test_tenant_name'
-             'node'])
+
-        self.validate_credentials_response(
+        self.execute(['fuel', 'node'])
-            mkeystone_cli.Client.call_args[1],
+        self.validate_credentials_response(mkeystone_cli,
-            username='a',
+                                           username='test_user',
-            password='b',
+                                           password='test_password',
-            tenant_name='t'
+                                           tenant_name='test_tenant_name')
-        )
+
-        self.execute(
+    def test_credentials_cli(self, mkeystone_cli):
-            ['fuel', 'node', '--user=a', '--password=b', '--tenant=t'])
+        self.useFixture(fixtures.EnvironmentVariable('OS_USERNAME'))
-        self.validate_credentials_response(
+        self.useFixture(fixtures.EnvironmentVariable('OS_PASSWORD'))
-            mkeystone_cli.Client.call_args[1],
+        self.useFixture(fixtures.EnvironmentVariable('OS_TENANT_NAME'))
-            username='a',
+
-            password='b',
+        self.execute(['fuel', '--os-username=a', '--os-tenant-name=admin',
-            tenant_name='t'
+                      '--os-password=b', 'node'])
-        )
+        self.validate_credentials_response(mkeystone_cli,
-        self.execute(
+                                           username='a',
-            ['fuel', 'node', '--user', 'a', '--password', 'b',
+                                           password='b',
-             '--tenant', 't'])
+                                           tenant_name='admin')
-        self.validate_credentials_response(
+
-            mkeystone_cli.Client.call_args[1],
+    def test_authentication_env_variables(self, mkeystone_cli):
-            username='a',
+        self.useFixture(fixtures.EnvironmentVariable('OS_USERNAME', 'name'))
-            password='b',
+        self.useFixture(fixtures.EnvironmentVariable('OS_PASSWORD', 'pass'))
-            tenant_name='t'
+        self.useFixture(fixtures.EnvironmentVariable('OS_TENANT_NAME', 'ten'))
-        )
+
+        self.execute(['fuel', 'node'])
+        self.validate_credentials_response(mkeystone_cli,
+                                           username='name',
+                                           password='pass',
+                                           tenant_name='ten')
+
+    def test_credentials_override(self, mkeystone_cli):
+        self.useFixture(fixtures.EnvironmentVariable('OS_USERNAME'))
+        self.useFixture(fixtures.EnvironmentVariable('OS_PASSWORD', 'var_p'))
+        self.useFixture(fixtures.EnvironmentVariable('OS_TENANT_NAME', 'va_t'))
+
+        conf = fuelclient_settings.get_settings()
+        conf.config['OS_USERNAME'] = 'conf_user'
+        conf.config['OS_PASSWORD'] = 'conf_password'
+        conf.config['OS_TENANT_NAME'] = 'conf_tenant_name'
+
+        self.execute(['fuel', '--os-tenant-name=cli_tenant', 'node'])
+        self.validate_credentials_response(mkeystone_cli,
+                                           username='conf_user',
+                                           password='var_p',
+                                           tenant_name='cli_tenant')

fuelclient/tests/unit/v2/cli/test_engine.py

@@ -108,3 +108,18 @@ class BaseCLITest(oslo_base.BaseTestCase):
                                             expected_data,
                                             mock.ANY,
                                             mock.ANY)
+
+    @mock.patch('fuelclient.fuelclient_settings.'
+                'FuelClientSettings.update_from_command_line_options')
+    def test_settings_override_called(self, m_update):
+        cmd = ('--os-password tpass --os-username tname --os-tenant-name tten '
+               'node list')
+
+        self.exec_command(cmd)
+
+        m_update.assert_called_once_with(mock.ANY)
+        passed_settings = m_update.call_args[0][0]
+
+        self.assertEqual('tname', passed_settings.os_username)
+        self.assertEqual('tpass', passed_settings.os_password)
+        self.assertEqual('tten', passed_settings.os_tenant_name)

fuelclient/utils.py

@@ -194,3 +194,22 @@ def safe_deserialize(loader):
                                          ''.format(e.__class__.__name__,
                                                    six.text_type(e)))
     return wrapper
+
+
+def add_os_cli_parameters(parser):
+    parser.add_argument(
+        '--os-auth-url', metavar='<auth-url>',
+        help='Authentication URL, defaults to env[OS_AUTH_URL].')
+
+    parser.add_argument(
+        '--os-tenant-name', metavar='<auth-tenant-name>',
+        help='Authentication tenant name, defaults to '
+             'env[OS_TENANT_NAME].')
+
+    parser.add_argument(
+        '--os-username', metavar='<auth-username>',
+        help='Authentication username, defaults to env[OS_USERNAME].')
+
+    parser.add_argument(
+        '--os-password', metavar='<auth-password>',
+        help='Authentication password, defaults to env[OS_PASSWORD].')

tools/prepare_nailgun.sh

@@ -109,8 +109,9 @@ EOL
 # Connection settings
 SERVER_ADDRESS: "127.0.0.1"
 SERVER_PORT: "${NAILGUN_PORT}"
-KEYSTONE_USER: "admin"
+OS_USERNAME: "admin"
-KEYSTONE_PASS: "admin"
+OS_PASSWORD: "admin"
+OS_TENANT_NAME: "admin"
 EOL
 }