Skip to content

Add kubeflow deploy doc#95

Open
typhoonzero wants to merge 6 commits intomasterfrom
add_kubeflow_deploy_doc
Open

Add kubeflow deploy doc#95
typhoonzero wants to merge 6 commits intomasterfrom
add_kubeflow_deploy_doc

Conversation

@typhoonzero
Copy link
Contributor

@typhoonzero typhoonzero commented Feb 6, 2026

Summary by CodeRabbit

  • Documentation
    • Added a comprehensive Kubeflow installation and deployment guide for Alauda AI v2+, covering Chart Plugins (kfbase, kfp, kftraining deprecation, kubeflow-trainer, model registry), environment prerequisites, OAuth2-proxy/Dex redirection, Istio ext-authz notes, RBAC/AuthorizationPolicy and namespace binding guidance, component onboarding with YAML examples, step-by-step deployment/uninstallation flows, and post-deployment UI and access considerations.

@coderabbitai
Copy link

coderabbitai bot commented Feb 6, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a new installation guide docs/en/installation/kubeflow.mdx documenting Kubeflow Chart Plugins (kfbase, chart-kubeflow-model-registry, kfp, kftraining/kubeflow-trainer), environment prerequisites, OAuth2 proxy / Dex / IstioExtAuthz notes, component onboarding with violet, deployment procedures, and RBAC/AuthorizationPolicy guidance.

Changes

Cohort / File(s) Summary
Kubeflow Installation Guide
docs/en/installation/kubeflow.mdx
New comprehensive documentation covering Kubeflow Chart Plugins for Alauda AI v2+, prerequisites, OAuth2 proxy (dex) and IstioExtAuthz integration, cluster patch steps, component onboarding via violet, deployment workflows for kfbase/kfp/kftraining(kubeflow-trainer)/chart-kubeflow-model-registry, RBAC/namespace binding, and post-deployment notes and caveats.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • zhaomingkun1030

Poem

