NO-JIRA: [RHCOS10] Migrate base images and operand images to UBI10/RHEL10 - #103
NO-JIRA: [RHCOS10] Migrate base images and operand images to UBI10/RHEL10#103PillaiManish wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@PillaiManish: This pull request explicitly references no jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/test e2e-operator-fips e2e-operator-rhcos10-fips e2e-operator-rhcos10 |
WalkthroughMigrates build and runtime base images from UBI9 to UBI10 and replaces SPIRE/SPIFFE operand image references from ghcr.io to quay.io/rh-ee-rausingh, plus related manifest, config, test, utility, and documentation updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: PillaiManish 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/controller/spiffe-csi-driver/daemonset_test.go (1)
169-170: Cover the env override instead of pinning another default image string.This will need another manual edit on the next image bump and still doesn’t prove
RELATED_IMAGE_SPIFFE_CSI_INIT_CONTAINERis honored. Set the env var in the test and assert against that override value.Proposed test refactor
+const expectedInitContainerImage = "example.com/test/spiffe-csi-init:test" + func TestGenerateSpiffeCsiDriverDaemonSet(t *testing.T) { + t.Setenv(utils.SpiffeCSIInitContainerImageEnv, expectedInitContainerImage) + // Mock the utility functions that are called in the main function // These would need to be properly mocked in a real test environment @@ - if container.Image != "registry.redhat.io/ubi10/ubi:10.1" { - t.Errorf("Expected init container image 'registry.redhat.io/ubi10/ubi:10.1', got '%s'", container.Image) + if container.Image != expectedInitContainerImage { + t.Errorf("Expected init container image '%s', got '%s'", expectedInitContainerImage, container.Image) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/spiffe-csi-driver/daemonset_test.go` around lines 169 - 170, The test currently asserts container.Image equals a pinned default string; instead set the RELATED_IMAGE_SPIFFE_CSI_INIT_CONTAINER env var at the start of the test to a unique override value, recreate or call the function that builds the DaemonSet so container (the local variable under test) picks up the env override, then assert container.Image equals that override value; ensure you restore/clear RELATED_IMAGE_SPIFFE_CSI_INIT_CONTAINER (or use t.Setenv) to avoid leaking the env to other tests and update any references to container.Image assertion in daemonset_test.go accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/manager/manager.yaml`:
- Around line 95-96: Add a SpiffeHelperImageEnv constant and a
GetSpiffeHelperImage() accessor so the RELATED_IMAGE_SPIFFE_HELPER env var is
actually used: define SpiffeHelperImageEnv alongside the other image constants
(matching the naming pattern used for SpireServerImageEnv,
SpiffeCSIInitContainerImageEnv, etc.), add a corresponding field/getter on the
related images helper (e.g., implement GetSpiffeHelperImage() on the same type
that provides the other getters), and wire that getter where related images are
injected so the manifest env var is consumed; alternatively remove the
RELATED_IMAGE_SPIFFE_HELPER entry from the manifest if you choose not to add
SpiffeHelperImageEnv/GetSpiffeHelperImage().
In `@Dockerfile`:
- Line 17: Replace the invalid runtime image reference in the Dockerfile FROM
instruction: change the existing "FROM registry.redhat.io/ubi10:10.1" to include
the image name so it reads "FROM registry.redhat.io/ubi10/ubi:10.1"; update the
Dockerfile's FROM line (the FROM instruction) to match the registry format used
elsewhere.
---
Nitpick comments:
In `@pkg/controller/spiffe-csi-driver/daemonset_test.go`:
- Around line 169-170: The test currently asserts container.Image equals a
pinned default string; instead set the RELATED_IMAGE_SPIFFE_CSI_INIT_CONTAINER
env var at the start of the test to a unique override value, recreate or call
the function that builds the DaemonSet so container (the local variable under
test) picks up the env override, then assert container.Image equals that
override value; ensure you restore/clear RELATED_IMAGE_SPIFFE_CSI_INIT_CONTAINER
(or use t.Setenv) to avoid leaking the env to other tests and update any
references to container.Image assertion in daemonset_test.go accordingly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7134392e-1383-424f-af70-20073a46812b
📒 Files selected for processing (7)
.work/compliance/rhcos10/PR2-ubi10-migration.mdDockerfilebundle/manifests/zero-trust-workload-identity-manager.clusterserviceversion.yamlconfig/manager/manager.yamlpkg/controller/spiffe-csi-driver/daemonset_test.gopkg/controller/utils/relatedImages.gotest/e2e/utils/constants.go
| - name: RELATED_IMAGE_SPIFFE_HELPER | ||
| value: quay.io/rh-ee-rausingh/zero-trust-workload-identity-manager-spiffe-spiffe-helper:v0.10.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether RELATED_IMAGE_SPIFFE_HELPER is wired through Go code.
rg -n -C2 'RELATED_IMAGE_SPIFFE_HELPER|SpiffeHelperImageEnv\b|func GetSpiffeHelperImage\('Repository: openshift/zero-trust-workload-identity-manager
Length of output: 1296
🏁 Script executed:
# Check what actually exists in constants.go around the mentioned lines
cat -n pkg/controller/utils/constants.go | head -70 | tail -20Repository: openshift/zero-trust-workload-identity-manager
Length of output: 1270
🏁 Script executed:
# Check what getters exist in relatedImages.go
grep -n "^func Get" pkg/controller/utils/relatedImages.goRepository: openshift/zero-trust-workload-identity-manager
Length of output: 409
🏁 Script executed:
# Search for any references to spiffe-helper or SpiffeHelper in Go code
rg -n "spiffe.?helper|SpiffeHelper" --type goRepository: openshift/zero-trust-workload-identity-manager
Length of output: 1933
🏁 Script executed:
# Check if there's any other pattern for consuming RELATED_IMAGE_* variables
rg -n "RELATED_IMAGE_" --type go -A 2 -B 2 | head -50Repository: openshift/zero-trust-workload-identity-manager
Length of output: 1095
Add SpiffeHelperImageEnv to constants and GetSpiffeHelperImage() to relatedImages, or remove this unused env var.
The RELATED_IMAGE_SPIFFE_HELPER env var is added to the manifest, but the controller does not wire it. pkg/controller/utils/constants.go (lines 56–62) defines env var constants for all other images (SpireServerImageEnv, SpiffeCSIInitContainerImageEnv, etc.) but has no SpiffeHelperImageEnv. Similarly, pkg/controller/utils/relatedImages.go has getters for all other images but no GetSpiffeHelperImage(). This is dead configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/manager/manager.yaml` around lines 95 - 96, Add a SpiffeHelperImageEnv
constant and a GetSpiffeHelperImage() accessor so the
RELATED_IMAGE_SPIFFE_HELPER env var is actually used: define
SpiffeHelperImageEnv alongside the other image constants (matching the naming
pattern used for SpireServerImageEnv, SpiffeCSIInitContainerImageEnv, etc.), add
a corresponding field/getter on the related images helper (e.g., implement
GetSpiffeHelperImage() on the same type that provides the other getters), and
wire that getter where related images are injected so the manifest env var is
consumed; alternatively remove the RELATED_IMAGE_SPIFFE_HELPER entry from the
manifest if you choose not to add SpiffeHelperImageEnv/GetSpiffeHelperImage().
| go build -mod=mod -a -o zero-trust-workload-identity-manager ./cmd/zero-trust-workload-identity-manager/main.go | ||
|
|
||
| FROM registry.access.redhat.com/ubi9-minimal:9.4 | ||
| FROM registry.redhat.io/ubi10:10.1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and inspect the Dockerfile
fd -t f "Dockerfile" --exec wc -l {} \;Repository: openshift/zero-trust-workload-identity-manager
Length of output: 594
🏁 Script executed:
# Show current Dockerfile content
cat -n DockerfileRepository: openshift/zero-trust-workload-identity-manager
Length of output: 844
🏁 Script executed:
# Check git diff to see what changed
git diff HEAD Dockerfile | head -100Repository: openshift/zero-trust-workload-identity-manager
Length of output: 72
🏁 Script executed:
# Search for all UBI10 references in the repository
rg "registry\.redhat\.io/ubi" --no-heading -nRepository: openshift/zero-trust-workload-identity-manager
Length of output: 928
🏁 Script executed:
# Check git log to understand the context of changes
git log --oneline -10Repository: openshift/zero-trust-workload-identity-manager
Length of output: 171
🌐 Web query:
Red Hat UBI10 container image registry path format registry.redhat.io/ubi10
💡 Result:
The registry path format for Red Hat UBI 10 (Universal Base Image 10) container images on registry.redhat.io is registry.redhat.io/ubi10/<image-name>:<tag>. This requires authentication via a Red Hat subscription. Examples: - registry.redhat.io/ubi10/ubi:latest (standard UBI image) - registry.redhat.io/ubi10/ubi-minimal:latest - registry.redhat.io/ubi10/ubi-init:latest - registry.redhat.io/ubi10/s2i-base (as seen in updates) UBI images are also available without authentication from registry.access.redhat.com/ubi10/<image-name>:<tag> for public use. To pull from registry.redhat.io, authenticate first: podman login registry.redhat.io using your customer portal credentials.
Citations:
- 1: https://docs.redhat.com/es/documentation/red_hat_enterprise_linux/10/html/building_running_and_managing_containers/working-with-container-registries
- 2: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/10/html/composing_a_customized_rhel_system_image/creating-and-registering-a-container-to-embed-to-image
- 3: https://access.redhat.com/articles/4238681
- 4: https://access.redhat.com/errata/RHBA-2026:4156
Fix the runtime image reference path to include the image name.
Line 17 uses registry.redhat.io/ubi10:10.1, which is an invalid registry path. Red Hat's UBI10 image registry requires the format registry.redhat.io/ubi10/<image-name>:<tag>. This reference will fail to pull. The codebase consistently uses registry.redhat.io/ubi10/ubi:10.1 elsewhere (config/manager, tests, manifests, and relatedImages). Update line 17 to match: FROM registry.redhat.io/ubi10/ubi:10.1.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` at line 17, Replace the invalid runtime image reference in the
Dockerfile FROM instruction: change the existing "FROM
registry.redhat.io/ubi10:10.1" to include the image name so it reads "FROM
registry.redhat.io/ubi10/ubi:10.1"; update the Dockerfile's FROM line (the FROM
instruction) to match the registry format used elsewhere.
|
/retest |
…EL10 Update operator Dockerfile and all operand/related images for native RHCOS10 compatibility. Operator image changes: - Dockerfile builder: rhel-9-golang-1.23-openshift-4.18 → registry.redhat.io/ubi10/go-toolset:10.1 - Dockerfile runtime: ubi9-minimal:9.4 → registry.redhat.io/ubi10:10.1 - vendor/build-machinery-go/.ci-operator.yaml: rhel-9-release-golang-1.23 → rhel-10-release-golang-1.24 Operand image changes (ghcr.io/spiffe → quay.io/rh-ee-rausingh mirrors): - spire-server:1.13.3, spire-agent:1.13.3, spiffe-csi-driver:0.2.8 - oidc-discovery-provider:1.13.3, spire-controller-manager:0.6.3 - spiffe-helper:v0.10.0 (new) - init container: ubi9:latest → ubi10-minimal:10.1 Made-with: Cursor
b8472bc to
6fa4f93
Compare
|
/test e2e-operator-fips e2e-operator-rhcos10-fips e2e-operator-rhcos10 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/zero-trust-workload-identity-manager.clusterserviceversion.yaml`:
- Around line 710-711: The CSV added RELATED_IMAGE_SPIFFE_HELPER but the
operator code doesn't declare or include that key in the RELATED_IMAGE plumbing;
add a constant named RELATED_IMAGE_SPIFFE_HELPER to
pkg/controller/utils/constants.go and then wire it into the related images
resolution in pkg/controller/utils/relatedImages.go by adding the same key to
the relatedImages map/processing (the same place other RELATED_IMAGE_* entries
are handled, e.g., where GetRelatedImages()/relatedImages map is defined) so the
operator reads the env var and resolves/overrides the helper image just like the
other RELATED_IMAGE variables.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7ac11b8-e98c-42a3-abd1-6e2d1305280d
📒 Files selected for processing (7)
.work/compliance/rhcos10/PR2-ubi10-migration.mdDockerfilebundle/manifests/zero-trust-workload-identity-manager.clusterserviceversion.yamlconfig/manager/manager.yamlpkg/controller/spiffe-csi-driver/daemonset_test.gopkg/controller/utils/relatedImages.gotest/e2e/utils/constants.go
✅ Files skipped from review due to trivial changes (4)
- pkg/controller/spiffe-csi-driver/daemonset_test.go
- pkg/controller/utils/relatedImages.go
- test/e2e/utils/constants.go
- .work/compliance/rhcos10/PR2-ubi10-migration.md
🚧 Files skipped from review as they are similar to previous changes (2)
- Dockerfile
- config/manager/manager.yaml
| - name: RELATED_IMAGE_SPIFFE_HELPER | ||
| value: quay.io/rh-ee-rausingh/zero-trust-workload-identity-manager-spiffe-spiffe-helper:v0.10.0 |
There was a problem hiding this comment.
RELATED_IMAGE_SPIFFE_HELPER is added in CSV but not wired into operator image resolution
Line 710 and Line 800 introduce helper image config, but the operator utility layer does not define/read this env var (pkg/controller/utils/constants.go:55-62, pkg/controller/utils/relatedImages.go:1-59). This leaves the helper image outside the existing RELATED_IMAGE plumbing and can break maintainability/disconnected image override expectations.
Proposed wiring changes
diff --git a/pkg/controller/utils/constants.go b/pkg/controller/utils/constants.go
@@
SpireControllerManagerImageEnv = "RELATED_IMAGE_SPIRE_CONTROLLER_MANAGER"
NodeDriverRegistrarImageEnv = "RELATED_IMAGE_NODE_DRIVER_REGISTRAR"
SpiffeCSIInitContainerImageEnv = "RELATED_IMAGE_SPIFFE_CSI_INIT_CONTAINER"
+ SpiffeHelperImageEnv = "RELATED_IMAGE_SPIFFE_HELPER"diff --git a/pkg/controller/utils/relatedImages.go b/pkg/controller/utils/relatedImages.go
@@
func GetSpiffeCsiInitContainerImage() string {
containerImage := os.Getenv(SpiffeCSIInitContainerImageEnv)
if containerImage == "" {
return "registry.redhat.io/ubi10/ubi:10.1"
}
return containerImage
}
+
+func GetSpiffeHelperImage() string {
+ spiffeHelperImage := os.Getenv(SpiffeHelperImageEnv)
+ if spiffeHelperImage == "" {
+ return ""
+ }
+ return spiffeHelperImage
+}As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
Also applies to: 800-801
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@bundle/manifests/zero-trust-workload-identity-manager.clusterserviceversion.yaml`
around lines 710 - 711, The CSV added RELATED_IMAGE_SPIFFE_HELPER but the
operator code doesn't declare or include that key in the RELATED_IMAGE plumbing;
add a constant named RELATED_IMAGE_SPIFFE_HELPER to
pkg/controller/utils/constants.go and then wire it into the related images
resolution in pkg/controller/utils/relatedImages.go by adding the same key to
the relatedImages map/processing (the same place other RELATED_IMAGE_* entries
are handled, e.g., where GetRelatedImages()/relatedImages map is defined) so the
operator reads the env var and resolves/overrides the helper image just like the
other RELATED_IMAGE variables.
|
/test e2e-operator |
|
/retest |
|
/test e2e-operator |
anirudhAgniRedhat
left a comment
anirudhAgniRedhat
left a comment
There was a problem hiding this comment.
/test e2e-operator
|
@PillaiManish @rausingh-rh the image for controller manager is not pullable please make the registry public |
|
/retest |
|
@PillaiManish: all tests passed! 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
Update operator Dockerfile and all operand/related images for native RHCOS10 compatibility.
Operator image changes:
Operand image changes (ghcr.io/spiffe → quay.io/rh-ee-rausingh mirrors):
Made-with: Cursor