cfg: allow options to be marked as required

Implements blueprint cfg-required-options

Add a 'required' flag to option schemas:

  StrOpt('foo', required=True)

which causes a RequiredOptError exception to be raised if the
user fails to supply a value for the option on the CLI or in
a config file.

Change-Id: Ied7bb25f0c1582c4991d0f212f4871b9358b73fb

2 files changed, 140 insertions, 2 deletions

diff --git a/openstack/common/cfg.py b/openstack/common/cfg.py
index 08efeb7..1aaa69d 100644
--- a/openstack/common/cfg.py
+++ b/openstack/common/cfg.py
@@ -213,6 +213,9 @@ as the leftover arguments, but will instead return::
 
 i.e. argument parsing is stopped at the first non-option argument.
 
+Options may be declared as required so that an error is raised if the user
+does not supply a value for the option.
+
 Options may be declared as secret so that their values are not leaked into
 log files:
 
@@ -291,6 +294,21 @@ class DuplicateOptError(Error):
         return "duplicate option: %s" % self.opt_name
 
 
+class RequiredOptError(Error):
+    """Raised if an option is required but no value is supplied by the user."""
+
+    def __init__(self, opt_name, group=None):
+        self.opt_name = opt_name
+        self.group = group
+
+    def __str__(self):
+        if self.group is None:
+            return "value required for option: %s" % self.opt_name
+        else:
+            return "value required for option: %s.%s" % (self.group.name,
+                                                         self.opt_name)
+
+
 class TemplateSubstitutionError(Error):
     """Raised if an error occurs substituting a variable in an opt value."""
 
@@ -452,7 +470,7 @@ class Opt(object):
     multi = False
 
     def __init__(self, name, dest=None, short=None, default=None,
-                 metavar=None, help=None, secret=False):
+                 metavar=None, help=None, secret=False, required=False):
         """Construct an Opt object.
 
         The only required parameter is the option's name. However, it is
@@ -465,6 +483,7 @@ class Opt(object):
         :param metavar: the option argument to show in --help
         :param help: an explanation of how the option is used
         :param secret: true iff the value should be obfuscated in log output
+        :param required: true iff a value must be supplied for this option
         """
         self.name = name
         if dest is None:
@@ -476,6 +495,7 @@ class Opt(object):
         self.metavar = metavar
         self.help = help
         self.secret = secret
+        self.required = required
 
     def _get_from_config_parser(self, cparser, section):
         """Retrieves the option value from a MultiConfigParser object.
@@ -909,7 +929,8 @@ class ConfigOpts(collections.Mapping):
 
         :params args: command line arguments (defaults to sys.argv[1:])
         :returns: the list of arguments left over after parsing options
-        :raises: SystemExit, ConfigFilesNotFoundError, ConfigFileParseError
+        :raises: SystemExit, ConfigFilesNotFoundError, ConfigFileParseError,
+                 RequiredOptError
         """
         self.reset()
 
@@ -928,6 +949,8 @@ class ConfigOpts(collections.Mapping):
 
         self._parse_config_files(from_file + from_dir)
 
+        self._check_required_opts()
+
         return args
 
     def __getattr__(self, name):
@@ -1298,6 +1321,28 @@ class ConfigOpts(collections.Mapping):
             not_read_ok = filter(lambda f: f not in read_ok, config_files)
             raise ConfigFilesNotFoundError(not_read_ok)
 
