From 98eb8e28c95f101829e038a635b0413714ca0717 Mon Sep 17 00:00:00 2001 From: Siddharth Sharma Date: Wed, 10 Jun 2026 11:02:12 -0400 Subject: [PATCH 1/2] Filter Konflux golang-builder search by hermetic flag When searching for existing golang-builder builds in the Konflux DB, consider the effective network mode from ocp-build-data config. If the builder is configured as hermetic, only match hermetic builds. - Extract resolve_network_mode() into artcommonlib for shared use - Add _get_effective_network_mode() to update_golang pipeline - Add hermetic field to search_builds_by_fields where clause - Fix boolean handling in search_builds_by_fields (was using String column type for all values, now uses Boolean for bool values) Co-Authored-By: Claude Opus 4.6 rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED --- artcommon/artcommonlib/konflux/konflux_db.py | 2 + artcommon/artcommonlib/metadata.py | 32 ++- pyartcd/pyartcd/pipelines/update_golang.py | 37 ++++ pyartcd/tests/pipelines/test_update_golang.py | 207 ++++++++++++++++++ 4 files changed, 268 insertions(+), 10 deletions(-) diff --git a/artcommon/artcommonlib/konflux/konflux_db.py b/artcommon/artcommonlib/konflux/konflux_db.py index fce893ea34..ba628bb01b 100644 --- a/artcommon/artcommonlib/konflux/konflux_db.py +++ b/artcommon/artcommonlib/konflux/konflux_db.py @@ -805,6 +805,8 @@ async def search_builds_by_fields( # Translating into queries like "AND outcome IN ('success', 'failed')" col_value = [str(outcome) for outcome in col_value] base_clauses.append(Column(col_name, String).in_(col_value)) + elif isinstance(col_value, bool): + base_clauses.append(Column(col_name, Boolean) == col_value) else: base_clauses.append(Column(col_name, String) == col_value) else: diff --git a/artcommon/artcommonlib/metadata.py b/artcommon/artcommonlib/metadata.py index 405bca2c82..72e279a26f 100644 --- a/artcommon/artcommonlib/metadata.py +++ b/artcommon/artcommonlib/metadata.py @@ -21,6 +21,27 @@ CONFIG_MODE_DEFAULT = CONFIG_MODES[0] +VALID_NETWORK_MODES = ["hermetic", "internal-only", "open"] + + +def resolve_network_mode( + cli_override: str | None = None, + image_config_network_mode: str | None = None, + group_config_network_mode: str | None = None, +) -> str: + """Resolve the effective Konflux network mode from multiple config sources. + + Precedence: cli_override > image_config > group_config > "open" + """ + if cli_override: + network_mode = cli_override + else: + network_mode = image_config_network_mode or group_config_network_mode or "open" + + if network_mode not in VALID_NETWORK_MODES: + raise ValueError(f"Invalid network mode: {network_mode}. Valid modes: {VALID_NETWORK_MODES}") + return network_mode + class MetadataBase(object): def __init__(self, meta_type, runtime, data_obj): @@ -617,18 +638,9 @@ def run_async(): def get_konflux_network_mode(self) -> str: runtime_override = getattr(self.runtime, "network_mode_override", None) - if runtime_override: - return runtime_override - group_config_network_mode = self.runtime.group_config.konflux.get("network_mode") image_config_network_mode = self.config.konflux.get("network_mode") - - network_mode = image_config_network_mode or group_config_network_mode or "open" - - valid_network_modes = ["hermetic", "internal-only", "open"] - if network_mode not in valid_network_modes: - raise ValueError(f"Invalid network mode; {network_mode}. Valid modes: {valid_network_modes}") - return network_mode + return resolve_network_mode(runtime_override, image_config_network_mode, group_config_network_mode) @property def bridge_bug_mirroring_enabled(self) -> bool: diff --git a/pyartcd/pyartcd/pipelines/update_golang.py b/pyartcd/pyartcd/pipelines/update_golang.py index bd1021217f..df92357f97 100644 --- a/pyartcd/pyartcd/pipelines/update_golang.py +++ b/pyartcd/pyartcd/pipelines/update_golang.py @@ -19,6 +19,7 @@ from artcommonlib.github_auth import get_github_client_for_org from artcommonlib.konflux.konflux_build_record import ArtifactType, Engine, KonfluxBuildOutcome, KonfluxBuildRecord from artcommonlib.konflux.konflux_db import KonfluxDb +from artcommonlib.metadata import resolve_network_mode from artcommonlib.release_util import isolate_el_version_in_release from artcommonlib.rpm_utils import parse_nvr from artcommonlib.util import new_roundtrip_yaml_handler @@ -573,6 +574,11 @@ async def get_existing_builders_konflux( """ _LOGGER.info(f"Checking if {GOLANG_BUILDER_IMAGE_NAME} builds exist in Konflux for given golang builds") + el_hermetic_map: dict[int, bool] = {} + for el_v in el_nvr_map: + network_mode = self._get_effective_network_mode(el_v, go_version) + el_hermetic_map[el_v] = network_mode == "hermetic" + builder_records: dict[int, KonfluxBuildRecord] = {} extra_patterns = {'nvr': f"{GOLANG_BUILDER_CVE_COMPONENT}-v{go_version}"} build_records = await asyncio.gather( @@ -585,6 +591,7 @@ async def get_existing_builders_konflux( "artifact_type": str(ArtifactType.IMAGE), "outcome": str(KonfluxBuildOutcome.SUCCESS), "engine": str(Engine.KONFLUX), + "hermetic": el_hermetic_map[el_v], }, extra_patterns=extra_patterns, limit=1, @@ -972,6 +979,36 @@ def _get_doozer_group_and_image(self, el_v, go_version): group += f'@{self.data_gitref}' return group, image_key + def _get_effective_network_mode(self, el_v: int, go_version: str) -> str: + if self.network_mode: + return self.network_mode + + if self.use_new_golang_branch: + branch = self.GOLANG_DATA_BRANCH + image_key = self.get_golang_image_key(el_v, go_version) + else: + branch = self.get_golang_branch(el_v, go_version) + image_key = GOLANG_BUILDER_IMAGE_NAME + + ref = self.data_gitref if self.data_gitref else branch + repo = self._get_upstream_ocp_build_data_repo() + + try: + image_config = self._load_yaml_from_repo(repo, f"images/{image_key}.yml", ref) + except Exception: + image_config = {} + image_config_network_mode = (image_config.get("konflux") or {}).get("network_mode") + + try: + group_config = self._load_yaml_from_repo(repo, "group.yml", ref) + except Exception: + group_config = {} + group_config_network_mode = (group_config.get("konflux") or {}).get("network_mode") + + network_mode = resolve_network_mode(None, image_config_network_mode, group_config_network_mode) + _LOGGER.info(f"Effective network mode for el{el_v} golang-builder: {network_mode}") + return network_mode + def verify_golang_builder_repo(self, el_v, go_version): if self.use_new_golang_branch: branch = self.GOLANG_DATA_BRANCH diff --git a/pyartcd/tests/pipelines/test_update_golang.py b/pyartcd/tests/pipelines/test_update_golang.py index 034f388915..93b836223f 100644 --- a/pyartcd/tests/pipelines/test_update_golang.py +++ b/pyartcd/tests/pipelines/test_update_golang.py @@ -1197,6 +1197,8 @@ async def mock_search_builds(*_args, **_kwargs): "golang-1.20.12-2.el8": {("ignored-builder", "ignored-version", "ignored-release")} } + pipeline._get_effective_network_mode = Mock(return_value="open") + el_nvr_map = {8: "golang-1.20.12-2.el8"} builder_nvrs = await pipeline.get_existing_builders_konflux(el_nvr_map, "1.20.12") @@ -1241,6 +1243,8 @@ async def mock_search_builds(*_args, **_kwargs): "golang-1.25.7-1.el9_5": {("openshift-golang-builder", "v1.25.7", "202602170955.g5015a16.el9")} } + pipeline._get_effective_network_mode = Mock(return_value="open") + el_nvr_map = {9: "golang-1.25.7-1.el9"} builder_nvrs = await pipeline.get_existing_builders_konflux(el_nvr_map, "1.25.7") @@ -1500,5 +1504,208 @@ async def test_rebase_and_build_konflux(self, mock_cmd_assert, mock_konflux_db): self.assertEqual(mock_cmd_assert.call_count, 2) +class TestGetEffectiveNetworkMode(IsolatedAsyncioTestCase): + @patch("pyartcd.pipelines.update_golang.KonfluxDb") + def test_cli_override(self, mock_konflux_db): + mock_runtime = Mock(dry_run=False, working_dir=Path("/tmp/working")) + mock_runtime.new_slack_client.return_value = Mock() + pipeline = UpdateGolangPipeline( + runtime=mock_runtime, + ocp_version="4.16", + cves=None, + force_update_tracker=False, + go_nvrs=["golang-1.22.9-1.el9"], + art_jira="ART-1234", + tag_builds=False, + network_mode="hermetic", + ) + result = pipeline._get_effective_network_mode(9, "1.22.9") + self.assertEqual(result, "hermetic") + + @patch("pyartcd.pipelines.update_golang.get_github_client_for_org") + @patch("pyartcd.pipelines.update_golang.KonfluxDb") + def test_image_config(self, mock_konflux_db, mock_get_github_client): + mock_runtime = Mock(dry_run=False, working_dir=Path("/tmp/working")) + mock_runtime.new_slack_client.return_value = Mock() + + upstream_repo = Mock() + upstream_repo.get_contents.side_effect = lambda path, ref: Mock( + decoded_content={ + "images/openshift-golang-builder.yml": b"konflux:\n network_mode: hermetic\n", + "group.yml": b"konflux:\n network_mode: open\n", + }[path] + ) + mock_get_github_client.return_value.get_repo.return_value = upstream_repo + + pipeline = UpdateGolangPipeline( + runtime=mock_runtime, + ocp_version="4.16", + cves=None, + force_update_tracker=False, + go_nvrs=["golang-1.22.9-1.el9"], + art_jira="ART-1234", + tag_builds=False, + ) + result = pipeline._get_effective_network_mode(9, "1.22.9") + self.assertEqual(result, "hermetic") + + @patch("pyartcd.pipelines.update_golang.get_github_client_for_org") + @patch("pyartcd.pipelines.update_golang.KonfluxDb") + def test_group_config_fallback(self, mock_konflux_db, mock_get_github_client): + mock_runtime = Mock(dry_run=False, working_dir=Path("/tmp/working")) + mock_runtime.new_slack_client.return_value = Mock() + + upstream_repo = Mock() + upstream_repo.get_contents.side_effect = lambda path, ref: Mock( + decoded_content={ + "images/openshift-golang-builder.yml": b"{}\n", + "group.yml": b"konflux:\n network_mode: hermetic\n", + }[path] + ) + mock_get_github_client.return_value.get_repo.return_value = upstream_repo + + pipeline = UpdateGolangPipeline( + runtime=mock_runtime, + ocp_version="4.16", + cves=None, + force_update_tracker=False, + go_nvrs=["golang-1.22.9-1.el9"], + art_jira="ART-1234", + tag_builds=False, + ) + result = pipeline._get_effective_network_mode(9, "1.22.9") + self.assertEqual(result, "hermetic") + + @patch("pyartcd.pipelines.update_golang.get_github_client_for_org") + @patch("pyartcd.pipelines.update_golang.KonfluxDb") + def test_default_open(self, mock_konflux_db, mock_get_github_client): + mock_runtime = Mock(dry_run=False, working_dir=Path("/tmp/working")) + mock_runtime.new_slack_client.return_value = Mock() + + upstream_repo = Mock() + upstream_repo.get_contents.side_effect = lambda path, ref: Mock( + decoded_content={ + "images/openshift-golang-builder.yml": b"{}\n", + "group.yml": b"{}\n", + }[path] + ) + mock_get_github_client.return_value.get_repo.return_value = upstream_repo + + pipeline = UpdateGolangPipeline( + runtime=mock_runtime, + ocp_version="4.16", + cves=None, + force_update_tracker=False, + go_nvrs=["golang-1.22.9-1.el9"], + art_jira="ART-1234", + tag_builds=False, + ) + result = pipeline._get_effective_network_mode(9, "1.22.9") + self.assertEqual(result, "open") + + @patch("pyartcd.pipelines.update_golang.get_github_client_for_org") + @patch("pyartcd.pipelines.update_golang.KonfluxDb") + def test_image_config_overrides_group(self, mock_konflux_db, mock_get_github_client): + mock_runtime = Mock(dry_run=False, working_dir=Path("/tmp/working")) + mock_runtime.new_slack_client.return_value = Mock() + + upstream_repo = Mock() + upstream_repo.get_contents.side_effect = lambda path, ref: Mock( + decoded_content={ + "images/openshift-golang-builder.yml": b"konflux:\n network_mode: open\n", + "group.yml": b"konflux:\n network_mode: hermetic\n", + }[path] + ) + mock_get_github_client.return_value.get_repo.return_value = upstream_repo + + pipeline = UpdateGolangPipeline( + runtime=mock_runtime, + ocp_version="4.16", + cves=None, + force_update_tracker=False, + go_nvrs=["golang-1.22.9-1.el9"], + art_jira="ART-1234", + tag_builds=False, + ) + result = pipeline._get_effective_network_mode(9, "1.22.9") + self.assertEqual(result, "open") + + +class TestGetExistingBuildersKonfluxHermeticFilter(IsolatedAsyncioTestCase): + @patch("pyartcd.pipelines.update_golang.KonfluxDb") + @patch("pyartcd.pipelines.update_golang.elliottutil.get_golang_container_nvrs_for_konflux_record") + async def test_hermetic_filter(self, mock_get_golang_nvrs, mock_konflux_db_class): + mock_runtime = Mock(dry_run=False, working_dir=Path("/tmp/working")) + mock_runtime.new_slack_client.return_value = Mock() + + mock_db_instance = Mock() + mock_konflux_db_class.return_value = mock_db_instance + + pipeline = UpdateGolangPipeline( + runtime=mock_runtime, + ocp_version="4.16", + cves=None, + force_update_tracker=False, + go_nvrs=["golang-1.22.9-1.el9"], + art_jira="ART-1234", + tag_builds=False, + build_system="konflux", + ) + + mock_build_record = Mock(spec=KonfluxBuildRecord) + mock_build_record.nvr = "openshift-golang-builder-v1.22.9-202403212137.el9.g144a3f8" + + async def mock_search_builds(*_args, **_kwargs): + yield mock_build_record + + mock_db_instance.search_builds_by_fields = Mock(side_effect=mock_search_builds) + mock_get_golang_nvrs.return_value = {"golang-1.22.9-1.el9": {("ignored", "ignored", "ignored")}} + + pipeline._get_effective_network_mode = Mock(return_value="hermetic") + + el_nvr_map = {9: "golang-1.22.9-1.el9"} + await pipeline.get_existing_builders_konflux(el_nvr_map, "1.22.9") + + where = mock_db_instance.search_builds_by_fields.call_args.kwargs["where"] + self.assertTrue(where["hermetic"]) + + @patch("pyartcd.pipelines.update_golang.KonfluxDb") + @patch("pyartcd.pipelines.update_golang.elliottutil.get_golang_container_nvrs_for_konflux_record") + async def test_open_filter(self, mock_get_golang_nvrs, mock_konflux_db_class): + mock_runtime = Mock(dry_run=False, working_dir=Path("/tmp/working")) + mock_runtime.new_slack_client.return_value = Mock() + + mock_db_instance = Mock() + mock_konflux_db_class.return_value = mock_db_instance + + pipeline = UpdateGolangPipeline( + runtime=mock_runtime, + ocp_version="4.16", + cves=None, + force_update_tracker=False, + go_nvrs=["golang-1.22.9-1.el9"], + art_jira="ART-1234", + tag_builds=False, + build_system="konflux", + ) + + mock_build_record = Mock(spec=KonfluxBuildRecord) + mock_build_record.nvr = "openshift-golang-builder-v1.22.9-202403212137.el9.g144a3f8" + + async def mock_search_builds(*_args, **_kwargs): + yield mock_build_record + + mock_db_instance.search_builds_by_fields = Mock(side_effect=mock_search_builds) + mock_get_golang_nvrs.return_value = {"golang-1.22.9-1.el9": {("ignored", "ignored", "ignored")}} + + pipeline._get_effective_network_mode = Mock(return_value="open") + + el_nvr_map = {9: "golang-1.22.9-1.el9"} + await pipeline.get_existing_builders_konflux(el_nvr_map, "1.22.9") + + where = mock_db_instance.search_builds_by_fields.call_args.kwargs["where"] + self.assertFalse(where["hermetic"]) + + if __name__ == "__main__": unittest.main() From 2b76493376bf202f65b17c87e33ea6360fac18e4 Mon Sep 17 00:00:00 2001 From: Siddharth Sharma Date: Wed, 10 Jun 2026 12:05:06 -0400 Subject: [PATCH 2/2] Fix test_image tests: mock get_konflux_network_mode instead of using isinstance guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review feedback, the function signature says `str | None` — passing MagicMock is a test bug, not a function bug. Mock `get_konflux_network_mode` in the 4 lockfile tests that use MagicMock configs, matching the pattern already used by the passing tests. Co-Authored-By: Claude Opus 4.6 rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED --- doozer/tests/test_image.py | 20 ++++++++++++++++---- pyartcd/pyartcd/pipelines/update_golang.py | 6 ++++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/doozer/tests/test_image.py b/doozer/tests/test_image.py index 832ce2a6b7..c427fd3a9b 100644 --- a/doozer/tests/test_image.py +++ b/doozer/tests/test_image.py @@ -280,7 +280,10 @@ def test_lockfile_enabled_metadata_override_false(self): metadata.runtime.group_config.konflux.cachi2.lockfile.enabled = True - with patch.object(ImageMetadata, "is_cachi2_enabled", return_value=True): + with ( + patch.object(ImageMetadata, "is_cachi2_enabled", return_value=True), + patch.object(ImageMetadata, "get_konflux_network_mode", return_value="hermetic"), + ): result = metadata.is_lockfile_generation_enabled() self.assertFalse(result) @@ -314,7 +317,10 @@ def test_lockfile_enabled_group_override_false(self): metadata.runtime.group_config.konflux.cachi2.lockfile.enabled = False - with patch.object(ImageMetadata, "is_cachi2_enabled", return_value=True): + with ( + patch.object(ImageMetadata, "is_cachi2_enabled", return_value=True), + patch.object(ImageMetadata, "get_konflux_network_mode", return_value="hermetic"), + ): result = metadata.is_lockfile_generation_enabled() self.assertFalse(result) @@ -347,7 +353,10 @@ def test_lockfile_enabled_cachi2_disabled(self): metadata.runtime.group_config.konflux.cachi2.lockfile.enabled = True - with patch.object(ImageMetadata, "is_cachi2_enabled", return_value=False): + with ( + patch.object(ImageMetadata, "is_cachi2_enabled", return_value=False), + patch.object(ImageMetadata, "get_konflux_network_mode", return_value="hermetic"), + ): result = metadata.is_lockfile_generation_enabled() self.assertFalse(result) @@ -362,7 +371,10 @@ def test_lockfile_enabled_all_missing(self): metadata.runtime.group_config.konflux.cachi2.lockfile.enabled = Missing - with patch.object(ImageMetadata, "is_cachi2_enabled", return_value=Missing): + with ( + patch.object(ImageMetadata, "is_cachi2_enabled", return_value=Missing), + patch.object(ImageMetadata, "get_konflux_network_mode", return_value="hermetic"), + ): result = metadata.is_lockfile_generation_enabled() self.assertFalse(result) diff --git a/pyartcd/pyartcd/pipelines/update_golang.py b/pyartcd/pyartcd/pipelines/update_golang.py index df92357f97..d59c16ec6c 100644 --- a/pyartcd/pyartcd/pipelines/update_golang.py +++ b/pyartcd/pyartcd/pipelines/update_golang.py @@ -995,13 +995,15 @@ def _get_effective_network_mode(self, el_v: int, go_version: str) -> str: try: image_config = self._load_yaml_from_repo(repo, f"images/{image_key}.yml", ref) - except Exception: + except Exception as e: + _LOGGER.warning(f"Failed to load image config for {image_key}: {e}") image_config = {} image_config_network_mode = (image_config.get("konflux") or {}).get("network_mode") try: group_config = self._load_yaml_from_repo(repo, "group.yml", ref) - except Exception: + except Exception as e: + _LOGGER.warning(f"Failed to load group config from {ref}: {e}") group_config = {} group_config_network_mode = (group_config.get("konflux") or {}).get("network_mode")