Skip to content

feat: New tests for image signing and split infra tests to smaller ones #1222

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

Merged
dbasunag merged 3 commits into from
Mar 16, 2026
Merged

feat: New tests for image signing and split infra tests to smaller ones #1222

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
157ee1f
feat: New tests for image signing and split infra tests to smaller ones
dbasunag Mar 15, 2026
9434a22
Merge branch 'main' into signing_image_tests
dbasunag Mar 16, 2026
0257962
Merge branch 'main' into signing_image_tests
dbasunag Mar 16, 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
6 changes: 5 additions & 1 deletion Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ ENV PATH="$PATH:$BIN_DIR"

# Install system dependencies using dnf
RUN dnf update -y \
&& dnf install -y python3 python3-pip ssh gnupg curl gpg wget vim httpd-tools rsync openssl openssl-devel\
&& dnf install -y python3 python3-pip ssh gnupg curl gpg wget vim httpd-tools rsync openssl openssl-devel skopeo\
&& dnf clean all \
&& rm -rf /var/cache/dnf

Expand All @@ -22,6 +22,10 @@ RUN curl -sSL "https://github.com/fullstorydev/grpcurl/releases/download/v1.9.2/
&& tar xvf /tmp/grpcurl_1.2.tar.gz --no-same-owner \
&& mv grpcurl /usr/bin/grpcurl

# Install cosign
RUN curl -sSL "https://github.com/sigstore/cosign/releases/download/v2.4.2/cosign-linux-amd64" --output /usr/bin/cosign \
Copy link
Copy Markdown
Member

@jonburdo jonburdo Mar 16, 2026

Choose a reason for hiding this comment

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

Actually, sorry - I just realized - this should be rh cosign - not upstream sigstore

&& chmod +x /usr/bin/cosign
Comment on lines +25 to +27
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Verify the downloaded cosign binary before installing it (CWE-494).

High severity. This curl -sSL writes whatever the remote endpoint returns straight into /usr/bin/cosign, and the build never verifies a checksum or even fails on HTTP 4xx/5xx. A compromised release asset, proxy, or transient GitHub error can bake arbitrary content into the image that CI later executes.

🔧 Remediation 
+# Pin and verify the release asset before installing it.
+ARG COSIGN_VERSION=2.4.2
+ARG COSIGN_SHA256=<published-sha256>
-# Install cosign
-RUN curl -sSL "https://github.com/sigstore/cosign/releases/download/v2.4.2/cosign-linux-amd64" --output /usr/bin/cosign \
-    && chmod +x /usr/bin/cosign
+# Install cosign
+RUN curl -fsSLo /tmp/cosign "https://github.com/sigstore/cosign/releases/download/v${COSIGN_VERSION}/cosign-linux-amd64" \
+    && echo "${COSIGN_SHA256}  /tmp/cosign" | sha256sum -c - \
+    && install -m 0755 /tmp/cosign /usr/bin/cosign \
+    && rm -f /tmp/cosign

As per coding guidelines, **: REVIEW PRIORITIES: 1. Security vulnerabilities (provide severity, exploit scenario, and remediation code)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 25 - 27, The Dockerfile RUN that downloads the
cosign binary currently writes remote content directly to /usr/bin/cosign
without verification; change the RUN that fetches the cosign release (the curl
line that writes /usr/bin/cosign for v2.4.2) to: 1) use curl --fail/-f so HTTP
errors fail the build, 2) download the corresponding checksum or signature
artifact (SHA256SUMS or cosign-linux-amd64.sum) from the same release, 3) verify
the downloaded binary using shasum/sha256sum (or verify the signature with gpg
if a signed checksum is provided), and 4) only install (move/chmod) the binary
into /usr/bin/cosign after checksum/signature verification succeeds; ensure the
verification step references the same release URL and aborts the build on
mismatch.


RUN useradd -ms /bin/bash $USER
USER $USER
WORKDIR $HOME_DIR
Expand Down
180 changes: 167 additions & 13 deletions tests/model_registry/model_registry/python_client/signing/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,21 @@
from ocp_resources.config_map import ConfigMap
from ocp_resources.deployment import Deployment
from ocp_resources.namespace import Namespace
from ocp_resources.pod import Pod
from ocp_resources.service import Service
from ocp_resources.subscription import Subscription
from ocp_utilities.operators import install_operator, uninstall_operator
from pyhelper_utils.shell import run_command
from pytest_testconfig import config as py_config
from simple_logger.logger import get_logger
from timeout_sampler import TimeoutSampler

