From 78a843d39490c03eb4e46c2d266661ba496d79e1 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Feb 10 2025 14:07:55 +0000
Subject: [PATCH 1/2] move config map and add unit tests


---

diff --git a/kojihub/kojixmlrpc.py b/kojihub/kojixmlrpc.py
index ca938c2..2d1e8af 100644
--- a/kojihub/kojixmlrpc.py
+++ b/kojihub/kojixmlrpc.py
@@ -416,6 +416,104 @@ def error_reply(start_response, status, response, extra_headers=None):
     return [response]
 
 
+# Known configuration options, used by load_config
+config_map = [
+    # option, type, default
+    ['DBName', 'string', None],
+    ['DBUser', 'string', None],
+    ['DBHost', 'string', None],
+    ['DBhost', 'string', None],   # alias for backwards compatibility
+    ['DBPort', 'integer', None],
+    ['DBPass', 'string', None],
+    ['DBConnectionString', 'string', None],
+    ['KojiDir', 'string', None],
+
+    ['ProxyPrincipals', 'string', ''],
+    ['HostPrincipalFormat', 'string', None],
+    ['AllowedKrbRealms', 'string', '*'],
+    # TODO:  this option should be turned True in 1.34
+    ['DisableURLSessions', 'boolean', False],
+
+    ['DNUsernameComponent', 'string', 'CN'],
+    ['ProxyDNs', 'string', ''],
+
+    ['CheckClientIP', 'boolean', True],
+
+    ['LoginCreatesUser', 'boolean', True],
+    ['AllowProxyAuthType', 'boolean', False],
+    ['KojiWebURL', 'string', 'http://localhost.localdomain/koji'],
+    ['EmailDomain', 'string', None],
+    ['NotifyOnSuccess', 'boolean', True],
+    ['DisableNotifications', 'boolean', False],
+
+    ['Plugins', 'string', ''],
+    ['PluginPath', 'string', '/usr/lib/koji-hub-plugins'],
+
+    ['KojiDebug', 'boolean', False],
+    ['KojiTraceback', 'string', None],
+    ['VerbosePolicy', 'boolean', False],
+
+    ['LogLevel', 'string', 'WARNING'],
+    ['LogFormat', 'string',
+     '%(asctime)s [%(levelname)s] m=%(method)s u=%(user_name)s p=%(process)s r=%(remoteaddr)s '
+     '%(name)s: %(message)s'],
+
+    ['MissingPolicyOk', 'boolean', True],
+    ['EnableMaven', 'boolean', False],
+    ['EnableWin', 'boolean', False],
+
+    ['RLIMIT_AS', 'string', None],
+    ['RLIMIT_CORE', 'string', None],
+    ['RLIMIT_CPU', 'string', None],
+    ['RLIMIT_DATA', 'string', None],
+    ['RLIMIT_FSIZE', 'string', None],
+    ['RLIMIT_MEMLOCK', 'string', None],
+    ['RLIMIT_NOFILE', 'string', None],
+    ['RLIMIT_NPROC', 'string', None],
+    ['RLIMIT_OFILE', 'string', None],  # alias for RLIMIT_NOFILE
+    ['RLIMIT_RSS', 'string', None],
+    ['RLIMIT_STACK', 'string', None],
+
+    ['MemoryWarnThreshold', 'integer', 5000],
+    ['MaxRequestLength', 'integer', 4194304],
+
+    ['LockOut', 'boolean', False],
+    ['ServerOffline', 'boolean', False],
+    ['OfflineMessage', 'string', None],
+
+    ['MaxNameLengthInternal', 'integer', 256],
+    ['RegexNameInternal', 'string', r'^[A-Za-z0-9/_.+-]+$'],
+    ['RegexUserName', 'string', r'^[A-Za-z0-9/_.@-]+$'],
+
+    ['RPMDefaultChecksums', 'string', 'md5 sha256'],
+
+    ['SessionRenewalTimeout', 'integer', 1440],
+
+    # scheduler options
+    ['MaxJobs', 'integer', 15],
+    ['CapacityOvercommit', 'integer', 5],
+    ['ReadyTimeout', 'integer', 180],
+    ['AssignTimeout', 'integer', 300],
+    ['SoftRefusalTimeout', 'integer', 900],
+    ['HostTimeout', 'integer', 900],
+    ['RunInterval', 'integer', 60],
+
+    # repo options
+    ['MaxRepoTasks', 'integer', 10],
+    ['MaxRepoTasksMaven', 'integer', 2],
+    ['RepoRetries', 'integer', 3],
+    ['RequestCleanTime', 'integer', 60 * 24],  # in minutes
+    ['AllowNewRepo', 'bool', True],
+    ['RepoLag', 'int', 3600],
+    ['RepoAutoLag', 'int', 7200],
+    ['RepoLagWindow', 'int', 600],
+    ['RepoQueueUser', 'str', 'kojira'],
+    ['DebuginfoTags', 'str', ''],
+    ['SourceTags', 'str', ''],
+    ['SeparateSourceTags', 'str', ''],
+]
+
+
 def load_config(environ):
     """Load configuration options
 
@@ -437,111 +535,19 @@ def load_config(environ):
     cfdir = environ.get('koji.hub.ConfigDir', '/etc/koji-hub/hub.conf.d')
     config = koji.read_config_files([cfdir, (cf, True)], raw=True)
 
-    cfgmap = [
-        # option, type, default
-        ['DBName', 'string', None],
-        ['DBUser', 'string', None],
-        ['DBHost', 'string', None],
-        ['DBhost', 'string', None],   # alias for backwards compatibility
-        ['DBPort', 'integer', None],
-        ['DBPass', 'string', None],
-        ['DBConnectionString', 'string', None],
-        ['KojiDir', 'string', None],
-
-        ['ProxyPrincipals', 'string', ''],
-        ['HostPrincipalFormat', 'string', None],
-        ['AllowedKrbRealms', 'string', '*'],
-        # TODO:  this option should be turned True in 1.34
-        ['DisableURLSessions', 'boolean', False],
-
-        ['DNUsernameComponent', 'string', 'CN'],
-        ['ProxyDNs', 'string', ''],
-
-        ['CheckClientIP', 'boolean', True],
-
-        ['LoginCreatesUser', 'boolean', True],
-        ['AllowProxyAuthType', 'boolean', False],
-        ['KojiWebURL', 'string', 'http://localhost.localdomain/koji'],
-        ['EmailDomain', 'string', None],
-        ['NotifyOnSuccess', 'boolean', True],
-        ['DisableNotifications', 'boolean', False],
-
-        ['Plugins', 'string', ''],
-        ['PluginPath', 'string', '/usr/lib/koji-hub-plugins'],
-
-        ['KojiDebug', 'boolean', False],
-        ['KojiTraceback', 'string', None],
-        ['VerbosePolicy', 'boolean', False],
-
-        ['LogLevel', 'string', 'WARNING'],
-        ['LogFormat', 'string',
-         '%(asctime)s [%(levelname)s] m=%(method)s u=%(user_name)s p=%(process)s r=%(remoteaddr)s '
-         '%(name)s: %(message)s'],
-
-        ['MissingPolicyOk', 'boolean', True],
-        ['EnableMaven', 'boolean', False],
-        ['EnableWin', 'boolean', False],
-
-        ['RLIMIT_AS', 'string', None],
-        ['RLIMIT_CORE', 'string', None],
-        ['RLIMIT_CPU', 'string', None],
-        ['RLIMIT_DATA', 'string', None],
-        ['RLIMIT_FSIZE', 'string', None],
-        ['RLIMIT_MEMLOCK', 'string', None],
-        ['RLIMIT_NOFILE', 'string', None],
-        ['RLIMIT_NPROC', 'string', None],
-        ['RLIMIT_OFILE', 'string', None],  # alias for RLIMIT_NOFILE
-        ['RLIMIT_RSS', 'string', None],
-        ['RLIMIT_STACK', 'string', None],
-
-        ['MemoryWarnThreshold', 'integer', 5000],
-        ['MaxRequestLength', 'integer', 4194304],
-
-        ['LockOut', 'boolean', False],
-        ['ServerOffline', 'boolean', False],
-        ['OfflineMessage', 'string', None],
-
-        ['MaxNameLengthInternal', 'integer', 256],
-        ['RegexNameInternal', 'string', r'^[A-Za-z0-9/_.+-]+$'],
-        ['RegexUserName', 'string', r'^[A-Za-z0-9/_.@-]+$'],
-
-        ['RPMDefaultChecksums', 'string', 'md5 sha256'],
-
-        ['SessionRenewalTimeout', 'integer', 1440],
-
-        # scheduler options
-        ['MaxJobs', 'integer', 15],
-        ['CapacityOvercommit', 'integer', 5],
-        ['ReadyTimeout', 'integer', 180],
-        ['AssignTimeout', 'integer', 300],
-        ['SoftRefusalTimeout', 'integer', 900],
-        ['HostTimeout', 'integer', 900],
-        ['RunInterval', 'integer', 60],
-
-        # repo options
-        ['MaxRepoTasks', 'integer', 10],
-        ['MaxRepoTasksMaven', 'integer', 2],
-        ['RepoRetries', 'integer', 3],
-        ['RequestCleanTime', 'integer', 60 * 24],  # in minutes
-        ['AllowNewRepo', 'bool', True],
-        ['RepoLag', 'integer', 3600],
-        ['RepoAutoLag', 'integer', 7200],
-        ['RepoLagWindow', 'integer', 600],
-        ['RepoQueueUser', 'str', 'kojira'],
-        ['DebuginfoTags', 'str', ''],
-        ['SourceTags', 'str', ''],
-        ['SeparateSourceTags', 'str', ''],
-    ]
     opts = {}
-    for name, dtype, default in cfgmap:
+    for name, dtype, default in config_map:
         key = ('hub', name)
         if config and config.has_option(*key):
             if dtype == 'integer':
                 opts[name] = config.getint(*key)
             elif dtype == 'boolean':
                 opts[name] = config.getboolean(*key)
-            else:
+            elif dtype == 'string':
                 opts[name] = config.get(*key)
+            else:
+                # anything else is an error in the map definition
+                raise ValueError(f'Invalid data type {dtype} for {name} option')
             continue
         opts[name] = default
     if opts['DBHost'] is None:
diff --git a/tests/test_hub/test_config.py b/tests/test_hub/test_config.py
new file mode 100644
index 0000000..65337ab
--- /dev/null
+++ b/tests/test_hub/test_config.py
@@ -0,0 +1,103 @@
+from unittest import mock
+import configparser
+import os
+import shutil
+import tempfile
+import unittest
+
+import koji
+import kojihub
+from kojihub import kojixmlrpc
+
+
+class TestHubConfig(unittest.TestCase):
+
+    def setUp(self):
+        self.context = mock.patch('kojihub.kojihub.context').start()
+        self.tempdir = tempfile.mkdtemp()
+        self.environ = {
+            'koji.hub.ConfigFile': self.tempdir + '/hub.conf',
+            'koji.hub.ConfigDir': self.tempdir + '/hub.conf.d',
+        }
+        # make an empty .d dir
+        os.mkdir(self.tempdir + '/hub.conf.d')
+        self.write_config({})
+
+    def tearDown(self):
+        shutil.rmtree(self.tempdir)
+        mock.patch.stopall()
+
+    def write_config(self, data):
+        """Write given values to test config file"""
+        cfg = configparser.RawConfigParser()
+        cfg.add_section('hub')
+        for key in data:
+            cfg.set('hub', key, data[key])
+        with open(self.tempdir + '/hub.conf', 'wt') as fp:
+            cfg.write(fp)
+
+    def write_config_string(self, config):
+        with open(self.tempdir + '/hub.conf', 'wt') as fp:
+            fp.write(config)
+
+    def test_defaults(self):
+        # blank config should get us all default opts
+        opts = kojixmlrpc.load_config(self.environ)
+
+        for name, dtype, default in kojixmlrpc.config_map:
+            self.assertIn(name, opts)
+            value = opts[name]
+            self.assertEqual(value, default)
+
+    def test_values(self):
+        config_data = {
+            'CheckClientIP': False,
+            'DBHost': 'localhost',
+            'DBPort': 1234,
+        }
+        self.write_config(config_data)
+
+        opts = kojixmlrpc.load_config(self.environ)
+
+        for key in config_data:
+            self.assertEqual(config_data[key], opts[key])
+
+    def test_kojidir(self):
+        config_data = {
+            'KojiDir': self.tempdir,
+        }
+        self.write_config(config_data)
+
+        opts = kojixmlrpc.load_config(self.environ)
+
+        self.assertEqual(config_data['KojiDir'], opts['KojiDir'])
+        self.assertEqual(config_data['KojiDir'], koji.BASEDIR)
+        self.assertEqual(config_data['KojiDir'], koji.pathinfo.topdir)
+
+    def test_invalid_dtype(self):
+        bad_row = ['BadOpt', 'badtype', None]
+        self.write_config({'BadOpt': '1234'})
+
+        with mock.patch('kojihub.kojixmlrpc.config_map', new=kojixmlrpc.config_map + [bad_row]):
+            with self.assertRaises(ValueError) as ex:
+                kojixmlrpc.load_config(self.environ)
+
+        expected = 'Invalid data type badtype for BadOpt option'
+        self.assertEqual(str(ex.exception), expected)
+
+
+    def test_policy(self):
+        config = '''
+[policy]
+channel =
+    has req_channel :: req
+    is_child_task :: parent
+    method newRepo :: use createrepo
+    all :: use default
+'''
+        self.write_config_string(config)
+
+        kojixmlrpc.load_config(self.environ)
+
+
+# the end

From b7ce3334e5d03039a7346e54084a1f89bbe44080 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Feb 10 2025 14:07:59 +0000
Subject: [PATCH 2/2] test config dtypes


---

diff --git a/kojihub/kojixmlrpc.py b/kojihub/kojixmlrpc.py
index 2d1e8af..809da60 100644
--- a/kojihub/kojixmlrpc.py
+++ b/kojihub/kojixmlrpc.py
@@ -503,14 +503,14 @@ config_map = [
     ['MaxRepoTasksMaven', 'integer', 2],
     ['RepoRetries', 'integer', 3],
     ['RequestCleanTime', 'integer', 60 * 24],  # in minutes
-    ['AllowNewRepo', 'bool', True],
-    ['RepoLag', 'int', 3600],
-    ['RepoAutoLag', 'int', 7200],
-    ['RepoLagWindow', 'int', 600],
-    ['RepoQueueUser', 'str', 'kojira'],
-    ['DebuginfoTags', 'str', ''],
-    ['SourceTags', 'str', ''],
-    ['SeparateSourceTags', 'str', ''],
+    ['AllowNewRepo', 'boolean', True],
+    ['RepoLag', 'integer', 3600],
+    ['RepoAutoLag', 'integer', 7200],
+    ['RepoLagWindow', 'integer', 600],
+    ['RepoQueueUser', 'string', 'kojira'],
+    ['DebuginfoTags', 'string', ''],
+    ['SourceTags', 'string', ''],
+    ['SeparateSourceTags', 'string', ''],
 ]
 
 
diff --git a/tests/test_hub/test_config.py b/tests/test_hub/test_config.py
index 65337ab..b3a530e 100644
--- a/tests/test_hub/test_config.py
+++ b/tests/test_hub/test_config.py
@@ -85,7 +85,6 @@ class TestHubConfig(unittest.TestCase):
         expected = 'Invalid data type badtype for BadOpt option'
         self.assertEqual(str(ex.exception), expected)
 
-
     def test_policy(self):
         config = '''
 [policy]
@@ -99,5 +98,10 @@ channel =
 
         kojixmlrpc.load_config(self.environ)
 
+    def test_map(self):
+        for row in kojixmlrpc.config_map:
+            name, dtype, default = row
+            self.assertIn(dtype, ('integer', 'boolean', 'string'))
+
 
 # the end

