From e3072c3d5f8b25a8df513c00aa1390a342c20d2f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Hozza?= <thozza@redhat.com>
Date: Mon, 28 Nov 2022 13:51:37 +0100
Subject: [PATCH] osbuild: don't use `util.get_repo_urls()`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Don't use `util.get_repo_urls()` to resolve provided repositories, but
implement osbuild-specific variant of the function named
`_get_repo_urls(). The reason is that the function from `utils`
transforms repositories defined as dicts to strings, which is
undesired for osbuild. The requirement for osbuild is to preserve the
dict as is, just to resolve the string in `baseurl` to the actual
repository URL.

Add a unit test covering the newly added function. It is inspired by a
similar test from `test_util.py`.

Signed-off-by: Tomáš Hozza <thozza@redhat.com>
---
 pungi/phases/osbuild.py     | 31 ++++++++++++++++++-
 tests/test_osbuild_phase.py | 62 +++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/pungi/phases/osbuild.py b/pungi/phases/osbuild.py
index 6e52e9c5..2bef1636 100644
--- a/pungi/phases/osbuild.py
+++ b/pungi/phases/osbuild.py
@@ -27,6 +27,35 @@ class OSBuildPhase(
             arches = set(image_conf["arches"]) & arches
         return sorted(arches)
 
+    @staticmethod
+    def _get_repo_urls(compose, repos, arch="$basearch"):
+        """
+        Get list of repos with resolved repo URLs. Preserve repos defined
+        as dicts.
+        """
+        resolved_repos = []
+
+        for repo in repos:
+            if isinstance(repo, dict):
+                try:
+                    url = repo["baseurl"]
+                except KeyError:
+                    raise RuntimeError(
+                        "`baseurl` is required in repo dict %s" % str(repo)
+                    )
+                url = util.get_repo_url(compose, url, arch=arch)
+                if url is None:
+                    raise RuntimeError("Failed to resolve repo URL for %s" % str(repo))
+                repo["baseurl"] = url
+                resolved_repos.append(repo)
+            else:
+                repo = util.get_repo_url(compose, repo, arch=arch)
+                if repo is None:
+                    raise RuntimeError("Failed to resolve repo URL for %s" % repo)
+                resolved_repos.append(repo)
+
+        return resolved_repos
+
     def _get_repo(self, image_conf, variant):
         """
         Get a list of repos. First included are those explicitly listed in
@@ -38,7 +67,7 @@ class OSBuildPhase(
         if not variant.is_empty and variant.uid not in repos:
             repos.append(variant.uid)
 
-        return util.get_repo_urls(self.compose, repos, arch="$arch")
+        return OSBuildPhase._get_repo_urls(self.compose, repos, arch="$arch")
 
     def run(self):
         for variant in self.compose.get_variants():
diff --git a/tests/test_osbuild_phase.py b/tests/test_osbuild_phase.py
index c53f7529..e545a39e 100644
--- a/tests/test_osbuild_phase.py
+++ b/tests/test_osbuild_phase.py
@@ -3,14 +3,76 @@
 import mock
 
 import os
+import shutil
+import tempfile
+import unittest
 
 import koji as orig_koji
 
 from tests import helpers
+from pungi import compose
 from pungi.phases import osbuild
 from pungi.checks import validate
 
 
+class OSBuildPhaseHelperFuncsTest(unittest.TestCase):
+    @mock.patch("pungi.compose.ComposeInfo")
+    def setUp(self, ci):
+        self.tmp_dir = tempfile.mkdtemp()
+        conf = {"translate_paths": [(self.tmp_dir, "http://example.com")]}
+        ci.return_value.compose.respin = 0
+        ci.return_value.compose.id = "RHEL-8.0-20180101.n.0"
+        ci.return_value.compose.date = "20160101"
+        ci.return_value.compose.type = "nightly"
+        ci.return_value.compose.type_suffix = ".n"
+        ci.return_value.compose.label = "RC-1.0"
+        ci.return_value.compose.label_major_version = "1"
+
+        compose_dir = os.path.join(self.tmp_dir, ci.return_value.compose.id)
+        self.compose = compose.Compose(conf, compose_dir)
+        server_variant = mock.Mock(uid="Server", type="variant")
+        client_variant = mock.Mock(uid="Client", type="variant")
+        self.compose.all_variants = {
+            "Server": server_variant,
+            "Client": client_variant,
+        }
+
+    def tearDown(self):
+        shutil.rmtree(self.tmp_dir)
+
+    def test__get_repo_urls(self):
+        repos = [
+            "http://example.com/repo",
+            "Server",
+            {
+                "baseurl": "Client",
+                "package_sets": ["build"],
+            },
+            {
+                "baseurl": "ftp://example.com/linux/repo",
+                "package_sets": ["build"],
+            },
+        ]
+
+        expect = [
+            "http://example.com/repo",
+            "http://example.com/RHEL-8.0-20180101.n.0/compose/Server/$basearch/os",
+            {
+                "baseurl": "http://example.com/RHEL-8.0-20180101.n.0/compose/Client/"
+                + "$basearch/os",
+                "package_sets": ["build"],
+            },
+            {
+                "baseurl": "ftp://example.com/linux/repo",
+                "package_sets": ["build"],
+            },
+        ]
+
+        self.assertEqual(
+            osbuild.OSBuildPhase._get_repo_urls(self.compose, repos), expect
+        )
+
+
 class OSBuildPhaseTest(helpers.PungiTestCase):
     @mock.patch("pungi.phases.osbuild.ThreadPool")
     def test_run(self, ThreadPool):