Fix notebook template for RHOAI 3.0 compatibility #484
Conversation
- Changed annotation from 'inject-oauth' to 'inject-auth' (RHOAI 3.
WalkthroughA test resource file is updated to replace OAuth-specific annotations and container configurations with simplified auth mechanism. The oauth-proxy container, related volumes, and logout URL annotation are removed. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![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)
tests/common/resources/custom-nb-small.yaml(1 hunks)
🔇 Additional comments (1)
tests/common/resources/custom-nb-small.yaml (1)
88-102: Simplified volume and container configuration is appropriate.The removal of the oauth-proxy container and related volumes (oauth-config, tls-certificates) is expected, as RHOAI 3.0's notebook controller now automatically manages these components. The remaining configuration is cleaner and leverages the platform's built-in capabilities.
| metadata: | ||
| annotations: | ||
| notebooks.opendatahub.io/inject-oauth: "true" | ||
| notebooks.opendatahub.io/inject-auth: "true" |
There was a problem hiding this comment.
🧩 Analysis chain
Annotation update aligns with RHOAI 3.0 requirements.
The change from inject-oauth to inject-auth is correct and reflects the RHOAI 3.0 annotation naming convention. Per the PR objectives, the notebook controller now automatically handles OAuth proxy injection and secret creation.
To ensure consistency across the codebase, please verify there are no other notebook template or configuration files still referencing the old inject-oauth annotation:
🏁 Script executed:
#!/bin/bash
# Description: Search for any remaining references to the old inject-oauth annotation
# Search for old annotation pattern in YAML and template files
rg -n 'inject-oauth' --type=yaml --type=tf -g '*.yaml' -g '*.template' -g '*.yml'
# Also search in any documentation that might reference this
rg -n 'inject-oauth' --type=md -g '*.md'Length of output: 120
🏁 Script executed:
#!/bin/bash
# Broader search for all references to both old and new annotations
echo "=== Files with old inject-oauth annotation ==="
rg -l 'inject-oauth' --type=yaml --type=yml
echo ""
echo "=== Files with new inject-auth annotation ==="
rg -l 'inject-auth' --type=yaml --type=yml
echo ""
echo "=== Count of old annotation references ==="
rg 'inject-oauth' --type=yaml --type=yml | wc -l
echo ""
echo "=== Count of new annotation references ==="
rg 'inject-auth' --type=yaml --type=yml | wc -lLength of output: 297
🏁 Script executed:
#!/bin/bash
# Broader search for all references to both old and new annotations (corrected syntax)
echo "=== Files with old inject-oauth annotation ==="
rg -l 'inject-oauth' --type=yaml
echo ""
echo "=== Files with new inject-auth annotation ==="
rg -l 'inject-auth' --type=yaml
echo ""
echo "=== All files with old annotation (with line numbers) ==="
rg -n 'inject-oauth' --type=yaml
echo ""
echo "=== All files with new annotation (with line numbers) ==="
rg -n 'inject-auth' --type=yamlLength of output: 541
Update incomplete: second notebook file still uses old inject-oauth annotation.
The annotation change in tests/common/resources/custom-nb-small.yaml is correct for RHOAI 3.0, but the migration is incomplete. The file examples/stable-diffusion-dreambooth/yaml/distributed/workbench.yaml:5 still references the old inject-oauth annotation and must be updated to inject-auth to maintain consistency across the codebase.
🤖 Prompt for AI Agents
In tests/common/resources/custom-nb-small.yaml around line 12 the annotation was
updated to notebooks.opendatahub.io/inject-auth but the migration is incomplete:
open examples/stable-diffusion-dreambooth/yaml/distributed/workbench.yaml
(around line 5) and replace the old annotation key inject-oauth with the new
notebooks.opendatahub.io/inject-auth (or the same short form used elsewhere) so
the example uses the updated annotation; scan the file for any other occurrences
of inject-oauth and update them to inject-auth to keep consistency.
|
/lgtm |
|
@ChughShilpa please review and approve |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sutaakar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Description
Problem:
Root Cause
How Has This Been Tested?
Summary by CodeRabbit