feat: add support to pass Oauth proxy image for downstream #2275
Conversation
WalkthroughA new parameter, Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ 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
CodeRabbit Configuration File (
|
|
[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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/controller/components/workbenches/workbenches.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: kahowell
PR: opendatahub-io/opendatahub-operator#2220
File: bundle.rhoai/manifests/rhods-operator.clusterserviceversion.yaml:1548-1550
Timestamp: 2025-07-29T18:44:42.749Z
Learning: In the opendatahub-operator repository's rhoai branch, the CSV manifest intentionally uses `quay.io/opendatahub/opendatahub-operator:latest` as the image reference even though it's for the rhods-operator, and this configuration difference from the main branch is by design.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build/push catalog image
- GitHub Check: golangci-lint
- GitHub Check: Run tests and collect coverage on internal and pkg
- GitHub Check: Run tests and collect coverage on tests/integration
🔇 Additional comments (1)
internal/controller/components/workbenches/workbenches.go (1)
55-57: Addition of oauth-proxy image param — LGTMThis aligns with the PR objective to enable downstream override. No behavioral risk in this file.
| "odh-notebook-controller-image": "RELATED_IMAGE_ODH_NOTEBOOK_CONTROLLER_IMAGE", | ||
| "oauth-proxy-image": "RELATED_IMAGE_OSE_OAUTH_PROXY_IMAGE", | ||
| }); err != nil { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify wiring: RELATED_IMAGE env/CSV + downstream placeholder
Please confirm:
- Operator env/CSV includes RELATED_IMAGE_OSE_OAUTH_PROXY_IMAGE (env var and relatedImages entry), or ApplyParams will inject an empty value at build time.
- The downstream manifest’s params.env actually defines oauth-proxy-image (blocked on the linked kubeflow PR).
- Consider whether the KF Notebook Controller also needs an oauth-proxy override; if yes, mirror this mapping in the kfNbc ApplyParams block.
Run this to validate in-repo wiring (CSV/env/duplicates). Note: the params.env placeholder likely lives in the kubeflow repo; absence here can be expected until that PR merges.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Search for the new RELATED_IMAGE env var occurrences"
rg -n 'RELATED_IMAGE_OSE_OAUTH_PROXY_IMAGE' || true
echo
echo "2) Check CSVs for relatedImages/env wiring mentioning oauth-proxy"
fd -a 'clusterserviceversion.yaml' | while read -r f; do
echo "--- $f"
rg -n -N 'relatedImages|oauth|OSE_OAUTH_PROXY' "$f" -A 3 -B 3 || true
done
echo
echo "3) Check manager manifests for the env var"
fd -a 'manager.yaml' | while read -r f; do
echo "--- $f"
rg -n 'RELATED_IMAGE_OSE_OAUTH_PROXY_IMAGE|env:' "$f" -A 5 -B 5 || true
done
echo
echo "4) Look for the new params key within this repo (may be absent if only in kubeflow repo)"
rg -n 'oauth-proxy-image' || true
echo
echo "5) Detect possibly conflicting/alternate RELATED_IMAGE names for oauth proxy"
rg -n 'RELATED_IMAGE_.*OAUTH.*' || trueLength of output: 3953
Inject OAuth proxy image env var & CSV relatedImages entry
- File: config/manager/manager.yaml
Underspec.template.spec.containers[].env, inject the new env var so ApplyParams can substitute it:- name: RELATED_IMAGE_OSE_OAUTH_PROXY_IMAGE value: "${RELATED_IMAGE_OSE_OAUTH_PROXY_IMAGE}"
- File: bundle/manifests/opendatahub-operator.clusterserviceversion.yaml
(and config/manifests/bases/opendatahub-operator.clusterserviceversion.yaml)
Add underspec.relatedImages:spec: relatedImages: + - name: oauth-proxy + image: "${RELATED_IMAGE_OSE_OAUTH_PROXY_IMAGE}"
- Downstream CSV (kubeflow repo) must define
oauth-proxy-imagein itsparams.envso the placeholder is populated (pending the linked PR). - If the KF Notebook Controller also needs an oauth-proxy override, mirror this
oauth-proxy→RELATED_IMAGE_OSE_OAUTH_PROXY_IMAGEmapping in its ApplyParams block as well.
🤖 Prompt for AI Agents
In internal/controller/components/workbenches/workbenches.go around lines 55 to
57, you need to inject the OAuth proxy image environment variable and update the
CSV relatedImages entry. Add the env var RELATED_IMAGE_OSE_OAUTH_PROXY_IMAGE
with the value placeholder "${RELATED_IMAGE_OSE_OAUTH_PROXY_IMAGE}" in
config/manager/manager.yaml under spec.template.spec.containers[].env. Also, add
an entry for oauth-proxy-image under spec.relatedImages in
bundle/manifests/opendatahub-operator.clusterserviceversion.yaml and
config/manifests/bases/opendatahub-operator.clusterserviceversion.yaml. Ensure
the downstream CSV in the kubeflow repo defines oauth-proxy-image in its
params.env for placeholder substitution. If the KF Notebook Controller requires
an oauth-proxy override, add the oauth-proxy to
RELATED_IMAGE_OSE_OAUTH_PROXY_IMAGE mapping in its ApplyParams block as well.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2275 +/- ##
==========================================
- Coverage 41.92% 41.92% -0.01%
==========================================
Files 144 144
Lines 11387 11388 +1
==========================================
Hits 4774 4774
- Misses 6212 6213 +1
Partials 401 401 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/cc @jstourac |
|
Please make PR from your own forked git repo's branch, not branch out directly from "origin" |
|
The opendatahub-io/kubeflow#671 has been merged and synced all the way through down to the rhds repo. This PR can be resumed and moved on. |
Description
Add support to override OAuth proxy image for odh notebook controller
DO NOT MERGE until opendatahub-io/kubeflow#671 is merged
https://issues.redhat.com/browse/RHOAIENG-31618
https://issues.redhat.com/browse/RHOAIENG-31730
How Has This Been Tested?
Screenshot or short clip
Merge criteria
Summary by CodeRabbit