🐰 I hop through docs with bright intent,
Plugins lined up, each step well-meant,
Dex and violet guide the way,
Namespaces bloom where configs stay,
Deployments hum — a rabbit's content.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add kubeflow deploy doc' directly and specifically describes the main change—adding documentation for Kubeflow deployment—which matches the changeset that introduces a comprehensive Kubeflow installation guide.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add_kubeflow_deploy_doc

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Fix all issues with AI agents
In `@docs/en/installation/kubeflow.mdx`:
- Line 176: Replace the hard-coded namespace string
"cluster.local/ns/kubeflow-new/sa/ml-pipeline-ui" with a clear placeholder (for
example "cluster.local/ns/<kubeflow-namespace>/sa/ml-pipeline-ui") or add a
brief clarifying sentence near that occurrence explaining what "kubeflow-new"
refers to (i.e., the Kubeflow installation namespace) so readers know to
substitute their environment-specific namespace; update the text around the
symbol to use the placeholder or the explanatory note consistently.
- Line 19: The line that references an internal Confluence link for obtaining
the platform Dex CA certificate must be replaced with publicly accessible
guidance: remove or replace the confluence.alauda.cn URL and either (1) inline
the necessary steps to obtain the platform Dex CA certificate (a short
step-by-step list explaining where to find/export the CA cert from the platform
and any CLI commands or UI paths), or (2) link to a public documentation page or
official upstream source that provides the same instructions; update the
sentence mentioning "platform dex CA certificate" accordingly so external
readers can follow the steps without access to internal resources.
- Around line 183-185: The docs use two inconsistent names for the same plugin —
"kftrainer" and "kftraining" — which will confuse users; pick one canonical
identifier and update the occurrences accordingly (or explicitly state that
"kftrainer" is the display name and "kftraining" is the package name). Edit the
text referencing kftrainer/kftraining in the Kubeflow section (lines referencing
"kftrainer", "kftraining", "KubeflowTrainingOperator", and "Cluster Plugins") so
all mentions either use the unified name or include a parenthetical
clarification like "kftrainer (display name) / kftraining (package name)" to
make the relationship clear to users searching the Cluster Plugins UI.
- Line 21: Single-line summary: Replace the non-actionable "please consult
relevant colleagues" with actionable guidance for ServiceMesh v2 configuration.
Update the sentence in the docs (the paragraph referencing Platform Management
-> Resource Management and the spec field for ServiceMesh v1) to either (a) link
to the ServiceMesh v2 configuration guide, (b) inline the v2 steps if short and
stable, or (c) specify the exact internal team or channel to contact (e.g.,
Platform Team docs or Slack channel), so external readers have a clear next
step; ensure the text references "ServiceMesh v1" and "ServiceMesh v2"
explicitly and that the link or contact replaces the current phrase.
- Around line 33-51: The PEM block beginning with "-----BEGIN CERTIFICATE-----"
and ending with "-----END CERTIFICATE-----" contains a real CA certificate;
replace that entire PEM block with a clearly labeled placeholder (e.g.,
"-----BEGIN CERTIFICATE-----\n<PLACEHOLDER CERTIFICATE FOR DOCUMENTATION
ONLY>\n-----END CERTIFICATE-----") and add a short inline note that this is a
dummy/example certificate for documentation purposes only so no real PKI
material is published.
🧹 Nitpick comments (2)
docs/en/installation/kubeflow.mdx (2)

122-131: Add yaml language identifier to fenced code blocks for syntax highlighting.

This code block (and the one at line 137) contains YAML but lacks a language identifier, which means no syntax highlighting will be applied. Add yaml to improve readability:

-```
+```yaml
 apiVersion: kubeflow.org/v1beta1
 kind: Profile

Same applies to the code block starting at line 137.


108-108: Consider using a placeholder instead of a hardcoded example IP.

The redirect URI uses a hardcoded IP (192.168.139.133:30665) that differs from the example platform address on line 82 (192.168.171.123). While these may intentionally refer to different clusters, using a placeholder like <your-cluster-address>:<nodeport> (with an explanatory note) would be clearer and more consistent.

Comment on lines 33 to 51
-----BEGIN CERTIFICATE-----
MIIDKzCCAhOgAwIBAgIRAK9C9PuDXtYFvybudWQkN4UwDQYJKoZIhvcNAQELBQAw
EDEOMAwGA1UEChMFY3BhYXMwHhcNMjUwMzEwMDkxODAzWhcNMzUwMzA4MDkxODAz
WjASMRAwDgYDVQQKEwdrdWJlLWNhMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB
CgKCAQEAmChGjtwWOPvj0Ca3TkuPxxx6jg4oDTAPqyowT2pcaVeNhFwoMmCCkFXm
7brFKXCc7IE1kHq5dbRCn+UwCA46g7zvz8b7SY/0qRymwTlYqRILDZacwWHUSJSD
cDyK297V+Ig5oIno6fTa2FWSJBqyxqivZ3lzf1XpsiwSPPXol+LclUne0fDiM98C
dBQWKDYadwlcluuPUHULthA3OjcKGpmyV7cyTHPcRjBSmkAmuL0bQhbWhkB8G9oe
4cp2joo/qVsSzeUepkHeTD9PPk1AZ59FE8DDgL0FRREE7vou6g7fbOZL98pC4ldg
ZIY/EB5v38uR6J25uzLPFSf75vbwHwIDAQABo34wfDAOBgNVHQ8BAf8EBAMCBaAw
DAYDVR0TAQH/BAIwADAfBgNVHSMEGDAWgBQk8E8JWyAANbALLaeAxZ17adgq/TA7
BgNVHREENDAyggVjcGFhc4ILZXhhbXBsZS5jb22HBH8AAAGHEAAAAAAAAAAAAAAA
AAAAAAGHBMCoq/MwDQYJKoZIhvcNAQELBQADggEBAIXo0V2jMeRd4cw5p3FWoFno
VWno7Cy7ENvVjgfQymcWbGi6fXWvkDBUPCmqv5bosUVyAOJ/p92g861nCAo3jxoZ
voCTDN4xU+t0xs2hMTKHsSB7v3n18rBtqcVpUvm1it/NyeOU4HiYfPTPkRVugGf4
gtYknrU6Skt9BkiNy+2Jcsb6V3mAJ5GQzbT0qPL1vKWkBB9oCbjMwJggsW+TdKgY
KJuII0m6JNDUlKLCazLL8OvXq84Nu+cJ6QaNOT0gBRIWSPA+UbAsibbFnf0VOeeU
WforZLredR6GKc2qMdKdcW4G+8fRSWcx0gEIRquoQH1P7yIEJ3xOGoxQfIRVpls=
-----END CERTIFICATE-----
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Embedded certificate appears to be a real CA certificate — use a placeholder instead.

This PEM block looks like an actual internal CA certificate rather than a placeholder. Publishing real certificates in documentation (especially in a public repo) is a security concern — it leaks internal PKI details and could aid targeted attacks. Replace it with a clearly marked placeholder, e.g.:

      - path: spec.values.pilot.jwksResolverExtraRootCA
        value: |
          -----BEGIN CERTIFICATE-----
-          MIIDKzCCAhOgAwIBAgIRAK9C9PuDXtYFvybudWQkN4UwDQYJKoZIhvcNAQELBQAw
-          ...
-          WforZLredR6GKc2qMdKdcW4G+8fRSWcx0gEIRquoQH1P7yIEJ3xOGoxQfIRVpls=
+          <YOUR_DEX_CA_CERTIFICATE_BASE64_HERE>
          -----END CERTIFICATE-----
🤖 Prompt for AI Agents
In `@docs/en/installation/kubeflow.mdx` around lines 33 - 51, The PEM block
beginning with "-----BEGIN CERTIFICATE-----" and ending with "-----END
CERTIFICATE-----" contains a real CA certificate; replace that entire PEM block
with a clearly labeled placeholder (e.g., "-----BEGIN
CERTIFICATE-----\n<PLACEHOLDER CERTIFICATE FOR DOCUMENTATION ONLY>\n-----END
CERTIFICATE-----") and add a short inline note that this is a dummy/example
certificate for documentation purposes only so no real PKI material is
published.


## required for kubeflow pipelines
## TEMPLATE: "cluster.local/ns/<KUBEFLOW_NAMESPACE>/sa/<KFP_UI_SERVICE_ACCOUNT>"
- "cluster.local/ns/kubeflow-new/sa/ml-pipeline-ui"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

kubeflow-new looks like an environment-specific namespace — should this be a placeholder?

The principal on line 176 references kubeflow-new as the namespace (cluster.local/ns/kubeflow-new/sa/ml-pipeline-ui), but the rest of the document doesn't mention this namespace. If this is meant to be the Kubeflow installation namespace, consider using a placeholder like <kubeflow-namespace> or documenting what kubeflow-new refers to, so users configure it correctly for their environment.

🤖 Prompt for AI Agents
In `@docs/en/installation/kubeflow.mdx` at line 176, Replace the hard-coded
namespace string "cluster.local/ns/kubeflow-new/sa/ml-pipeline-ui" with a clear
placeholder (for example
"cluster.local/ns/<kubeflow-namespace>/sa/ml-pipeline-ui") or add a brief
clarifying sentence near that occurrence explaining what "kubeflow-new" refers
to (i.e., the Kubeflow installation namespace) so readers know to substitute
their environment-specific namespace; update the text around the symbol to use
the placeholder or the explanatory note consistently.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 6, 2026

Deploying alauda-ai with  Cloudflare Pages  Cloudflare Pages

Latest commit: e7a0ac6
Status: ✅  Deploy successful!
Preview URL: https://32f4f4cd.alauda-ai.pages.dev
Branch Preview URL: https://add-kubeflow-deploy-doc.alauda-ai.pages.dev

View logs

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@docs/en/installation/kubeflow.mdx`:
- Line 19: Capitalize the first word of the sentence "obtain the platform dex CA
certificate for later use:" so it reads "Obtain the platform dex CA certificate
for later use:"; update this sentence in docs/en/installation/kubeflow.mdx to
start with an uppercase "O".
🧹 Nitpick comments (5)
docs/en/installation/kubeflow.mdx (5)

85-91: Consider using a placeholder for the IP address.

192.168.171.123 appears to be an environment-specific address. Using a placeholder like <platform-address> would be more consistent with the <platform_password> placeholder already used on the same line, and clearer for readers who need to substitute their own values.

📝 Proposed fix
-violet push --platform-address="https://192.168.171.123" \
+violet push --platform-address="https://<platform_address>" \

109-114: Replace hardcoded IP in redirect URI with a placeholder.

192.168.139.133:30665 is environment-specific. Use a placeholder so readers know to substitute their own value, consistent with the comment on line 112.

📝 Proposed fix
      redirectURIs:
      - ...
      # Add the following line, note: the redirect address must be consistent with oidcRedirectURL in values:
-      - https://192.168.139.133:30665/*
+      - https://<your-kubeflow-host>:<port>/*

127-136: Add yaml language specifier to the code block.

This code block contains a Kubernetes YAML manifest but lacks a language identifier, which will result in no syntax highlighting. The same issue applies to the code block at line 142.

📝 Proposed fix
-```
+```yaml
 apiVersion: kubeflow.org/v1beta1
 kind: Profile

142-186: Add yaml language specifier to the code block.

Same as the earlier block — this YAML manifest should have the yaml language identifier for proper syntax highlighting.

📝 Proposed fix
-```
+```yaml
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding

138-138: Rephrase the heading to be more concise.

Using a conditional sentence as a section heading is unusual and verbose. Consider a shorter, scannable heading.

📝 Proposed fix
-### 3. If binding user to an already created namespace, you also need to complete the following configuration:
+### 3. Bind User to an Existing Namespace

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@docs/en/installation/kubeflow.mdx`:
- Line 70: The X-Auth-Request-Redirect header hardcodes the http scheme which
can downgrade HTTPS deployments; update the header value for
X-Auth-Request-Redirect to use https://%REQ(Host)%%REQ(:PATH)% (or a
protocol-agnostic/existing proxy variable if available) and add a short note in
the surrounding text explaining when to use https vs http and that the scheme
must match the platform's public URL; locate the header line containing
"X-Auth-Request-Redirect" and adjust the value accordingly and update the nearby
examples that assume HTTPS to remain consistent.
🧹 Nitpick comments (6)
docs/en/installation/kubeflow.mdx (6)

5-8: Inconsistent capitalization of "Kubeflow" in plugin descriptions.

Line 5 capitalizes "Kubeflow base components" while lines 6–8 use lowercase "kubeflow". Consider capitalizing consistently for a polished look.

📝 Proposed fix
-- kfmr: kubeflow model registry instance (Helm Chart)
-- kfp: kubeflow pipeline
-- kftraining: kubeflow training operator (deprecated)
+- kfmr: Kubeflow model registry instance (Helm Chart)
+- kfp: Kubeflow pipeline
+- kftraining: Kubeflow training operator (deprecated)

85-91: Use placeholders instead of hardcoded IP/email in the example command.

Lines 87–88 use a specific IP address and email that differ from other examples in the doc (e.g., line 113 uses a different IP). Despite the comment on line 86, using explicit placeholders would be clearer and avoid users accidentally copying the wrong address.

📝 Proposed fix
 # Note: replace your platform addr, username, password and cluster name.
-violet push --platform-address="https://192.168.171.123" \
-  --platform-username="admin@cpaas.io" \
+violet push --platform-address="https://<PLATFORM_ADDRESS>" \
+  --platform-username="<PLATFORM_USERNAME>" \
   --platform-password="<platform_password>" \
   <your downloaded plugin package file>

109-114: Use a placeholder for the environment-specific redirect URI.

The redirect URI on line 113 uses a hardcoded IP and port. Replace with a placeholder so users don't accidentally copy the wrong address.

📝 Proposed fix
       redirectURIs:
       - ...
       # Add the following line, note: the redirect address must be consistent with oidcRedirectURL in values:
-      - https://192.168.139.133:30665/*
+      - https://<KUBEFLOW_HOST>:<PORT>/*

127-136: Add yaml language identifier to the fenced code block for syntax highlighting.

The code block starting at line 127 lacks a language identifier. This also applies to the code block at line 142.

📝 Proposed fix (line 127)
-```
+```yaml
 apiVersion: kubeflow.org/v1beta1

138-138: Heading is overly long and reads like a sentence — consider shortening.

Section headings should be concise. Consider rephrasing:

-### 3. If binding user to an already created namespace, you also need to complete the following configuration:
+### 3. Bind a User to an Existing Namespace

Then move the explanatory text into the section body.


202-207: Capitalize "MySQL" as a proper product name.

Lines 203–205 use lowercase "mysql" in parameter descriptions. "MySQL" is a proper product name and should be capitalized consistently.

📝 Proposed fix
-- mysqlStorageClass: The mysql storage class used by Model Registry. Needs to be a storage class supported by the target deployment cluster.
-- mysqlStorageSize: The mysql storage size used by Model Registry.
-- mysqlDataBase: Database name (will be created automatically).
+- mysqlStorageClass: The MySQL storage class used by Model Registry. Needs to be a storage class supported by the target deployment cluster.
+- mysqlStorageSize: The MySQL storage size used by Model Registry.
+- mysqlDataBase: The database name (will be created automatically).

- x-auth-request-email
- x-auth-request-access-token
includeAdditionalHeadersInCheck:
X-Auth-Request-Redirect: http://%REQ(Host)%%REQ(:PATH)%
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n docs/en/installation/kubeflow.mdx | sed -n '50,130p'

Repository: alauda/aml-docs

Length of output: 4012


Redirect scheme is hardcoded to http:// — should this be https://?

The X-Auth-Request-Redirect header uses http:// but the rest of the document uses HTTPS URLs (lines 87, 113). If the platform is served over HTTPS, this would redirect the user to an HTTP URL after authentication, potentially breaking the OAuth flow or causing a security downgrade.

Consider using https:// or, if the scheme varies, documenting when each should be used:

Suggested diff
-              X-Auth-Request-Redirect: http://%REQ(Host)%%REQ(:PATH)%
+              X-Auth-Request-Redirect: https://%REQ(Host)%%REQ(:PATH)%
🤖 Prompt for AI Agents
In `@docs/en/installation/kubeflow.mdx` at line 70, The X-Auth-Request-Redirect
header hardcodes the http scheme which can downgrade HTTPS deployments; update
the header value for X-Auth-Request-Redirect to use
https://%REQ(Host)%%REQ(:PATH)% (or a protocol-agnostic/existing proxy variable
if available) and add a short note in the surrounding text explaining when to
use https vs http and that the scheme must match the platform's public URL;
locate the header line containing "X-Auth-Request-Redirect" and adjust the value
accordingly and update the nearby examples that assume HTTPS to remain
consistent.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@docs/en/installation/kubeflow.mdx`:
- Around line 94-98: Update the intro paragraph to either list the "volcano"
plugin alongside the other plugins or explicitly mark it as optional;
specifically reference the "volcano" plugin and clarify it is only required when
enabling volcano scheduler support for "kftraining" so readers know it's not
mandatory for all Kubeflow onboardings. Ensure the introduction text that
enumerates plugins includes either the volcano entry or a parenthetical note
linking volcano to optional kftraining scheduler support.
- Around line 110-121: Update the documentation block that instructs editing the
ConfigMap under cpaas-system: specify the exact ConfigMap name (e.g., "dex" or
"dex-config") so users can find it, remove the duplicate "note" so the line
reads only "Note:", and replace the hardcoded host/port in the redirectURIs
example (currently "192.168.139.133:30665") with a placeholder like
"<KUBEFLOW_HOST>:<PORT>", while keeping the reference to oidcRedirectURL and the
redirectURIs key unchanged.
🧹 Nitpick comments (2)
docs/en/installation/kubeflow.mdx (2)

134-143: Add yaml language identifier to the code block.

This code block contains a Kubernetes YAML manifest but lacks a language identifier. Adding yaml enables syntax highlighting and improves readability in the rendered documentation.

-```
+```yaml
 apiVersion: kubeflow.org/v1beta1
 kind: Profile

The same applies to the code block starting at line 149.


145-147: Section heading reads as a conditional clause rather than a title.

"3. If binding user to an already created namespace, you also need to complete the following configuration:" is awkward as a heading. Consider rephrasing for clarity:

-### 3. If binding user to an already created namespace, you also need to complete the following configuration:
+### 3. Bind a User to an Existing Namespace

The conditional context can be explained in the body text instead.

Comment on lines 94 to 98
- kfbase: Kubeflow Base functionality
- chart-kubeflow-model-registry: Kubeflow Model Registry
- kfp: Kubeflow Pipeline functionality
- kftraining: Kubeflow Training Operator (deprecated)
- volcano: volcano scheduler
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

volcano plugin not mentioned in the introduction.

Line 98 lists volcano: volcano scheduler as a plugin to onboard, but the introduction (lines 5–8) doesn't mention it. Either add it to the introduction or clarify that it's an optional dependency (only needed when enabling volcano scheduler support for kftraining, per line 100).

🤖 Prompt for AI Agents
In `@docs/en/installation/kubeflow.mdx` around lines 94 - 98, Update the intro
paragraph to either list the "volcano" plugin alongside the other plugins or
explicitly mark it as optional; specifically reference the "volcano" plugin and
clarify it is only required when enabling volcano scheduler support for
"kftraining" so readers know it's not mandatory for all Kubeflow onboardings.
Ensure the introduction text that enumerates plugins includes either the volcano
entry or a parenthetical note linking volcano to optional kftraining scheduler
support.

Comment on lines +110 to +121
In **Administrator** - **Clusters** - **Resources**, select `Global` cluster,
find the `ConfigMap` resource in the `cpaas-system` namespace, and click the
edit button to add the following configuration under `redirectURIs`:

> **Note: note: the redirect host and port must be the same with `oidcRedirectURL` configured when installing the "Kubeflow Base" plugin.**

```yaml
redirectURIs:
- ...
# Add the following line,
- https://192.168.139.133:30665/*
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing ConfigMap name, hardcoded IP, and duplicate "note" in this section.

