KojiResolver: Filter module builds based on the real stream name.

Query Koji for the real stream name of each module and keep only those matching
requested `stream`.

This needs to be done, because MBS stores the stream name in the "version" field in Koji,
but the "version" field cannot contain "-" character. Therefore MBS replaces all "-"
with "_". This makes it impossible to reconstruct the original stream name from the
"version" field.

We therefore need to ask for real original stream name here and filter out modules based
on this real stream name.

module_build_service/resolver/KojiResolver.py

@@ -59,6 +59,59 @@ class KojiResolver(DBResolver):
 
         return result
 
+    def _filter_based_on_real_stream_name(self, koji_session, module_builds, stream):
+        """
+        Query Koji for real stream name of each module and keep only those matching `stream`.
+
+        This needs to be done, because MBS stores the stream name in the "version" field in Koji,
+        but the "version" field cannot contain "-" character. Therefore MBS replaces all "-"
+        with "_". This makes it impossible to reconstruct the original stream name from the
+        "version" field.
+
+        We therefore need to ask for real original stream name here and filter out modules based
+        on this real stream name.
+
+        :param KojiSession koji_session: Koji session.
+        :param list module_builds: List of builds as returned by KojiSession.listTagged method.
+        :param str stream: The requested stream name.
+        :return list: Filtered list of builds.
+        """
+        # We need to import here because of circular dependencies.
+        from module_build_service.builder.KojiModuleBuilder import koji_multicall_map
+
+        # Return early if there are no module builds.
+        if not module_builds:
+            return []
+
+        # Prepare list of build ids to pass them to Koji multicall later.
+        build_ids = [b["build_id"] for b in module_builds]
+
+        # Get the Koji builds from Koji.
+        koji_builds = koji_multicall_map(koji_session, koji_session.getBuild, build_ids)
+        if not koji_builds:
+            raise RuntimeError("Error during Koji multicall when filtering KojiResolver builds.")
+
+        # Filter out modules with different stream in the Koji build metadata.
+        ret = []
+        for module_build, koji_build in zip(module_builds, koji_builds):
+            koji_build_stream = koji_build.get("extra", {}).get("typeinfo", {}).get("module", {}).\
+                get("stream")
+            if not koji_build_stream:
+                log.warning(
+                    "Not filtering out Koji build with id %d - it has no \"stream\" set in its "
+                    "metadata." % koji_build["build_id"])
+                ret.append(module_build)
+                continue
+
+            if koji_build_stream == stream:
+                ret.append(module_build)
+            else:
+                log.info(
+                    "Filtering out Koji build %d - its stream \"%s\" does not match the requested "
+                    "stream \"%s\"" % (koji_build["build_id"], stream, koji_build_stream))
+
+        return ret
+
     def get_buildrequired_koji_builds(self, name, stream, base_module_mmd):
         """
         Returns list of Koji build dicts of all module builds with `name` and `stream` which are
@@ -89,13 +142,23 @@ class KojiResolver(DBResolver):
         module_builds = koji_session.listTagged(
             tag, inherit=True, type="module", package=name, event=event["id"])
 
-        # Filter out different streams
+        # Filter out different streams. Note that the stream name in the b["version"] is
+        # normalized. This makes it impossible to find out its original value. We therefore
+        # filter out only completely different stream names here to reduce the `module_builds`
+        # dramatically, but the resulting `module_builds` list might still contain unwanted
+        # streams. We will get rid of them using the `_filter_based_on_real_stream_name` method
+        # later.
+        # Example of such streams: "fedora-30" and "fedora_30". They will both be normalized to
+        # "fedora_30".
         normalized_stream = stream.replace("-", "_")
         module_builds = [b for b in module_builds if b["version"] == normalized_stream]
 
         # Filter out builds inherited from non-top tag
         module_builds = self._filter_inherited(koji_session, module_builds, tag, event)
 
+        # Filter out modules based on the real stream name.
+        module_builds = self._filter_based_on_real_stream_name(koji_session, module_builds, stream)
+
         # Find the latest builds of all modules. This does the following:
         # - Sorts the module_builds descending by Koji NVR (which maps to NSV
         #   for modules). Split release into modular version and context, and

tests/test_resolver/test_koji.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # SPDX-License-Identifier: MIT
 import pytest
-from mock import patch
+from mock import patch, MagicMock
 from datetime import datetime
 
 import module_build_service.resolver as mbs_resolver
@@ -70,6 +70,7 @@ class TestLocalResolverModule:
 
         # No package with such name tagged.
         koji_session.listTagged.return_value = []
+        koji_session.multiCall.return_value = [[]]
 
         self._create_test_modules(db_session)
         platform = db_session.query(ModuleBuild).filter_by(stream="f30.1.3").one()
@@ -95,6 +96,9 @@ class TestLocalResolverModule:
                 "release": "20170109091357.7c29193d", "tag_name": "foo-test"
             }]
 
+        koji_session.multiCall.return_value = [
+            [build] for build in koji_session.listTagged.return_value]
+
         self._create_test_modules(db_session)
         platform = db_session.query(ModuleBuild).filter_by(stream="f30.1.3").one()
         resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji")
@@ -119,6 +123,9 @@ class TestLocalResolverModule:
                 "release": "20170109091357.7c29193d", "tag_name": "foo-test"
             }]
 
+        koji_session.multiCall.return_value = [
+            [build] for build in koji_session.listTagged.return_value]
+
         self._create_test_modules(db_session)
         platform = db_session.query(ModuleBuild).filter_by(stream="f30.1.3").one()
         resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji")
@@ -152,6 +159,9 @@ class TestLocalResolverModule:
                 "release": "20160109091357.7c29193d", "tag_name": "foo-test"
             }]
 
+        koji_session.multiCall.return_value = [
+            [build] for build in koji_session.listTagged.return_value]
+
         self._create_test_modules(db_session)
         platform = db_session.query(ModuleBuild).filter_by(stream="f30.1.3").one()
         resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji")
@@ -177,6 +187,9 @@ class TestLocalResolverModule:
                 "release": "20170109091357.7c29193d", "tag_name": "foo-test"
             }]
 
+        koji_session.multiCall.return_value = [
+            [build] for build in koji_session.listTagged.return_value]
+
         self._create_test_modules(db_session)
         platform = db_session.query(ModuleBuild).filter_by(stream="f30.1.3").one()
         resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji")
@@ -216,6 +229,44 @@ class TestLocalResolverModule:
             "testmodule-master-20170110091357.7c29193d",
             "testmodule-2-20180109091357.7c29193d"}
 
+    @patch("module_build_service.builder.KojiModuleBuilder.koji_multicall_map")
+    def test_filter_based_on_real_stream_name(self, koji_multicall_map, db_session):
+        koji_session = MagicMock()
+        koji_multicall_map.return_value = [
+            {"build_id": 124, "extra": {"typeinfo": {"module": {"stream": "foo-test"}}}},
+            {"build_id": 125, "extra": {"typeinfo": {"module": {"stream": "foo_test"}}}},
+            {"build_id": 126, "extra": {"typeinfo": {"module": {"stream": "foo-test"}}}},
+            {"build_id": 127, "extra": {"typeinfo": {"module": {}}}},
+        ]
+
+        builds = [
+            {"build_id": 124, "name": "testmodule", "version": "foo_test"},
+            {"build_id": 125, "name": "testmodule", "version": "foo_test"},
+            {"build_id": 126, "name": "testmodule", "version": "foo_test"},
+            {"build_id": 127, "name": "testmodule", "version": "foo_test"},
+        ]
+
+        resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji")
+        new_builds = resolver._filter_based_on_real_stream_name(koji_session, builds, "foo-test")
+
+        build_ids = {b["build_id"] for b in new_builds}
+        assert build_ids == {124, 126, 127}
+
+    @patch("module_build_service.builder.KojiModuleBuilder.koji_multicall_map")
+    def test_filter_based_on_real_stream_name_multicall_error(
+            self, koji_multicall_map, db_session):
+        koji_session = MagicMock()
+        koji_multicall_map.return_value = None
+
+        builds = [
+            {"build_id": 124, "name": "testmodule", "version": "foo_test"},
+        ]
+
+        expected_error = "Error during Koji multicall when filtering KojiResolver builds."
+        resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji")
+        with pytest.raises(RuntimeError, match=expected_error):
+            resolver._filter_based_on_real_stream_name(koji_session, builds, "foo-test")
+
     def test_get_compatible_base_module_modulemds_fallback_to_dbresolver(self, db_session):
         tests.init_data(1, multiple_stream_versions=True)
         resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji")

tests/test_utils/test_utils.py

@@ -1694,6 +1694,9 @@ class TestUtilsModuleReuse:
                 "release": "20170109091357.78e4a6fd", "tag_name": "module-fedora-27-build"
             }]
 
+        koji_session.multiCall.return_value = [
+            [build] for build in koji_session.listTagged.return_value]
+
         # Mark platform:f28 as KojiResolver ready by defining "koji_tag_with_modules".
         # Also define the "virtual_streams" to possibly confuse the get_reusable_module.
         platform_f28 = db_session.query(models.ModuleBuild).filter_by(name="platform").one()