Add release validation tests for image-references#2165
Add release validation tests for image-references#2165Joeavaikath wants to merge 7 commits intoopenshift:oadp-devfrom
Conversation
Validates that bundle/image-references entries match CSV RELATED_IMAGE_* env vars and use the correct release version tag. Skips gracefully on non-release branches. Supports an exceptions list for images with independent versioning. Signed-off-by: Joseph <jvaikath@redhat.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds YAML-backed image-references validators and tests, adds RELATED_IMAGE_KUBEVIRT_DATAMOVER_PLUGIN to CSV and manager deployment, and moves gopkg.in/yaml.v3 to a direct go.mod requirement. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 8 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Joeavaikath The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/cherry-pick oadp-1.6 |
|
@Joeavaikath: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/retest |
|
on the oadp-1.6 branch |
Signed-off-by: Joseph <jvaikath@redhat.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bundle/manifests/oadp-operator.clusterserviceversion.yaml`:
- Line 1706: There are two spec.relatedImages entries for the same name
"openshift-adp-controller-manager-kubevirt-velero-plugin" with different tags;
remove the duplicate so each spec.relatedImages.name maps to exactly one image.
Locate both entries named
"openshift-adp-controller-manager-kubevirt-velero-plugin" and keep the canonical
image tag (choose and retain the intended tag, e.g., ":oadp-1.6" or ":latest"),
delete the other entry, and ensure no other relatedImages contain the same name
to avoid ambiguous release metadata resolution.
- Around line 1483-1484: The CSV currently defines the environment variable
RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN twice; remove the duplicate entry so there
is a single env var declaration for RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN in the
container/env list, keeping the branch-appropriate image tag (the intended value
from the original declaration) and delete the redundant block (the second
occurrence shown in the diff) so deployment behavior is deterministic.
In `@config/manager/manager.yaml`:
- Around line 105-106: Remove the duplicate RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN
environment variable entry and ensure only a single declaration remains; locate
the second occurrence of RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN in the
manager.yaml env list, delete that duplicate entry (or consolidate if the value
differs), and keep the authoritative value in the original
RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN declaration so the operator resolves a
single, deterministic plugin tag at runtime.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d8a70d7-8fe8-425d-b60e-dfe65e25be28
📒 Files selected for processing (3)
bundle/manifests/oadp-operator.clusterserviceversion.yamlconfig/manager/manager.yamlgo.mod
| - name: RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN | ||
| value: quay.io/konveyor/kubevirt-velero-plugin:oadp-1.6 |
There was a problem hiding this comment.
Remove duplicate RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN definition.
Line 105 introduces a second entry for the same env var already declared earlier (Line 83). This makes the effective value order-dependent and can cause the operator to resolve the wrong plugin tag at runtime.
Proposed fix
@@
- - name: RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN
- value: quay.io/konveyor/kubevirt-velero-plugin:latest
+ - name: RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN
+ value: quay.io/konveyor/kubevirt-velero-plugin:oadp-1.6
@@
- - name: RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN
- value: quay.io/konveyor/kubevirt-velero-plugin:oadp-1.6🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/manager/manager.yaml` around lines 105 - 106, Remove the duplicate
RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN environment variable entry and ensure only
a single declaration remains; locate the second occurrence of
RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN in the manager.yaml env list, delete that
duplicate entry (or consolidate if the value differs), and keep the
authoritative value in the original RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN
declaration so the operator resolves a single, deterministic plugin tag at
runtime.
Signed-off-by: Joseph <jvaikath@redhat.com>
Signed-off-by: Joseph <jvaikath@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/release/image_references_test.go (1)
23-37: Optional: extract shared bundle-loading setup into a tiny helper.Line 24–27 and Line 32–35 duplicate the same setup; a small helper would reduce repetition in these top-level integration tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/release/image_references_test.go` around lines 23 - 37, Both tests TestImageReferencesMatchCSV and TestCSVMatchImageReferences duplicate bundle-loading setup (repoRoot, readBundleFile, imageRefsRelPath, csvRelPath); extract that into a small helper function (e.g., loadImageAndCSV or setupBundleData) that returns irData and csvData and call that helper from both tests, then pass the returned values into reportErrors(ValidateImageReferencesMatchCSV(...)) and reportErrors(ValidateCSVMatchImageReferences(...)) to remove the repeated lines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/release/helpers_test.go`:
- Around line 25-39: The branchVersion function currently treats any branch with
prefix oadpBranchPrefix (e.g., "oadp-dev") as a release branch and fatals on git
errors; update branch detection to only accept release-style names (for example
match a stricter pattern like start with oadp- followed by digits/semantic parts
or explicitly exclude "oadp-dev") when checking both the PULL_BASE_REF env var
and the git HEAD result, and change the git lookup error handling in
branchVersion (replace t.Fatalf) to return an empty string so tests gracefully
skip instead of failing; locate and modify the branchVersion function and its
uses of oadpBranchPrefix, the env var check, the branch variable check, and the
exec.Command error path accordingly.
---
Nitpick comments:
In `@tests/release/image_references_test.go`:
- Around line 23-37: Both tests TestImageReferencesMatchCSV and
TestCSVMatchImageReferences duplicate bundle-loading setup (repoRoot,
readBundleFile, imageRefsRelPath, csvRelPath); extract that into a small helper
function (e.g., loadImageAndCSV or setupBundleData) that returns irData and
csvData and call that helper from both tests, then pass the returned values into
reportErrors(ValidateImageReferencesMatchCSV(...)) and
reportErrors(ValidateCSVMatchImageReferences(...)) to remove the repeated lines.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: afd7ce6b-462a-4639-838a-2d6038579a0d
📒 Files selected for processing (4)
tests/release/helpers_test.gotests/release/image_references.gotests/release/image_references_test.gotests/release/types.go
✅ Files skipped from review due to trivial changes (1)
- tests/release/types.go
Signed-off-by: Joseph <jvaikath@redhat.com>
|
@Joeavaikath: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
Why the changes were made
Adds automated validation that
bundle/image-referencesstays in sync with the CSV on release branches::oadp-1.6on theoadp-1.6branch). Derives version fromPULL_BASE_REF(Prow) or the git branch name.image-referenceshas a correspondingRELATED_IMAGE_*env var in the CSV (or is a container image).Both Prow-facing tests skip gracefully on non-release branches. The core validation logic is extracted into pure functions with comprehensive unit tests.
When tested against
oadp-1.6, this already caught a real discrepancy:oadp-kubevirt-velero-plugin-rhel9andoadp-kubevirt-datamover-plugin-rhel9are inimage-referencesbut missing from the CSV'sRELATED_IMAGE_*env vars.How to test the changes made
Summary by CodeRabbit
New Features
Chores
Tests