From ff2357702f5620a2f267f0300464325737f911ed Mon Sep 17 00:00:00 2001
From: Ondřej Nosek <onosek@redhat.com>
Date: May 18 2020 23:34:28 +0000
Subject: Remove deprecated support for kojiconfig


JIRA: RHELCMP-567
Relates: #498

Signed-off-by: Ondřej Nosek <onosek@redhat.com>

---

diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py
index 5d5d55d..1f9cce9 100644
--- a/pyrpkg/__init__.py
+++ b/pyrpkg/__init__.py
@@ -88,9 +88,8 @@ class Commands(object):
     """
 
     def __init__(self, path, lookaside, lookasidehash, lookaside_cgi,
-                 gitbaseurl, anongiturl, branchre, kojiconfig,
-                 build_client,
-                 koji_config_type='config', user=None,
+                 gitbaseurl, anongiturl, branchre, kojiprofile,
+                 build_client, user=None,
                  dist=None, target=None, quiet=False,
                  distgit_namespaced=False, realms=None, lookaside_namespaced=False,
                  git_excludes=None):
@@ -114,11 +113,7 @@ class Commands(object):
         # The regex of branches we care about
         self.branchre = branchre
         # The location of the buildsys config file
-        self._compat_kojiconfig = koji_config_type == 'config'
-        if self._compat_kojiconfig:
-            self.kojiconfig = os.path.expanduser(kojiconfig)
-        else:
-            self.kojiprofile = kojiconfig
+        self.kojiprofile = kojiprofile
         # Koji profile of buildsys to build packages
         # The buildsys client to use
         self.build_client = build_client
@@ -270,78 +265,6 @@ class Commands(object):
             self.load_kojisession(anon=True)
         return self._anon_kojisession
 
-    def read_koji_config(self):
-        """Read Koji config from Koji configuration files or profile"""
-        if self._compat_kojiconfig:
-            return self._deprecated_read_koji_config()
-        else:
-            return koji.read_config(self.kojiprofile)
-
-    def _deprecated_read_koji_config(self):
-        """Read Koji config from Koji configuration files"""
-
-        # Stealing a bunch of code from /usr/bin/koji here, too bad it isn't
-        # in a more usable library form
-        defaults = {
-            'anon_retry': True,
-            'authtype': None,
-            'ca': '~/.koji/clientca.crt',
-            'cert': '~/.koji/client.crt',
-            'debug': None,
-            'debug_xmlrpc': None,
-            'keepalive': True,
-            'krbservice': None,
-            'max_retries': None,
-            'offline_retry_interval': None,
-            'offline_retry': None,
-            'retry_interval': None,
-            'serverca': '~/.koji/serverca.crt',
-            'server': None,
-            'timeout': None,
-            'topurl': 'http://localhost/kojiroot',
-            'use_fast_upload': None,
-            'weburl': 'http://localhost/koji',
-            }
-
-        # Process the configs in order, global, user, then any option passed
-        config = configparser.ConfigParser()
-        confs = [self.kojiconfig,
-                 os.path.expanduser('~/.koji/config')]
-        config.read(confs)
-
-        build_client_name = os.path.basename(self.build_client)
-
-        config_val_methods = {
-            'anon_retry': config.getboolean,
-            'debug': config.getboolean,
-            'debug_xmlrpc': config.getboolean,
-            'keepalive': config.getboolean,
-            'offline_retry': config.getboolean,
-            'use_fast_upload': config.getboolean,
-            'max_retries': config.getint,
-            'offline_retry_interval': config.getint,
-            'retry_interval': config.getint,
-            'timeout': config.getint,
-            }
-
-        if config.has_section(build_client_name):
-            for name, value in config.items(build_client_name):
-                if name not in defaults:
-                    continue
-                get_method = config_val_methods.get(name)
-                defaults[name] = get_method(build_client_name, name) if get_method else value
-
-        if not defaults['server']:
-            raise rpkgError('No server defined in: %s' % ', '.join(confs))
-
-        # Expand out the directory options
-        for name in ('cert', 'ca', 'serverca'):
-            path = defaults[name]
-            if path:
-                defaults[name] = os.path.expanduser(path)
-
-        return defaults
-
     def login_koji_session(self, koji_config, session):
         """Login Koji session"""
 
@@ -391,7 +314,8 @@ class Commands(object):
 
         The koji session can be logged in or anonymous
         """
