From 1ad9bacd650e665989d4eb58011ea2a788efe690 Mon Sep 17 00:00:00 2001 From: Roman Prykhodchenko Date: Thu, 15 Oct 2015 12:10:48 +0200 Subject: [PATCH] Generate user settings file Since relying on a system-wide /etc/... configuration file is not a good approach for a user-side application it's necessary to not use that file by default. This patch changes default settings file to ~/.config/fuel/fuel_client.yaml and adds generation of one with default values if it does not exist. Specifying of a custom settings file using FUELCLIENT_CUSTOM_SETTINGS variable is still possible. DocImpact Closes-bug: #1458361 Change-Id: Ia17c8edbdd71a74318d3237a47d4b5650934c378 --- .testr.conf | 1 + MANIFEST.in | 2 +- fuelclient/cli/error.py | 6 +- fuelclient/cli/parser.py | 12 ---- ...lclient_settings.yaml => fuel_client.yaml} | 4 +- fuelclient/fuelclient_settings.py | 61 ++++++++++------ fuelclient/tests/unit/common/test_config.py | 69 +++++++++++++++++++ test-requirements.txt | 1 + 8 files changed, 117 insertions(+), 39 deletions(-) rename fuelclient/{fuelclient_settings.yaml => fuel_client.yaml} (89%) create mode 100644 fuelclient/tests/unit/common/test_config.py diff --git a/.testr.conf b/.testr.conf index d676cbd..00e20ad 100644 --- a/.testr.conf +++ b/.testr.conf @@ -2,3 +2,4 @@ test_command=OS_STDOUT_CAPTURE=1 OS_STDERR_CAPTURE=1 OS_LOG_CAPTURE=1 ${PYTHON:-python} -m subunit.run discover -t ./ ${OS_TEST_PATH:-./fuelclient/tests/unit} $LISTOPT $IDOPTION test_id_option=--load-list $IDFILE test_list_option=--list +test_run_concurrency=echo 1 diff --git a/MANIFEST.in b/MANIFEST.in index 12e5915..30fa462 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1 +1 @@ -include fuelclient/fuelclient_settings.yaml +include fuelclient/fuel_client.yaml diff --git a/fuelclient/cli/error.py b/fuelclient/cli/error.py index e7e24b8..4aa4cf5 100644 --- a/fuelclient/cli/error.py +++ b/fuelclient/cli/error.py @@ -116,15 +116,13 @@ def exceptions_decorator(func): except requests.ConnectionError: exit_with_error(""" Can't connect to Nailgun server! - Please modify "SERVER_ADDRESS" and "LISTEN_PORT" - in the file /etc/fuel/client/config.yaml""") + Please check connection settings in your configuration file.""") except Unauthorized: exit_with_error(""" Unauthorized: need authentication! Please provide user and password via client fuel --user=user --password=pass [action] - or modify "KEYSTONE_USER" and "KEYSTONE_PASS" in - /etc/fuel/client/config.yaml""") + or modify your credentials in your configuration file.""") except FuelClientException as exc: exit_with_error(exc.message) diff --git a/fuelclient/cli/parser.py b/fuelclient/cli/parser.py index 2769a34..a0a80b6 100644 --- a/fuelclient/cli/parser.py +++ b/fuelclient/cli/parser.py @@ -35,18 +35,6 @@ class Parser(object): self.args = argv self.parser = argparse.ArgumentParser( usage=""" - Default configuration for Fuel Client uses the - following parameters: - - SERVER_ADDRESS: "127.0.0.1" - LISTEN_PORT: "8000" - KEYSTONE_USER: "admin" - KEYSTONE_PASS: "admin" - - These options can be changed by putting some or all of them - into a yaml-formatted text file and specifying its full path - in the FUELCLIENT_CUSTOM_SETTINGS environment variable. - fuel [optional args] [action] [flags] DEPRECATION WARNING: diff --git a/fuelclient/fuelclient_settings.yaml b/fuelclient/fuel_client.yaml similarity index 89% rename from fuelclient/fuelclient_settings.yaml rename to fuelclient/fuel_client.yaml index cd71796..1dfdd6a 100644 --- a/fuelclient/fuelclient_settings.yaml +++ b/fuelclient/fuel_client.yaml @@ -1,8 +1,8 @@ # Connection settings SERVER_ADDRESS: "127.0.0.1" LISTEN_PORT: "8000" -KEYSTONE_USER: "admin" -KEYSTONE_PASS: "admin" +KEYSTONE_USER: +KEYSTONE_PASS: HTTP_PROXY: null HTTP_TIMEOUT: 10 diff --git a/fuelclient/fuelclient_settings.py b/fuelclient/fuelclient_settings.py index d1ea813..338eb2b 100644 --- a/fuelclient/fuelclient_settings.py +++ b/fuelclient/fuelclient_settings.py @@ -15,7 +15,8 @@ # under the License. import os -import sys +import pkg_resources +import shutil import six import yaml @@ -44,30 +45,34 @@ class FuelClientSettings(object): def __init__(self): settings_files = [] + user_conf_dir = os.getenv('XDG_CONFIG_HOME', + os.path.expanduser('~/.config/')) + # Look up for a default file distributed with the source code - project_path = os.path.dirname(__file__) - project_settings_file = os.path.join(project_path, - 'fuelclient_settings.yaml') - external_default_settings = '/etc/fuel/client/config.yaml' - external_user_settings = os.environ.get('FUELCLIENT_CUSTOM_SETTINGS') + default_settings = pkg_resources.resource_filename('fuelclient', + 'fuel_client.yaml') - # NOTE(romcheg): when external default settings file is removed - # this deprecation warning should be removed as well. - if os.path.exists(external_default_settings) and \ - external_default_settings != external_user_settings: - six.print_('DEPRECATION WARNING: {0} exists and will be ' - 'used as the source for settings. This behavior is ' - 'deprecated. Please specify the path to your custom ' - 'settings file in the FUELCLIENT_CUSTOM_SETTINGS ' - 'environment variable.'.format( - external_default_settings), - file=sys.stderr) + user_settings = os.path.join(user_conf_dir, 'fuel', 'fuel_client.yaml') + custom_settings = os.getenv('FUELCLIENT_CUSTOM_SETTINGS') - self._add_file_if_exists(project_settings_file, settings_files) - self._add_file_if_exists(external_default_settings, settings_files) + if not os.path.exists(user_settings) and not custom_settings: + self.populate_default_settings(default_settings, user_settings) + six.print_('Settings for Fuel Client have been saved to {0}.\n' + 'Consider changing default values to the ones which ' + 'are appropriate for you.'.format(user_settings)) + + self._add_file_if_exists(default_settings, settings_files) + self._add_file_if_exists(user_settings, settings_files) + + # NOTE(romcheg): this file is kept temporarily to + # keep master branch stable. + # As soon as puppet manifests utilize new config + # generation, this line should be removed + self._add_file_if_exists('/etc/fuel/client/config.yaml', + settings_files) # Add a custom settings file specified by user - self._add_file_if_exists(external_user_settings, settings_files) + self._add_file_if_exists(custom_settings, settings_files) self.config = {} for sf in settings_files: @@ -96,6 +101,22 @@ class FuelClientSettings(object): if k in os.environ: self.config[k] = os.environ[k] + def populate_default_settings(self, source, destination): + """Puts default configuration file to a user's home directory.""" + + try: + dst_dir = os.path.dirname(destination) + + if not os.path.exists(dst_dir): + os.makedirs(dst_dir, 0o700) + + shutil.copy(source, destination) + os.chmod(destination, 0o600) + except (IOError, OSError): + msg = ('Could not save settings to {0}. Please make sure the ' + 'directory is writable') + raise error.SettingsException(msg.format(dst_dir)) + def dump(self): return yaml.dump(self.config) diff --git a/fuelclient/tests/unit/common/test_config.py b/fuelclient/tests/unit/common/test_config.py new file mode 100644 index 0000000..ac07990 --- /dev/null +++ b/fuelclient/tests/unit/common/test_config.py @@ -0,0 +1,69 @@ +# -*- coding: utf-8 -*- +# +# Copyright 2015 Mirantis, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import os + +import fixtures +import mock + +from fuelclient.cli import error +from fuelclient import fuelclient_settings +from fuelclient.tests.unit.v1 import base + + +class TestSettings(base.UnitTestCase): + + @mock.patch('os.makedirs') + @mock.patch('shutil.copy') + @mock.patch('os.chmod') + @mock.patch('os.path.exists') + def test_config_generation(self, m_exists, m_chmod, m_copy, m_makedirs): + project_dir = os.path.dirname(fuelclient_settings.__file__) + + expected_fmode = 0o600 + expected_dmode = 0o700 + expected_default = os.path.join(project_dir, + 'fuel_client.yaml') + expected_path = os.path.expanduser('~/.config/fuel/fuel_client.yaml') + conf_home = os.path.expanduser('~/.config/') + conf_dir = os.path.dirname(expected_path) + + fuelclient_settings._SETTINGS = None + m_exists.return_value = False + f_confdir = fixtures.EnvironmentVariable('XDG_CONFIG_HOME', conf_home) + f_settings = fixtures.EnvironmentVariable('FUELCLIENT_CUSTOM_SETTINGS') + + self.useFixture(f_confdir) + self.useFixture(f_settings) + + fuelclient_settings.get_settings() + + m_makedirs.assert_called_once_with(conf_dir, expected_dmode) + m_copy.assert_called_once_with(expected_default, expected_path) + m_chmod.assert_called_once_with(expected_path, expected_fmode) + + @mock.patch('os.makedirs') + @mock.patch('os.path.exists') + def test_config_generation_write_error(self, m_exists, m_makedirs): + fuelclient_settings._SETTINGS = None + m_exists.return_value = False + m_makedirs.side_effect = OSError('[Errno 13] Permission denied') + + f_settings = fixtures.EnvironmentVariable('FUELCLIENT_CUSTOM_SETTINGS') + self.useFixture(f_settings) + + self.assertRaises(error.SettingsException, + fuelclient_settings.get_settings) diff --git a/test-requirements.txt b/test-requirements.txt index b1d481f..c2fb6fc 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,3 +1,4 @@ +fixtures>=0.3.14,<1.3.0 hacking>=0.10.0,<0.11 mock>=1.0 oslotest>=1.5.1,<1.6.0 # Apache-2.0