docs: add CI plugin image sync guidance to common.go - #2346
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaovilai The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe change adds documentation for synchronizing plugin image constants, ChangesImage Synchronization Documentation
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
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 `@pkg/common/common.go`:
- Around line 70-87: Correct the synchronization guidance associated with the
image constants near RegistryImage: either add the missing
RELATED_IMAGE_REGISTRY mapping in config/manager/manager.yaml or exclude
RegistryImage from the contract. Standardize the CI image tag example as latest
rather than :latest, and state that operator.substitutions and base_images are
required only when defined by the affected branch configuration, not as a
universal pair.
🪄 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: Enterprise
Run ID: 947b55a1-cb01-4463-894c-4df6975d4c7e
📒 Files selected for processing (1)
pkg/common/common.go
| // Each image constant below corresponds to a RELATED_IMAGE_* environment variable | ||
| // in config/manager/manager.yaml. In production, OLM injects these from the CSV; | ||
| // during CI e2e tests, ci-operator substitutes them with freshly-built CI images | ||
| // so that tests always run against the latest plugin code. | ||
| // | ||
| // To keep CI substitutions in sync, every image here must have a matching pair of | ||
| // entries in the openshift/release ci-operator config for each OADP release branch: | ||
| // | ||
| // 1. A base_images entry that imports the image from the CI registry | ||
| // (namespace: konveyor, name: <plugin>, tag: <branch or :latest>). | ||
| // | ||
| // 2. An operator.substitutions entry that replaces the quay.io pullspec | ||
| // in the CSV with the CI image reference. | ||
| // | ||
| // When adding, removing, or renaming a plugin image constant: | ||
| // - Update the corresponding RELATED_IMAGE_* env var in config/manager/manager.yaml | ||
| // - Update the base_images + operator.substitutions in openshift/release | ||
| // ci-operator/config/openshift/oadp-operator/ for every affected branch config |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Image constants:"
rg -n '^[[:space:]]*[A-Za-z0-9]+Image[[:space:]]*=' pkg/common/common.go
echo "RELATED_IMAGE mappings:"
fd --type f --hidden --glob 'manager.yaml' . \
| while read -r file; do
rg -n -C 2 'RELATED_IMAGE_|image:' "$file"
done
echo "OpenShift CI configurations:"
fd --type f --hidden --glob '*.yaml' . \
| rg 'ci-operator/config/openshift/oadp-operator/' \
| while read -r file; do
rg -n -C 3 'base_images:|substitutions:|RELATED_IMAGE_|quay.io/' "$file"
doneRepository: openshift/oadp-operator
Length of output: 3665
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked candidate files:"
git ls-files \
'pkg/common/common.go' \
'*manager.yaml' \
'*ci-operator*' \
'*oadp-operator*' \
| sed -n '1,160p'
echo
echo "Common constants and usages:"
cat -n pkg/common/common.go | sed -n '60,115p'
rg -n -C 2 'VeleroImage|OpenshiftPluginImage|AWSPluginImage|LegacyAWSPluginImage|AzurePluginImage|GCPPluginImage|RegistryImage|KubeVirtPluginImage|KubeVirtDatamoverPluginImage|HypershiftPluginImage' .
echo
echo "Manager configuration:"
fd --type f --hidden --glob 'manager.yaml' . -x sh -c 'echo "--- $1"; cat -n "$1"' sh {} \;
echo
echo "CI configuration references:"
rg -n -C 3 'base_images|operator:|substitutions|RELATED_IMAGE_|quay.io/(konveyor|redhat-user-workloads)' . \
-g '*.yaml' -g '*.yml' -g '*.json' || trueRepository: openshift/oadp-operator
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
common = Path("pkg/common/common.go").read_text()
manager_files = list(Path(".").rglob("manager.yaml"))
constants = dict(re.findall(
r'^\s*(\w+Image)\s*=\s*"([^"]+)"',
common,
re.M,
))
print("constant_count:", len(constants))
for name, pullspec in constants.items():
env = "RELATED_IMAGE_" + re.sub(r'(?<!^)(?=[A-Z])', '_', name[:-5]).upper()
print(f"{name}\t{env}\t{pullspec}")
for path in manager_files:
text = path.read_text()
print(f"\nmanager_file: {path}")
for m in re.finditer(r'name:\s*(RELATED_IMAGE_[A-Z0-9_]+)\s*\n\s*value:\s*([^\s]+)', text):
print(f"{m.group(1)}\t{m.group(2)}")
ci_files = [
p for p in Path(".").rglob("*")
if p.is_file() and "ci-operator/config/openshift/oadp-operator/" in p.as_posix()
]
print("\nci_file_count:", len(ci_files))
for path in ci_files:
print(path)
PYRepository: openshift/oadp-operator
Length of output: 2705
🌐 Web query:
site:github.com/openshift/release "ci-operator/config/openshift/oadp-operator" "base_images" "substitutions"
💡 Result:
In the context of OpenShift's ci-operator configuration files—such as those found under ci-operator/config/openshift/oadp-operator/—base_images are used to define the source container images required for building, testing, or running the operator's lifecycle [1][2][3]. While there is no specific top-level key explicitly named substitutions within the standard ci-operator configuration schema, image substitution and overriding in OpenShift CI typically occur through a few established mechanisms: 1. Base Image Definitions: The base_images block identifies specific container images (referenced by name, namespace, and tag) that serve as dependencies [1][2][3][4]. These can be referenced in images build steps or test definitions [1][3][5]. 2. Environment Variable Injection: Configuration files often use shell-style variable expansion (e.g., $(BRANCH)) to inject dynamic values into the configuration at runtime [6]. This is frequently used to align image tags with specific OpenShift release branches [6]. 3. Release-Based Overrides: CI jobs often use environment variables like RELEASE_IMAGE_LATEST or RELEASE_IMAGE_INITIAL to substitute entire release payload images, which the ci-operator can then resolve or utilize as a source for base_images [6]. 4. Image Mirroring and Mapping: Outside of the individual config file, the OpenShift CI infrastructure uses image-mirror-mappings (configured in the ci namespace) to transparently redirect image pulls from one registry or repository to another, effectively substituting the underlying image source without changing the operator's ci-operator config [5]. If you are encountering a reference to "substitutions" in documentation or a specific script, it likely refers to these dynamic injection points or the external infrastructure-level mirroring mappings that ensure the correct images are pulled during the CI process [6][5].
Citations:
- 1: https://github.com/openshift/release/blob/52b67a8df670129a188f6be99c1b64ac8e85a2a5/ci-operator/config/openshift/origin/openshift-origin-master.yaml
- 2: https://github.com/openshift/release/blob/main/ci-operator/config/openshift/hypershift/openshift-hypershift-main.yaml
- 3: https://github.com/openshift/release/blob/master/ci-operator/config/openshift/cincinnati/openshift-cincinnati-master.yaml
- 4: https://github.com/openshift/release/blob/main/ci-operator/config/stolostron/multicloud-operators-foundation/stolostron-multicloud-operators-foundation-main.yaml
- 5: Add OADP 1.5 image build and mirroring for hypershift-oadp-plugin release#80356
- 6: https://github.com/openshift/release/blob/main/ci-operator/jobs/openshift/release/openshift-release-infra-periodics.yaml
🌐 Web query:
OpenShift ci-operator configuration base_images tag field operator substitutions syntax
💡 Result:
In OpenShift ci-operator configurations, the operator.substitutions field is used to replace specific image pull specifications within operator manifests with images built or imported during the CI process [1][2]. This mechanism ensures that operator bundles, which are often created for testing, reference the correct, freshly built versions of operands and operators rather than hardcoded external pull specs [1]. The configuration for operator substitutions follows this structure under the operator top-level key [2]: operator: substitutions: - pullspec: <string_to_replace> with: <replacement_string> Field Details: - pullspec: This defines the exact string (the pull specification) found in the operator manifests that needs to be replaced [2]. - with: This defines the replacement string. Typically, this is a reference to a CI-managed image in the format pipeline:<image_name> [1][2]. How it relates to base_images and image resolution: While base_images are used to import external or pre-existing images into the CI pipeline (making them available as ImageStreamTags) [3][4], operator substitutions specifically bridge the gap between static manifests and these dynamic CI images [1]. When the BundleSourceStep runs, it scans the operator manifests, identifies occurrences of the strings defined in the pullspec field, and replaces them with the actual pull specification of the corresponding image in the pipeline ImageStream [5][1]. Example: If you have an operand imported via base_images and an operator built in your repository (defined in the images section), your configuration might look like this [1]: base_images: operand: namespace: ocp name: operand tag: latest images: tested-operator:... operator: substitutions: - pullspec: "quay.io/example/operand:1.0" with: "pipeline:operand" - pullspec: "quay.io/example/operator:1.0" with: "pipeline:tested-operator" In this example, any occurrence of the external quay.io pull specs in the operator manifests will be replaced by the resolved pull specifications of the pipeline:operand and pipeline:tested-operator images [1][2].
Citations:
- 1: https://docs.ci.openshift.org/how-tos/testing-operator-sdk-operators/
- 2: https://steps.ci.openshift.org/ci-operator-reference
- 3: https://github.com/openshift/ci-operator/blob/master/CONFIGURATION.md
- 4: https://docs.ci.openshift.org/architecture/ci-operator/
- 5: https://docs.ci.openshift.org/internals/steps/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
import urllib.request
url = "https://api.github.com/repos/openshift/release/contents/ci-operator/config/openshift/oadp-operator"
request = urllib.request.Request(url, headers={"Accept": "application/vnd.github+json"})
try:
with urllib.request.urlopen(request, timeout=20) as response:
data = json.load(response)
except Exception as exc:
print(f"external_lookup_error: {exc}")
raise SystemExit(0)
for item in data:
print(item.get("name", ""))
PY
echo
echo "Local ci-operator schema references:"
rg -n -C 3 'base_images:|operator:|substitutions:|tag:' . \
-g '*.yaml' -g '*.yml' \
| sed -n '1,240p' || trueRepository: openshift/oadp-operator
Length of output: 27565
🌐 Web query:
site:github.com/openshift/release/ci-operator/config/openshift/oadp-operator base_images operator substitutions
💡 Result:
The matching OpenShift CI configuration is:
- Directory:
openshift/release/ci-operator/config/openshift/oadp-operator(github.com) - OADP 1.6 / OCP 5.0:
openshift-oadp-operator-oadp-1.6__5.0.yaml- Uses bundle substitution:
quay.io/konveyor/oadp-operator:oadp-1.6- replaced with
oadp-operator-1.6(github.com)
- Uses bundle substitution:
- Development / OCP 5.0:
openshift-oadp-operator-oadp-dev__5.0.yaml- Uses:
quay.io/konveyor/oadp-operator:latest- replaced with
oadp-operator(github.com)
- Uses:
No base_images entry is present in these files.
Citations:
- 1: https://github.com/openshift/release/tree/master/ci-operator/config/openshift/oadp-operator
- 2: https://github.com/openshift/release/blob/main/ci-operator/config/openshift/oadp-operator/openshift-oadp-operator-oadp-1.6__5.0.yaml
- 3: https://github.com/openshift/release/blob/main/ci-operator/config/openshift/oadp-operator/openshift-oadp-operator-oadp-dev__5.0.yaml
🏁 Script executed:
#!/bin/bash
set -euo pipefail
api='https://api.github.com/repos/openshift/release/contents/ci-operator/config/openshift/oadp-operator'
json="$(curl -ksSfL "$api")"
python3 - "$json" <<'PY'
import json, sys
items = json.loads(sys.argv[1])
for item in items:
if item.get("name", "").endswith((".yaml", ".yml")):
print(item["name"])
PY
echo
for file in \
openshift-oadp-operator-oadp-1.6__5.0.yaml \
openshift-oadp-operator-oadp-dev__5.0.yaml
do
url="https://raw.githubusercontent.com/openshift/release/master/ci-operator/config/openshift/oadp-operator/$file"
echo "--- $file"
content="$(curl -ksSfL "$url" || true)"
if [ -z "$content" ]; then
echo "not found"
continue
fi
printf '%s\n' "$content" | rg -n -C 3 \
'^(base_images:|operator:| substitutions:| - pullspec:| with:)|quay.io/konveyor/|pipeline:|RELATED_IMAGE_' \
| sed -n '1,220p'
doneRepository: openshift/oadp-operator
Length of output: 263
Correct the image synchronization contract.
RegistryImage has no RELATED_IMAGE_REGISTRY entry in config/manager/manager.yaml. Exclude it from this contract or add the missing mapping. Use tag: latest, not tag: :latest. The cited OADP configurations use operator.substitutions without base_images, so do not require both entries unless each branch defines them.
🤖 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 `@pkg/common/common.go` around lines 70 - 87, Correct the synchronization
guidance associated with the image constants near RegistryImage: either add the
missing RELATED_IMAGE_REGISTRY mapping in config/manager/manager.yaml or exclude
RegistryImage from the contract. Standardize the CI image tag example as latest
rather than :latest, and state that operator.substitutions and base_images are
required only when defined by the affected branch configuration, not as a
universal pair.
There was a problem hiding this comment.
Thanks for the review. Addressing each point:
1. RegistryImage has no RELATED_IMAGE_REGISTRY in manager.yaml — This is a pre-existing state, not introduced by this PR. The comment says "each image constant below corresponds to a RELATED_IMAGE_*" which is aspirational guidance for the general pattern. Whether RegistryImage specifically needs a RELATED_IMAGE_ mapping is a separate concern tracked in #2343. Not changing the guidance here.
2. tag: :latest → tag: latest — Valid catch. The ci-operator YAML tag field takes the bare value latest without a colon prefix. Will fix.
3. base_images + operator.substitutions pairing — The guidance is correct for the pattern being introduced in openshift/release#82762. Plugin images from promoted CI builds (not built from source in the current repo) require both: base_images to import from the CI registry, and operator.substitutions to rewrite the CSV pullspec. The existing substitution-only pattern applies to the operator image itself (built from source via images stanza, referenced as pipeline:<name>), which is a different case. No change needed.
There was a problem hiding this comment.
@redhat-chai-bot, thank you for the clarification.
The tag: latest correction is valid.
The base_images and operator.substitutions guidance is valid for promoted plugin images. Please state that scope in the comment. The current text says that each image constant below follows this contract. That statement includes RegistryImage, which has no corresponding RELATED_IMAGE_* variable.
The fact that RegistryImage predates this PR does not make the new universal statement accurate. Please either exclude RegistryImage from the contract or change the contract to apply only to plugin images that have RELATED_IMAGE_* variables.
✏️ Learnings added
Learnt from: redhat-chai-bot
Repo: openshift/oadp-operator PR: 2346
File: pkg/common/common.go:70-87
Timestamp: 2026-08-01T03:57:00.124Z
Learning: For OADP CI configuration, promoted plugin images that are not built from source in the oadp-operator repository require both `base_images` and `operator.substitutions`. The existing substitution-only pattern applies to the operator image built through the `images` stanza and referenced as `pipeline:<name>`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
9ab9a49 to
8052cf3
Compare
Adds a Go comment block above
// Imagesinpkg/common/common.goexplaining:RELATED_IMAGE_*env var → needsbase_images+operator.substitutionsin openshift/releaseSupersedes #2345 (which targeted
oadp-1.6and included a separatedocs/guide +config/manager/manager.yamlchanges) — this PR is comment-only, targetsoadp-dev, nodocs/file.pairs to openshift/release#82762
Note
Proposed by chai-bot (redhat-chai-bot), opened as draft via Claude Code. AI-generated — review for accuracy.
Summary by CodeRabbit