From 935da7c2460849d6b32856e2d96c17e0e6b4807e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= <lsedlar@redhat.com>
Date: Thu, 26 Oct 2023 09:50:05 +0200
Subject: [PATCH] pkgset: Ignore duplicated module builds
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If the module tag contains the same module build multiple times (because
it's in multiple tags in the inheritance), Pungi will not process that
correctly and try to include the same NSVC in the compose multiple
times. That leads to a crash.

This patch adds another step to the inheritance filter to ensure the
result contains each module only once.

JIRA: RHELCMP-12768
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
---
 pungi/phases/pkgset/sources/source_koji.py | 11 +++-
 tests/test_pkgset_source_koji.py           | 58 +++++++++++++++-------
 2 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/pungi/phases/pkgset/sources/source_koji.py b/pungi/phases/pkgset/sources/source_koji.py
index e15c8d08..bd9cb5d5 100644
--- a/pungi/phases/pkgset/sources/source_koji.py
+++ b/pungi/phases/pkgset/sources/source_koji.py
@@ -487,7 +487,16 @@ def filter_inherited(koji_proxy, event, module_builds, top_tag):
         # And keep only builds from that topmost tag
         result.extend(build for build in builds if build["tag_name"] == tag)
 
-    return result
+    # If the same module was inherited multiple times, it will be in result
+    # multiple times. We need to deduplicate.
+    deduplicated_result = []
+    included_nvrs = set()
+    for build in result:
+        if build["nvr"] not in included_nvrs:
+            deduplicated_result.append(build)
+            included_nvrs.add(build["nvr"])
+
+    return deduplicated_result
 
 
 def filter_by_whitelist(compose, module_builds, input_modules, expected_modules):
diff --git a/tests/test_pkgset_source_koji.py b/tests/test_pkgset_source_koji.py
index 26c12854..44bbff9d 100644
--- a/tests/test_pkgset_source_koji.py
+++ b/tests/test_pkgset_source_koji.py
@@ -31,6 +31,17 @@ TAG_INFO = {
 }
 
 
+def _mk_module_build(r, t):
+    """Create a dict as returned Koji buildinfo."""
+    return {
+        "name": "foo",
+        "version": "1",
+        "release": r,
+        "nvr": "foo-1-%s" % r,
+        "tag_name": t,
+    }
+
+
 class TestGetKojiEvent(helpers.PungiTestCase):
     def setUp(self):
         super(TestGetKojiEvent, self).setUp()
@@ -547,19 +558,15 @@ class TestFilterInherited(unittest.TestCase):
             {"name": "middle-tag"},
             {"name": "bottom-tag"},
         ]
-        module_builds = [
-            {"name": "foo", "version": "1", "release": "1", "tag_name": "top-tag"},
-            {"name": "foo", "version": "1", "release": "2", "tag_name": "bottom-tag"},
-            {"name": "foo", "version": "1", "release": "3", "tag_name": "middle-tag"},
-        ]
+
+        m1 = _mk_module_build("1", "top-tag")
+        m2 = _mk_module_build("2", "middle-tag")
+        m3 = _mk_module_build("3", "bottom-tag")
+        module_builds = [m1, m2, m3]
 
         result = source_koji.filter_inherited(koji_proxy, event, module_builds, top_tag)
 
-        six.assertCountEqual(
-            self,
-            result,
-            [{"name": "foo", "version": "1", "release": "1", "tag_name": "top-tag"}],
-        )
+        six.assertCountEqual(self, result, [m1])
         self.assertEqual(
             koji_proxy.mock_calls,
             [mock.call.getFullInheritance("top-tag", event=123456)],
@@ -574,18 +581,33 @@ class TestFilterInherited(unittest.TestCase):
             {"name": "middle-tag"},
             {"name": "bottom-tag"},
         ]
-        module_builds = [
-            {"name": "foo", "version": "1", "release": "2", "tag_name": "bottom-tag"},
-            {"name": "foo", "version": "1", "release": "3", "tag_name": "middle-tag"},
-        ]
+        m2 = _mk_module_build("2", "bottom-tag")
+        m3 = _mk_module_build("3", "middle-tag")
+        module_builds = [m2, m3]
 
         result = source_koji.filter_inherited(koji_proxy, event, module_builds, top_tag)
 
-        six.assertCountEqual(
-            self,
-            result,
-            [{"name": "foo", "version": "1", "release": "3", "tag_name": "middle-tag"}],
+        six.assertCountEqual(self, result, [m3])
+        self.assertEqual(
+            koji_proxy.mock_calls,
+            [mock.call.getFullInheritance("top-tag", event=123456)],
         )
+
+    def test_build_in_multiple_tags(self):
+        event = {"id": 123456}
+        koji_proxy = mock.Mock()
+        top_tag = "top-tag"
+
+        koji_proxy.getFullInheritance.return_value = [
+            {"name": "middle-tag"},
+            {"name": "bottom-tag"},
+        ]
+        m = _mk_module_build("1", "middle-tag")
+        module_builds = [m, m]
+
+        result = source_koji.filter_inherited(koji_proxy, event, module_builds, top_tag)
+
+        six.assertCountEqual(self, result, [m])
         self.assertEqual(
             koji_proxy.mock_calls,
             [mock.call.getFullInheritance("top-tag", event=123456)],