-        koji_config = self.read_koji_config()
+        # Read Koji config from Koji profile
+        koji_config = koji.read_config(self.kojiprofile)
 
         # save the weburl and topurl for later use as well
         self._kojiweburl = koji_config['weburl']
@@ -3133,7 +3057,7 @@ class Commands(object):
         return koji_result
 
     def container_build_koji(self, target_override=False, opts={},
-                             kojiconfig=None, kojiprofile=None,
+                             kojiprofile=None,
                              build_client=None,
                              koji_task_watcher=None,
                              nowait=False,
@@ -3145,15 +3069,7 @@ class Commands(object):
             else self.flatpak_build_target if flatpak \
             else self.container_build_target
 
-        # This is for backward-compatibility of deprecated kojiconfig.
-        # Signature of container_build_koji is not changed in case someone
-        # reuses this method in his app and keep it unbroken.
-        # Why to check names of kojiconfig and kojiprofile on Commands? Please
-        # see also Commands.__init__
-        if self._compat_kojiconfig:
-            koji_session_backup = (self.build_client, self.kojiconfig)
-        else:
-            koji_session_backup = (self.build_client, self.kojiprofile)
+        koji_session_backup = (self.build_client, self.kojiprofile)
 
         rv = 0  # return value of koji task. It is returned as a method result.
         try:
@@ -3209,10 +3125,7 @@ class Commands(object):
                     log_result(self.log.info, result)
 
         finally:
-            if self._compat_kojiconfig:
-                self.build_client, self.kojiconfig = koji_session_backup
-            else:
-                self.build_client, self.kojiprofile = koji_session_backup
+            self.build_client, self.kojiprofile = koji_session_backup
             self.load_kojisession()
 
         return rv
diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py
index f34871f..8721b4e 100644
--- a/pyrpkg/cli.py
+++ b/pyrpkg/cli.py
@@ -235,24 +235,14 @@ class cliClient(object):
                   for realm in items.get("kerberos_realms", '').split(',')
                   if realm]
 
-        kojiconfig = None
-
-        if self.config.has_option(self.name, 'kojiconfig'):
-            kojiconfig = self.config.get(self.name, 'kojiconfig')
-            koji_config_type = 'config'
-            self.log.warning(
-                'Deprecation warning: kojiconfig is deprecated. Instead, '
-                'kojiprofile should be used.')
-
-        # kojiprofile has higher priority to be used if both kojiconfig and
-        # kojiprofile exist at same time.
+        kojiprofile = None
         if self.config.has_option(self.name, 'kojiprofile'):
-            kojiconfig = self.config.get(self.name, 'kojiprofile')
-            koji_config_type = 'profile'
+            kojiprofile = self.config.get(self.name, 'kojiprofile')
 
