From ffd5194a3be8cd5fced038cc25ea1d8f78be1ea9 Mon Sep 17 00:00:00 2001 From: Christian Hugo Date: Thu, 3 Nov 2016 17:01:35 +0100 Subject: [PATCH] Raise ValueError if a config section does not exist Instead of printing the error message and calling sys.exit() when a section not exists or reading the file failed rais an Exception from readconfig. Depending on the Value or IO-Error, the caller can decide if he wants to exit or continue. If an Exception reaches the wsgi utilities it bubbles all the way up. Change-Id: Ieb444f8c34e37f49bea21c3caf1c6c2d7bee5fb4 Closes-Bug: 1578321 --- swift/common/daemon.py | 10 ++++++++-- swift/common/utils.py | 12 +++++++----- swift/obj/auditor.py | 5 +++-- test/__init__.py | 7 +++---- test/unit/common/middleware/crypto/test_keymaster.py | 10 ++++++++++ test/unit/common/test_daemon.py | 9 +++++++++ test/unit/common/test_utils.py | 9 ++++++--- 7 files changed, 46 insertions(+), 16 deletions(-) diff --git a/swift/common/daemon.py b/swift/common/daemon.py index 7e30075184..39fc66c805 100644 --- a/swift/common/daemon.py +++ b/swift/common/daemon.py @@ -14,6 +14,7 @@ # limitations under the License. import os +import sys import time import signal from re import sub @@ -74,8 +75,13 @@ def run_daemon(klass, conf_file, section_name='', once=False, **kwargs): if section_name is '': section_name = sub(r'([a-z])([A-Z])', r'\1-\2', klass.__name__).lower() - conf = utils.readconf(conf_file, section_name, - log_name=kwargs.get('log_name')) + try: + conf = utils.readconf(conf_file, section_name, + log_name=kwargs.get('log_name')) + except (ValueError, IOError) as e: + # The message will be printed to stderr + # and results in an exit code of 1. + sys.exit(e) # once on command line (i.e. daemonize=false) will over-ride config once = once or not utils.config_true_value(conf.get('daemonize', 'true')) diff --git a/swift/common/utils.py b/swift/common/utils.py index 44f93f7bc2..94121e5b4f 100644 --- a/swift/common/utils.py +++ b/swift/common/utils.py @@ -2416,6 +2416,8 @@ def readconf(conf_path, section_name=None, log_name=None, defaults=None, not defined) :param defaults: dict of default values to pre-populate the config with :returns: dict of config items + :raises ValueError: if section_name does not exist + :raises IOError: if reading the file failed """ if defaults is None: defaults = {} @@ -2432,15 +2434,15 @@ def readconf(conf_path, section_name=None, log_name=None, defaults=None, else: success = c.read(conf_path) if not success: - print(_("Unable to read config from %s") % conf_path) - sys.exit(1) + raise IOError(_("Unable to read config from %s") % + conf_path) if section_name: if c.has_section(section_name): conf = dict(c.items(section_name)) else: - print(_("Unable to find %(section)s config section in %(conf)s") % - {'section': section_name, 'conf': conf_path}) - sys.exit(1) + raise ValueError( + _("Unable to find %(section)s config section in %(conf)s") % + {'section': section_name, 'conf': conf_path}) if "log_name" not in conf: if log_name is not None: conf['log_name'] = log_name diff --git a/swift/obj/auditor.py b/swift/obj/auditor.py index f2a38774e5..f08d6f0933 100644 --- a/swift/obj/auditor.py +++ b/swift/obj/auditor.py @@ -54,9 +54,10 @@ class AuditorWorker(object): # configure for their replicator replicator_config = readconf(self.conf['__file__'], 'object-replicator') - except (KeyError, SystemExit): + except (KeyError, ValueError, IOError): # if we can't parse the real config (generally a KeyError on - # __file__, or SystemExit on no object-replicator section) we use + # __file__, or ValueError on no object-replicator section, or + # IOError if reading the file failed) we use # a very conservative default for rsync_timeout default_rsync_timeout = 86400 else: diff --git a/test/__init__.py b/test/__init__.py index b3ebefe70c..10364a1a08 100644 --- a/test/__init__.py +++ b/test/__init__.py @@ -61,14 +61,13 @@ def get_config(section_name=None, defaults=None): '/etc/swift/test.conf') try: config = readconf(config_file, section_name) - except SystemExit: + except IOError: if not os.path.exists(config_file): print('Unable to read test config %s - file not found' % config_file, file=sys.stderr) elif not os.access(config_file, os.R_OK): print('Unable to read test config %s - permission denied' % config_file, file=sys.stderr) - else: - print('Unable to read test config %s - section %s not found' - % (config_file, section_name), file=sys.stderr) + except ValueError as e: + print(e) return config diff --git a/test/unit/common/middleware/crypto/test_keymaster.py b/test/unit/common/middleware/crypto/test_keymaster.py index 40ed32d7ab..71fa06009c 100644 --- a/test/unit/common/middleware/crypto/test_keymaster.py +++ b/test/unit/common/middleware/crypto/test_keymaster.py @@ -19,6 +19,7 @@ import os import mock import unittest +from getpass import getuser from swift.common import swob from swift.common.middleware.crypto import keymaster from swift.common.middleware.crypto.crypto_utils import CRYPTO_KEY_CALLBACK @@ -26,6 +27,7 @@ from swift.common.swob import Request from test.unit.common.middleware.helpers import FakeSwift, FakeAppThatExcepts from test.unit.common.middleware.crypto.crypto_helpers import ( TEST_KEYMASTER_CONF) +from test.unit import tmpfile def capture_start_response(): @@ -124,6 +126,14 @@ class TestKeymaster(unittest.TestCase): start_response, _ = capture_start_response() self.assertRaises(Exception, app, req.environ, start_response) + def test_missing_conf_section(self): + sample_conf = "[default]\nuser = %s\n" % getuser() + with tmpfile(sample_conf) as conf_file: + self.assertRaisesRegexp( + ValueError, 'Unable to find keymaster config section in.*', + keymaster.KeyMaster, self.swift, { + 'keymaster_config_path': conf_file}) + def test_root_secret(self): for secret in (os.urandom(32), os.urandom(33), os.urandom(50)): encoded_secret = base64.b64encode(secret) diff --git a/test/unit/common/test_daemon.py b/test/unit/common/test_daemon.py index a42d643471..92376ad8b2 100644 --- a/test/unit/common/test_daemon.py +++ b/test/unit/common/test_daemon.py @@ -124,6 +124,15 @@ class TestRunDaemon(unittest.TestCase): daemon.run_daemon(MyDaemon, conf_file, logger=logger) self.assertTrue('user quit' in sio.getvalue().lower()) + # test missing section + sample_conf = "[default]\nuser = %s\n" % getuser() + with tmpfile(sample_conf) as conf_file: + self.assertRaisesRegexp(SystemExit, + 'Unable to find my-daemon ' + 'config section in.*', + daemon.run_daemon, MyDaemon, + conf_file, once=True) + if __name__ == '__main__': unittest.main() diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index 0b25a2bf1c..bad704ce28 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -1983,9 +1983,12 @@ log_name = yarr''' expected = {'__file__': conffile, 'log_name': 'section1', 'foo': 'bar', 'bar': 'baz'} self.assertEqual(result, expected) - self.assertRaises(SystemExit, utils.readconf, temppath, 'section3') + + self.assertRaisesRegexp( + ValueError, 'Unable to find section3 config section in.*', + utils.readconf, temppath, 'section3') os.unlink(temppath) - self.assertRaises(SystemExit, utils.readconf, temppath) + self.assertRaises(IOError, utils.readconf, temppath) def test_readconf_raw(self): conf = '''[section1] @@ -2009,7 +2012,7 @@ log_name = %(yarr)s''' 'section2': {'log_name': '%(yarr)s'}} self.assertEqual(result, expected) os.unlink(temppath) - self.assertRaises(SystemExit, utils.readconf, temppath) + self.assertRaises(IOError, utils.readconf, temppath) def test_readconf_dir(self): config_dir = {