Skip CSV relatedImages check for universal training images

[sutaakar]

Summary

• Skip CSV spec.relatedImages verification for universal training images (odh-th06-*) in TestRunTrainJobWithDefaultClusterTrainingRuntimes, as these images are no longer listed in the operator CSV
• Non-universal images (odh-training-*) continue to be verified against CSV relatedImages

Test plan

• Ran TestRunTrainJobWithDefaultClusterTrainingRuntimes with fix — all 5 unique runtimes pass
• Ran without fix — fails on first universal image (odh-th06-cuda130-torch210-py312 not in CSV)
• Verified non-universal images (odh-training-cuda128-torch29-py312, odh-training-rocm64-torch29-py312) still checked against CSV

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Improved test validation for container images by conditionally skipping verification for specific universal image types while maintaining existing checks for other images. Enhanced test logging to provide better visibility into validation skips.

[coderabbitai]

📝 Walkthrough

Walkthrough

A strings package import is added to tests/trainer/cluster_training_runtimes_test.go. Inside verifyPodContainerImages, a new conditional branch checks whether a container image URL contains the substring "/odh-th06-". If matched, the test logs that the Product CSV relatedImages check is being skipped and calls continue, bypassing the existing assertion. All other images continue through the original CSV containment check.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

---

CWE-697 / test integrity concern — hardcoded substring bypass.

The string "/odh-th06-" is a hardcoded, opaque identifier used to exempt images from supply-chain provenance verification (csv.Spec.RelatedImages). This pattern:

• Silently widens the trust boundary. Any image whose registry path happens to contain "/odh-th06-" — including a compromised or unintended image — will never be checked against the operator's declared relatedImages.
• No expiration or scope guard. There is no comment, constant, issue link, or bounded condition explaining when this exemption should be removed. Permanent substring-based exemptions in security assertions are a maintenance debt that tends to expand (CWE-1357).
• String matching is fragile. strings.Contains on a full image URL is path-order-dependent and can be trivially satisfied by an attacker-controlled registry subdirectory name.

Direct questions to answer before merging:

1. What is the authoritative definition of a "universal image" and why does "/odh-th06-" encode it?
2. Is there a tracking issue for restoring this assertion once these images are listed in the CSV?
3. Should this be a named constant or a well-documented allow-list rather than an inline strings.Contains?

🚥 Pre-merge checks | ✅ 9 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Contribution Quality And Spam Detection	⚠️ Warning	PR exhibits security theater (CWE-20 claimed in test code processing only K8s-sourced, system-controlled image data) and contribution pattern signals (single-file minimal change, no test coverage,...	Require justification: why is substring matching on orchestration system data a supply-chain concern? Add unit test coverage for skip logic. Consider if this fix addresses real exploitability or just theoretical path-like inspection of t...

✅ Passed checks (9 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly describes the main change: skipping CSV relatedImages validation for universal training images identified by the 'odh-th06-' prefix.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No Hardcoded Secrets	✅ Passed	No hardcoded secrets detected. Changes contain only image name prefix matching ("odh-th06-"), standard library import (strings), and test logging statements.
No Weak Cryptography	✅ Passed	No weak cryptographic primitives (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or non-constant-time secret comparisons detected in the PR changes.
No Injection Vectors	✅ Passed	No injection vectors detected. Code uses strings.Contains() for conditional logic on API-sourced image names. No SQL concatenation, shell execution, eval/exec, unsafe deserialization, or XSS patter...
No Privileged Containers	✅ Passed	PR modifies only a Go test file (tests/trainer/cluster_training_runtimes_test.go), not Kubernetes manifests, Helm templates, or Dockerfiles. No privileged container configurations are present.
No Sensitive Data In Logs	✅ Passed	Container image URLs logged (e.g., "odh-th06-cuda130-torch210-py312@sha256:...") are not sensitive data; they are public deployment artifacts. No passwords, tokens, API keys, PII, session IDs, or c...

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@tests/trainer/cluster_training_runtimes_test.go`:
- Around line 301-305: The substring matching in the condition using
strings.Contains(image, "/odh-th06-") is overly broad and could allow an
attacker-controlled path containing the substring /odh-th06- anywhere in the
image URL to bypass the CSV relatedImages validation. Replace the substring
matching with a more precise check that validates the actual image name prefix
(the portion after the last slash in the image URL) rather than matching an
arbitrary substring anywhere in the full URL path. This ensures only legitimate
universal images with the th06 designation skip the validation check.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 2084d34e-3889-4e37-b31b-5e26376e1386

📥 Commits

Reviewing files that changed from the base of the PR and between 22a0545 and 6c04ab6.

📒 Files selected for processing (1)

• tests/trainer/cluster_training_runtimes_test.go

[ChughShilpa]

/lgtm

[sutaakar]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ChughShilpa, sutaakar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [sutaakar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment