[dashboard] sync with upstream & enhancements - #1603
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR removes the TenantSecretsTable API type and REST storage, updates apiserver registration and RBAC, adds OpenAPI-derived multilineString schema generation for custom forms (with tests), updates dashboard UI/resource names from tenantsecretstables to tenantsecrets, adjusts UI patches and Dockerfile build refs, and refreshes dashboard configuration and images. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Dashboard UI
participant Ctrl as customformsoverride Controller
participant OpenAPI as OpenAPI JSON
participant K8s as Kubernetes API
UI->>Ctrl: ensureCustomFormsOverride()
Ctrl->>OpenAPI: parse openAPISchema (string)
alt parse OK
OpenAPI-->>Ctrl: properties map
Ctrl->>Ctrl: processSpecProperties() (recursive)
Ctrl-->>Ctrl: convert eligible "string" -> "multilineString"
Ctrl->>K8s: CreateOrUpdate with generated schema
K8s-->>Ctrl: success
else parse error / empty
OpenAPI-->>Ctrl: error/empty
Ctrl->>K8s: CreateOrUpdate with empty schema (fallback)
K8s-->>Ctrl: success
end
Ctrl-->>UI: override applied
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
💤 Files with no reviewable changes (8)
🚧 Files skipped from review as they are similar to previous changes (4)
🔇 Additional comments (6)
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 |
Summary of ChangesHello Andrei Kvapil (@kvaps), I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on synchronizing the dashboard components with upstream changes and enhancing the user interface's interaction with OpenAPI schemas. It streamlines resource management by removing the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces several enhancements and refactorings for the dashboard. Key changes include replacing the virtual TenantSecretsTable resource with a flatMap column on TenantSecret for a better UI experience, and adding a feature to automatically render string fields in forms as multiline text areas. The PR also includes updates to sync with upstream dependencies.
My review has identified a critical merge conflict in a patch file that must be resolved. I've also included several suggestions to improve code clarity, robustness, and adherence to Go best practices.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pkg/generated/openapi/zz_generated.openapi.gois excluded by!**/generated/**
📒 Files selected for processing (18)
internal/controller/dashboard/customformsoverride.go(2 hunks)internal/controller/dashboard/customformsoverride_test.go(1 hunks)internal/controller/dashboard/factory.go(1 hunks)internal/controller/dashboard/static_helpers.go(2 hunks)internal/controller/dashboard/static_refactored.go(1 hunks)packages/apps/tenant/templates/tenant.yaml(0 hunks)packages/system/dashboard/images/openapi-ui-k8s-bff/Dockerfile(1 hunks)packages/system/dashboard/images/openapi-ui/Dockerfile(2 hunks)packages/system/dashboard/images/openapi-ui/openapi-k8s-toolkit/patches/additional-properties-types.diff(0 hunks)packages/system/dashboard/images/openapi-ui/openapi-ui/patches/namespaces.diff(2 hunks)packages/system/dashboard/images/openapi-ui/openapi-ui/patches/remove-inside-link.diff(0 hunks)packages/system/dashboard/templates/web.yaml(2 hunks)packages/system/dashboard/values.yaml(1 hunks)pkg/apis/core/v1alpha1/register.go(1 hunks)pkg/apis/core/v1alpha1/tenantsecretstable_types.go(0 hunks)pkg/apis/core/v1alpha1/zz_generated.deepcopy.go(0 hunks)pkg/apiserver/apiserver.go(0 hunks)pkg/registry/core/tenantsecretstable/rest.go(0 hunks)
💤 Files with no reviewable changes (7)
- pkg/apiserver/apiserver.go
- packages/system/dashboard/images/openapi-ui/openapi-ui/patches/remove-inside-link.diff
- packages/apps/tenant/templates/tenant.yaml
- pkg/apis/core/v1alpha1/zz_generated.deepcopy.go
- pkg/apis/core/v1alpha1/tenantsecretstable_types.go
- pkg/registry/core/tenantsecretstable/rest.go
- packages/system/dashboard/images/openapi-ui/openapi-k8s-toolkit/patches/additional-properties-types.diff
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-14T16:58:01.478Z
Learnt from: nbykov0
Repo: cozystack/cozystack PR: 932
File: packages/system/dashboard/images/kubeapps-apis/Dockerfile:15-17
Timestamp: 2025-05-14T16:58:01.478Z
Learning: When working with Dockerfiles for cross-architecture builds in the kubeapps-apis component, the `TARGETOS` and `TARGETARCH` build arguments should be declared without default values so that Docker's buildx system can automatically set these variables based on the specified target platform.
Applied to files:
packages/system/dashboard/images/openapi-ui/Dockerfilepackages/system/dashboard/images/openapi-ui-k8s-bff/Dockerfile
📚 Learning: 2025-05-14T16:54:50.368Z
Learnt from: nbykov0
Repo: cozystack/cozystack PR: 932
File: packages/apps/kubernetes/images/cluster-autoscaler/Dockerfile:3-4
Timestamp: 2025-05-14T16:54:50.368Z
Learning: In Dockerfiles for the cozystack project, ARG ordering where a variable like ${TARGETARCH} is referenced before being declared can still work correctly in practice because Docker's BuildKit may evaluate the variable only when it's actually used in a FROM statement.
Applied to files:
packages/system/dashboard/images/openapi-ui-k8s-bff/Dockerfile
⏰ 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: Build
🔇 Additional comments (11)
pkg/apis/core/v1alpha1/register.go (1)
66-66: LGTM!The log message accurately reflects the removal of TenantSecretsTable and correctly lists the remaining static kinds being registered.
packages/system/dashboard/images/openapi-ui-k8s-bff/Dockerfile (1)
6-6: LGTM!The commit reference update aligns with the upstream sync in PR objectives. The build structure and process remain sound.
packages/system/dashboard/images/openapi-ui/Dockerfile (2)
20-20: Update the comment to reflect the actual source repository.Line 20 states the import is from
cozystack/openapi-ui, but line 26 usesPRO-Robotech/openapi-ui. Update the comment for clarity, or confirm whether this repository change is intentional.Also applies to: 26-26
22-22: Verify that disabling patch application is correct and won't break the build.Patches for the openapi-ui stage are now commented out (lines 28–29), and git is no longer installed in the builder stage (line 22). This change aligns with the source repository change but could break the build if the patches are still required. Confirm that the new openapi-ui source from PRO-Robotech does not require these patches or that they are no longer compatible.
Also applies to: 28-29
packages/system/dashboard/templates/web.yaml (2)
97-98: Verify HIDE_INSIDE is a documented and supported configuration option.A new environment variable
HIDE_INSIDEwith value"true"is being added to the OpenAPI UI web container. This appears to be a new feature or configuration flag.Ensure that:
- The OpenAPI UI image ({{ .Values.openapiUI.image | quote }}) supports this environment variable
- The intended behavior of
HIDE_INSIDE=trueis documented and aligns with the PR's dashboard enhancement objectives
47-50: Original review comment is based on incorrect assumptions about environment variable formats.The changes follow standard Kubernetes and OpenAPI UI conventions. Kubernetes env vars are strings—pass the boolean as the literal string "true" or "false" (lowercase) and the app should parse it as a boolean. The lowercase format (
"true"and"false") is correct and standard; no case sensitivity handling issues exist.No consuming code in the codebase references these variables—they are container environment variables. The LOGGER_WITH_HEADERS change to
"false"is an intentional configuration adjustment to disable header logging, not a breaking change requiring verification. HIDE_INSIDE is a recognized OpenAPI UI configuration option.Likely an incorrect or invalid review comment.
packages/system/dashboard/values.yaml (1)
2-2: Image digests for OpenAPI UI components are properly formatted and configured.All three image references have been validated: SHA256 digests are correctly formatted (64 hexadecimal characters), YAML syntax is valid, and corresponding Dockerfiles exist in the repository. The digest pinning follows security best practices for reproducible deployments.
internal/controller/dashboard/static_helpers.go (2)
125-138: LGTM - Resource rename aligned with API consolidation.The resource name update from
tenantsecretstablestotenantsecretsis consistent with the broader API refactor described in the PR summary. The existing column configuration logic is preserved correctly.
1049-1056: Manual verification needed: Confirm frontend dashboard supports flatMap column type.The
createFlatMapColumnhelper is implemented correctly and follows the established pattern. However, the frontend code that processes the "flatMap" column type could not be located in the codebase searches. The usage at line 179 ofstatic_refactored.goshows explicit column declarations with synthetic field names (_flatMapData_Key,_flatMapData_Value) alongside the flatMap column, but without access to the frontend dashboard processing logic, we cannot confirm:
- The dashboard correctly handles the "flatMap" column type
- The synthetic fields are properly expanded and rendered
Please verify that the dashboard frontend (likely in a separate frontend repository or compiled assets) correctly processes flatMap columns and generates the expected field expansion for the tenant secrets data table.
internal/controller/dashboard/factory.go (1)
287-310: LGTM - API endpoint and customization ID updated consistently.The
secretsTabfunction correctly updates both thefetchUrlandcustomizationIdto referencetenantsecretsinstead oftenantsecretstables, aligning with the API resource consolidation across the codebase.internal/controller/dashboard/static_refactored.go (1)
176-183: No issues found; flatMap implementation is properly integrated.The
createFlatMapColumnfunction is correctly implemented and the changes to tenantsecrets display follow the established codebase patterns. The customizationId references match betweenfactory.goandstatic_refactored.go, and the configuration properly expands secret data objects into separate rows via the flatMap column with synthetic field references_flatMapData_Keyand_flatMapData_Value. This is backend dashboard configuration code; the original verification request for frontend-dependent components does not apply to this codebase structure.
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
897d85f to
b45f4a6
Compare
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release-0.37
git worktree add -d .worktree/backport-1603-to-release-0.37 origin/release-0.37
cd .worktree/backport-1603-to-release-0.37
git switch --create backport-1603-to-release-0.37
git cherry-pick -x b45f4a6545b7835c9521c8e0f6904ce450b95eae |
Signed-off-by: Andrei Kvapil <kvapss@gmail.com> <!-- Thank you for making a contribution! Here are some tips for you: - Start the PR title with the [label] of Cozystack component: - For system components: [platform], [system], [linstor], [cilium], [kube-ovn], [dashboard], [cluster-api], etc. - For managed apps: [apps], [tenant], [kubernetes], [postgres], [virtual-machine] etc. - For development and maintenance: [tests], [ci], [docs], [maintenance]. - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `backport` if it's a bugfix that needs to be backported to a previous version. --> - Move patches to upstream: `namespaces` and `hide inside` - Introduce flatMap logic - Remove `tenantsecretstables` resource - Extend dashboard-controller to specify `multilineString` for any string without enum in spec (previusly it was for all strings) <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same [label] as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note [dashboard] sync with upstream & enhancements ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **New Features** * Enhanced OpenAPI form handling: string fields now better support multiline input. * **Improvements** * Secrets UI and API alignment: secrets display and data keys updated for consistency. * Form generation improved for nested objects and arrays. * Deployment defaults adjusted (logger flags normalized; inside feature hidden via env). * **Removed** * Removed the "Inside" header menu item and the legacy secrets-table API/resource. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Signed-off-by: Andrei Kvapil kvapss@gmail.com
What this PR does
namespacesandhide insidetenantsecretstablesresourcemultilineStringfor any string without enum in spec (previusly it was for all strings)Release note
Summary by CodeRabbit
New Features
Improvements
Removed