From bd16059492b74f1711e873bf26b0c7491a974c6d Mon Sep 17 00:00:00 2001
From: "Jay S. Bryant" <jsbryant@us.ibm.com>
Date: Tue, 17 Dec 2013 19:22:06 -0600
Subject: [PATCH] Throw exception if --config-dir doesn't exist

Currently an invalid --config-dir option is silently ignored.
This means that a user could accidentally enter the wrong
directory or have a typo which would cause the desired
config files to not be used and no indication of the error would be
produced.

This change adds a ConfigDirNotFoundError exception that can be
raised when the requested --config-dir isn't found.

While working out the unit tests for this it was discovered that
the test_config_dir_tilde test case was not correct.  It could still
pass when it should fail.  This commit includes a fix for that issue
as well.

Closes-bug: 1255354
Change-Id: I386b2419d04572b14ccf11b527f871a237304e53
---
 oslo/config/cfg.py | 18 ++++++++++++++++-
 tests/test_cfg.py  | 50 +++++++++++++++++++++++++++++-----------------
 2 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/oslo/config/cfg.py b/oslo/config/cfg.py
index 653bed42..b5485b4a 100644
--- a/oslo/config/cfg.py
+++ b/oslo/config/cfg.py
@@ -387,6 +387,16 @@ class ConfigFilesNotFoundError(Error):
                 ",".join(self.config_files))
 
 
+class ConfigDirNotFoundError(Error):
+    """Raised if the requested config-dir is not found."""
+
+    def __init__(self, config_dir):
+        self.config_dir = config_dir
+
+    def __str__(self):
+        return ('Failed to read config file directory: %s' % self.config_dir)
+
+
 class ConfigFileParseError(Error):
     """Raised if there is an error parsing a config file."""
 
@@ -1045,10 +1055,16 @@ class _ConfigDirOpt(Opt):
         def __call__(self, parser, namespace, values, option_string=None):
             """Handle a --config-dir command line argument.
 
-            :raises: ConfigFileParseError, ConfigFileValueError
+            :raises: ConfigFileParseError, ConfigFileValueError,
+                     ConfigDirNotFoundError
             """
             setattr(namespace, self.dest, values)
 
+            values = os.path.expanduser(values)
+
+            if not os.path.exists(values):
+                raise ConfigDirNotFoundError(values)
+
             config_dir_glob = os.path.join(values, '*.conf')
 
             for config_file in sorted(glob.glob(config_dir_glob)):
diff --git a/tests/test_cfg.py b/tests/test_cfg.py
index 249f2197..ed354f6f 100644
--- a/tests/test_cfg.py
+++ b/tests/test_cfg.py
@@ -75,6 +75,10 @@ class ExceptionsTestCase(utils.BaseTestCase):
         msg = str(cfg.ConfigFilesNotFoundError(['foo', 'bar']))
         self.assertEqual(msg, 'Failed to read some config files: foo,bar')
 
+    def test_config_dir_not_found_error(self):
+        msg = str(cfg.ConfigDirNotFoundError('foobar'))
+        self.assertEqual(msg, 'Failed to read config file directory: foobar')
+
     def test_config_file_parse_error(self):
         msg = str(cfg.ConfigFileParseError('foo', 'foobar'))
         self.assertEqual(msg, 'Failed to parse foo: foobar')
@@ -2011,6 +2015,14 @@ class ConfigDirTestCase(BaseTestCase):
         self.assertTrue(hasattr(self.conf.snafu, 'bell'))
         self.assertEqual(self.conf.snafu.bell, 'whistle-11')
 
+    def test_config_dir_doesnt_exist(self):
+        tmpdir = '/tmp/foo'
+
+        self.assertRaises(cfg.ConfigDirNotFoundError,
+                          self.conf,
+                          ['--config-dir', tmpdir]
+                          )
+
 
 class ReparseTestCase(BaseTestCase):
 
@@ -2762,27 +2774,29 @@ class TildeExpansionTestCase(BaseTestCase):
 
     def test_config_dir_tilde(self):
         homedir = os.path.expanduser('~')
-        tmpdir = tempfile.mktemp(dir=homedir,
-                                 prefix='cfg-',
-                                 suffix='.d')
-        tmpfile = os.path.join(tmpdir, 'foo.conf')
-        tmpbase = os.path.basename(tmpfile)
-
-        self.useFixture(fixtures.MonkeyPatch(
-                        'glob.glob',
-                        lambda p: [tmpfile]))
-
         try:
-            self.conf(['--config-dir',
-                       os.path.join('~', os.path.basename(tmpdir))])
-        except cfg.ConfigFilesNotFoundError as cfnfe:
-            self.assertTrue(os.path.expanduser('~') in str(cfnfe))
+            tmpdir = tempfile.mkdtemp(dir=homedir,
+                                      prefix='cfg-',
+                                      suffix='.d')
+            tmpfile = os.path.join(tmpdir, 'foo.conf')
 
-        self.useFixture(fixtures.MonkeyPatch(
-            'os.path.exists',
-            lambda p: p == tmpfile))
+            self.useFixture(fixtures.MonkeyPatch(
+                            'glob.glob',
+                            lambda p: [tmpfile]))
 
-        self.assertEqual(self.conf.find_file(tmpbase), tmpfile)
+            e = self.assertRaises(cfg.ConfigFilesNotFoundError,
+                                  self.conf,
+                                  ['--config-dir',
+                                   os.path.join('~',
+                                                os.path.basename(tmpdir))]
+                                  )
+            self.assertIn(tmpdir, str(e))
+        finally:
+            try:
+                shutil.rmtree(tmpdir)
+            except OSError as exc:
+                if exc.errno != 2:
+                    raise
 
 
 class SubCommandTestCase(BaseTestCase):