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
This commit is contained in:
Christian Hugo 2016-11-03 17:01:35 +01:00 committed by Tim Burke
parent a30351bdfc
commit ffd5194a3b
7 changed files with 46 additions and 16 deletions

View File

@ -14,6 +14,7 @@
# limitations under the License. # limitations under the License.
import os import os
import sys
import time import time
import signal import signal
from re import sub from re import sub
@ -74,8 +75,13 @@ def run_daemon(klass, conf_file, section_name='', once=False, **kwargs):
if section_name is '': if section_name is '':
section_name = sub(r'([a-z])([A-Z])', r'\1-\2', section_name = sub(r'([a-z])([A-Z])', r'\1-\2',
klass.__name__).lower() klass.__name__).lower()
conf = utils.readconf(conf_file, section_name, try:
log_name=kwargs.get('log_name')) 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 on command line (i.e. daemonize=false) will over-ride config
once = once or not utils.config_true_value(conf.get('daemonize', 'true')) once = once or not utils.config_true_value(conf.get('daemonize', 'true'))

View File

@ -2416,6 +2416,8 @@ def readconf(conf_path, section_name=None, log_name=None, defaults=None,
not defined) not defined)
:param defaults: dict of default values to pre-populate the config with :param defaults: dict of default values to pre-populate the config with
:returns: dict of config items :returns: dict of config items
:raises ValueError: if section_name does not exist
:raises IOError: if reading the file failed
""" """
if defaults is None: if defaults is None:
defaults = {} defaults = {}
@ -2432,15 +2434,15 @@ def readconf(conf_path, section_name=None, log_name=None, defaults=None,
else: else:
success = c.read(conf_path) success = c.read(conf_path)
if not success: if not success:
print(_("Unable to read config from %s") % conf_path) raise IOError(_("Unable to read config from %s") %
sys.exit(1) conf_path)
if section_name: if section_name:
if c.has_section(section_name): if c.has_section(section_name):
conf = dict(c.items(section_name)) conf = dict(c.items(section_name))
else: else:
print(_("Unable to find %(section)s config section in %(conf)s") % raise ValueError(
{'section': section_name, 'conf': conf_path}) _("Unable to find %(section)s config section in %(conf)s") %
sys.exit(1) {'section': section_name, 'conf': conf_path})
if "log_name" not in conf: if "log_name" not in conf:
if log_name is not None: if log_name is not None:
conf['log_name'] = log_name conf['log_name'] = log_name

View File

@ -54,9 +54,10 @@ class AuditorWorker(object):
# configure for their replicator # configure for their replicator
replicator_config = readconf(self.conf['__file__'], replicator_config = readconf(self.conf['__file__'],
'object-replicator') 'object-replicator')
except (KeyError, SystemExit): except (KeyError, ValueError, IOError):
# if we can't parse the real config (generally a KeyError on # 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 # a very conservative default for rsync_timeout
default_rsync_timeout = 86400 default_rsync_timeout = 86400
else: else:

View File

@ -61,14 +61,13 @@ def get_config(section_name=None, defaults=None):
'/etc/swift/test.conf') '/etc/swift/test.conf')
try: try:
config = readconf(config_file, section_name) config = readconf(config_file, section_name)
except SystemExit: except IOError:
if not os.path.exists(config_file): if not os.path.exists(config_file):
print('Unable to read test config %s - file not found' print('Unable to read test config %s - file not found'
% config_file, file=sys.stderr) % config_file, file=sys.stderr)
elif not os.access(config_file, os.R_OK): elif not os.access(config_file, os.R_OK):
print('Unable to read test config %s - permission denied' print('Unable to read test config %s - permission denied'
% config_file, file=sys.stderr) % config_file, file=sys.stderr)
else: except ValueError as e:
print('Unable to read test config %s - section %s not found' print(e)
% (config_file, section_name), file=sys.stderr)
return config return config

View File

@ -19,6 +19,7 @@ import os
import mock import mock
import unittest import unittest
from getpass import getuser
from swift.common import swob from swift.common import swob
from swift.common.middleware.crypto import keymaster from swift.common.middleware.crypto import keymaster
from swift.common.middleware.crypto.crypto_utils import CRYPTO_KEY_CALLBACK 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.helpers import FakeSwift, FakeAppThatExcepts
from test.unit.common.middleware.crypto.crypto_helpers import ( from test.unit.common.middleware.crypto.crypto_helpers import (
TEST_KEYMASTER_CONF) TEST_KEYMASTER_CONF)
from test.unit import tmpfile
def capture_start_response(): def capture_start_response():
@ -124,6 +126,14 @@ class TestKeymaster(unittest.TestCase):
start_response, _ = capture_start_response() start_response, _ = capture_start_response()
self.assertRaises(Exception, app, req.environ, 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): def test_root_secret(self):
for secret in (os.urandom(32), os.urandom(33), os.urandom(50)): for secret in (os.urandom(32), os.urandom(33), os.urandom(50)):
encoded_secret = base64.b64encode(secret) encoded_secret = base64.b64encode(secret)

View File

@ -124,6 +124,15 @@ class TestRunDaemon(unittest.TestCase):
daemon.run_daemon(MyDaemon, conf_file, logger=logger) daemon.run_daemon(MyDaemon, conf_file, logger=logger)
self.assertTrue('user quit' in sio.getvalue().lower()) 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__': if __name__ == '__main__':
unittest.main() unittest.main()

View File

@ -1983,9 +1983,12 @@ log_name = yarr'''
expected = {'__file__': conffile, 'log_name': 'section1', expected = {'__file__': conffile, 'log_name': 'section1',
'foo': 'bar', 'bar': 'baz'} 'foo': 'bar', 'bar': 'baz'}
self.assertEqual(result, expected) 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) os.unlink(temppath)
self.assertRaises(SystemExit, utils.readconf, temppath) self.assertRaises(IOError, utils.readconf, temppath)
def test_readconf_raw(self): def test_readconf_raw(self):
conf = '''[section1] conf = '''[section1]
@ -2009,7 +2012,7 @@ log_name = %(yarr)s'''
'section2': {'log_name': '%(yarr)s'}} 'section2': {'log_name': '%(yarr)s'}}
self.assertEqual(result, expected) self.assertEqual(result, expected)
os.unlink(temppath) os.unlink(temppath)
self.assertRaises(SystemExit, utils.readconf, temppath) self.assertRaises(IOError, utils.readconf, temppath)
def test_readconf_dir(self): def test_readconf_dir(self):
config_dir = { config_dir = {