-
Notifications
You must be signed in to change notification settings - Fork 9
add k8s rbac & project api #320
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: master
Are you sure you want to change the base?
Conversation
WalkthroughThe PR introduces comprehensive Kubernetes API documentation across multiple API groups (Connector, Project, RBAC, ServiceAccount, User) with corresponding MDX index and resource pages. It also adds a CustomResourceDefinition for the User resource in the auth.alauda.io group. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Areas requiring attention:
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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 |
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: 3
🧹 Nitpick comments (1)
docs/shared/crds/auth.alauda.io_users.yaml (1)
107-107: Inconsistent timestamp field format.Line 107:
last_login_timeis typed as string with no format specification. For consistency and validation, consider specifyingformat: date-timeto ensure proper timestamp validation and serialization, aligning with theexpiredfield structure (lines 71-76).Apply this diff for consistency:
last_login_time: + format: date-time type: string
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
docs/en/apis/kubernetes_apis/connector/connector.mdx(1 hunks)docs/en/apis/kubernetes_apis/connector/index.mdx(1 hunks)docs/en/apis/kubernetes_apis/project/index.mdx(1 hunks)docs/en/apis/kubernetes_apis/project/project.mdx(1 hunks)docs/en/apis/kubernetes_apis/rbac/clusterrole.mdx(1 hunks)docs/en/apis/kubernetes_apis/rbac/clusterrolebinding.mdx(1 hunks)docs/en/apis/kubernetes_apis/rbac/index.mdx(1 hunks)docs/en/apis/kubernetes_apis/rbac/role.mdx(1 hunks)docs/en/apis/kubernetes_apis/rbac/rolebinding.mdx(1 hunks)docs/en/apis/kubernetes_apis/serviceaccount/index.mdx(1 hunks)docs/en/apis/kubernetes_apis/serviceaccount/serviceaccount.mdx(1 hunks)docs/en/apis/kubernetes_apis/user/index.mdx(1 hunks)docs/en/apis/kubernetes_apis/user/user.mdx(1 hunks)docs/shared/crds/auth.alauda.io_users.yaml(1 hunks)
⏰ 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). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (12)
docs/en/apis/kubernetes_apis/connector/index.mdx (1)
1-6: Empty YAML front matter may omit necessary metadata.The file has empty front matter delimiters (lines 1-2). Typically, MDX pages include metadata such as
title,sidebar_label, or custom fields. Verify if this is intentional per your documentation template standards or if metadata should be populated.docs/en/apis/kubernetes_apis/connector/connector.mdx (1)
1-5: LGTM!The Connector API documentation is well-structured with appropriate OpenAPIPath entries for listing and retrieving individual connectors.
docs/en/apis/kubernetes_apis/rbac/rolebinding.mdx (1)
1-5: LGTM!The RoleBinding documentation correctly uses namespace-scoped paths with pathPrefix for cluster context. The API endpoints are accurate for the resource type.
docs/en/apis/kubernetes_apis/rbac/clusterrole.mdx (1)
1-5: LGTM!The ClusterRole documentation correctly uses cluster-scoped paths (no namespace segment) with appropriate pathPrefix. The API endpoints are accurate.
docs/en/apis/kubernetes_apis/project/index.mdx (1)
1-6: Empty YAML front matter may omit necessary metadata.Similar to other index files, this page has empty front matter. Verify if metadata (e.g., title, sidebar_label) should be populated per your documentation standards.
docs/en/apis/kubernetes_apis/user/index.mdx (1)
1-6: Empty YAML front matter may omit necessary metadata.This file follows the same pattern as other index files with empty front matter. Confirm whether metadata should be populated per your documentation template standards.
docs/en/apis/kubernetes_apis/serviceaccount/index.mdx (1)
1-6: Empty YAML front matter may omit necessary metadata.Consistent with other index pages in this PR, this file has empty front matter. Verify if metadata should be included per your documentation standards.
docs/en/apis/kubernetes_apis/rbac/clusterrolebinding.mdx (1)
1-5: Fix incorrect API paths for cluster-scoped resource.ClusterRoleBinding is a cluster-scoped Kubernetes resource and should not include
/namespaces/{namespace}/in its API paths. The current paths are incompatible with Kubernetes API conventions. Correct paths should omit the namespace segment, matching the structure used inclusterrole.mdx.Apply this diff to correct the API paths:
# ClusterRoleBinding [rbac.authorization.k8s.io/v1] -<OpenAPIPath path="/apis/rbac.authorization.k8s.io/v1/namespaces/{namespace}/clusterrolebindings" pathPrefix="/kubernetes/{cluster}" /> +<OpenAPIPath path="/apis/rbac.authorization.k8s.io/v1/clusterrolebindings" pathPrefix="/kubernetes/{cluster}" /> -<OpenAPIPath path="/apis/rbac.authorization.k8s.io/v1/namespaces/{namespace}/clusterrolebindings/{name}" pathPrefix="/kubernetes/{cluster}" /> +<OpenAPIPath path="/apis/rbac.authorization.k8s.io/v1/clusterrolebindings/{name}" pathPrefix="/kubernetes/{cluster}" />Likely an incorrect or invalid review comment.
docs/en/apis/kubernetes_apis/rbac/index.mdx (1)
1-2: Empty YAML front matter with no metadata.The front matter contains only delimiters (
---\n---) with no key-value pairs. If metadata (e.g., sidebar labels, description) is needed, populate it; otherwise, consider removing the front matter entirely for clarity.docs/en/apis/kubernetes_apis/serviceaccount/serviceaccount.mdx (1)
1-5: Documentation structure looks good.Consistent with standard Kubernetes API documentation pattern. ServiceAccount entries are properly namespaced with the
/kubernetes/{cluster}path prefix, and no custom CRD declaration is needed.docs/en/apis/kubernetes_apis/rbac/role.mdx (1)
1-5: Documentation structure is consistent and well-formed.Standard Kubernetes RBAC Role endpoints are properly documented with cluster path prefix. No issues identified.
docs/en/apis/kubernetes_apis/user/user.mdx (1)
1-6: Documentation structure is consistent with custom API pattern.User API documentation follows the established pattern for custom APIs under the auth.alauda.io group. OpenAPI paths and CRD declaration are properly configured.
| # Project [auth.alauda.io/v1] | ||
| <OpenAPIPath path="/apis/auth.alauda.io/v1/projects" /> | ||
|
|
||
| <OpenAPIPath path="/apis/auth.alauda.io/v1/projects/{name}" /> | ||
|
|
||
| <K8sCrd name="projects.auth.alauda.io" /> |
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.
🧩 Analysis chain
Verify corresponding CRD file exists for projects.auth.alauda.io.
Line 6 references a CRD declaration for projects.auth.alauda.io, but the corresponding CRD YAML file is not included in this PR for review. Ensure docs/shared/crds/auth.alauda.io_projects.yaml exists and is properly formatted.
🏁 Script executed:
#!/bin/bash
# Verify the Project CRD file exists and is valid YAML
fd -e yaml docs/shared/crds | grep -i project
cat docs/shared/crds/auth.alauda.io_projects.yaml 2>/dev/null | head -20Length of output: 369
🏁 Script executed:
#!/bin/bash
# Verify the Project CRD file exists using corrected fd syntax
echo "=== Searching for Project CRD files ==="
fd --full-path 'auth.alauda.io_projects.yaml' docs/shared/crds 2>/dev/null
echo "=== Checking if docs/shared/crds directory exists ==="
ls -la docs/shared/crds 2>&1 | head -30
echo "=== Listing all YAML files in CRDs directory ==="
fd . docs/shared/crds -e yaml 2>&1 | head -20Length of output: 2568
The CRD file for projects.auth.alauda.io is missing.
Line 6 references <K8sCrd name="projects.auth.alauda.io" />, but the corresponding CRD YAML file docs/shared/crds/auth.alauda.io_projects.yaml does not exist in the repository. The docs/shared/crds/ directory contains 16 CRD files, including auth.alauda.io_users.yaml, but no projects CRD. Either create the missing CRD file or remove the K8sCrd reference if it is not yet implemented.
🤖 Prompt for AI Agents
In docs/en/apis/kubernetes_apis/project/project.mdx around lines 1-6, the page
references a CRD via <K8sCrd name="projects.auth.alauda.io" /> but the
corresponding YAML file docs/shared/crds/auth.alauda.io_projects.yaml is
missing; either add that CRD YAML to docs/shared/crds/ (matching naming
convention and contents of other CRDs) and commit it, or remove/replace the
<K8sCrd ... /> tag from this MDX if the CRD is not ready, ensuring the page
builds without broken references.
| description: Extra contains additional arbitrary metadata for the user from | ||
| third-party systems | ||
| type: object | ||
| x-kubernetes-preserve-unknown-fields: true |
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.
Broad use of x-kubernetes-preserve-unknown-fields at multiple levels.
The CRD uses x-kubernetes-preserve-unknown-fields: true at both the spec level (line 85) and root level (line 134). While this enables flexibility for third-party integrations (noted in the extra field comment), it also:
- Allows arbitrary fields that may mask schema validation issues
- Increases the risk of unintended data acceptance
- Makes schema evolution harder to track
Consider narrowing the preserve-unknown-fields scope to only the extra field if possible, or document the rationale for the broader application.
Also applies to: 134-134
| old_password: | ||
| type: string | ||
| password: | ||
| type: string |
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.
The password and old_password fields are stored as plain strings in the User resource spec. Kubernetes resources are persisted in etcd by default, which is not the appropriate place for credential storage. Credentials should be managed via Kubernetes Secrets.
Consider:
- Removing password fields from the spec and managing them separately via Secrets.
- If passwords must be included, ensure etcd encryption and access controls are strictly configured.
- Document the security implications and expected access restrictions.
Deploying alauda-container-platform with
|
| Latest commit: |
6594ef3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c527a7c6.alauda-container-platform.pages.dev |
| Branch Preview URL: | https://cherry-pick-ait-62790-to-mas.alauda-container-platform.pages.dev |
Summary by CodeRabbit