diff --git a/fuelclient/cli/actions/base.py b/fuelclient/cli/actions/base.py index 71c6a756..f99ec451 100644 --- a/fuelclient/cli/actions/base.py +++ b/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: diff --git a/fuelclient/cli/parser.py b/fuelclient/cli/parser.py index 8b2e37d2..2c8d3c48 100644 --- a/fuelclient/cli/parser.py +++ b/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"]) diff --git a/fuelclient/client.py b/fuelclient/client.py index 36057bc1..e3786c75 100644 --- a/fuelclient/client.py +++ b/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.password, new_pass) + self.keystone_client.users.update_own_password(conf.OS_PASSWORD, + 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() diff --git a/fuelclient/fuel_client.yaml b/fuelclient/fuel_client.yaml index 8ca62030..c89b3f3d 100644 --- a/fuelclient/fuel_client.yaml +++ b/fuelclient/fuel_client.yaml @@ -1,8 +1,12 @@ # Connection settings SERVER_ADDRESS: "127.0.0.1" SERVER_PORT: "8000" -KEYSTONE_USER: -KEYSTONE_PASS: +OS_USERNAME: +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 diff --git a/fuelclient/fuelclient_settings.py b/fuelclient/fuelclient_settings.py index e95cf33b..46776001 100644 --- a/fuelclient/fuelclient_settings.py +++ b/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): - """Looks for deprecated options and prints warnings if finds any.""" + def _print_deprecation_warning(self, old_option, new_option=None): + """Print deprecation warning for an option.""" - dep_msg = ('DEPRECATION WARNING: {old} parameter was deprecated and ' - 'will not be supported in the next version of ' - 'python-fuelclient.{repl}') - repl_msg = ' Please replace this parameter with {0}' + deprecation_tpl = ('DEPRECATION WARNING: {} parameter was ' + 'deprecated and will not be supported in the next ' + 'version of python-fuelclient.') + 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: - replace = repl_msg.format(DEPRECATION_TABLE.get(opt)) + new_opt = 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) diff --git a/fuelclient/main.py b/fuelclient/main.py index f87b3187..47611eb6 100644 --- a/fuelclient/main.py +++ b/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( diff --git a/fuelclient/tests/unit/common/test_config.py b/fuelclient/tests/unit/common/test_config.py index 123766cc..78da19ea 100644 --- a/fuelclient/tests/unit/common/test_config.py +++ b/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 ' - 'deprecated and will not be supported in the next ' - 'version of python-fuelclient. Please replace this ' - 'parameter with SERVER_PORT') + expected_warings = [mock.call('DEPRECATION WARNING: LISTEN_PORT ' + 'parameter was deprecated and will not ' + 'be supported in the next version of ' + '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) diff --git a/fuelclient/tests/unit/v1/test_authentication.py b/fuelclient/tests/unit/v1/test_authentication.py index e5568b8c..5e1948c2 100644 --- a/fuelclient/tests/unit/v1/test_authentication.py +++ b/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 -from mock import patch -from six.moves import urllib +import fixtures +import mock 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.execute( - ['fuel', '--user=a', '--password=b', 'node']) - self.validate_credentials_response( - mkeystone_cli.Client.call_args[1], - username='a', - password='b', - tenant_name='admin' - ) - self.execute( - ['fuel', '--user=a', '--password', 'b', 'node']) - self.validate_credentials_response( - mkeystone_cli.Client.call_args[1], - username='a', - password='b', - tenant_name='admin' - ) - self.execute( - ['fuel', '--user=a', '--password=b', '--tenant=t', 'node']) - self.validate_credentials_response( - mkeystone_cli.Client.call_args[1], - username='a', - password='b', - tenant_name='t' - ) - self.execute( - ['fuel', '--user', 'a', '--password', 'b', '--tenant', 't', - 'node']) - self.validate_credentials_response( - mkeystone_cli.Client.call_args[1], - username='a', - password='b', - tenant_name='t' - ) - self.execute( - ['fuel', 'node', '--user=a', '--password=b', '--tenant=t']) - self.validate_credentials_response( - mkeystone_cli.Client.call_args[1], - username='a', - password='b', - tenant_name='t' - ) - self.execute( - ['fuel', 'node', '--user', 'a', '--password', 'b', - '--tenant', 't']) - self.validate_credentials_response( - mkeystone_cli.Client.call_args[1], - username='a', - password='b', - tenant_name='t' - ) + self.useFixture(fixtures.MockPatchObject(fuelclient_settings, + '_SETTINGS', + None)) + + def validate_credentials_response(self, m_client, username=None, + password=None, tenant_name=None): + """Checks whether keystone was called properly.""" + + conf = fuelclient_settings.get_settings() + + expected_url = 'http://{}:{}{}'.format(conf.SERVER_ADDRESS, + conf.SERVER_PORT, + '/keystone/v2.0') + m_client.__init__assert_called_once_with(auth_url=expected_url, + username=username, + password=password, + tenant_name=tenant_name) + + def test_credentials_settings(self, mkeystone_cli): + self.useFixture(fixtures.EnvironmentVariable('OS_USERNAME')) + self.useFixture(fixtures.EnvironmentVariable('OS_PASSWORD')) + self.useFixture(fixtures.EnvironmentVariable('OS_TENANT_NAME')) + + conf = fuelclient_settings.get_settings() + conf.config['OS_USERNAME'] = 'test_user' + conf.config['OS_PASSWORD'] = 'test_password' + conf.config['OS_TENANT_NAME'] = 'test_tenant_name' + + self.execute(['fuel', 'node']) + self.validate_credentials_response(mkeystone_cli, + username='test_user', + password='test_password', + tenant_name='test_tenant_name') + + def test_credentials_cli(self, mkeystone_cli): + self.useFixture(fixtures.EnvironmentVariable('OS_USERNAME')) + self.useFixture(fixtures.EnvironmentVariable('OS_PASSWORD')) + self.useFixture(fixtures.EnvironmentVariable('OS_TENANT_NAME')) + + self.execute(['fuel', '--os-username=a', '--os-tenant-name=admin', + '--os-password=b', 'node']) + self.validate_credentials_response(mkeystone_cli, + username='a', + password='b', + tenant_name='admin') + + def test_authentication_env_variables(self, mkeystone_cli): + self.useFixture(fixtures.EnvironmentVariable('OS_USERNAME', 'name')) + self.useFixture(fixtures.EnvironmentVariable('OS_PASSWORD', 'pass')) + 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') diff --git a/fuelclient/tests/unit/v2/cli/test_engine.py b/fuelclient/tests/unit/v2/cli/test_engine.py index 6b30e229..e7888e84 100644 --- a/fuelclient/tests/unit/v2/cli/test_engine.py +++ b/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) diff --git a/fuelclient/utils.py b/fuelclient/utils.py index 5dfcaa08..cc0e6f96 100644 --- a/fuelclient/utils.py +++ b/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='', + help='Authentication URL, defaults to env[OS_AUTH_URL].') + + parser.add_argument( + '--os-tenant-name', metavar='', + help='Authentication tenant name, defaults to ' + 'env[OS_TENANT_NAME].') + + parser.add_argument( + '--os-username', metavar='', + help='Authentication username, defaults to env[OS_USERNAME].') + + parser.add_argument( + '--os-password', metavar='', + help='Authentication password, defaults to env[OS_PASSWORD].') diff --git a/tools/prepare_nailgun.sh b/tools/prepare_nailgun.sh index c97b4053..b509e1be 100644 --- a/tools/prepare_nailgun.sh +++ b/tools/prepare_nailgun.sh @@ -109,8 +109,9 @@ EOL # Connection settings SERVER_ADDRESS: "127.0.0.1" SERVER_PORT: "${NAILGUN_PORT}" -KEYSTONE_USER: "admin" -KEYSTONE_PASS: "admin" +OS_USERNAME: "admin" +OS_PASSWORD: "admin" +OS_TENANT_NAME: "admin" EOL }