Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 6 additions & 16 deletions doozer/doozerlib/backend/konflux_image_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ class KonfluxImageBuilderConfig:
skip_tasks: tuple[str, ...] = ()
dry_run: bool = False
build_priority: Optional[str] = None
ec_policy_configuration: str = constants.KONFLUX_DEFAULT_EC_POLICY_CONFIGURATION
prega_ec_policy_configuration: str = constants.KONFLUX_PREGA_EC_POLICY_CONFIGURATION
ec_policy_configuration: Optional[str] = None
prega_ec_policy_configuration: Optional[str] = None
skip_ec_verify: bool = False


Expand Down Expand Up @@ -287,9 +287,7 @@ async def build(self, metadata: ImageMetadata, git_auth_secret: Optional[str] =
record["image_tag"] = image_tag

# Validate SLSA attestation and source image signature
# Skip for non-OCP groups (e.g., OKD) as they may not have attestations/signatures
is_ocp_group = self._config.group_name.startswith("openshift-")
if is_ocp_group:
if metadata.runtime.variant is not BuildVariant.OKD:
try:
# use image_digest here to be precise, image_pullspec can collide in case of golang-builder images
await self._validate_build_attestation_and_signature(
Expand All @@ -300,24 +298,16 @@ async def build(self, metadata: ImageMetadata, git_auth_secret: Optional[str] =
f"Failed to get SLA attestation / source signature from konflux for image {definitive_image_pullspec}, marking build as {KonfluxBuildOutcome.BUILD_ERROR}. Error: {e}"
)
outcome = KonfluxBuildOutcome.BUILD_ERROR
else:
logger.info(
"Skipping SLSA attestation validation for %s: non-OCP group '%s'",
metadata.distgit_key,
self._config.group_name,
)
elif not self._config.dry_run:
# This should never happen in real builds - only expected in dry-run mode
raise IOError("PipelineRun succeeded but IMAGE_URL or IMAGE_DIGEST missing from results")

# Run enterprise-contract (EC) verification after a successful build
# TODO: Expand EC verification to layered products
# TODO: Expose EC failure links (ITS/PLR URLs) via Slack notification or dashboard column
is_ocp_group = self._config.group_name.startswith("openshift-")
should_run_ec = (
outcome is KonfluxBuildOutcome.SUCCESS
and metadata.runtime.variant is not BuildVariant.OKD
and is_ocp_group
and self._config.ec_policy_configuration is not None
Comment on lines 307 to +310

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Gate on the phase-specific EC policy.

The predicate checks only ec_policy_configuration, but Lines 327-329 select the PREGA policy for pre-release builds. This causes both incorrect outcomes: a standard-only configuration can call EC verification with ec_policy=None, while a PREGA-only configuration is skipped despite having a usable policy. Resolve the policy before gating and use that same value for the skip log.

Proposed direction
-                    and self._config.ec_policy_configuration is not None
+                    and ec_policy is not None
...
-                    elif self._config.ec_policy_configuration is None:
+                    elif ec_policy is None:

Ensure ec_policy is resolved from the lifecycle phase before evaluating should_run_ec.

Also applies to: 361-363

🤖 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 `@doozer/doozerlib/backend/konflux_image_builder.py` around lines 307 - 310,
Update the EC verification flow around the should_run_ec predicate to resolve
the phase-specific ec_policy first, selecting the PREGA policy for pre-release
builds and the standard policy otherwise. Gate should_run_ec on that resolved
policy being present, and reuse the same ec_policy value in the skip log and
verification call. Apply the equivalent change to the second affected flow.

and not self._config.skip_ec_verify
and metadata.for_release
and image_pullspec # EC requires actual build results
Expand Down Expand Up @@ -368,9 +358,9 @@ async def build(self, metadata: ImageMetadata, git_auth_secret: Optional[str] =
logger.info(
"Skipping EC verification for %s: --skip-ec-verify flag is set", metadata.distgit_key
)
elif not is_ocp_group:
elif self._config.ec_policy_configuration is None:
logger.info(
"Skipping EC verification for %s: non-OCP group '%s'",
"Skipping EC verification for %s: no EC policy configured for group '%s'",
metadata.distgit_key,
self._config.group_name,
)
Expand Down
9 changes: 5 additions & 4 deletions doozer/doozerlib/cli/images_konflux.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,12 +306,13 @@ async def run(self):
else:
group = runtime.group

product = runtime.product
if runtime.assembly == "test":
ec_policy = constants.KONFLUX_TEST_EC_POLICY_CONFIGURATION
prega_ec_policy = constants.KONFLUX_TEST_PREGA_EC_POLICY_CONFIGURATION
ec_policy = constants.PRODUCT_TEST_EC_POLICY_MAP.get(product)
prega_ec_policy = constants.PRODUCT_TEST_PREGA_EC_POLICY_MAP.get(product, ec_policy)
else:
ec_policy = constants.KONFLUX_DEFAULT_EC_POLICY_CONFIGURATION
prega_ec_policy = constants.KONFLUX_PREGA_EC_POLICY_CONFIGURATION
ec_policy = constants.PRODUCT_EC_POLICY_MAP.get(product)
prega_ec_policy = constants.PRODUCT_PREGA_EC_POLICY_MAP.get(product, ec_policy)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

config = KonfluxImageBuilderConfig(
base_dir=Path(runtime.working_dir, constants.WORKING_SUBDIR_KONFLUX_BUILD_SOURCES),
Expand Down
31 changes: 29 additions & 2 deletions doozer/doozerlib/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@
ART_BUILD_FAILURES_URL = 'https://art-build-failures-art-build-failures.apps.artc2023.pc3z.p1.openshiftapps.com'

# Enterprise Contract (EC) verification pipeline constants
# TODO: Expand EC verification to layered products (logging, oadp, mta, rhmtc, quay, cert-manager, etc.)
# Currently scoped to OCP only.
KONFLUX_EC_PIPELINE_GIT_URL = "https://github.com/konflux-ci/build-definitions"
KONFLUX_EC_PIPELINE_REVISION = "main"
KONFLUX_EC_PIPELINE_PATH = "pipelines/enterprise-contract.yaml"
Expand All @@ -80,6 +78,35 @@
# https://gitlab.cee.redhat.com/releng/konflux-release-data/-/merge_requests/19219
KONFLUX_TEST_EC_POLICY_CONFIGURATION = "ocp-art-tenant/conforma-build-stage-test"
KONFLUX_TEST_PREGA_EC_POLICY_CONFIGURATION = "ocp-art-tenant/conforma-build-ec-stage-test"

# Product-to-EC-policy mappings for build-time ITS verification.
# Maps product name -> "namespace/policy-name" for each assembly type.
PRODUCT_EC_POLICY_MAP = {
"logging": "art-logging-tenant/conforma-build-stage",
"mta": "art-mta-tenant/conforma-build-stage",
"oadp": "art-oadp-tenant/conforma-build-stage",
"ocp": "ocp-art-tenant/conforma-build-stage",
"openshift-logging": "art-logging-tenant/conforma-build-stage",
"rhmtc": "art-mtc-tenant/conforma-build-stage",
}

PRODUCT_TEST_EC_POLICY_MAP = {
"logging": "art-logging-tenant/conforma-build-stage-test",
"mta": "art-mta-tenant/conforma-build-stage-test",
"oadp": "art-oadp-tenant/conforma-build-stage-test",
"ocp": "ocp-art-tenant/conforma-build-stage-test",
"openshift-logging": "art-logging-tenant/conforma-build-stage-test",
"rhmtc": "art-mtc-tenant/conforma-build-stage-test",
}

PRODUCT_PREGA_EC_POLICY_MAP = {
"ocp": "ocp-art-tenant/conforma-build-ec-stage",
}

PRODUCT_TEST_PREGA_EC_POLICY_MAP = {
"ocp": "ocp-art-tenant/conforma-build-ec-stage-test",
}

# Base image release EC policies (ReleasePlanAdmission policy name suffix). Selection is by
# software_lifecycle.phase in resolve_konflux_base_image_release_targets — ART-19498.
# Prod: registry-ocp-art-base-prod | Pre-release: registry-ocp-art-base-ec-prod
Expand Down
10 changes: 7 additions & 3 deletions doozer/tests/backend/test_konflux_ec_verification.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ def _make_config(**overrides) -> KonfluxImageBuilderConfig:
dry_run=True,
build_priority="5",
skip_ec_verify=False,
ec_policy_configuration=constants.KONFLUX_DEFAULT_EC_POLICY_CONFIGURATION,
prega_ec_policy_configuration=constants.KONFLUX_PREGA_EC_POLICY_CONFIGURATION,
)
defaults.update(overrides)
return KonfluxImageBuilderConfig(**defaults)
Expand Down Expand Up @@ -177,9 +179,11 @@ async def test_ec_skipped_when_skip_flag_set(self, mock_kc_init):
verify_ec = await self._run_build_and_get_ec_calls(config, metadata, mock_kc_init)
verify_ec.assert_not_called()

async def test_ec_skipped_for_non_ocp_group(self, mock_kc_init):
"""EC verification should be skipped for non-OCP groups (e.g. logging)."""
config = _make_config(group_name="logging-6.2")
async def test_ec_skipped_when_no_policy_configured(self, mock_kc_init):
"""EC verification should be skipped when no EC policy is configured for the product."""
config = _make_config(
group_name="logging-6.2", ec_policy_configuration=None, prega_ec_policy_configuration=None
)
metadata = _make_metadata(for_release=True)

verify_ec = await self._run_build_and_get_ec_calls(config, metadata, mock_kc_init)
Expand Down
11 changes: 8 additions & 3 deletions doozer/tests/backend/test_konflux_image_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from unittest.mock import AsyncMock, MagicMock, patch

from artcommonlib.konflux.konflux_build_record import KonfluxBuildOutcome
from artcommonlib.variants import BuildVariant
from doozerlib.backend.konflux_image_builder import (
KonfluxImageBuilder,
KonfluxImageBuilderConfig,
Expand All @@ -27,13 +28,17 @@ def setUp(self):
self.mock_konflux_client_factory.return_value = self.mock_konflux_client
self.mock_konflux_client.resource_url.return_value = "https://example.com/pipelinerun"

from doozerlib import constants as doozer_constants

self.builder = KonfluxImageBuilder(
KonfluxImageBuilderConfig(
base_dir=Path(self.temp_dir.name),
group_name="openshift-4.17",
namespace="test-namespace",
plr_template="test-template",
build_priority="5",
ec_policy_configuration=doozer_constants.KONFLUX_DEFAULT_EC_POLICY_CONFIGURATION,
prega_ec_policy_configuration=doozer_constants.KONFLUX_PREGA_EC_POLICY_CONFIGURATION,
)
)

Expand Down Expand Up @@ -115,9 +120,8 @@ async def test_build_uses_definitive_pullspec_for_attestation_validation(self):

mock_validate.assert_awaited_once_with("quay.io/test/image@sha256:testdigest", "test-image")

async def test_build_skips_slsa_validation_for_non_ocp_groups(self):
"""Test that SLSA attestation validation is skipped for non-OCP groups like OKD."""
# Create a builder with an OKD group name
async def test_build_skips_slsa_validation_for_okd_variant(self):
"""Test that SLSA attestation validation is skipped for OKD variant."""
okd_builder = KonfluxImageBuilder(
KonfluxImageBuilderConfig(
base_dir=Path(self.temp_dir.name),
Expand All @@ -129,6 +133,7 @@ async def test_build_skips_slsa_validation_for_non_ocp_groups(self):
)

metadata = self._metadata()
metadata.runtime.variant = BuildVariant.OKD
dest_dir = okd_builder._config.base_dir.joinpath(metadata.qualified_key)
dest_dir.mkdir(parents=True)

Expand Down
9 changes: 9 additions & 0 deletions pyartcd/pyartcd/pipelines/build_layered_products.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def __init__(
data_gitref: Optional[str] = None,
network_mode: Optional[str] = None,
plr_template: Optional[str] = None,
skip_ec_verify: bool = False,
logger: Optional[logging.Logger] = None,
):
self.runtime = runtime
Expand All @@ -54,6 +55,7 @@ def __init__(
self.data_gitref = data_gitref
self.network_mode = network_mode
self.plr_template = plr_template
self.skip_ec_verify = skip_ec_verify
self._logger = logger or runtime.logger
# Operator NVRs that were dropped from the bundle build trigger because they are embargoed.
# The caller uses this to mark the Jenkins job UNSTABLE instead of a plain SUCCESS.
Expand Down Expand Up @@ -357,6 +359,8 @@ async def _build(
build_cmd.extend(['--plr-template', plr_template_url])
if self.network_mode:
build_cmd.extend(['--network-mode', self.network_mode])
if self.skip_ec_verify:
build_cmd.append('--skip-ec-verify')
if self.runtime.dry_run:
build_cmd.append("--dry-run")

Expand Down Expand Up @@ -423,6 +427,9 @@ async def _build(
type=click.Choice(['hermetic', 'internal-only', 'open']),
help='Override network mode for Konflux builds. Takes precedence over image and group config settings.',
)
@click.option(
"--skip-ec-verify", is_flag=True, default=False, help="Skip Enterprise Contract verification for built images"
)
@click.option("--ignore-locks", is_flag=True, default=False, help="(For testing) Do not wait for locks")
@click.option(
'--plr-template',
Expand All @@ -445,6 +452,7 @@ async def build_layered_products(
kubeconfig: Optional[str],
data_gitref: Optional[str],
network_mode: Optional[str],
skip_ec_verify: bool,
ignore_locks: bool,
plr_template: str,
):
Expand All @@ -464,6 +472,7 @@ async def build_layered_products(
data_gitref=data_gitref,
network_mode=network_mode,
plr_template=plr_template,
skip_ec_verify=skip_ec_verify,
)

lock_identifier = jenkins.get_build_path_or_random()
Expand Down