From 8120e03d40cadf06dd3e1ad3eda6fd97627b98fa Mon Sep 17 00:00:00 2001
From: Yu Ming Zhu <yzhu@redhat.com>
Date: Oct 09 2019 06:12:02 +0000
Subject: [PATCH 1/4] extract read_config_files util for config parsing


---

diff --git a/builder/kojid b/builder/kojid
index 8a308d0..e81d28c 100755
--- a/builder/kojid
+++ b/builder/kojid
@@ -47,7 +47,6 @@ import time
 import traceback
 import xml.dom.minidom
 import zipfile
-from six.moves.configparser import RawConfigParser
 from fnmatch import fnmatch
 from gzip import GzipFile
 from optparse import OptionParser, SUPPRESS_HELP
@@ -6100,8 +6099,7 @@ def get_options():
         assert False  # pragma: no cover
 
     # load local config
-    config = RawConfigParser()
-    config.read(options.configFile)
+    config = koji.read_config_files(options.configFile)
     for x in config.sections():
         if x != 'kojid':
             quit('invalid section found in config file: %s' % x)
diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py
index 513ee22..1fdd4bb 100644
--- a/cli/koji_cli/commands.py
+++ b/cli/koji_cli/commands.py
@@ -5904,10 +5904,8 @@ def handle_image_build(options, session, args):
     (task_options, args) = parser.parse_args(args)
 
     if task_options.config:
-        if not os.path.exists(task_options.config):
-            parser.error(_("%s not found!" % task_options.config))
         section = 'image-build'
-        config = koji.read_config_files(task_options.config)
+        config = koji.read_config_files(task_options.config, strict=True)
         if not config.has_section(section):
             parser.error(_("single section called [%s] is required" % section))
         # pluck out the positional arguments first
diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py
index 18b11ec..f0d9fb8 100644
--- a/hub/kojixmlrpc.py
+++ b/hub/kojixmlrpc.py
@@ -20,7 +20,6 @@
 
 from __future__ import absolute_import
 from __future__ import division
-from six.moves.configparser import RawConfigParser
 import datetime
 import inspect
 import logging
@@ -396,17 +395,8 @@ def load_config(environ):
     #get our config file(s)
     cf = environ.get('koji.hub.ConfigFile', '/etc/koji-hub/hub.conf')
     cfdir = environ.get('koji.hub.ConfigDir', '/etc/koji-hub/hub.conf.d')