from tests.model_registry.model_registry.python_client.signing.constants import (
SECURESIGN_API_VERSION,
SECURESIGN_NAME,
SECURESIGN_NAMESPACE,
SIGNING_OCI_REPO_NAME,
SIGNING_OCI_TAG,
TAS_CONNECTION_TYPE_NAME,
)
from tests.model_registry.model_registry.python_client.signing.utils import (
Expand All @@ -33,8 +39,9 @@
get_root_checksum,
get_tas_service_urls,
)
from utilities.constants import OPENSHIFT_OPERATORS, Timeout
from utilities.constants import OPENSHIFT_OPERATORS, Labels, ModelCarImage, OCIRegistry, Timeout
from utilities.infra import get_openshift_token, is_managed_cluster
from utilities.resources.route import Route
from utilities.resources.securesign import Securesign

LOGGER = get_logger(name=__name__)
Expand Down Expand Up @@ -314,7 +321,89 @@ def tas_connection_type(admin_client: DynamicClient, securesign_instance: Secure
LOGGER.info(f"TAS Connection Type '{TAS_CONNECTION_TYPE_NAME}' deleted from namespace '{app_namespace}'")


@pytest.fixture(scope="package")
@pytest.fixture(scope="class")
def oci_registry_pod(
admin_client: DynamicClient,
oci_namespace: Namespace,
) -> Generator[Pod, Any]:
"""Create a simple OCI registry (Zot) pod with local emptyDir storage.

Unlike oci_registry_pod_with_minio, this does not require MinIO — data is
stored in an emptyDir volume, which is sufficient for signing test scenarios.

Args:
admin_client: Kubernetes dynamic client
oci_namespace: Namespace for the OCI registry pod

Yields:
Pod: Ready OCI registry pod
"""
with Pod(
client=admin_client,
name=OCIRegistry.Metadata.NAME,
namespace=oci_namespace.name,
containers=[
{
"env": [
{"name": "ZOT_HTTP_ADDRESS", "value": OCIRegistry.Metadata.DEFAULT_HTTP_ADDRESS},
{"name": "ZOT_HTTP_PORT", "value": str(OCIRegistry.Metadata.DEFAULT_PORT)},
{"name": "ZOT_LOG_LEVEL", "value": "info"},
],
"image": OCIRegistry.PodConfig.REGISTRY_IMAGE,
"name": OCIRegistry.Metadata.NAME,
"securityContext": {
"allowPrivilegeEscalation": False,
"capabilities": {"drop": ["ALL"]},
"runAsNonRoot": True,
"seccompProfile": {"type": "RuntimeDefault"},
},
"volumeMounts": [
{
"name": "zot-data",
"mountPath": "/var/lib/registry",
}
],
}
],
volumes=[
{
"name": "zot-data",
"emptyDir": {},
}
],
label={
Labels.Openshift.APP: OCIRegistry.Metadata.NAME,
"maistra.io/expose-route": "true",
},
) as oci_pod:
oci_pod.wait_for_condition(condition="Ready", status="True")
yield oci_pod


@pytest.fixture(scope="class")
def ai_hub_oci_registry_route(admin_client: DynamicClient, oci_registry_service: Service) -> Generator[Route, Any]:
"""Override the default Route with edge TLS termination.

Cosign requires HTTPS. Edge termination lets the OpenShift router handle TLS
and forward plain HTTP to the Zot backend.
"""
with Route(
client=admin_client,
name=OCIRegistry.Metadata.NAME,
namespace=oci_registry_service.namespace,
to={"kind": "Service", "name": oci_registry_service.name},
tls={"termination": "edge", "insecureEdgeTerminationPolicy": "Redirect"},
) as oci_route:
yield oci_route


@pytest.fixture(scope="class")
def ai_hub_oci_registry_host(ai_hub_oci_registry_route: Route) -> str:
"""Get the OCI registry host from the route."""
return ai_hub_oci_registry_route.instance.spec.host
Comment on lines +383 to +403
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Wait for the Route to be admitted before returning its host.

ai_hub_oci_registry_host reads instance.spec.host immediately after creating the Route. That value is assigned asynchronously by OpenShift, so the next fixture can intermittently build https://None/v2/ or start probing a host that is not admitted yet.

As per coding guidelines, **: REVIEW PRIORITIES: 3. Bug-prone patterns and error handling gaps

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/model_registry/model_registry/python_client/signing/conftest.py` around
lines 383 - 403, The ai_hub_oci_registry_host fixture returns
route.instance.spec.host immediately but OpenShift assigns the host
asynchronously; change ai_hub_oci_registry_host to wait until the Route is
admitted and the host is populated before returning. Poll the Route resource
created by ai_hub_oci_registry_route (use the Route instance object returned by
the Route context manager) and wait for instance.status.ingress[*].conditions
(or equivalent) to show an "Admitted" condition with status "True" and for
instance.spec.host to be non-empty, with a short timeout and retry/backoff;
return the host only after these checks pass so downstream tests don't get None
or unadmitted hosts.



@pytest.fixture(scope="class")
def downloaded_model_dir() -> Path:
"""Download a test model from Hugging Face to a temporary directory.

Expand All @@ -334,7 +423,7 @@ def downloaded_model_dir() -> Path:
return model_dir


@pytest.fixture(scope="package")
@pytest.fixture(scope="class")
def set_environment_variables(securesign_instance: Securesign) -> Generator[None, Any]:
"""
Create a service account token and save it to a temporary directory.
Expand Down Expand Up @@ -403,6 +492,81 @@ def signer(set_environment_variables) -> Signer:
return signer


@pytest.fixture(scope="class")
def copied_model_to_oci_registry(
oci_registry_pod: Pod,
ai_hub_oci_registry_host: str,
) -> Generator[str, Any]:
"""Copy ModelCarImage.MNIST_8_1 from quay.io to the local OCI registry using skopeo.

Sets COSIGN_ALLOW_INSECURE_REGISTRY so cosign can access the registry over
the edge-terminated Route with a self-signed certificate.

Args:
oci_registry_pod: OCI registry pod fixture ensuring registry is running
ai_hub_oci_registry_host: OCI registry hostname from route

Yields:
str: The destination image reference with digest (e.g. "{host}/{repo}@sha256:...")
"""
# Wait for the OCI registry to be reachable via the Route
registry_url = f"https://{ai_hub_oci_registry_host}"
LOGGER.info(f"Waiting for OCI registry to be reachable at {registry_url}/v2/")
for sample in TimeoutSampler(
wait_timeout=120,
sleep=5,
func=requests.get,
url=f"{registry_url}/v2/",
timeout=5,
verify=False,
):
if sample.ok:
LOGGER.info("OCI registry is reachable")
break

source_image = ModelCarImage.MNIST_8_1.removeprefix("oci://")
dest_ref = f"{ai_hub_oci_registry_host}/{SIGNING_OCI_REPO_NAME}:{SIGNING_OCI_TAG}"

LOGGER.info(f"Copying image from docker://{source_image} to docker://{dest_ref}")
run_command(
command=[
"skopeo",
"copy",
"--dest-tls-verify=false",
f"docker://{source_image}",
f"docker://{dest_ref}",
],
check=True,
)
LOGGER.info(f"Image copied successfully to {dest_ref}")

# Get the digest of the pushed image
_, inspect_out, _ = run_command(
command=[
"skopeo",
"inspect",
"--tls-verify=false",
f"docker://{dest_ref}",
],
check=True,
)
digest = json.loads(inspect_out).get("Digest", "")
LOGGER.info(f"Pushed image {inspect_out} digest: {digest}")

dest_with_digest = f"{ai_hub_oci_registry_host}/{SIGNING_OCI_REPO_NAME}@{digest}"
LOGGER.info(f"Full image reference: {dest_with_digest}")

# Set cosign env var to allow insecure registry access (self-signed cert from edge Route)
os.environ["COSIGN_ALLOW_INSECURE_REGISTRY"] = "true"
LOGGER.info("Set COSIGN_ALLOW_INSECURE_REGISTRY=true")

yield dest_with_digest

# Cleanup
os.environ.pop("COSIGN_ALLOW_INSECURE_REGISTRY", None)
LOGGER.info("Cleaned up COSIGN_ALLOW_INSECURE_REGISTRY")


@pytest.fixture(scope="function")
def signed_model(signer, downloaded_model_dir) -> Path:
"""
Expand All @@ -413,13 +577,3 @@ def signed_model(signer, downloaded_model_dir) -> Path:
LOGGER.info("Model signed successfully")

return downloaded_model_dir


@pytest.fixture(scope="function")
def verified_model(signer, downloaded_model_dir) -> None:
"""
Verify a signed model.
"""
LOGGER.info(f"Verifying signed model in directory: {downloaded_model_dir}")
signer.verify_model(model_path=str(downloaded_model_dir))
LOGGER.info("Model verified successfully")
4 changes: 4 additions & 0 deletions tests/model_registry/model_registry/python_client/signing/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,7 @@

# TAS Connection Type ConfigMap name
TAS_CONNECTION_TYPE_NAME = "tas-securesign-v1"

# OCI Registry configuration for signed model storage
SIGNING_OCI_REPO_NAME = "signing-test/signed-model"
SIGNING_OCI_TAG = "latest"
Loading