-        if not kojiconfig:
-            raise rpkgError('Missing kojiconfig and kojiprofile to load Koji '
-                            'session. One of them must be specified.')
+        if not kojiprofile:
+            raise rpkgError('Missing kojiprofile to load Koji session. Check '
+                            'if there is deprecated kojiconfig instead of '
+                            'kojiprofile in the config.')
 
         if '%(module)s' in items['gitbaseurl']:
             self.log.warning(
@@ -277,9 +267,8 @@ class cliClient(object):
                                        items['gitbaseurl'],
                                        items['anongiturl'],
                                        items['branchre'],
-                                       kojiconfig,
+                                       kojiprofile,
                                        items['build_client'],
-                                       koji_config_type=koji_config_type,
                                        user=self.args.user,
                                        dist=self.args.dist or self.args.release,
                                        target=target,
@@ -2070,22 +2059,13 @@ class cliClient(object):
                   "Using %(option)s from [%(root.section)s]"
         err_args = {"plugin.section": section_name, "root.section": self.name}
 
-        kojiconfig = kojiprofile = None
-
-        if self.cmd._compat_kojiconfig:
-            if self.config.has_option(section_name, "kojiconfig"):
-                kojiconfig = self.config.get(section_name, "kojiconfig")
-            else:
-                err_args["option"] = "kojiconfig"
-                self.log.debug(err_msg % err_args)
-                kojiconfig = self.config.get(self.name, "kojiconfig")
+        kojiprofile = None
+        if self.config.has_option(section_name, "kojiprofile"):
+            kojiprofile = self.config.get(section_name, "kojiprofile")
         else:
-            if self.config.has_option(section_name, "kojiprofile"):
-                kojiprofile = self.config.get(section_name, "kojiprofile")
-            else:
-                err_args["option"] = "kojiprofile"
-                self.log.debug(err_msg % err_args)
-                kojiprofile = self.config.get(self.name, "kojiprofile")
+            err_args["option"] = "kojiprofile"
+            self.log.debug(err_msg % err_args)
+            kojiprofile = self.config.get(self.name, "kojiprofile")
 
         if self.config.has_option(section_name, "build_client"):
             build_client = self.config.get(section_name, "build_client")
@@ -2103,7 +2083,6 @@ class cliClient(object):
         rv = self.cmd.container_build_koji(
             target_override,
             opts=opts,
-            kojiconfig=kojiconfig,
             kojiprofile=kojiprofile,
             build_client=build_client,
             koji_task_watcher=koji_cli.lib.watch_tasks,
diff --git a/tests/fixtures/rpkg-deprecated-kojiconfig.conf b/tests/fixtures/rpkg-deprecated-kojiconfig.conf
deleted file mode 100644
index 232afe9..0000000
--- a/tests/fixtures/rpkg-deprecated-kojiconfig.conf
+++ /dev/null
@@ -1,11 +0,0 @@
-[rpkg]
-lookaside = http://localhost/repo/pkgs
-lookasidehash = md5
-lookaside_cgi = https://localhost/repo/pkgs/upload.cgi
-gitbaseurl = ssh://%(user)s@localhost/%(repo)s
-anongiturl = git://localhost/%(repo)s
-branchre = f\d$|f\d\d$|el\d$|olpc\d$|master$
-kojiconfig = /path/to/koji.conf
-build_client = koji
-clone_config_rpms =
-  bz.default-component %(repo)s
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 5215a7d..f127d47 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -144,55 +144,6 @@ class TestModuleNameOption(CliTestCase):
         self.assertEqual(cmd.ns_repo_name, 'user/project/foo')
 
 
-class TestKojiConfigBackwardCompatibility(CliTestCase):
-    """Test backward compatibility of kojiconfig and kojiprofile
-
-    Remove this test case after deprecated kojiconfig is removed eventually.
-    """
-
-    create_repo_per_test = False
-
-    @patch('pyrpkg.Commands._deprecated_read_koji_config')
-    @patch('pyrpkg.koji.read_config')
-    def test_use_deprecated_kojiconfig(self,
-                                       read_config,
-                                       _deprecated_read_koji_config):
-        cli_cmd = ['rpkg', '--path', self.cloned_repo_path, 'build']
-
-        cfg_file = os.path.join(os.path.dirname(__file__),
-                                'fixtures',
-                                'rpkg-deprecated-kojiconfig.conf')
-
-        with patch('sys.argv', new=cli_cmd):
-            cli = self.new_cli(cfg_file)
-
-        cli.cmd.read_koji_config()
-
-        self.assertFalse(hasattr(cli.cmd, 'kojiprofile'))
-        self.assertEqual(utils.kojiconfig, cli.cmd.kojiconfig)
-        self.assertTrue(cli.cmd._compat_kojiconfig)
-
-        read_config.assert_not_called()
-        _deprecated_read_koji_config.assert_called_once()
-
-    @patch('pyrpkg.Commands._deprecated_read_koji_config')
-    @patch('pyrpkg.koji.read_config')
-    def test_use_kojiprofile(self, read_config, _deprecated_read_koji_config):
-        cli_cmd = ['rpkg', '--path', self.cloned_repo_path, 'build']
-
-        with patch('sys.argv', new=cli_cmd):
-            cli = self.new_cli()
-
-        cli.cmd.read_koji_config()
-
-        self.assertFalse(hasattr(cli.cmd, 'kojiconfig'))
-        self.assertEqual(utils.kojiprofile, cli.cmd.kojiprofile)
-        self.assertFalse(cli.cmd._compat_kojiconfig)
-
-        read_config.assert_called_once_with(utils.kojiprofile)
-        _deprecated_read_koji_config.assert_not_called()
-
-
 class TestContainerBuildWithKoji(CliTestCase):
     """Test container_build with koji"""
 
@@ -234,7 +185,6 @@ class TestContainerBuildWithKoji(CliTestCase):
                 'compose_ids': None,
                 'skip_build': False
             },
-            kojiconfig=None,
             kojiprofile='koji',
             build_client=utils.build_client,
             koji_task_watcher=koji_cli.lib.watch_tasks,
@@ -267,7 +217,6 @@ class TestContainerBuildWithKoji(CliTestCase):
                 'compose_ids': None,
                 'skip_build': False
             },
