-
Notifications
You must be signed in to change notification settings - Fork 44
[RHOAIENG-30871] bump the oauth proxy image to RHEL9 and latest #669
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.
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
base: main
Are you sure you want to change the base?
Conversation
Skipping CI for Draft Pull Request. |
WalkthroughThe changes update the OAuth proxy container image used by the odh-notebook-controller, both in its deployment configuration and in the Go source code constant. The image reference is switched from "ose-oauth-proxy" to "ose-oauth-proxy-rhel9" with updated SHA256 digests and documentation links. No other logic or configuration is altered. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #669 +/- ##
===========================================
+ Coverage 57.46% 67.67% +10.21%
===========================================
Files 11 9 -2
Lines 3082 2116 -966
===========================================
- Hits 1771 1432 -339
+ Misses 1141 555 -586
+ Partials 170 129 -41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/odh-notebook-controller/controllers/notebook_oauth.go (1)
44-48
: Comment text mentions v4.14 AMD64 while the constant now points to a RHEL9 image – please realign the docsThe explanatory comment still claims the digest is “v4.14 image for AMD64”.
Since the reference has been switched to theose-oauth-proxy-rhel9
repository, the note is potentially misleading and may confuse anyone validating the digest or troubleshooting multi-arch pulls.- // OAuthProxyImage uses sha256 manifest list digest value of v4.14 image for AMD64 as default … + // OAuthProxyImage uses the sha256 digest of the RHEL 9 based OAuth-proxy image (OCP 4.14), + // pointing to the multi-arch manifest list. Update this comment when the digest changes.While touching this, please double-check that the digest (
ca21e218…
) is indeed the manifest-list and not a single-arch blob; otherwise clusters on non-amd64 hardware will fail to pull the sidecar.components/odh-notebook-controller/config/manager/manager.yaml (1)
28-29
: Deployment arg updated – keep code & manifest versions synchronised going forwardThe
--oauth-proxy-image
argument correctly mirrors the new digest. Note that this value is duplicated in Go (OAuthProxyImage
) and here in YAML; any future bump will require touching both places to avoid drift. Consider sourcing the image from a single location (e.g. via an env var or Helm/Kustomize substitution) to reduce maintenance overhead.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/odh-notebook-controller/config/manager/manager.yaml
(1 hunks)components/odh-notebook-controller/controllers/notebook_oauth.go
(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-06-29T12:32:38.270Z
Learning: The `ci/prow/odh-notebook-controller-unit` test in the opendatahub-io/kubeflow repository is a known flaky test with existing Jira tickets RHOAIENG-15909 and RHOAIENG-15907 tracking the issue. Test failures from this CI job are often not related to code changes and typically pass on rerun.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-14T11:32:05.952Z
Learning: PR #649 in opendatahub-io/kubeflow partially addressed context.Background() replacement but left 4 instances in Eventually blocks within OAuth-related tests in notebook_controller_test.go that still need to be replaced with the global ctx variable.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository. The files need updates to the build_root.image_stream_tag.tag from golang-1.23 to golang-1.24 and base_images.ubi_minimal.tag from "8" to "9".
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:36:30.551Z
Learning: Step 6 of the DEPENDENCIES.md file in opendatahub-io/kubeflow repository requires updating the Go version and UBI version in the external openshift/release repository's CI operator configuration file at ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-master.yaml, specifically updating the build_root.image_stream_tag.tag to use the correct Go version and base_images.ubi_minimal.tag to match the UBI version changes.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#605
File: components/odh-notebook-controller/controllers/notebook_controller_test.go:1126-1131
Timestamp: 2025-07-02T04:00:16.948Z
Learning: In the opendatahub-io/kubeflow repository's notebook controller tests, OAuth finalizer tests should be streamlined into single test blocks rather than multiple "It" blocks, as checking finalizer removal races with object deletion and is typically unobservable in Kubernetes.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository.
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-15T19:46:04.900Z
Learning: Prow CI logs for opendatahub-io/notebooks repository follow this URL pattern: HTML page at https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/{repo}/{pr_number}/{job_name}/{job_id} and raw logs at https://storage.googleapis.com/test-platform-results/pr-logs/pull/{repo}/{pr_number}/{job_name}/{job_id}/build-log.txt
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:179-179
Timestamp: 2025-06-25T06:54:57.600Z
Learning: In the file `components/odh-notebook-controller/controllers/notebook_runtime_test.go`, there is a suggestion to repurpose `testCase.ConfigMap` as `expectedConfigMap` in the test structure to improve clarity by separating input from expected results. This improvement is deferred to be addressed as part of GitHub issue #634.
📚 Learning: step 6 of the dependencies.md file in opendatahub-io/kubeflow repository requires updating the go ve...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:36:30.551Z
Learning: Step 6 of the DEPENDENCIES.md file in opendatahub-io/kubeflow repository requires updating the Go version and UBI version in the external openshift/release repository's CI operator configuration file at ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-master.yaml, specifically updating the build_root.image_stream_tag.tag to use the correct Go version and base_images.ubi_minimal.tag to match the UBI version changes.
Applied to files:
components/odh-notebook-controller/config/manager/manager.yaml
components/odh-notebook-controller/controllers/notebook_oauth.go
📚 Learning: step 6 in the dependencies.md file for opendatahub-io/kubeflow refers to updating go version in the ...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-17T17:37:03.652Z
Learning: Step 6 in the DEPENDENCIES.md file for opendatahub-io/kubeflow refers to updating Go version in the openshift/release repository configuration files at https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-main.yaml and https://github.com/openshift/release/blob/master/ci-operator/config/opendatahub-io/kubeflow/opendatahub-io-kubeflow-v1.10-branch.yaml, not the local CI/CD files in the kubeflow repository. The files need updates to the build_root.image_stream_tag.tag from golang-1.23 to golang-1.24 and base_images.ubi_minimal.tag from "8" to "9".
Applied to files:
components/odh-notebook-controller/controllers/notebook_oauth.go
📚 Learning: pr #649 in opendatahub-io/kubeflow partially addressed context.background() replacement but left 4 i...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#0
File: :0-0
Timestamp: 2025-07-14T11:32:05.952Z
Learning: PR #649 in opendatahub-io/kubeflow partially addressed context.Background() replacement but left 4 instances in Eventually blocks within OAuth-related tests in notebook_controller_test.go that still need to be replaced with the global ctx variable.
Applied to files:
components/odh-notebook-controller/controllers/notebook_oauth.go
📚 Learning: in the file `components/odh-notebook-controller/controllers/notebook_runtime_test.go`, there is a su...
Learnt from: jiridanek
PR: opendatahub-io/kubeflow#623
File: components/odh-notebook-controller/controllers/notebook_runtime_test.go:179-179
Timestamp: 2025-06-25T06:54:57.600Z
Learning: In the file `components/odh-notebook-controller/controllers/notebook_runtime_test.go`, there is a suggestion to repurpose `testCase.ConfigMap` as `expectedConfigMap` in the test structure to improve clarity by separating input from expected results. This improvement is deferred to be addressed as part of GitHub issue #634.
Applied to files:
components/odh-notebook-controller/controllers/notebook_oauth.go
🔇 Additional comments (1)
components/odh-notebook-controller/controllers/notebook_oauth.go (1)
45-48
: LGTM on the constant update – matches the image used in the Deployment
OAuthProxyImage
now aligns with the RHEL 9 container required by the Deployment manifest. No functional concerns.
/retest |
https://issues.redhat.com/browse/RHOAIENG-30871
Description
How Has This Been Tested?
quay.io/jstourac/opendatahub-operator@sha256:32547e3bbc66f91d01c6c5f340f0513688f143919d9b8c4985de9fd1ff76df02
registry.redhat.io/openshift4/ose-oauth-proxy-rhel9@sha256:ca21e218e26c46e3c63d926241846f8f307fd4a586cc4b04147da49af6018ef5
We should probably check with the OCP 4.14 as the oldest supported OCP version too.
Merge criteria:
Summary by CodeRabbit