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 <aarefiev@mirantis.com>
Closes-Bug: 1502027
Change-Id: I63cdfbf4eb44c07f7fefa8301e9cda1289654c94

11 files changed, 56 insertions, 23 deletions

diff --git a/ironic/api/app.py b/ironic/api/app.py
index d3fdd531a..779f839cb 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 9fd1b6f4d..8a26735fa 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 dee9b6955..e3b4a589c 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 7194bdf20..003453d39 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 975ad3491..04f0de48c 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 4a4e8d3b0..d20585c18 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 b06ac5cd1..4eb2c8506 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 eebbba413..e3c856e33 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 f83ce9c31..7cd3816e3 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 6bb9b9725..c018f16c1 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 ded48def1..31d793961 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)