-            kojiconfig=None,
             kojiprofile='koji',
             build_client=utils.build_client,
             koji_task_watcher=koji_cli.lib.watch_tasks,
@@ -299,7 +248,6 @@ class TestContainerBuildWithKoji(CliTestCase):
                 'compose_ids': None,
                 'skip_build': False
             },
-            kojiconfig=None,
             kojiprofile='koji',
             build_client=utils.build_client,
             koji_task_watcher=koji_cli.lib.watch_tasks,
@@ -307,48 +255,6 @@ class TestContainerBuildWithKoji(CliTestCase):
             flatpak=False
         )
 
-    def test_using_deprecated_kojiconfig(self):
-        """test_build_using_deprecated_kojiconfig
-
-        This is for ensuring container_build works with deprecated kojiconfig.
-        This test can be delete after kojiconfig is removed eventually.
-        """
-        cli_cmd = ['rpkg', '--path', self.cloned_repo_path,
-                   '--name', 'mycontainer',
-                   'container-build']
-
-        cfg_file = os.path.join(os.path.dirname(__file__),
-                                'fixtures',
-                                'rpkg-deprecated-kojiconfig.conf')
-
-        with patch('sys.argv', new=cli_cmd):
-            cli = self.new_cli(cfg_file)
-            cli.container_build_koji()
-
-        self.mock_container_build_koji.assert_called_once_with(
-            False,
-            opts={
-                'scratch': False,
-                'quiet': False,
-                'release': None,
-                'isolated': False,
-                'koji_parent_build': None,
-                'yum_repourls': None,
-                'dependency_replacements': None,
-                'git_branch': 'eng-rhel-7',
-                'arches': None,
-                'signing_intent': None,
-                'compose_ids': None,
-                'skip_build': False
-            },
-            kojiconfig='/path/to/koji.conf',
-            kojiprofile=None,
-            build_client=utils.build_client,
-            koji_task_watcher=koji_cli.lib.watch_tasks,
-            nowait=False,
-            flatpak=False
-        )
-
     def test_use_container_build_own_config(self):
         cli_cmd = ['rpkg', '--path', self.cloned_repo_path,
                    'container-build']
@@ -397,7 +303,6 @@ class TestContainerBuildWithKoji(CliTestCase):
                 'arches': None,
                 'skip_build': False
             },
-            kojiconfig=None,
             kojiprofile='koji',
             build_client=utils.build_client,
             koji_task_watcher=koji_cli.lib.watch_tasks,