-    if cfdir:
-        configs = koji.config_directory_contents(cfdir)
-    else:
-        configs = []
-    if cf and os.path.isfile(cf):
-        configs.append(cf)
-    if configs:
-        config = RawConfigParser()
-        config.read(configs)
-    else:
-        config = None
+    config = koji.read_config_files([cfdir, cf], raw=True, strict=True)
+
     cfgmap = [
         #option, type, default
         ['DBName', 'string', None],
diff --git a/koji/__init__.py b/koji/__init__.py
index db1ed2e..05add50 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -1685,7 +1685,8 @@ def config_directory_contents(dir_name):
             if not name.endswith('.conf'):
                 continue
             config_full_name = os.path.join(dir_name, name)
-            configs.append(config_full_name)
+            if os.path.isfile(config_full_name):
+                configs.append(config_full_name)
     return configs
 
 
@@ -1726,60 +1727,50 @@ def read_config(profile_name, user_config=None):
 
     #note: later config files override earlier ones
 
+    strict = False
     # /etc/koji.conf.d
-    configs = config_directory_contents('/etc/koji.conf.d')
+    configs = ['/etc/koji.conf.d']
 
     # /etc/koji.conf
-    if os.access('/etc/koji.conf', os.F_OK):
-        configs.append('/etc/koji.conf')
+    configs.append('/etc/koji.conf')
 
     # User specific configuration
     if user_config:
         # Config file specified on command line
-        fn = os.path.expanduser(user_config)
-        if os.path.isdir(fn):
-            # Specified config is a directory
-            contents = config_directory_contents(fn)
-            if not contents:
-                raise ConfigurationError("No config files found in directory: %s" % fn)
-            configs.extend(contents)
-        else:
-            # Specified config is a file
-            if not os.access(fn, os.F_OK):
-                raise ConfigurationError("No such file: %s" % fn)
-            configs.append(fn)
+        # The existence will be checked
+        configs.append(os.path.expanduser(user_config))
+        strict = True
     else:
-        # User config
-        user_config_dir = os.path.expanduser("~/.koji/config.d")
-        configs.extend(config_directory_contents(user_config_dir))
-        fn = os.path.expanduser("~/.koji/config")
-        if os.access(fn, os.F_OK):
-            configs.append(fn)
+        # User config dir
+        configs.append(os.path.expanduser("~/.koji/config.d"))
+        # User config file
+        configs.append(os.path.expanduser("~/.koji/config"))
+
+    config = read_config_files(configs, strict=strict)
 
     # Load the configs in a particular order
     got_conf = False
-    for configFile in configs:
-        config = read_config_files(configFile)
-        if config.has_section(profile_name):
-            got_conf = True
-            for name, value in config.items(profile_name):
-                #note the config_defaults dictionary also serves to indicate which
-                #options *can* be set via the config file. Such options should
-                #not have a default value set in the option parser.
-                if name in result:
-                    if name in ('anon_retry', 'offline_retry', 'use_fast_upload',
-                                'krb_rdns', 'debug', 'debug', 'debug_xmlrpc', 'krb_canon_host'):
-                        result[name] = config.getboolean(profile_name, name)
-                    elif name in ('max_retries', 'retry_interval',
-                                  'offline_retry_interval', 'poll_interval',
-                                  'timeout', 'auth_timeout',
-                                  'upload_blocksize', 'pyver'):
-                        try:
-                            result[name] = int(value)
-                        except ValueError:
-                            raise ConfigurationError("value for %s config option must be a valid integer" % name)
-                    else:
-                        result[name] = value
+    if config.has_section(profile_name):
+        got_conf = True
+        for name, value in config.items(profile_name):
+            #note the config_defaults dictionary also serves to indicate which
+            #options *can* be set via the config file. Such options should
+            #not have a default value set in the option parser.
+            if name in result:
+                if name in ('anon_retry', 'offline_retry',
+                            'use_fast_upload', 'krb_rdns', 'debug',
+                            'debug_xmlrpc', 'krb_canon_host'):
+                    result[name] = config.getboolean(profile_name, name)
+                elif name in ('max_retries', 'retry_interval',
+                              'offline_retry_interval', 'poll_interval',
+                              'timeout', 'auth_timeout',
+                              'upload_blocksize', 'pyver'):
+                    try:
+                        result[name] = int(value)
+                    except ValueError:
+                        raise ConfigurationError("value for %s config option must be a valid integer" % name)
+                else:
+                    result[name] = value
 
     # Check if the specified profile had a config specified
     if configs and not got_conf:
@@ -1852,17 +1843,20 @@ def get_profile_module(profile_name, config=None):
     return mod
 
 
-def read_config_files(config_files, raw=False):
+def read_config_files(config_files, default=None, raw=False, strict=False):
     """Use parser to read config file(s)
 
-    :param config_files: config file(s) to read (required).
+    :param config_files: config file(s) to read (required). Config file could
+                         be file or directory, and order is preserved
     :type config_files: str or list
+    :param str default: default config file which is loaded before
+                        config_files. If specified, it must be readable.
     :param bool raw: enable 'raw' parsing (no interpolation). Default: False
+    :param bool strict: enable reading check for each item in config_files.
+                        Default: False
 
     :return: object of parser which contains parsed content
     """
-    if not isinstance(config_files, (list, tuple)):
-        config_files = [config_files]
     if raw:
         parser = six.moves.configparser.RawConfigParser
     elif six.PY2:
@@ -1872,12 +1866,33 @@ def read_config_files(config_files, raw=False):
         # deprecated alias
         parser = six.moves.configparser.ConfigParser
     config = parser()
-    for config_file in config_files:
-        with open(config_file, 'r') as f:
+    if default:
+        with open(default, 'r') as f:
             if six.PY2:
                 config.readfp(f)
             else:
                 config.read_file(f)
+    if not isinstance(config_files, (list, tuple)):
+        config_files = [config_files]
+    cfgs = []
+    # append dir contents
+    for config_file in config_files:
+        if os.path.isdir(config_file):
+            fns = config_directory_contents(config_file)
+            if fns:
+                cfgs.extend(fns)
+            else:
+                logging.debug("No config files found in directory: %s"
+                              % config_file)
+        else:
+            cfgs.append(config_file)
+    # checking access if strict is True
+    if strict:
+        for cf in cfgs:
+            if not os.path.isfile(cf) or not os.access(cf, os.F_OK):
+                raise ConfigurationError("Config file %s can't be opened."
+                                         % cf)
+    config.read(cfgs)
     return config
 
 
diff --git a/plugins/builder/runroot.py b/plugins/builder/runroot.py
index c667089..5490d96 100644
--- a/plugins/builder/runroot.py
+++ b/plugins/builder/runroot.py
@@ -49,8 +49,7 @@ class RunRootTask(koji.tasks.BaseTaskHandler):
         return res
 
     def _read_config(self):
-        cp = six.moves.configparser.SafeConfigParser()
-        cp.read(CONFIG_FILE)
+        cp = koji.read_config_files(CONFIG_FILE)
         self.config = {
            'default_mounts': [],
            'safe_roots': [],
diff --git a/plugins/builder/save_failed_tree.py b/plugins/builder/save_failed_tree.py
index 7c579a7..f449088 100644
--- a/plugins/builder/save_failed_tree.py
+++ b/plugins/builder/save_failed_tree.py
@@ -3,7 +3,6 @@ import fnmatch
 import os
 import sys
 import tarfile
-import six.moves.configparser
 
 import koji
 import koji.tasks as tasks
@@ -28,8 +27,7 @@ def omit_paths3(tarinfo):
 
 def read_config():
     global config
-    cp = six.moves.configparser.SafeConfigParser()
-    cp.read(CONFIG_FILE)
+    cp = koji.read_config_files(CONFIG_FILE)
     config = {
         'path_filters': [],
         'volume': None,
diff --git a/plugins/hub/protonmsg.py b/plugins/hub/protonmsg.py
index 4953529..8a1953c 100644
--- a/plugins/hub/protonmsg.py
+++ b/plugins/hub/protonmsg.py
@@ -269,7 +269,7 @@ def send_queued_msgs(cbtype, *args, **kws):
     log = logging.getLogger('koji.plugin.protonmsg')
     global CONFIG
     if not CONFIG:
-        CONFIG = koji.read_config_files(CONFIG_FILE)
+        CONFIG = koji.read_config_files(CONFIG_FILE, strict=True)
     urls = CONFIG.get('broker', 'urls').split()
     test_mode = False
     if CONFIG.has_option('broker', 'test_mode'):
diff --git a/plugins/hub/rpm2maven.py b/plugins/hub/rpm2maven.py
index 717df54..bb42340 100644
--- a/plugins/hub/rpm2maven.py
+++ b/plugins/hub/rpm2maven.py
@@ -10,16 +10,15 @@ import koji
 from koji.context import context
 from koji.plugin import callback
 from koji.util import rmtree
-import six.moves.configparser
 import fnmatch
 import os
-import shutil
 import subprocess
 
 CONFIG_FILE = '/etc/koji-hub/plugins/rpm2maven.conf'
 
 config = None
 
+
 @callback('postImport')
 def maven_import(cbtype, *args, **kws):
     global config
@@ -32,8 +31,7 @@ def maven_import(cbtype, *args, **kws):
     filepath = kws['filepath']
 
     if not config:
-        config = six.moves.configparser.SafeConfigParser()
-        config.read(CONFIG_FILE)
+        config = koji.read_config_files(CONFIG_FILE, strict=True)
     name_patterns = config.get('patterns', 'rpm_names').split()
     for pattern in name_patterns:
         if fnmatch.fnmatch(rpminfo['name'], pattern):
@@ -52,6 +50,7 @@ def maven_import(cbtype, *args, **kws):
         if os.path.exists(tmpdir):
             rmtree(tmpdir)
 
+
 def expand_rpm(filepath, tmpdir):
     devnull = open('/dev/null', 'r+')
     rpm2cpio = subprocess.Popen(['/usr/bin/rpm2cpio', filepath],
@@ -69,6 +68,7 @@ def expand_rpm(filepath, tmpdir):
               (filepath, rpm2cpio.wait(), cpio.wait()))
     devnull.close()
 
+
 def scan_and_import(buildinfo, rpminfo, tmpdir):
     global config
     path_patterns = config.get('patterns', 'artifact_paths').split()
diff --git a/plugins/hub/save_failed_tree.py b/plugins/hub/save_failed_tree.py
index 103bb95..12c0f47 100644
--- a/plugins/hub/save_failed_tree.py
+++ b/plugins/hub/save_failed_tree.py
@@ -1,6 +1,5 @@
 from __future__ import absolute_import
 import sys
-import six.moves.configparser
 import koji
 from koji.context import context
 from koji.plugin import export
@@ -29,8 +28,7 @@ def saveFailedTree(buildrootID, full=False, **opts):
 
     # read configuration only once
     if config is None:
-        config = six.moves.configparser.SafeConfigParser()
-        config.read(CONFIG_FILE)
+        config = koji.read_config_files(CONFIG_FILE, strict=True)
         allowed_methods = config.get('permissions', 'allowed_methods').split()
         if len(allowed_methods) == 1 and allowed_methods[0] == '*':
             allowed_methods = '*'
diff --git a/tests/test_cli/data/image-build-config-empty.conf b/tests/test_cli/data/image-build-config-empty.conf
new file mode 100644
index 0000000..8d3bc94
--- /dev/null
+++ b/tests/test_cli/data/image-build-config-empty.conf
@@ -0,0 +1 @@
+# empty config file
\ No newline at end of file
diff --git a/tests/test_cli/test_image_build.py b/tests/test_cli/test_image_build.py
index 15f240a..f9211ca 100644
--- a/tests/test_cli/test_image_build.py
+++ b/tests/test_cli/test_image_build.py
@@ -12,7 +12,10 @@ import koji
 from koji_cli.commands import handle_image_build, _build_image_oz
 from . import utils
 
-ConfigParser = six.moves.configparser.ConfigParser
+if six.PY2:
+    ConfigParser = six.moves.configparser.SafeConfigParser
+else:
+    ConfigParser = six.moves.configparser.ConfigParser
 
 TASK_OPTIONS = {
     "background": None,
@@ -240,28 +243,32 @@ class TestImageBuild(utils.CliTestCase):
         """Test handle_image_build argument with --config cases"""
 
         # Case 1, config file not exist case
-        self.assert_system_exit(
-                handle_image_build,
-                self.options,
-                self.session,
-                ['--config', '/nonexistent-file-755684354'],
-                stderr=self.format_error_message('/nonexistent-file-755684354 not found!'),
-                activate_session=None)
+        with self.assertRaises(koji.ConfigurationError) as cm:
+            handle_image_build(self.options,
+                               self.session,
+                               ['--config', '/nonexistent-file-755684354'])
+        self.assertEqual(cm.exception.args[0],
+                         "Config file /nonexistent-file-755684354 can't be opened.")
 
-        # Case 2, no image-build section in config file
 
+        # Case 2, no image-build section in config file
         expected = "single section called [%s] is required" % "image-build"
 
         self.configparser.return_value = ConfigParser()
+
         self.assert_system_exit(
             handle_image_build,
             self.options,
             self.session,
-            ['--config', '/dev/null'],
+            ['--config',
+             os.path.join(os.path.dirname(__file__),
+                          'data/image-build-config-empty.conf')],
             stderr=self.format_error_message(expected),
             activate_session=None)
 
-        config_file = os.path.join(os.path.dirname(__file__), 'data/image-build-config.conf')
+        config_file = os.path.join(os.path.dirname(__file__),
+                                   'data/image-build-config.conf')
+        # Case 3, normal
         handle_image_build(
             self.options,
             self.session,
diff --git a/tests/test_lib/test_utils.py b/tests/test_lib/test_utils.py
index 8af441b..5ba168d 100644
--- a/tests/test_lib/test_utils.py
+++ b/tests/test_lib/test_utils.py
@@ -162,49 +162,131 @@ class MiscFunctionTestCase(unittest.TestCase):
 class ConfigFileTestCase(unittest.TestCase):
     """Test config file reading functions"""
 
-    @mock_open()
-    @mock.patch("six.moves.configparser.ConfigParser", spec=True)
-    @mock.patch("six.moves.configparser.RawConfigParser", spec=True)
-    @mock.patch("six.moves.configparser.SafeConfigParser", spec=True)
-    def test_read_config_files(self, scp_clz, rcp_clz, cp_clz, open_mock):
+    def setUp(self):
+        self.manager = mock.MagicMock()
+        self.manager.logging = mock.patch('koji.logging').start()
+        self.manager.isdir = mock.patch("os.path.isdir").start()
+        self.manager.isfile = mock.patch("os.path.isfile").start()
+        self.manager.access = mock.patch("os.access", return_value=True).start()
+        self.manager.open = mock_open().start()
+        self.manager.cp_clz = mock.patch("six.moves.configparser.ConfigParser",
+                                         spec=True).start()
+        self.manager.scp_clz = mock.patch("six.moves.configparser.SafeConfigParser",
+                                         spec=True).start()
+        self.manager.rcp_clz = mock.patch("six.moves.configparser.RawConfigParser",
+                                         spec=True).start()
+        self.mocks = [self.manager.logging,
+                      self.manager.isdir,
+                      self.manager.isfile,
+                      self.manager.access,
+                      self.manager.open,
+                      self.manager.cp_clz,
+                      self.manager.scp_clz,
+                      self.manager.rcp_clz]
+
+    def reset_mock(self):
+        for m in self.mocks:
+            m.reset_mock()
+
+    def tearDown(self):
+        mock.patch.stopall()
+
+    def test_read_config_files(self):
         files = 'test1.conf'
-        conf = koji.read_config_files(files)
-        open_mock.assert_called_once_with(files, 'r')
+        default = 'default.conf'
+        self.manager.isdir.return_value = False
+        conf = koji.read_config_files(files, default=default)
+        self.manager.isdir.assert_called_once_with(files)
+        self.manager.open.assert_called_once_with(default, 'r')
         if six.PY2:
             self.assertTrue(isinstance(conf,
                                        six.moves.configparser.SafeConfigParser.__class__))
-            scp_clz.assert_called_once()
-            scp_clz.return_value.readfp.assert_called_once()
+            self.manager.scp_clz.assert_called_once()
+            self.manager.scp_clz.return_value.readfp.assert_called_once()
+            self.manager.scp_clz.return_value.read.assert_called_once_with([files])
         else:
             self.assertTrue(isinstance(conf,
                                        six.moves.configparser.ConfigParser.__class__))
-            cp_clz.assert_called_once()
-            cp_clz.return_value.read_file.assert_called_once()
+            self.manager.cp_clz.assert_called_once()
+            self.manager.cp_clz.return_value.read_file.assert_called_once()
+            self.manager.cp_clz.return_value.read.assert_called_once_with([files])
 
-        open_mock.reset_mock()
-        cp_clz.reset_mock()
-        scp_clz.reset_mock()
+        # no default
+        self.reset_mock()
+        koji.read_config_files(files)
+        self.manager.open.assert_not_called()
+
+        # list as config_files
+        self.reset_mock()
         files = ['test1.conf', 'test2.conf']
         koji.read_config_files(files)
-        open_mock.assert_has_calls([call('test1.conf', 'r'),
-                                    call('test2.conf', 'r')],
-                                   any_order=True)
+        self.manager.open.assert_not_called()
+
         if six.PY2:
-            scp_clz.assert_called_once()
-            self.assertEqual(scp_clz.return_value.readfp.call_count, 2)
+            parser_clz = self.manager.scp_clz
         else:
-            cp_clz.assert_called_once()
-            self.assertEqual(cp_clz.return_value.read_file.call_count, 2)
+            parser_clz = self.manager.cp_clz
+        parser_clz.assert_called_once()
+        parser_clz.return_value.read.assert_called_once_with(files)
 
-        open_mock.reset_mock()
-        cp_clz.reset_mock()
-        scp_clz.reset_mock()
+        # raw
+        self.reset_mock()
         conf = koji.read_config_files(files, raw=True)
         self.assertTrue(isinstance(conf,
                                    six.moves.configparser.RawConfigParser.__class__))
-        cp_clz.assert_not_called()
-        scp_clz.assert_not_called()
-        rcp_clz.assert_called_once()
+        self.manager.cp_clz.assert_not_called()
+        self.manager.scp_clz.assert_not_called()
+        self.manager.rcp_clz.assert_called_once()
+
+        # strict
+        # case1, not a file
+        self.reset_mock()
+        self.manager.isfile.side_effect = [True, False]
+        with self.assertRaises(koji.ConfigurationError) as cm:
+            koji.read_config_files(files, strict=True)
+        self.assertEqual(cm.exception.args[0],
+                         "Config file test2.conf can't be opened.")
+        self.manager.open.assert_not_called()
+
+        # case2, inaccessible
+        self.reset_mock()
+        self.manager.isfile.side_effect = None
+        self.manager.isfile.return_value = True
+        self.manager.access.side_effect = [True, False]
+        with self.assertRaises(koji.ConfigurationError) as cm:
+            koji.read_config_files(files, strict=True)
+        self.assertEqual(cm.exception.args[0],
+                         "Config file test2.conf can't be opened.")
+        self.manager.open.assert_not_called()
+
+        # directories
+        self.reset_mock()
+        files = ['test1.conf', 'testdir1', 'test2.conf', 'testdir2']
+        self.manager.isdir.side_effect = [False, True, False, True]
+        self.manager.isfile.side_effect = lambda f: False \
+            if f == 'test1-4.dir.conf' else True
+        self.manager.access.side_effect = None
+        self.manager.access.return_value = True
+        with mock.patch("os.listdir", side_effect=[['test1-2.conf',
+                                                    'test1-1.conf',
+                                                    'test1-3.txt',
+                                                    'test1-4.dir.conf'],
+                                                   []]) as listdir_mock:
+            conf = koji.read_config_files(files)
+        listdir_mock.assert_has_calls([call('testdir1'), call('testdir2')])
+        if six.PY2:
+            parser_clz = self.manager.scp_clz
+        else:
+            parser_clz = self.manager.cp_clz
+        parser_clz.assert_called_once()
+        parser_clz.return_value.read.assert_called_once_with(
+            ['test1.conf',
+             'testdir1/test1-1.conf',
+             'testdir1/test1-2.conf',
+             'testdir1/test1-4.dir.conf',
+             'test2.conf'])
+        self.manager.logging.debug.assert_called_once_with(
+            "No config files found in directory: testdir2")
 
 
 class MavenUtilTestCase(unittest.TestCase):
diff --git a/util/koji-gc b/util/koji-gc
index 02a29f1..b464d67 100755
--- a/util/koji-gc
+++ b/util/koji-gc
@@ -108,62 +108,50 @@ def get_options():
 
     defaults = parser.get_default_values()
 
-    config = six.moves.configparser.ConfigParser()
-    cf = getattr(options, 'config_file', None)
-    if cf:
-        if not os.access(cf, os.F_OK):
-            parser.error(_("No such file: %s") % cf)
-            assert False  # pragma: no cover
-    else:
-        cf = '/etc/koji-gc/koji-gc.conf'
-        if not os.access(cf, os.F_OK):
-            cf = None
-    if not cf:
-        print("no config file")
-        config = None
-    else:
-        config.read(cf)
-        # List of values read from config file to update default parser values
-        cfgmap = [
-            # name, alias, type
-            ['keytab', None, 'string'],
-            ['principal', None, 'string'],
-            ['krbservice', None, 'string'],
-            ['krb_rdns', None, 'boolean'],
-            ['krb_canon_host', None, 'boolean'],
-            ['runas', None, 'string'],
-            ['user', None, 'string'],
-            ['password', None, 'string'],
-            ['noauth', None, 'boolean'],
-            ['cert', None, 'string'],
-            ['ca', None, 'string'],  # FIXME: remove in next major release
-            ['serverca', None, 'string'],
-            ['server', None, 'string'],
-            ['weburl', None, 'string'],
-            ['smtp_host', None, 'string'],
-            ['from_addr', None, 'string'],
-            ['email_template', None, 'string'],
-            ['email_domain', None, 'string'],
-            ['mail', None, 'boolean'],
-            ['delay', None, 'string'],
-            ['unprotected_keys', None, 'string'],
-            ['grace_period', None, 'string'],
-            ['trashcan_tag', None, 'string'],
-            ['no_ssl_verify', None, 'boolean'],
-            ['timeout', None, 'integer'],
-            ]
-        for name, alias, type in cfgmap:
-            if alias is None:
-                alias = ('main', name)
-            if config.has_option(*alias):
-                if options.debug:
-                    print("Using option %s from config file" % (alias,))
-                if type == 'integer':
-                    setattr(defaults, name, config.getint(*alias))
-                elif type == 'boolean':
-                    setattr(defaults, name, config.getboolean(*alias))
-                else:
-                    setattr(defaults, name, config.get(*alias))
+    cf = getattr(options, 'config_file', '/etc/koji-gc/koji-gc.conf')
+    config = koji.read_config_files(cf)
+
+    # List of values read from config file to update default parser values
+    cfgmap = [
+        # name, alias, type
+        ['keytab', None, 'string'],
+        ['principal', None, 'string'],
+        ['krbservice', None, 'string'],
+        ['krb_rdns', None, 'boolean'],
+        ['krb_canon_host', None, 'boolean'],
+        ['runas', None, 'string'],
+        ['user', None, 'string'],
+        ['password', None, 'string'],
+        ['noauth', None, 'boolean'],
+        ['cert', None, 'string'],
+        ['ca', None, 'string'],  # FIXME: remove in next major release
+        ['serverca', None, 'string'],
+        ['server', None, 'string'],
+        ['weburl', None, 'string'],
+        ['smtp_host', None, 'string'],
+        ['from_addr', None, 'string'],
+        ['email_template', None, 'string'],
+        ['email_domain', None, 'string'],
+        ['mail', None, 'boolean'],
+        ['delay', None, 'string'],
+        ['unprotected_keys', None, 'string'],
+        ['grace_period', None, 'string'],
+        ['trashcan_tag', None, 'string'],
+        ['no_ssl_verify', None, 'boolean'],
+        ['timeout', None, 'integer'],
+        ]
+    for name, alias, type in cfgmap:
+        if alias is None:
+            alias = ('main', name)
+        if config.has_option(*alias):
+            if options.debug:
+                print("Using option %s from config file" % (alias,))
+            if type == 'integer':
+                setattr(defaults, name, config.getint(*alias))
+            elif type == 'boolean':
+                setattr(defaults, name, config.getboolean(*alias))
+            else:
+                setattr(defaults, name, config.get(*alias))
     #parse again with defaults
     (options, args) = parser.parse_args(values=defaults)
     options.config = config
@@ -187,8 +175,8 @@ def get_options():
     #parse key aliases
     options.key_aliases = {}
     try:
-        if config and config.has_option('main', 'key_aliases'):
-            for line in config.get('main','key_aliases').splitlines():
+        if config.has_option('main', 'key_aliases'):
+            for line in config.get('main', 'key_aliases').splitlines():
                 parts = line.split()
                 if len(parts) < 2:
                     continue
diff --git a/util/koji-shadow b/util/koji-shadow
index c0924fc..04cfdb0 100755
--- a/util/koji-shadow
+++ b/util/koji-shadow
@@ -31,7 +31,6 @@ except ImportError:  # pragma: no cover
     krbV = None
 import koji
 from koji.util import to_list
-import six.moves.configparser
 import fnmatch
 import optparse
 import os
@@ -154,56 +153,44 @@ def get_options():
     (options, args) = parser.parse_args()
 
     defaults = parser.get_default_values()
-    config = six.moves.configparser.ConfigParser()
-    cf = getattr(options, 'config_file', None)
-    if cf:
-        if not os.access(cf, os.F_OK):
-            parser.error(_("No such file: %s") % cf)
-            assert False  # pragma: no cover
-    else:
-        cf = '/etc/koji-shadow/koji-shadow.conf'
-        if not os.access(cf, os.F_OK):
-            cf = None
-    if not cf:
-        log("no config file")
-        config = None
-    else:
-        config.read(cf)
-        #allow config file to update defaults
-        for opt in parser.option_list:
-            if not opt.dest:
-                continue
-            name = opt.dest
-            alias = ('main', name)
-            if config.has_option(*alias):
-                log("Using option %s from config file" % (alias,))
-                if opt.action in ('store_true', 'store_false'):
-                    setattr(defaults, name, config.getboolean(*alias))
-                elif opt.action != 'store':
-                    pass
-                elif opt.type in ('int', 'long'):
-                    setattr(defaults, name, config.getint(*alias))
-                elif opt.type in ('float'):
-                    setattr(defaults, name, config.getfloat(*alias))
-                else:
-                    log(config.get(*alias))
-                    setattr(defaults, name, config.get(*alias))
-        #config file options without a cmdline equivalent
-        otheropts = [
-            #name, type, default
-            ['keytab', None, 'string'],
-            ['principal', None, 'string'],
-            ['runas', None, 'string'],
-            ['user', None, 'string'],
-            ['password', None, 'string'],
-            ['noauth', None, 'boolean'],
-            ['server', None, 'string'],
-            ['remote', None, 'string'],
-            ['max_jobs', None, 'int'],
-            ['serverca', None, 'string'],
-            ['auth_cert', None, 'string'],
-            ['arches', None, 'string'],
-            ]
+    cf = getattr(options, 'config_file', '/etc/koji-shadow/koji-shadow.conf')
+    config = koji.read_config_files(cf)
+
+    #allow config file to update defaults
+    for opt in parser.option_list:
+        if not opt.dest:
+            continue
+        name = opt.dest
+        alias = ('main', name)
+        if config.has_option(*alias):
+            log("Using option %s from config file" % (alias,))
+            if opt.action in ('store_true', 'store_false'):
+                setattr(defaults, name, config.getboolean(*alias))
+            elif opt.action != 'store':
+                pass
+            elif opt.type in ('int', 'long'):
+                setattr(defaults, name, config.getint(*alias))
+            elif opt.type in ('float'):
+                setattr(defaults, name, config.getfloat(*alias))
+            else:
+                log(config.get(*alias))
+                setattr(defaults, name, config.get(*alias))
+    #config file options without a cmdline equivalent
+    otheropts = [
+        #name, type, default
+        ['keytab', None, 'string'],
+        ['principal', None, 'string'],
+        ['runas', None, 'string'],
+        ['user', None, 'string'],
+        ['password', None, 'string'],
+        ['noauth', None, 'boolean'],
+        ['server', None, 'string'],
+        ['remote', None, 'string'],
+        ['max_jobs', None, 'int'],
+        ['serverca', None, 'string'],
+        ['auth_cert', None, 'string'],
+        ['arches', None, 'string'],
+        ]
 
 
     #parse again with updated defaults
diff --git a/util/kojira b/util/kojira
index 69fbf8c..9e0c9ec 100755
--- a/util/kojira
+++ b/util/kojira
@@ -27,7 +27,6 @@ import os
 import koji
 from koji.util import rmtree, parseStatus, to_list
 from optparse import OptionParser
-from six.moves.configparser import ConfigParser
 import errno
 import json
 import logging
@@ -901,8 +900,7 @@ def get_options():
     parser.add_option("--logfile", help="Specify logfile")
     (options, args) = parser.parse_args()
 
-    config = ConfigParser()
-    config.read(options.configFile)
+    config = koji.read_config_files(options.configFile)
     section = 'kojira'
     for x in config.sections():
         if x != section:
diff --git a/vm/kojikamid.py b/vm/kojikamid.py
index 6a41aa5..b3df301 100755
--- a/vm/kojikamid.py
+++ b/vm/kojikamid.py
@@ -28,7 +28,7 @@
 
 from __future__ import absolute_import
 from optparse import OptionParser
-from six.moves.configparser import ConfigParser
+from six.moves.configparser import ConfigParser, SafeConfigParser
 import os
 import subprocess
 import sys
@@ -212,7 +212,10 @@ class WindowsBuild(object):
         elif len(specfiles) > 1:
             raise BuildError('Multiple .ini files found')
 
-        conf = ConfigParser()
+        if six.PY2:
+            conf = SafeConfigParser()
+        else:
+            conf = ConfigParser()
         conf.read(os.path.join(self.spec_dir, specfiles[0]))
 
         # [naming] section
diff --git a/vm/kojivmd b/vm/kojivmd
index 4db4f2f..3e0f7b4 100755
--- a/vm/kojivmd
+++ b/vm/kojivmd
@@ -46,7 +46,6 @@ import base64
 import pwd
 import requests
 import fnmatch
-from six.moves.configparser import ConfigParser
 from contextlib import closing
 from optparse import OptionParser
 try:
@@ -102,8 +101,7 @@ def get_options():
         assert False  # pragma: no cover
 
     # load local config
-    config = ConfigParser()
-    config.read(options.configFile)
+    config = koji.read_config_files(options.configFile)
     for x in config.sections():
         if x != 'kojivmd':
             quit('invalid section found in config file: %s' % x)
diff --git a/www/kojiweb/wsgi_publisher.py b/www/kojiweb/wsgi_publisher.py
index abb3424..f7b7ffd 100644
--- a/www/kojiweb/wsgi_publisher.py
+++ b/www/kojiweb/wsgi_publisher.py
@@ -30,7 +30,6 @@ import pprint
 import sys
 import traceback
 
-from six.moves.configparser import RawConfigParser
 from koji.server import ServerError, ServerRedirect
 from koji.util import dslice, to_list
 import six
@@ -127,22 +126,12 @@ class Dispatcher(object):
         """
         cf = environ.get('koji.web.ConfigFile', '/etc/kojiweb/web.conf')
         cfdir = environ.get('koji.web.ConfigDir', '/etc/kojiweb/web.conf.d')
-        if cfdir:
-            configs = koji.config_directory_contents(cfdir)
-        else:
-            configs = []
-        if cf and os.path.isfile(cf):
-            configs.append(cf)
-        if configs:
-            config = RawConfigParser()
-            config.read(configs)
-        else:
-            raise koji.GenericError("Configuration missing")
+        config = koji.read_config_files([cfdir, cf], strict=True)
 
         opts = {}
         for name, dtype, default in self.cfgmap:
             key = ('web', name)
-            if config and config.has_option(*key):
+            if config.has_option(*key):
                 if dtype == 'integer':
                     opts[name] = config.getint(*key)
                 elif dtype == 'boolean':

From 98655e1d46457d1acf69c7553d6c97f769311753 Mon Sep 17 00:00:00 2001
From: Yu Ming Zhu <yzhu@redhat.com>
Date: Oct 09 2019 06:12:02 +0000
Subject: [PATCH 2/4] use RawConfigParser for kojid


---

diff --git a/builder/kojid b/builder/kojid
index e81d28c..82d1f45 100755
--- a/builder/kojid
+++ b/builder/kojid
@@ -6099,7 +6099,7 @@ def get_options():
         assert False  # pragma: no cover
 
     # load local config
-    config = koji.read_config_files(options.configFile)
+    config = koji.read_config_files(options.configFile, raw=True)
     for x in config.sections():
         if x != 'kojid':
             quit('invalid section found in config file: %s' % x)

From af448133f6e88ab8406bb8771377c337ba3a44f7 Mon Sep 17 00:00:00 2001
From: Yu Ming Zhu <yzhu@redhat.com>
Date: Oct 09 2019 06:12:02 +0000
Subject: [PATCH 3/4] fix test_runroot_builder tests


---

diff --git a/tests/test_plugins/test_runroot_builder.py b/tests/test_plugins/test_runroot_builder.py
index 42585e1..014cb00 100644
--- a/tests/test_plugins/test_runroot_builder.py
+++ b/tests/test_plugins/test_runroot_builder.py
@@ -24,6 +24,12 @@ def mock_open():
         return mock.patch('builtins.open')
 
 
+if six.PY2:
+    CONFIG_PARSER = 'six.moves.configparser.SafeConfigParser'
+else:
+    CONFIG_PARSER = 'six.moves.configparser.ConfigParser'
+
+
 CONFIG1 = {
         'paths': {
             'default_mounts': '/mnt/archive,/mnt/workdir',
@@ -96,11 +102,11 @@ class FakeConfigParser(object):
 
 
 class TestRunrootConfig(unittest.TestCase):
-    @mock.patch('six.moves.configparser.SafeConfigParser')
-    def test_bad_config_paths0(self, safe_config_parser):
+    @mock.patch(CONFIG_PARSER)
+    def test_bad_config_paths0(self, config_parser):
         cp = FakeConfigParser()
         del cp.CONFIG['path0']['mountpoint']
-        safe_config_parser.return_value = cp
+        config_parser.return_value = cp
         session = mock.MagicMock()
         options = mock.MagicMock()
         options.workdir = '/tmp/nonexistentdirectory'
@@ -109,11 +115,11 @@ class TestRunrootConfig(unittest.TestCase):
         self.assertEqual(cm.exception.args[0],
             "bad config: missing options in path0 section")
 
-    @mock.patch('six.moves.configparser.SafeConfigParser')
-    def test_bad_config_absolute_path(self, safe_config_parser):
+    @mock.patch(CONFIG_PARSER)
+    def test_bad_config_absolute_path(self, config_parser):
         cp = FakeConfigParser()
         cp.CONFIG['paths']['default_mounts'] = ''
-        safe_config_parser.return_value = cp
+        config_parser.return_value = cp
         session = mock.MagicMock()
         options = mock.MagicMock()
         options.workdir = '/tmp/nonexistentdirectory'
@@ -122,29 +128,29 @@ class TestRunrootConfig(unittest.TestCase):
         self.assertEqual(cm.exception.args[0],
             "bad config: all paths (default_mounts, safe_roots, path_subs) needs to be absolute: ")
 
-    @mock.patch('six.moves.configparser.SafeConfigParser')
-    def test_valid_config(self, safe_config_parser):
-        safe_config_parser.return_value = FakeConfigParser()
+    @mock.patch(CONFIG_PARSER)
+    def test_valid_config(self, config_parser):
+        config_parser.return_value = FakeConfigParser()
         session = mock.MagicMock()
         options = mock.MagicMock()
         options.workdir = '/tmp/nonexistentdirectory'
         runroot.RunRootTask(123, 'runroot', {}, session, options)
 
-    @mock.patch('six.moves.configparser.SafeConfigParser')
-    def test_valid_config_alt(self, safe_config_parser):
-        safe_config_parser.return_value = FakeConfigParser(CONFIG2)
+    @mock.patch(CONFIG_PARSER)
+    def test_valid_config_alt(self, config_parser):
+        config_parser.return_value = FakeConfigParser(CONFIG2)
         session = mock.MagicMock()
         options = mock.MagicMock()
         options.workdir = '/tmp/nonexistentdirectory'
         runroot.RunRootTask(123, 'runroot', {}, session, options)
 
-    @mock.patch('six.moves.configparser.SafeConfigParser')
-    def test_pathnum_gaps(self, safe_config_parser):
+    @mock.patch(CONFIG_PARSER)
+    def test_pathnum_gaps(self, config_parser):
         session = mock.MagicMock()
         options = mock.MagicMock()
         options.workdir = '/tmp/nonexistentdirectory'
         config = CONFIG2.copy()
-        safe_config_parser.return_value = FakeConfigParser(config)
+        config_parser.return_value = FakeConfigParser(config)
         task1 = runroot.RunRootTask(123, 'runroot', {}, session, options)
         # adjust the path numbers (but preserving order) and do it again
         config = CONFIG2.copy()
@@ -152,29 +158,29 @@ class TestRunrootConfig(unittest.TestCase):
         config['path999'] = config['path2']
         del config['path1']
         del config['path2']
-        safe_config_parser.return_value = FakeConfigParser(config)
+        config_parser.return_value = FakeConfigParser(config)
         task2 = runroot.RunRootTask(123, 'runroot', {}, session, options)
         # resulting processed config should be the same
         self.assertEqual(task1.config, task2.config)
         paths = list([CONFIG2[k] for k in ('path0', 'path1', 'path2')])
         self.assertEqual(task2.config['paths'], paths)
 
-    @mock.patch('six.moves.configparser.SafeConfigParser')
-    def test_bad_path_sub(self, safe_config_parser):
+    @mock.patch(CONFIG_PARSER)
+    def test_bad_path_sub(self, config_parser):
         session = mock.MagicMock()
         options = mock.MagicMock()
         options.workdir = '/tmp/nonexistentdirectory'
         config = copy.deepcopy(CONFIG2)
         config['paths']['path_subs'] += 'incorrect:format'
-        safe_config_parser.return_value = FakeConfigParser(config)
+        config_parser.return_value = FakeConfigParser(config)
         with self.assertRaises(koji.GenericError):
             runroot.RunRootTask(123, 'runroot', {}, session, options)
 
 
 class TestMounts(unittest.TestCase):
-    @mock.patch('six.moves.configparser.SafeConfigParser')
-    def setUp(self, safe_config_parser):
-        safe_config_parser.return_value = FakeConfigParser()
+    @mock.patch(CONFIG_PARSER)
+    def setUp(self, config_parser):
+        config_parser.return_value = FakeConfigParser()
         self.session = mock.MagicMock()
         options = mock.MagicMock()
         options.workdir = '/tmp/nonexistentdirectory'
@@ -324,8 +330,8 @@ class TestMounts(unittest.TestCase):
         os_unlink.assert_not_called()
 
 class TestHandler(unittest.TestCase):
-    @mock.patch('six.moves.configparser.SafeConfigParser')
-    def setUp(self, safe_config_parser):
+    @mock.patch(CONFIG_PARSER)
+    def setUp(self, config_parser):
         self.session = mock.MagicMock()
         self.br = mock.MagicMock()
         self.br.mock.return_value = 0

From 7dc3b2d2462419b57d52dd209c0a2057454843a5 Mon Sep 17 00:00:00 2001
From: Yuming Zhu <yzhu@redhat.com>
Date: Oct 09 2019 11:06:01 +0000
Subject: [PATCH 4/4] strict per item in config_files


---

diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py
index 1fdd4bb..5773a7c 100644
--- a/cli/koji_cli/commands.py
+++ b/cli/koji_cli/commands.py
@@ -5905,7 +5905,7 @@ def handle_image_build(options, session, args):
 
     if task_options.config:
         section = 'image-build'
-        config = koji.read_config_files(task_options.config, strict=True)
+        config = koji.read_config_files([(task_options.config, True)])
         if not config.has_section(section):
             parser.error(_("single section called [%s] is required" % section))
         # pluck out the positional arguments first
diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py
index f0d9fb8..e7df388 100644
--- a/hub/kojixmlrpc.py
+++ b/hub/kojixmlrpc.py
@@ -395,7 +395,7 @@ def load_config(environ):
     #get our config file(s)
     cf = environ.get('koji.hub.ConfigFile', '/etc/koji-hub/hub.conf')
     cfdir = environ.get('koji.hub.ConfigDir', '/etc/koji-hub/hub.conf.d')
-    config = koji.read_config_files([cfdir, cf], raw=True, strict=True)
+    config = koji.read_config_files([cfdir, (cf, True)], raw=True)
 
     cfgmap = [
         #option, type, default
diff --git a/koji/__init__.py b/koji/__init__.py
index 05add50..db61359 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -1673,7 +1673,7 @@ def openRemoteFile(relpath, topurl=None, topdir=None, tempdir=None):
     return fo
 
 
-def config_directory_contents(dir_name):
+def config_directory_contents(dir_name, strict=False):
     configs = []
     try:
         conf_dir_contents = os.listdir(dir_name)
@@ -1685,8 +1685,12 @@ def config_directory_contents(dir_name):
             if not name.endswith('.conf'):
                 continue
             config_full_name = os.path.join(dir_name, name)
-            if os.path.isfile(config_full_name):
+            if os.path.isfile(config_full_name) \
+               and os.access(config_full_name, os.F_OK):
                 configs.append(config_full_name)
+            elif strict:
+                raise ConfigurationError("Config file %s can't be opened."
+                                         % config_full_name)
     return configs
 
 
@@ -1727,7 +1731,6 @@ def read_config(profile_name, user_config=None):
 
     #note: later config files override earlier ones
 
-    strict = False
     # /etc/koji.conf.d
     configs = ['/etc/koji.conf.d']
 
@@ -1738,15 +1741,14 @@ def read_config(profile_name, user_config=None):
     if user_config:
         # Config file specified on command line
         # The existence will be checked
-        configs.append(os.path.expanduser(user_config))
-        strict = True
+        configs.append((os.path.expanduser(user_config), True))
     else:
         # User config dir
         configs.append(os.path.expanduser("~/.koji/config.d"))
         # User config file
         configs.append(os.path.expanduser("~/.koji/config"))
 
-    config = read_config_files(configs, strict=strict)
+    config = read_config_files(configs)
 
     # Load the configs in a particular order
     got_conf = False
@@ -1843,20 +1845,44 @@ def get_profile_module(profile_name, config=None):
     return mod
 
 
-def read_config_files(config_files, default=None, raw=False, strict=False):
+def read_config_files(config_files, raw=False):
     """Use parser to read config file(s)
 
     :param config_files: config file(s) to read (required). Config file could
-                         be file or directory, and order is preserved
-    :type config_files: str or list
-    :param str default: default config file which is loaded before
-                        config_files. If specified, it must be readable.
+                         be file or directory, and order is preserved.
+                         If it's a list/tuple of list/tuple, in each inner
+                         item, the 1st item is file/dir name, and the 2nd item
+                         is strict(False if not present), which indicate
+                         if checking that:
+                             1. is dir an empty directory
+                             2. dose file exist
+                             3. is file accessible
+                         raising ConfigurationError if any above is True.
+    :type config_files: str or list or tuple
     :param bool raw: enable 'raw' parsing (no interpolation). Default: False
-    :param bool strict: enable reading check for each item in config_files.
-                        Default: False
 
     :return: object of parser which contains parsed content
+
+    :raises: GenericError: config_files is not valid
+             ConfigurationError: See config_files if strict is true
+             OSError: Directory in config_files is not accessible
     """
+    if isinstance(config_files, six.string_types):
+        config_files = [(config_files, False)]
+    elif isinstance(config_files, (list, tuple)):
+        fcfgs = []
+        for i in config_files:
+            if isinstance(i, six.string_types):
+                fcfgs.append((i, False))
+            elif isinstance(i, (list, tuple)) and 0 < len(i) <= 2:
+                fcfgs.append((i[0], i[1] if len(i) == 2 else False))
+            else:
+                raise GenericError('invalid value: %s or type: %s'
+                                   % (i, type(i)))
+        config_files = fcfgs
+    else:
+        raise GenericError('invalid type: %s' % type(config_files))
+
     if raw:
         parser = six.moves.configparser.RawConfigParser
     elif six.PY2:
@@ -1866,32 +1892,21 @@ def read_config_files(config_files, default=None, raw=False, strict=False):
         # deprecated alias
         parser = six.moves.configparser.ConfigParser
     config = parser()
-    if default:
-        with open(default, 'r') as f:
-            if six.PY2:
-                config.readfp(f)
-            else:
-                config.read_file(f)
-    if not isinstance(config_files, (list, tuple)):
-        config_files = [config_files]
     cfgs = []
     # append dir contents
-    for config_file in config_files:
+    for config_file, strict in config_files:
         if os.path.isdir(config_file):
-            fns = config_directory_contents(config_file)
+            fns = config_directory_contents(config_file, strict=strict)
             if fns:
                 cfgs.extend(fns)
-            else:
-                logging.debug("No config files found in directory: %s"
-                              % config_file)
-        else:
+            elif strict:
+                raise ConfigurationError("No config files found in directory:"
+                                         " %s" % config_file)
+        elif os.path.isfile(config_file) and os.access(config_file, os.F_OK):
             cfgs.append(config_file)
-    # checking access if strict is True
-    if strict:
-        for cf in cfgs:
-            if not os.path.isfile(cf) or not os.access(cf, os.F_OK):
-                raise ConfigurationError("Config file %s can't be opened."
-                                         % cf)
+        elif strict:
+            raise ConfigurationError("Config file %s can't be opened."
+                                     % config_file)
     config.read(cfgs)
     return config
 
diff --git a/plugins/hub/protonmsg.py b/plugins/hub/protonmsg.py
index 8a1953c..c62d5ef 100644
--- a/plugins/hub/protonmsg.py
+++ b/plugins/hub/protonmsg.py
@@ -269,7 +269,7 @@ def send_queued_msgs(cbtype, *args, **kws):
     log = logging.getLogger('koji.plugin.protonmsg')
     global CONFIG
     if not CONFIG:
-        CONFIG = koji.read_config_files(CONFIG_FILE, strict=True)
+        CONFIG = koji.read_config_files([(CONFIG_FILE, True)])
     urls = CONFIG.get('broker', 'urls').split()
     test_mode = False
     if CONFIG.has_option('broker', 'test_mode'):
diff --git a/plugins/hub/rpm2maven.py b/plugins/hub/rpm2maven.py
index bb42340..cf16da9 100644
--- a/plugins/hub/rpm2maven.py
+++ b/plugins/hub/rpm2maven.py
@@ -31,7 +31,7 @@ def maven_import(cbtype, *args, **kws):
     filepath = kws['filepath']
 
     if not config:
-        config = koji.read_config_files(CONFIG_FILE, strict=True)
+        config = koji.read_config_files([(CONFIG_FILE, True)])
     name_patterns = config.get('patterns', 'rpm_names').split()
     for pattern in name_patterns:
         if fnmatch.fnmatch(rpminfo['name'], pattern):
diff --git a/plugins/hub/save_failed_tree.py b/plugins/hub/save_failed_tree.py
index 12c0f47..863e842 100644
--- a/plugins/hub/save_failed_tree.py
+++ b/plugins/hub/save_failed_tree.py
@@ -28,7 +28,7 @@ def saveFailedTree(buildrootID, full=False, **opts):
 
     # read configuration only once
     if config is None:
-        config = koji.read_config_files(CONFIG_FILE, strict=True)
+        config = koji.read_config_files([(CONFIG_FILE, True)])
         allowed_methods = config.get('permissions', 'allowed_methods').split()
         if len(allowed_methods) == 1 and allowed_methods[0] == '*':
             allowed_methods = '*'
diff --git a/tests/test_lib/test_utils.py b/tests/test_lib/test_utils.py
index 5ba168d..3b5df25 100644
--- a/tests/test_lib/test_utils.py
+++ b/tests/test_lib/test_utils.py
@@ -168,15 +168,17 @@ class ConfigFileTestCase(unittest.TestCase):
         self.manager.isdir = mock.patch("os.path.isdir").start()
         self.manager.isfile = mock.patch("os.path.isfile").start()
         self.manager.access = mock.patch("os.access", return_value=True).start()
-        self.manager.open = mock_open().start()
         self.manager.cp_clz = mock.patch("six.moves.configparser.ConfigParser",
                                          spec=True).start()
         self.manager.scp_clz = mock.patch("six.moves.configparser.SafeConfigParser",
                                          spec=True).start()
         self.manager.rcp_clz = mock.patch("six.moves.configparser.RawConfigParser",
                                          spec=True).start()
-        self.mocks = [self.manager.logging,
-                      self.manager.isdir,
+        if six.PY2:
+            self.real_parser_clz = self.manager.scp_clz
+        else:
+            self.real_parser_clz = self.manager.cp_clz
+        self.mocks = [self.manager.isdir,
                       self.manager.isfile,
                       self.manager.access,
                       self.manager.open,
@@ -192,42 +194,45 @@ class ConfigFileTestCase(unittest.TestCase):
         mock.patch.stopall()
 
     def test_read_config_files(self):
+
+        # bad config_files
+        for files in [0,
+                      False,
+                      set(),
+                      dict(),
+                      object(),
+                      ('string', True),
+                      [('str', True, 'str')],
+                      [tuple()],]:
+            with self.assertRaises(koji.GenericError):
+                koji.read_config_files(files)
+
+        # string as config_files
         files = 'test1.conf'
-        default = 'default.conf'
         self.manager.isdir.return_value = False
-        conf = koji.read_config_files(files, default=default)
+        conf = koji.read_config_files(files)
         self.manager.isdir.assert_called_once_with(files)
-        self.manager.open.assert_called_once_with(default, 'r')
         if six.PY2:
             self.assertTrue(isinstance(conf,
                                        six.moves.configparser.SafeConfigParser.__class__))
-            self.manager.scp_clz.assert_called_once()
-            self.manager.scp_clz.return_value.readfp.assert_called_once()
-            self.manager.scp_clz.return_value.read.assert_called_once_with([files])
         else:
             self.assertTrue(isinstance(conf,
                                        six.moves.configparser.ConfigParser.__class__))
-            self.manager.cp_clz.assert_called_once()
-            self.manager.cp_clz.return_value.read_file.assert_called_once()
-            self.manager.cp_clz.return_value.read.assert_called_once_with([files])
-
-        # no default
-        self.reset_mock()
-        koji.read_config_files(files)
-        self.manager.open.assert_not_called()
+        self.real_parser_clz.assert_called_once()
+        self.real_parser_clz.return_value.read.assert_called_once_with([files])
 
         # list as config_files
         self.reset_mock()
         files = ['test1.conf', 'test2.conf']
         koji.read_config_files(files)
-        self.manager.open.assert_not_called()
 
-        if six.PY2:
-            parser_clz = self.manager.scp_clz
-        else:
-            parser_clz = self.manager.cp_clz
-        parser_clz.assert_called_once()
-        parser_clz.return_value.read.assert_called_once_with(files)
+        self.real_parser_clz.assert_called_once()
+        self.real_parser_clz.return_value.read.assert_called_once_with(files)
+
+        # tuple as config_files
+        self.reset_mock()
+        files = ('test1.conf', 'test2.conf')
+        koji.read_config_files(files)
 
         # raw
         self.reset_mock()
@@ -241,31 +246,37 @@ class ConfigFileTestCase(unittest.TestCase):
         # strict
         # case1, not a file
         self.reset_mock()
-        self.manager.isfile.side_effect = [True, False]
+        files = [('test1.conf',), ('test2.conf', True)]
+        self.manager.isfile.return_value = False
         with self.assertRaises(koji.ConfigurationError) as cm:
-            koji.read_config_files(files, strict=True)
+            koji.read_config_files(files)
         self.assertEqual(cm.exception.args[0],
                          "Config file test2.conf can't be opened.")
-        self.manager.open.assert_not_called()
+
+        self.assertEqual(self.manager.isdir.call_count, 2)
+        self.assertEqual(self.manager.isfile.call_count, 2)
+        self.manager.access.assert_not_called()
 
         # case2, inaccessible
         self.reset_mock()
-        self.manager.isfile.side_effect = None
         self.manager.isfile.return_value = True
-        self.manager.access.side_effect = [True, False]
+        self.manager.access.return_value = False
         with self.assertRaises(koji.ConfigurationError) as cm:
-            koji.read_config_files(files, strict=True)
+            koji.read_config_files(files)
         self.assertEqual(cm.exception.args[0],
                          "Config file test2.conf can't be opened.")
-        self.manager.open.assert_not_called()
+        self.assertEqual(self.manager.isdir.call_count, 2)
+        self.assertEqual(self.manager.isfile.call_count, 2)
+        self.assertEqual(self.manager.access.call_count, 2)
 
         # directories
+        # strict==False
         self.reset_mock()
-        files = ['test1.conf', 'testdir1', 'test2.conf', 'testdir2']
-        self.manager.isdir.side_effect = [False, True, False, True]
+        files = ['test1.conf', 'gooddir', 'test2.conf', 'emptydir', 'nonexistdir']
+        self.manager.isdir.side_effect = lambda f: False \
+            if f in ['test1.conf', 'test2.conf', 'nonexistdir'] else True
         self.manager.isfile.side_effect = lambda f: False \
-            if f == 'test1-4.dir.conf' else True
-        self.manager.access.side_effect = None
+            if f in ['nonexistdir', 'gooddir/test1-4.dir.conf'] else True
         self.manager.access.return_value = True
         with mock.patch("os.listdir", side_effect=[['test1-2.conf',
                                                     'test1-1.conf',
@@ -273,20 +284,49 @@ class ConfigFileTestCase(unittest.TestCase):
                                                     'test1-4.dir.conf'],
                                                    []]) as listdir_mock:
             conf = koji.read_config_files(files)
-        listdir_mock.assert_has_calls([call('testdir1'), call('testdir2')])
-        if six.PY2:
-            parser_clz = self.manager.scp_clz
-        else:
-            parser_clz = self.manager.cp_clz
-        parser_clz.assert_called_once()
-        parser_clz.return_value.read.assert_called_once_with(
+        listdir_mock.assert_has_calls([call('gooddir'), call('emptydir')])
+        self.real_parser_clz.assert_called_once()
+        self.real_parser_clz.return_value.read.assert_called_once_with(
             ['test1.conf',
-             'testdir1/test1-1.conf',
-             'testdir1/test1-2.conf',
-             'testdir1/test1-4.dir.conf',
+             'gooddir/test1-1.conf',
+             'gooddir/test1-2.conf',
              'test2.conf'])
-        self.manager.logging.debug.assert_called_once_with(
-            "No config files found in directory: testdir2")
+        self.assertEqual(self.manager.isdir.call_count, 5)
+        self.assertEqual(self.manager.isfile.call_count, 6)
+        self.assertEqual(self.manager.access.call_count, 4)
+
+        # strict==True
+        # case1
+        self.reset_mock()
+        files[1] = ('gooddir', True)
+        with mock.patch("os.listdir", return_value=['test1-2.conf',
+                                                    'test1-1.conf',
+                                                    'test1-3.txt',
+                                                    'test1-4.dir.conf']
+                        ) as listdir_mock:
+            with self.assertRaises(koji.ConfigurationError) as cm:
+                conf = koji.read_config_files(files)
+        self.assertEqual(cm.exception.args[0],
+                         "Config file gooddir/test1-4.dir.conf can't be"
+                         " opened.")
+        listdir_mock.assert_called_once_with('gooddir')
+
+        # case2
+        self.reset_mock()
+        files[1] = ('gooddir', False)
+        files[3] = ('emptydir', True)
+        with mock.patch("os.listdir", side_effect=[['test1-2.conf',
+                                                    'test1-1.conf',
+                                                    'test1-3.txt',
+                                                    'test1-4.dir.conf'],
+                                                   []]
+                        ) as listdir_mock:
+            with self.assertRaises(koji.ConfigurationError) as cm:
+                conf = koji.read_config_files(files)
+        self.assertEqual(cm.exception.args[0],
+                         'No config files found in directory: emptydir')
+        self.assertEqual(listdir_mock.call_count, 2)
+
 
 
 class MavenUtilTestCase(unittest.TestCase):
diff --git a/www/kojiweb/wsgi_publisher.py b/www/kojiweb/wsgi_publisher.py
index f7b7ffd..9744722 100644
--- a/www/kojiweb/wsgi_publisher.py
+++ b/www/kojiweb/wsgi_publisher.py
@@ -126,7 +126,7 @@ class Dispatcher(object):
         """
         cf = environ.get('koji.web.ConfigFile', '/etc/kojiweb/web.conf')
         cfdir = environ.get('koji.web.ConfigDir', '/etc/kojiweb/web.conf.d')
-        config = koji.read_config_files([cfdir, cf], strict=True)
+        config = koji.read_config_files([cfdir, (cf, True)])
 
         opts = {}
         for name, dtype, default in self.cfgmap:

