Skip to content

Filter Konflux golang-builder search by hermetic flag #3048

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
thegreyd wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Filter Konflux golang-builder search by hermetic flag #3048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
98eb8e2
Filter Konflux golang-builder search by hermetic flag
thegreyd Jun 10, 2026
2b76493
Fix test_image tests: mock get_konflux_network_mode instead of using …
thegreyd Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions artcommon/artcommonlib/konflux/konflux_db.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
32 changes: 22 additions & 10 deletions artcommon/artcommonlib/metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -617,18 +638,9 @@ def run_async():

def get_konflux_network_mode(self) -> str:
runtime_override = getattr(self.runtime, "network_mode_override", None)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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:
Expand Down
20 changes: 16 additions & 4 deletions doozer/tests/test_image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand Down
39 changes: 39 additions & 0 deletions pyartcd/pyartcd/pipelines/update_golang.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand Down Expand Up @@ -972,6 +979,38 @@ 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 as e:
_LOGGER.warning(f"Failed to load image config for {image_key}: {e}")
image_config = {}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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 as e:
_LOGGER.warning(f"Failed to load group config from {ref}: {e}")
group_config = {}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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}")
Comment on lines +996 to +1011

@coderabbitai coderabbitai Bot Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Failing to read build-data currently fails open to non-hermetic matching.

At Line 1004 and Line 997, exceptions are converted to {}, which can make resolve_network_mode(...) fall back to "open" and query hermetic=False. That can incorrectly reuse a non-hermetic builder when hermetic config exists but could not be fetched.

🔧 Suggested fix 
-        try:
-            image_config = self._load_yaml_from_repo(repo, f"images/{image_key}.yml", ref)
-        except Exception as e:
-            _LOGGER.warning(f"Failed to load image config for {image_key}: {e}")
-            image_config = {}
+        try:
+            image_config = self._load_yaml_from_repo(repo, f"images/{image_key}.yml", ref)
+        except Exception as e:
+            _LOGGER.exception("Failed to load image config for %s at ref %s", image_key, ref)
+            raise RuntimeError(f"Unable to resolve network mode from images/{image_key}.yml at {ref}") from e
@@
-        try:
-            group_config = self._load_yaml_from_repo(repo, "group.yml", ref)
-        except Exception as e:
-            _LOGGER.warning(f"Failed to load group config from {ref}: {e}")
-            group_config = {}
+        try:
+            group_config = self._load_yaml_from_repo(repo, "group.yml", ref)
+        except Exception as e:
+            _LOGGER.exception("Failed to load group config at ref %s", ref)
+            raise RuntimeError(f"Unable to resolve network mode from group.yml at {ref}") from e
🧰 Tools
🪛 Ruff (0.15.15)

[warning] 998-998: Do not catch blind exception: Exception

(BLE001)

[warning] 1005-1005: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyartcd/pyartcd/pipelines/update_golang.py` around lines 996 - 1011, When
loading image/group YAML fails in _load_yaml_from_repo, don't swallow the
exception into {} because that lets
resolve_network_mode(image_config_network_mode, group_config_network_mode) fall
back to "open"; instead after logging the warning re-raise the exception (or
raise a new descriptive exception) so the pipeline fails closed and avoid
matching a non-hermetic builder; update the two except blocks around
_load_yaml_from_repo (the ones that set image_config = {} and group_config = {})
to log and then raise, so resolve_network_mode receives only valid config values
or the pipeline stops.

return network_mode

def verify_golang_builder_repo(self, el_v, go_version):
if self.use_new_golang_branch:
branch = self.GOLANG_DATA_BRANCH
Expand Down
207 changes: 207 additions & 0 deletions pyartcd/tests/pipelines/test_update_golang.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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()
Loading