+    def _do_check_required_opts(self, opts, group=None):
+        for info in opts.values():
+            default, opt, override = [info[k] for k in sorted(info.keys())]
+
+            if opt.required:
+                if (default is not None or
+                    override is not None):
+                    continue
+
+                if self._get(opt.name, group) is None:
+                    raise RequiredOptError(opt.name, group)
+
+    def _check_required_opts(self):
+        """Check that all opts marked as required have values specified.
+
+        :raises: RequiredOptError
+        """
+        self._do_check_required_opts(self._opts)
+
+        for group in self._groups.values():
+            self._do_check_required_opts(group._opts, group)
+
     class GroupAttr(collections.Mapping):
 
         """
diff --git a/tests/unit/test_cfg.py b/tests/unit/test_cfg.py
index 785a324..f16dfb4 100644
--- a/tests/unit/test_cfg.py
+++ b/tests/unit/test_cfg.py
@@ -52,6 +52,14 @@ class ExceptionsTestCase(unittest.TestCase):
         msg = str(DuplicateOptError('foo'))
         self.assertEquals(msg, 'duplicate option: foo')
 
+    def test_required_opt_error(self):
+        msg = str(RequiredOptError('foo'))
+        self.assertEquals(msg, 'value required for option: foo')
+
+    def test_required_opt_error_with_group(self):
+        msg = str(RequiredOptError('foo', OptGroup('bar')))
+        self.assertEquals(msg, 'value required for option: bar.foo')
+
     def test_template_substitution_error(self):
         msg = str(TemplateSubstitutionError('foobar'))
         self.assertEquals(msg, 'template substitution error: foobar')
@@ -863,6 +871,91 @@ class OverridesTestCase(BaseTestCase):
         self.assertEquals(self.conf.blaa.foo, 'bar')
 
 
+class RequiredOptsTestCase(BaseTestCase):
+
+    def setUp(self):
+        BaseTestCase.setUp(self)
+        self.conf.register_opt(StrOpt('boo', required=False))
+
+    def test_required_opt(self):
+        self.conf.register_opt(StrOpt('foo', required=True))
+
+        paths = self.create_tempfiles([('test',
+                                        '[DEFAULT]\n'
+                                        'foo = bar')])
+
+        self.conf(['--config-file', paths[0]])
+        self.assertTrue(hasattr(self.conf, 'foo'))
+        self.assertEquals(self.conf.foo, 'bar')
+
+    def test_required_cli_opt(self):
+        self.conf.register_cli_opt(StrOpt('foo', required=True))
+
+        self.conf(['--foo', 'bar'])
+
+        self.assertTrue(hasattr(self.conf, 'foo'))
+        self.assertEquals(self.conf.foo, 'bar')
+
+    def test_missing_required_opt(self):
+        self.conf.register_opt(StrOpt('foo', required=True))
+        self.assertRaises(RequiredOptError, self.conf, [])
+
+    def test_missing_required_cli_opt(self):
+        self.conf.register_cli_opt(StrOpt('foo', required=True))
+        self.assertRaises(RequiredOptError, self.conf, [])
+
+    def test_required_group_opt(self):
+        self.conf.register_group(OptGroup('blaa'))
+        self.conf.register_opt(StrOpt('foo', required=True), group='blaa')
+
+        paths = self.create_tempfiles([('test',
+                                        '[blaa]\n'
+                                        'foo = bar')])
+
+        self.conf(['--config-file', paths[0]])
+        self.assertTrue(hasattr(self.conf, 'blaa'))
+        self.assertTrue(hasattr(self.conf.blaa, 'foo'))
+        self.assertEquals(self.conf.blaa.foo, 'bar')
+
+    def test_required_cli_group_opt(self):
+        self.conf.register_group(OptGroup('blaa'))
+        self.conf.register_cli_opt(StrOpt('foo', required=True), group='blaa')
+
+        self.conf(['--blaa-foo', 'bar'])
+
+        self.assertTrue(hasattr(self.conf, 'blaa'))
+        self.assertTrue(hasattr(self.conf.blaa, 'foo'))
+        self.assertEquals(self.conf.blaa.foo, 'bar')
+
+    def test_missing_required_group_opt(self):
+        self.conf.register_group(OptGroup('blaa'))
+        self.conf.register_opt(StrOpt('foo', required=True), group='blaa')
+        self.assertRaises(RequiredOptError, self.conf, [])
+
+    def test_missing_required_cli_group_opt(self):
+        self.conf.register_group(OptGroup('blaa'))
+        self.conf.register_cli_opt(StrOpt('foo', required=True), group='blaa')
+        self.assertRaises(RequiredOptError, self.conf, [])
+
+    def test_required_opt_with_default(self):
+        self.conf.register_cli_opt(StrOpt('foo', required=True))
+        self.conf.set_default('foo', 'bar')
+
+        self.conf([])
+
+        self.assertTrue(hasattr(self.conf, 'foo'))
+        self.assertEquals(self.conf.foo, 'bar')
+
+    def test_required_opt_with_override(self):
+        self.conf.register_cli_opt(StrOpt('foo', required=True))
+        self.conf.set_override('foo', 'bar')
+
+        self.conf([])
+
+        self.assertTrue(hasattr(self.conf, 'foo'))
+        self.assertEquals(self.conf.foo, 'bar')
+
+
 class SadPathTestCase(BaseTestCase):
 
     def test_unknown_attr(self):