Three issues in this block:

  1. Line 111: Which ConfigMap? There are likely many in cpaas-system. Specify the resource name (e.g., dex or dex-config) so users can locate it.
  2. Line 114: "Note: note:" — double "note" with extra whitespace is a typo. Should be just "Note:".
  3. Line 120: The IP 192.168.139.133:30665 is hardcoded. Use a placeholder like <KUBEFLOW_HOST>:<PORT> to avoid confusion with the other hardcoded IP on line 88.
Suggested fixes
-> **Note:  note: the redirect host and port must be the same with `oidcRedirectURL` configured when installing the "Kubeflow Base" plugin.**
+> **Note: The redirect host and port must match the `oidcRedirectURL` configured when installing the "Kubeflow Base" plugin.**
-      - https://192.168.139.133:30665/*
+      - https://<KUBEFLOW_HOST>:<PORT>/*
🤖 Prompt for AI Agents
In `@docs/en/installation/kubeflow.mdx` around lines 110 - 121, Update the
documentation block that instructs editing the ConfigMap under cpaas-system:
specify the exact ConfigMap name (e.g., "dex" or "dex-config") so users can find
it, remove the duplicate "note" so the line reads only "Note:", and replace the
hardcoded host/port in the redirectURIs example (currently
"192.168.139.133:30665") with a placeholder like "<KUBEFLOW_HOST>:<PORT>", while
keeping the reference to oidcRedirectURL and the redirectURIs key unchanged.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@docs/en/installation/kubeflow.mdx`:
- Line 91: Replace the hardcoded IP in the example command using the same
placeholder used elsewhere: change the argument to use a placeholder (e.g.,
<PLATFORM_ADDRESS>) in the "violet push --platform-address" example so it reads
"violet push --platform-address=\"<PLATFORM_ADDRESS>\" ..." ensuring it matches
the placeholder referenced on the previous line and avoids embedding
environment-specific IPs.
🧹 Nitpick comments (3)
docs/en/installation/kubeflow.mdx (3)

137-146: YAML code blocks missing language identifier.

These YAML blocks use bare ``` fences without a language hint (e.g., ```yaml). This will result in no syntax highlighting in the rendered documentation.

Suggested fix (apply to both blocks)
-```
+```yaml
 apiVersion: kubeflow.org/v1beta1
-```
+```yaml
 apiVersion: rbac.authorization.k8s.io/v1

Also applies to: 152-196


1-10: Introduction doesn't mention volcano or LWS as optional/required dependencies.

Line 103 mentions volcano as needed for kftraining scheduler support, and line 17/230 mentions LWS as a dependency of kubeflow-trainer. Consider listing these in the introduction or adding a "Dependencies" subsection so users have a complete picture upfront.


148-148: Section heading style: numbered heading reads awkwardly as a conditional instruction.

### 3. If binding user to an already created namespace, you also need to complete the following configuration: is unusually long and conditional for a heading. Consider shortening it, e.g., ### 3. Bind User to an Existing Namespace.


```bash
# Note: replace your platform addr, username, password and cluster name.
violet push --platform-address="https://192.168.171.123" \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Hardcoded IP address in the example command.

192.168.171.123 is a specific environment IP. Consider using a placeholder like <PLATFORM_ADDRESS> consistent with the instruction on line 90.

Suggested fix
-violet push --platform-address="https://192.168.171.123" \
+violet push --platform-address="https://<PLATFORM_ADDRESS>" \
🤖 Prompt for AI Agents
In `@docs/en/installation/kubeflow.mdx` at line 91, Replace the hardcoded IP in
the example command using the same placeholder used elsewhere: change the
argument to use a placeholder (e.g., <PLATFORM_ADDRESS>) in the "violet push
--platform-address" example so it reads "violet push
--platform-address=\"<PLATFORM_ADDRESS>\" ..." ensuring it matches the
placeholder referenced on the previous line and avoids embedding
environment-specific IPs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant