From 1a4d589e285a9176f6aff9b69e03f29bdbc4cdfd Mon Sep 17 00:00:00 2001 From: Lin Tan Date: Tue, 22 Dec 2015 14:04:08 +0800 Subject: [PATCH] Add choices option to several options This will help user to avoid wrong configs, and oslo_config exception ConfigFileValueError will be raised if a wrong value was given in the config file. The list of options: [DEFAULT]/auth_strategy [glance]/auth_strategy [glance]/glance_protocol [neutron]/auth_strategy [amt]/protocol [irmc]/remote_image_share_type [irmc]/port [irmc]/auth_method [irmc]/sensor_method Co-Authored-By: Anton Arefiev Closes-Bug: 1502027 Change-Id: I63cdfbf4eb44c07f7fefa8301e9cda1289654c94 --- ironic/api/app.py | 1 + ironic/common/image_service.py | 2 ++ ironic/dhcp/neutron.py | 1 + ironic/drivers/modules/amt/common.py | 1 + ironic/drivers/modules/irmc/boot.py | 9 ++------- ironic/drivers/modules/irmc/common.py | 3 +++ ironic/tests/unit/common/test_image_service.py | 11 +++++++++++ ironic/tests/unit/dhcp/test_neutron.py | 14 +++++++------- .../unit/drivers/modules/amt/test_common.py | 5 +++++ .../unit/drivers/modules/irmc/test_boot.py | 18 +++++++++--------- .../unit/drivers/modules/irmc/test_common.py | 14 ++++++++++++++ ...hoice-to-some-options-9fb327c48e6bfda1.yaml | 15 +++++++++++++++ 12 files changed, 71 insertions(+), 23 deletions(-) create mode 100644 releasenotes/notes/add-choice-to-some-options-9fb327c48e6bfda1.yaml diff --git a/ironic/api/app.py b/ironic/api/app.py index d3fdd531a8..779f839cbe 100644 --- a/ironic/api/app.py +++ b/ironic/api/app.py @@ -30,6 +30,7 @@ api_opts = [ cfg.StrOpt( 'auth_strategy', default='keystone', + choices=['noauth', 'keystone'], help=_('Authentication strategy used by ironic-api: one of "keystone" ' 'or "noauth". "noauth" should not be used in a production ' 'environment because all authentication will be disabled.')), diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py index 9fd1b6f4d9..8a26735fa5 100644 --- a/ironic/common/image_service.py +++ b/ironic/common/image_service.py @@ -51,6 +51,7 @@ glance_opts = [ help=_('Default glance port.')), cfg.StrOpt('glance_protocol', default='http', + choices=['http', 'https'], help=_('Default protocol to use when connecting to glance. ' 'Set to https for SSL.')), cfg.ListOpt('glance_api_servers', @@ -67,6 +68,7 @@ glance_opts = [ 'glance.')), cfg.StrOpt('auth_strategy', default='keystone', + choices=['keystone', 'noauth'], help=_('Authentication strategy to use when connecting to ' 'glance. Only "keystone" and "noauth" are currently ' 'supported by ironic.')), diff --git a/ironic/dhcp/neutron.py b/ironic/dhcp/neutron.py index dee9b6955c..e3b4a589c5 100644 --- a/ironic/dhcp/neutron.py +++ b/ironic/dhcp/neutron.py @@ -44,6 +44,7 @@ neutron_opts = [ help=_('Client retries in the case of a failed request.')), cfg.StrOpt('auth_strategy', default='keystone', + choices=['keystone', 'noauth'], help=_('Default authentication strategy to use when connecting ' 'to neutron. Can be either "keystone" or "noauth". ' 'Running neutron in noauth mode (related to but not ' diff --git a/ironic/drivers/modules/amt/common.py b/ironic/drivers/modules/amt/common.py index 7194bdf20e..003453d393 100644 --- a/ironic/drivers/modules/amt/common.py +++ b/ironic/drivers/modules/amt/common.py @@ -50,6 +50,7 @@ COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES) opts = [ cfg.StrOpt('protocol', default='http', + choices=['http', 'https'], help=_('Protocol used for AMT endpoint, ' 'support http/https')), cfg.IntOpt('awake_interval', diff --git a/ironic/drivers/modules/irmc/boot.py b/ironic/drivers/modules/irmc/boot.py index 975ad3491c..04f0de48cb 100644 --- a/ironic/drivers/modules/irmc/boot.py +++ b/ironic/drivers/modules/irmc/boot.py @@ -57,6 +57,8 @@ opts = [ help=_('IP of remote image server')), cfg.StrOpt('remote_image_share_type', default='CIFS', + choices=['CIFS', 'NFS'], + ignore_case=True, help=_('Share type of virtual media, either "NFS" or "CIFS"')), cfg.StrOpt('remote_image_share_name', default='share', @@ -81,8 +83,6 @@ REQUIRED_PROPERTIES = { COMMON_PROPERTIES = REQUIRED_PROPERTIES -SUPPORTED_SHARE_TYPES = ('nfs', 'cifs') - def _parse_config_option(): """Parse config file options. @@ -97,11 +97,6 @@ def _parse_config_option(): _("Value '%s' for remote_image_share_root isn't a directory " "or doesn't exist.") % CONF.irmc.remote_image_share_root) - if CONF.irmc.remote_image_share_type.lower() not in SUPPORTED_SHARE_TYPES: - error_msgs.append( - _("Value '%s' for remote_image_share_type is not supported " - "value either 'NFS' or 'CIFS'.") % - CONF.irmc.remote_image_share_type) if error_msgs: msg = (_("The following errors were encountered while parsing " "config file:%s") % error_msgs) diff --git a/ironic/drivers/modules/irmc/common.py b/ironic/drivers/modules/irmc/common.py index 4a4e8d3b09..d20585c184 100644 --- a/ironic/drivers/modules/irmc/common.py +++ b/ironic/drivers/modules/irmc/common.py @@ -27,10 +27,12 @@ scci = importutils.try_import('scciclient.irmc.scci') opts = [ cfg.PortOpt('port', default=443, + choices=[443, 80], help=_('Port to be used for iRMC operations, either 80 or ' '443')), cfg.StrOpt('auth_method', default='basic', + choices=['basic', 'digest'], help=_('Authentication method to be used for iRMC operations, ' 'either "basic" or "digest"')), cfg.IntOpt('client_timeout', @@ -38,6 +40,7 @@ opts = [ help=_('Timeout (in seconds) for iRMC operations')), cfg.StrOpt('sensor_method', default='ipmitool', + choices=['ipmitool', 'scci'], help=_('Sensor data retrieval method, either ' '"ipmitool" or "scci"')), ] diff --git a/ironic/tests/unit/common/test_image_service.py b/ironic/tests/unit/common/test_image_service.py index b06ac5cd11..4eb2c85062 100644 --- a/ironic/tests/unit/common/test_image_service.py +++ b/ironic/tests/unit/common/test_image_service.py @@ -15,6 +15,7 @@ import os import shutil import mock +from oslo_config import cfg import requests import sendfile import six @@ -330,3 +331,13 @@ class ServiceGetterTestCase(base.TestCase): image_href = 'usenet://alt.binaries.dvd/image.qcow2' self.assertRaises(exception.ImageRefValidationFailed, image_service.get_image_service, image_href) + + def test_out_range_auth_strategy(self): + self.assertRaises(ValueError, cfg.CONF.set_override, + 'auth_strategy', 'fake', 'glance', + enforce_type=True) + + def test_out_range_glance_protocol(self): + self.assertRaises(ValueError, cfg.CONF.set_override, + 'glance_protocol', 'fake', 'glance', + enforce_type=True) diff --git a/ironic/tests/unit/dhcp/test_neutron.py b/ironic/tests/unit/dhcp/test_neutron.py index eebbba4135..e3c856e332 100644 --- a/ironic/tests/unit/dhcp/test_neutron.py +++ b/ironic/tests/unit/dhcp/test_neutron.py @@ -15,8 +15,10 @@ # under the License. import mock + from neutronclient.common import exceptions as neutron_client_exc from neutronclient.v2_0 import client +from oslo_config import cfg from oslo_utils import uuidutils from ironic.common import dhcp_factory @@ -63,13 +65,6 @@ class TestNeutron(db_base.DbTestCase): dhcp_factory.DHCPFactory._dhcp_provider = None - def test__build_client_invalid_auth_strategy(self): - self.config(auth_strategy='wrong_config', group='neutron') - token = 'test-token-123' - self.assertRaises(exception.ConfigInvalid, - neutron._build_client, - token=token) - @mock.patch.object(client.Client, "__init__") def test__build_client_with_token(self, mock_client_init): token = 'test-token-123' @@ -487,3 +482,8 @@ class TestNeutron(db_base.DbTestCase): list_mock.assert_called_once_with( network_id='00000000-0000-0000-0000-000000000000') delete_mock.assert_called_once_with(self.neutron_port['id']) + + def test_out_range_auth_strategy(self): + self.assertRaises(ValueError, cfg.CONF.set_override, + 'auth_strategy', 'fake', 'neutron', + enforce_type=True) diff --git a/ironic/tests/unit/drivers/modules/amt/test_common.py b/ironic/tests/unit/drivers/modules/amt/test_common.py index f83ce9c31a..7cd3816e31 100644 --- a/ironic/tests/unit/drivers/modules/amt/test_common.py +++ b/ironic/tests/unit/drivers/modules/amt/test_common.py @@ -209,3 +209,8 @@ class AwakeAMTInterfaceTestCase(db_base.DbTestCase): CONF.set_override('awake_interval', 0, 'amt') amt_common.awake_amt_interface(self.node) self.assertFalse(mock_ex.called) + + def test_out_range_protocol(self): + self.assertRaises(ValueError, cfg.CONF.set_override, + 'protocol', 'fake', 'amt', + enforce_type=True) diff --git a/ironic/tests/unit/drivers/modules/irmc/test_boot.py b/ironic/tests/unit/drivers/modules/irmc/test_boot.py index 6bb9b9725f..c018f16c18 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_boot.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_boot.py @@ -87,15 +87,6 @@ class IRMCDeployPrivateMethodsTestCase(db_base.DbTestCase): irmc_boot._parse_config_option) isdir_mock.assert_called_once_with('/non_existed_root') - @mock.patch.object(os.path, 'isdir', spec_set=True, autospec=True) - def test__parse_config_option_wrong_share_type(self, isdir_mock): - CONF.irmc.remote_image_share_type = 'NTFS' - isdir_mock.return_value = True - - self.assertRaises(exception.InvalidParameterValue, - irmc_boot._parse_config_option) - isdir_mock.assert_called_once_with('/remote_image_share_root') - @mock.patch.object(os.path, 'isfile', spec_set=True, autospec=True) def test__parse_driver_info_in_share(self, isfile_mock): """With required 'irmc_deploy_iso' in share.""" @@ -1029,3 +1020,12 @@ class IRMCVirtualMediaBootTestCase(db_base.DbTestCase): task, 'boot.iso') node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, persistent=True) + + def test_remote_image_share_type_values(self): + cfg.CONF.set_override('remote_image_share_type', 'cifs', 'irmc', + enforce_type=True) + cfg.CONF.set_override('remote_image_share_type', 'nfs', 'irmc', + enforce_type=True) + self.assertRaises(ValueError, cfg.CONF.set_override, + 'remote_image_share_type', 'fake', 'irmc', + enforce_type=True) diff --git a/ironic/tests/unit/drivers/modules/irmc/test_common.py b/ironic/tests/unit/drivers/modules/irmc/test_common.py index ded48def13..31d7939610 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_common.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_common.py @@ -18,6 +18,8 @@ Test class for common methods used by iRMC modules. import mock +from oslo_config import cfg + from ironic.common import exception from ironic.conductor import task_manager from ironic.drivers.modules.irmc import common as irmc_common @@ -166,3 +168,15 @@ class IRMCCommonMethodsTestCase(db_base.DbTestCase): auth_method=self.info['irmc_auth_method'], client_timeout=self.info['irmc_client_timeout']) self.assertEqual('get_report', returned_mock_scci_get_report) + + def test_out_range_port(self): + self.assertRaises(ValueError, cfg.CONF.set_override, + 'port', 60, 'irmc', enforce_type=True) + + def test_out_range_auth_method(self): + self.assertRaises(ValueError, cfg.CONF.set_override, + 'auth_method', 'fake', 'irmc', enforce_type=True) + + def test_out_range_sensor_method(self): + self.assertRaises(ValueError, cfg.CONF.set_override, + 'sensor_method', 'fake', 'irmc', enforce_type=True) diff --git a/releasenotes/notes/add-choice-to-some-options-9fb327c48e6bfda1.yaml b/releasenotes/notes/add-choice-to-some-options-9fb327c48e6bfda1.yaml new file mode 100644 index 0000000000..87b405e001 --- /dev/null +++ b/releasenotes/notes/add-choice-to-some-options-9fb327c48e6bfda1.yaml @@ -0,0 +1,15 @@ +--- +upgrade: + - Add ``choices`` parameter to config options. Invalid values will be + rejected when first accessing them, which can happen in the middle of + deployment. + * [DEFAULT]/auth_strategy [keystone, noauth] + * [glance]/auth_strategy [keystone, noauth] + * [glance]/glance_protocol [http, https] + * [neutron]/auth_strategy [keystone, noauth] + * [amt]/protocol [http, https] + * [irmc]/remote_image_share_type [CIFS, NFS] + * [irmc]/port [443, 80] + * [irmc]/auth_method [basic, digest] + * [irmc]/sensor_method [ipmitool, scci] +