Conversation
WalkthroughThis PR updates the Kubeflow installation docs: replaces the prior ConfigMap-based redirectURIs flow with a Platform Parameters (Platform Access URLs) flow, scopes oauth2-proxy certificate retrieval to the Global cluster, provides an explicit redirectURIs example (host/port with 30443), adjusts post-deployment navigation wording to Tools in Alauda AI, adds a YAML fragment for ServiceAccount/RoleBinding for namespace binding, and renames LWS plugin references to "Alauda Build of LeaderWorkerSet". Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
🧹 Nitpick comments (2)
docs/en/installation/kubeflow.mdx (2)
106-107: Use a neutral placeholder instead of a concrete IP in the redirect URI example.Line 107 currently shows a specific IP/port that can be copy-pasted accidentally and cause misconfiguration.
Suggested doc tweak
- - https://192.168.139.133:30443/* + - https://<oidc_redirect_host>:<oidc_redirect_port>/*🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/installation/kubeflow.mdx` around lines 106 - 107, Replace the concrete IP/port example "https://192.168.139.133:30443/*" with a neutral placeholder to avoid accidental copy-paste; update the redirect URI example referenced by oidcRedirectURL to something like "https://<HOST>:<PORT>/*" or "https://your-domain.example/*" so readers must substitute their actual host and port.
214-215: Polish note style for consistency.Line 214 can be slightly tightened for documentation style consistency (sentence case and direct wording).
Suggested wording
-> Note: make sure to install LWS (Alauda Build of LeaderWorkerSet) plugin before deploying +> Note: Make sure to install the LWS (Alauda Build of LeaderWorkerSet) plugin before deploying🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/installation/kubeflow.mdx` around lines 214 - 215, Replace the existing note line that reads "Note: make sure to install LWS (Alauda Build of LeaderWorkerSet) plugin before deploying kubeflow-trainer, as LWS is a dependency of kubeflow-trainer." with a tighter, sentence-case phrasing for consistency; update the wording to something like "Note: install LWS (Alauda Build of LeaderWorkerSet) before deploying kubeflow-trainer, as it is a dependency." to use sentence case, remove redundancy, and keep the meaning intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/en/installation/kubeflow.mdx`:
- Around line 106-107: Replace the concrete IP/port example
"https://192.168.139.133:30443/*" with a neutral placeholder to avoid accidental
copy-paste; update the redirect URI example referenced by oidcRedirectURL to
something like "https://<HOST>:<PORT>/*" or "https://your-domain.example/*" so
readers must substitute their actual host and port.
- Around line 214-215: Replace the existing note line that reads "Note: make
sure to install LWS (Alauda Build of LeaderWorkerSet) plugin before deploying
kubeflow-trainer, as LWS is a dependency of kubeflow-trainer." with a tighter,
sentence-case phrasing for consistency; update the wording to something like
"Note: install LWS (Alauda Build of LeaderWorkerSet) before deploying
kubeflow-trainer, as it is a dependency." to use sentence case, remove
redundancy, and keep the meaning intact.
Deploying alauda-ai with
|
| Latest commit: |
ad6378a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://07bd896c.alauda-ai.pages.dev |
| Branch Preview URL: | https://fix-kubeflow-docs.alauda-ai.pages.dev |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/en/installation/kubeflow.mdx (1)
22-27:⚠️ Potential issue | 🟡 MinorClarify the required
kubectlcluster context before fetching the certLine 22 says this is for the Global cluster, but Line 25 can still run against whatever context is currently active. Please explicitly instruct switching to the Global cluster first to avoid pulling the wrong
dex.tlssecret.Suggested doc tweak
-Obtain the platform dex CA certificate for later use in **Global** cluster: +Switch `kubectl` to the **Global** cluster, then obtain the platform dex CA certificate for later use: ```bash +kubectl config use-context <global-cluster-context> crt=$(kubectl get secret -n cpaas-system dex.tls -o jsonpath='{.data.tls\.crt}') echo -n $crt | base64 -d</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/en/installation/kubeflow.mdxaround lines 22 - 27, Clarify that the
kubectl commands must run against the Global cluster by adding an explicit step
to switch context before fetching the dex.tls secret: instruct the reader to run
kubectl config use-context (replace placeholder with
their Global cluster context) prior to the kubectl get secret -n cpaas-system
dex.tls ... and base64 decode steps so the dex.tls secret is retrieved from the
intended Global cluster context.</details> </blockquote></details> </blockquote></details>🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed. Outside diff comments: In `@docs/en/installation/kubeflow.mdx`: - Around line 22-27: Clarify that the kubectl commands must run against the Global cluster by adding an explicit step to switch context before fetching the dex.tls secret: instruct the reader to run kubectl config use-context <global-cluster-context> (replace placeholder with their Global cluster context) prior to the kubectl get secret -n cpaas-system dex.tls ... and base64 decode steps so the dex.tls secret is retrieved from the intended Global cluster context.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/en/installation/kubeflow.mdx (1)
126-145: YAML syntax and structure are correct.The new ServiceAccount and RoleBinding resources are properly formatted. Minor observation: this code block (and the one at line 110) uses plain triple backticks while the YAML block at line 33 uses
```yaml. Consider adding theyamllanguage identifier for syntax highlighting consistency.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/installation/kubeflow.mdx` around lines 126 - 145, The Markdown code fences for the Kubernetes resources (the ServiceAccount named "default-editor" and the RoleBinding referencing ClusterRole "kubeflow-edit" in namespace "kubeflow-admin-cpaas-io") need the YAML language identifier for consistent syntax highlighting; locate the triple-backtick fences that wrap the block containing the ServiceAccount/RoleBinding (and the similar block near the earlier example) and change ``` to ```yaml so the fenced blocks explicitly specify the yaml language.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/en/installation/kubeflow.mdx`:
- Around line 126-145: The Markdown code fences for the Kubernetes resources
(the ServiceAccount named "default-editor" and the RoleBinding referencing
ClusterRole "kubeflow-edit" in namespace "kubeflow-admin-cpaas-io") need the
YAML language identifier for consistent syntax highlighting; locate the
triple-backtick fences that wrap the block containing the
ServiceAccount/RoleBinding (and the similar block near the earlier example) and
change ``` to ```yaml so the fenced blocks explicitly specify the yaml language.
|
/test-pass |
* fix kubeflow install doc * update * update
2 participants
Summary by CodeRabbit