[dashboard] refactor dashboard configuration - #1457
Conversation
WalkthroughInitializes and labels dashboard static resources at manager startup, materializes static/dynamic dashboard CRs, cleans up orphaned dashboard resources during reconcile, refactors dashboard UI helpers into a unified factory, removes legacy dashboard-config Helm templates, and updates image digests and controller REST config (QPS/Burst). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant SETUP as Reconciler.SetupWithManager
participant MGR as controller.Manager
participant START as Manager.Runnable
participant DASH as Dashboard.Manager
participant K8S as Kubernetes API
Note over SETUP,MGR: SetupWithManager registers controller and startup runnable
SETUP->>MGR: add Runnable(initializeStaticResourcesOnce)
MGR->>START: run Runnable at startup
START->>DASH: InitializeStaticResources(ctx)
DASH->>DASH: CreateAllStaticResources()
loop each static resource
DASH->>K8S: CreateOrUpdate(static obj with dashboard labels)
K8S-->>DASH: upsert result
end
DASH-->>START: return (errors logged)
sequenceDiagram
autonumber
participant REC as Reconciler.Reconcile
participant DASH as Dashboard.Manager
participant K8S as Kubernetes API
Note over REC,DASH: After CRD reconciliation, cleanup is invoked
REC->>DASH: CleanupOrphanedResources(ctx)
DASH->>K8S: List resources matching dashboard-managed labels
DASH->>DASH: Build expected resource set from CRDs
loop each listed object
alt not in expected set
DASH->>K8S: Delete object
K8S-->>DASH: Delete result
else
Note right of DASH: keep object
end
end
DASH-->>REC: return (errors logged)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 introduces a significant architectural refactoring for dashboard configuration within 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 is a significant refactoring that moves the dashboard configuration from Helm charts into the Go controller. This is a commendable effort towards improving maintainability and testability. The code is now better structured with dedicated helper files. However, I have identified several issues, including a critical bug in the orphaned resource cleanup logic that could lead to data loss in the dashboard. There are also other high and medium severity issues related to resource updates, sorting logic, and concurrency patterns that need to be addressed to ensure the refactoring is robust and correct.
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
internal/controller/dashboard/sidebar.go (2)
101-106: Fix sort to match stated spec (Weight desc, then Label A→Z).The comparator currently sorts weight ascending, contradicting the comment/rules. Use descending for Weight.
Apply this diff:
- sort.Slice(categories[cat], func(i, j int) bool { - if categories[cat][i].Weight != categories[cat][j].Weight { - return categories[cat][i].Weight < categories[cat][j].Weight // lower weight first - } - return strings.ToLower(categories[cat][i].Label) < strings.ToLower(categories[cat][j].Label) - }) + sort.Slice(categories[cat], func(i, j int) bool { + if categories[cat][i].Weight != categories[cat][j].Weight { + return categories[cat][i].Weight > categories[cat][j].Weight // higher weight first (desc) + } + return strings.ToLower(categories[cat][i].Label) < strings.ToLower(categories[cat][j].Label) + })
114-126: Bring implementation in line with comment: add "Tenant Info" under Marketplace.Header says Marketplace section has two hardcoded entries, but only one is present. Add Tenant Info here (it’s duplicated under Administration as “Info”; choose one place or keep both intentionally).
Apply this diff:
menuItems := []any{ map[string]any{ "key": "marketplace", "label": "Marketplace", "children": []any{ map[string]any{ "key": "marketplace", "label": "Marketplace", "link": "/openapi-ui/{clusterName}/{namespace}/factory/marketplace", }, + map[string]any{ + "key": "tenant-info", + "label": "Tenant Info", + "link": "/openapi-ui/{clusterName}/{namespace}/factory/info-details/info", + }, }, }, }internal/controller/dashboard/customcolumns.go (1)
148-164: Return the actual OperationResult from CreateOrUpdate.The function discards op and always returns OperationResultNone, which can break reconciliation decisions upstream.
Apply this diff:
- _, err := controllerutil.CreateOrUpdate(ctx, m.client, obj, func() error { + op, err := controllerutil.CreateOrUpdate(ctx, m.client, obj, func() error { if err := controllerutil.SetOwnerReference(crd, obj, m.scheme); err != nil { return err } // Add dashboard labels to dynamic resources m.addDashboardLabels(obj, crd, ResourceTypeDynamic) b, err := json.Marshal(desired["spec"]) if err != nil { return err } obj.Spec = dashv1alpha1.ArbitrarySpec{JSON: apiextv1.JSON{Raw: b}} return nil }) - // Return OperationResultCreated/Updated is not available here with unstructured; we can mimic Updated when no error. - return controllerutil.OperationResultNone, err + return op, errinternal/controller/dashboard/factory.go (1)
355-388: keysOrder sorting never matches due to “spec.” prefix; also not stable
- orderMap keys strip "spec", but fields use JSONPath including "spec." → no matches.
- sort.Slice is not stable; fallback should preserve original order.
Fix by trimming "spec." on lookup and using SliceStable.
Apply this diff:
- // Sort fields based on their position in keysOrder - sort.Slice(fields, func(i, j int) bool { - posI, existsI := orderMap[fields[i].JSONPathSpec] - posJ, existsJ := orderMap[fields[j].JSONPathSpec] - - // If both exist in orderMap, sort by position - if existsI && existsJ { - return posI < posJ - } - // If only one exists, prioritize the one that exists - if existsI { - return true - } - if existsJ { - return false - } - // If neither exists, maintain original order (stable sort) - return i < j - }) + // Sort fields based on their position in keysOrder (stable; preserve original order otherwise) + sort.SliceStable(fields, func(i, j int) bool { + keyI := strings.TrimPrefix(fields[i].JSONPathSpec, "spec.") + keyJ := strings.TrimPrefix(fields[j].JSONPathSpec, "spec.") + posI, existsI := orderMap[keyI] + posJ, existsJ := orderMap[keyJ] + if existsI && existsJ { + return posI < posJ + } + if existsI { + return true + } + if existsJ { + return false + } + return false + })
🧹 Nitpick comments (15)
packages/system/dashboard/values.yaml (1)
2-4: Avoid:latesttags; prefer immutable, versioned tags (keep digest).Using
:latest@sha256:...can cause confusion during rollbacks and audit. Prefer a semver or dated tag with the digest for reproducibility.Please confirm these digests correspond to the intended release builds of openapi-ui and openapi-ui-k8s-bff. If you want, I can provide a small script to validate the manifests on GHCR.
internal/controller/dashboard/helpers.go (1)
25-45: Consider sourcing group/version from CRD instead of hardcoded defaults.pickGVK defaults to apps.cozystack.io/v1alpha1. If CRDs vary, prefer deriving group/version from the CRD to avoid mismatches.
Please confirm whether CRDs may use non-default group/version. If yes, I can adjust pickGVK to read them from Spec (or another reliable field) and update callers accordingly.
internal/controller/dashboard/sidebar.go (1)
57-59: Tighten typing of keysAndTags for clarity.Use map[string][]string to better reflect intent and avoid any casts downstream. JSON marshaling remains identical.
Apply this diff:
- categories := map[string][]item{} // category label -> children - keysAndTags := map[string]any{} // plural -> []string{ "<lower(kind)>-sidebar" } + categories := map[string][]item{} // category label -> children + keysAndTags := map[string][]string{} // plural -> ["<lower(kind)>-sidebar"]- // keysAndTags: plural -> [ "<lower(kind)>-sidebar" ] - keysAndTags[plural] = []any{fmt.Sprintf("%s-sidebar", strings.ToLower(kind))} + // keysAndTags: plural -> ["<lower(kind)>-sidebar"] + keysAndTags[plural] = []string{fmt.Sprintf("%s-sidebar", strings.ToLower(kind))}Also applies to: 95-97
internal/controller/cozystackresource_controller.go (1)
101-133: Consider sync.Once for one-time init.You can replace bool+mutex with sync.Once to simplify and prevent misuse.
internal/controller/dashboard/static_processor.go (1)
29-34: Redundant pre-labeling before CreateOrUpdate.Labeling before CreateOrUpdate is overwritten on update path; labeling inside mutate is sufficient.
Apply this diff:
- // Add dashboard labels to static resources - m.addDashboardLabels(resource, nil, ResourceTypeStatic) + // Labels will be applied inside mutate to both create and update pathsinternal/controller/dashboard/ui_helpers.go (4)
11-11: Tighten type: use string for id in contentCardWithTitleid is always treated as a string elsewhere; keeping it typed avoids accidental non‑string IDs.
Apply this diff:
-func contentCardWithTitle(id any, title string, style map[string]any, children []any) map[string]any { +func contentCardWithTitle(id string, title string, style map[string]any, children []any) map[string]any {
140-149: Auto-generate missing IDs for columns (consistency with other helpers)Other helpers auto-generate IDs; do the same here to keep IDs deterministic.
Apply this diff:
-func antdCol(id string, span float64, children []any) map[string]any { - return map[string]any{ +func antdCol(id string, span float64, children []any) map[string]any { + // Auto-generate ID if not provided + if id == "" { + id = generateContainerID("auto", "col") + } + return map[string]any{ "type": "antdCol", "data": map[string]any{ "id": id, "span": span, }, "children": children, } }
151-160: Auto-generate missing IDs for columns with styleMirror the ID auto-generation here as well.
Apply this diff:
-func antdColWithStyle(id string, style map[string]any, children []any) map[string]any { - return map[string]any{ +func antdColWithStyle(id string, style map[string]any, children []any) map[string]any { + // Auto-generate ID if not provided + if id == "" { + id = generateContainerID("auto", "col") + } + return map[string]any{ "type": "antdCol", "data": map[string]any{ "id": id, "style": style, }, "children": children, } }
183-196: Normalize numeric types in badge style to float64Elsewhere you standardize numeric fields as float64; keep this consistent to avoid type drift.
Apply this diff:
- "fontWeight": 400, + "fontWeight": float64(400), "lineHeight": "24px", - "minWidth": 24, + "minWidth": float64(24),internal/controller/dashboard/marketplacepanel.go (1)
72-83: Include spec.id for MarketplacePanel payloadOther specs include an id for determinism/consumers. Add one based on the resource name for consistency.
Apply this diff:
specMap := map[string]any{ + "id": generateSpecID(mp.Name), "description": d.Description, "name": displayName, "type": "nonCrd", "apiGroup": "apps.cozystack.io", "apiVersion": "v1alpha1", "typeName": app.Plural, // e.g., "buckets" "disabled": false, "hidden": false, "tags": tags, "icon": d.Icon, }Please confirm that the front-end’s MarketplacePanel schema expects/permits id on the root spec.
internal/controller/dashboard/static_helpers.go (3)
738-741: Normalize numeric types: gap should be float64Align with the float64 convention used elsewhere to avoid type inconsistency at render time.
Apply this diff:
- "gap": 6, + "gap": float64(6),And in createCustomColumnWithoutJsonPath:
- "gap": 6, + "gap": float64(6),Also applies to: 793-795
455-457: Handle JSON marshaling errors instead of ignoring themSwallowing errors can silently produce empty specs.
I recommend returning (obj, error) from these builders and bubbling json.Marshal errors. If you prefer to keep signatures, at least log on error and return a zero-value JSON (e.g., {}).
Also applies to: 476-478, 497-499
700-718: Consider migrating headers/tabs to unified helpers for consistencyThis legacy createFactorySpec path duplicates unified factory construction; prefer the unified approach where possible.
internal/controller/dashboard/unified_helpers.go (2)
21-31: Also normalize underscores in generateIDYou already provide sanitizeForID below; bring underscore replacement into generateID to keep IDs consistent.
Apply this diff:
// Remove any special characters that might cause issues id = strings.ReplaceAll(id, ".", "-") id = strings.ReplaceAll(id, "/", "-") id = strings.ReplaceAll(id, " ", "-") + id = strings.ReplaceAll(id, "_", "-")
263-271: Unused parameter in createResourceWithAutoIDresourceType isn’t used. Either incorporate it into the ID or drop the parameter to avoid confusion.
Would you like a follow-up PR to remove it after checking call sites?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (86)
internal/controller/cozystackresource_controller.go(4 hunks)internal/controller/dashboard/breadcrumb.go(1 hunks)internal/controller/dashboard/customcolumns.go(1 hunks)internal/controller/dashboard/customformsoverride.go(1 hunks)internal/controller/dashboard/customformsprefill.go(1 hunks)internal/controller/dashboard/factory.go(5 hunks)internal/controller/dashboard/helpers.go(1 hunks)internal/controller/dashboard/manager.go(2 hunks)internal/controller/dashboard/marketplacepanel.go(1 hunks)internal/controller/dashboard/sidebar.go(1 hunks)internal/controller/dashboard/static_helpers.go(1 hunks)internal/controller/dashboard/static_processor.go(1 hunks)internal/controller/dashboard/static_refactored.go(1 hunks)internal/controller/dashboard/tableurimapping.go(1 hunks)internal/controller/dashboard/ui_helpers.go(1 hunks)internal/controller/dashboard/unified_helpers.go(1 hunks)internal/controller/dashboard/webextras.go(0 hunks)packages/core/platform/bundles/paas-full.yaml(0 hunks)packages/core/platform/bundles/paas-hosted.yaml(0 hunks)packages/system/cozystack-controller/values.yaml(1 hunks)packages/system/dashboard-config/Chart.yaml(0 hunks)packages/system/dashboard-config/Makefile(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/configmap-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/namespace-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/node-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/pod-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/secret-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/service-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-api.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-builtin.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-project-api.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-project-builtin.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.apps.cozystack.io.workloadmonitors.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.core.cozystack.io.tenantsecretstables.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-ingress-details-rules.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-node-images.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-pod-details-list.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-status-conditions.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/hidden.metadata.tpl(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/icons.tpl(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/k8s.container.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1alpha1.cozystack.io.workloads.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/storage.k8s.io.v1.storageclasses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.namespaces.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumeclaims.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/cozy-marketplace.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/annotations.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/counters.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/labels.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/links.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/statuses.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/tables.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/taints.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/times.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/tolerations.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/namespace-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/node-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/pod-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/secret-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/service-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/workloadmonitor-details.yaml(0 hunks)packages/system/dashboard-config/templates/Navigation/navigaton.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/apps.cozystack.io.v1alpha1.virtualmachines.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/core.cozystack.io.v1alpha1.tenantnamespaces.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.namespaces.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/_helpers.tpl(0 hunks)packages/system/dashboard/values.yaml(1 hunks)
💤 Files with no reviewable changes (68)
- packages/system/dashboard-config/templates/Factory/helpers/links.tpl
- packages/system/dashboard-config/templates/TableUriMapping/v1.nodes.yaml
- packages/system/dashboard-config/templates/Factory/helpers/times.tpl
- packages/system/dashboard-config/templates/Factory/helpers/taints.tpl
- packages/system/dashboard-config/templates/TableUriMapping/networking.k8s.io.v1.ingresses.yaml
- packages/core/platform/bundles/paas-full.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-project-api.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.configmaps.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-project-builtin.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.pods.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1.services.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.pods.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-api.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1alpha1.cozystack.io.workloads.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/namespace-details.yaml
- packages/core/platform/bundles/paas-hosted.yaml
- packages/system/dashboard-config/templates/Factory/helpers/tables.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/icons.tpl
- packages/system/dashboard-config/templates/Factory/helpers/tolerations.tpl
- packages/system/dashboard-config/templates/Factory/helpers/annotations.tpl
- packages/system/dashboard-config/templates/TableUriMapping/apps.cozystack.io.v1alpha1.virtualmachines.yaml
- internal/controller/dashboard/webextras.go
- packages/system/dashboard-config/templates/CustomFormOverride/v1.configmaps.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.services.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/configmap-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.configmaps.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/hidden.metadata.tpl
- packages/system/dashboard-config/templates/_helpers.tpl
- packages/system/dashboard-config/templates/Breadcrumb/factory/node-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-pod-details-list.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-builtin.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.nodes.yaml
- packages/system/dashboard-config/templates/Factory/namespace-details.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.secrets.yaml
- packages/system/dashboard-config/templates/Factory/helpers/counters.tpl
- packages/system/dashboard-config/templates/CustomFormOverride/storage.k8s.io.v1.storageclasses.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-node-images.yaml
- packages/system/dashboard-config/templates/Factory/helpers/labels.tpl
- packages/system/dashboard-config/templates/TableUriMapping/v1.namespaces.yaml
- packages/system/dashboard-config/templates/Factory/service-details.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumeclaims.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.services.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.nodes.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumes.yaml
- packages/system/dashboard-config/Makefile
- packages/system/dashboard-config/templates/Breadcrumb/factory/pod-details.yaml
- packages/system/dashboard-config/templates/Factory/node-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-ingress-details-rules.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.namespaces.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/service-details.yaml
- packages/system/dashboard-config/templates/Factory/helpers/statuses.tpl
- packages/system/dashboard-config/templates/Navigation/navigaton.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.secrets.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.pods.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.apps.cozystack.io.workloadmonitors.yaml
- packages/system/dashboard-config/Chart.yaml
- packages/system/dashboard-config/templates/Factory/cozy-marketplace.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/secret-details.yaml
- packages/system/dashboard-config/templates/Factory/pod-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-status-conditions.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.core.cozystack.io.tenantsecretstables.yaml
- packages/system/dashboard-config/templates/TableUriMapping/core.cozystack.io.v1alpha1.tenantnamespaces.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.secrets.yaml
- packages/system/dashboard-config/templates/Factory/secret-details.yaml
- packages/system/dashboard-config/templates/Factory/workloadmonitor-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/k8s.container.yaml
🧰 Additional context used
🧬 Code graph analysis (15)
internal/controller/dashboard/sidebar.go (1)
internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(29-29)
internal/controller/dashboard/ui_helpers.go (1)
internal/controller/dashboard/unified_helpers.go (2)
BadgeSizeMedium(95-95)BadgeSizeLarge(96-96)
internal/controller/dashboard/customformsprefill.go (2)
internal/controller/dashboard/manager.go (2)
Manager(39-43)ResourceTypeDynamic(29-29)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/customformsoverride.go (2)
internal/controller/dashboard/manager.go (2)
Manager(39-43)ResourceTypeDynamic(29-29)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/static_helpers.go (1)
internal/controller/dashboard/unified_helpers.go (3)
BadgeSizeMedium(95-95)BadgeConfig(83-88)BadgeSizeLarge(96-96)
internal/controller/dashboard/customcolumns.go (1)
internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(29-29)
internal/controller/dashboard/breadcrumb.go (2)
internal/controller/dashboard/manager.go (2)
Manager(39-43)ResourceTypeDynamic(29-29)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/cozystackresource_controller.go (1)
internal/controller/dashboard/manager.go (2)
NewManager(54-60)WithCRDListFunc(49-51)
internal/controller/dashboard/marketplacepanel.go (2)
internal/controller/dashboard/manager.go (2)
Manager(39-43)ResourceTypeDynamic(29-29)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/manager.go (2)
api/v1alpha1/cozystackresourcedefinitions_types.go (2)
CozystackResourceDefinition(27-32)CozystackResourceDefinitionList(37-41)internal/controller/dashboard/static_refactored.go (1)
CreateAllStaticResources(1678-1712)
internal/controller/dashboard/factory.go (2)
internal/controller/dashboard/unified_helpers.go (3)
UnifiedResourceConfig(277-286)BadgeSizeLarge(96-96)BadgeSizeMedium(95-95)internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(29-29)
internal/controller/dashboard/static_processor.go (2)
internal/controller/dashboard/manager.go (2)
Manager(39-43)ResourceTypeStatic(28-28)internal/controller/dashboard/static_refactored.go (1)
CreateAllStaticResources(1678-1712)
internal/controller/dashboard/static_refactored.go (1)
internal/controller/dashboard/unified_helpers.go (2)
UnifiedResourceConfig(277-286)BadgeSizeLarge(96-96)
internal/controller/dashboard/tableurimapping.go (2)
internal/controller/dashboard/manager.go (1)
Manager(39-43)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/helpers.go (1)
api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
⏰ 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 (4)
packages/system/cozystack-controller/values.yaml (1)
2-2: Controller image bump looks fine; verify compatibility and availability.Confirm this digest exists on GHCR and that the controller version is compatible with the k8s/controller-runtime versions in this repo.
Would you like me to generate a quick script to check the GHCR manifest and list labels for this digest?
internal/controller/dashboard/customcolumns.go (1)
25-36: Helper functions present in internal/controller/dashboard/helpers.go Verified pickGVK, pickPlural, initialsFromKind, and hexColorForKind are defined in the same package.internal/controller/dashboard/tableurimapping.go (1)
9-13: Stub is safe and wired correctly
Only invoked by EnsureForCRD and links are managed by CustomColumnsOverride; no changes needed.internal/controller/dashboard/unified_helpers.go (1)
205-226: Verify 8-digit hex colors are supported by the UI rendererSome environments don’t support #RRGGBBAA; confirm front-end support or drop alpha.
| sort := make([]any, len(crd.Spec.Dashboard.KeysOrder)) | ||
| for i, v := range crd.Spec.Dashboard.KeysOrder { | ||
| sort[i] = v | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Nil-pointer risk when Dashboard is nil.
Accessing crd.Spec.Dashboard.KeysOrder without a nil check can panic.
Apply this diff:
- sort := make([]any, len(crd.Spec.Dashboard.KeysOrder))
- for i, v := range crd.Spec.Dashboard.KeysOrder {
- sort[i] = v
- }
+ var sort []any
+ if crd.Spec.Dashboard != nil && len(crd.Spec.Dashboard.KeysOrder) > 0 {
+ sort = make([]any, len(crd.Spec.Dashboard.KeysOrder))
+ for i, v := range crd.Spec.Dashboard.KeysOrder {
+ sort[i] = v
+ }
+ } else {
+ sort = []any{}
+ }Please add a test (or run an e2e) with a CRD lacking .spec.dashboard to confirm this path no longer panics.
🏁 Script executed:
#!/bin/bash
# Locate the definition of KeysOrder to inspect the Dashboard type
rg -n "KeysOrder" -g '*.go'Length of output: 906
🏁 Script executed:
#!/bin/bash
# Show Dashboard field definition in the CRD spec to verify if it's a pointer
rg -n -C3 "Dashboard" -g 'api/v1alpha1/cozystackresourcedefinitions_types.go'Length of output: 3307
Guard against nil crd.Spec.Dashboard before accessing KeysOrder
- internal/controller/dashboard/customformsoverride.go: wrap the KeysOrder access with a nil check (apply diff below)
- sort := make([]any, len(crd.Spec.Dashboard.KeysOrder))
- for i, v := range crd.Spec.Dashboard.KeysOrder {
- sort[i] = v
- }
+ var sort []any
+ if crd.Spec.Dashboard != nil && len(crd.Spec.Dashboard.KeysOrder) > 0 {
+ sort = make([]any, len(crd.Spec.Dashboard.KeysOrder))
+ for i, v := range crd.Spec.Dashboard.KeysOrder {
+ sort[i] = v
+ }
+ } else {
+ sort = []any{}
+ }- internal/controller/dashboard/factory.go (lines 30, 335): similarly guard
crd.Spec.Dashboard.KeysOrderagainst Dashboard being nil - Add a unit test or e2e with a CRD lacking
.spec.dashboardto confirm this no longer panics
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sort := make([]any, len(crd.Spec.Dashboard.KeysOrder)) | |
| for i, v := range crd.Spec.Dashboard.KeysOrder { | |
| sort[i] = v | |
| } | |
| // Replace unguarded access to crd.Spec.Dashboard.KeysOrder with a nil check | |
| var sort []any | |
| if crd.Spec.Dashboard != nil && len(crd.Spec.Dashboard.KeysOrder) > 0 { | |
| sort = make([]any, len(crd.Spec.Dashboard.KeysOrder)) | |
| for i, v := range crd.Spec.Dashboard.KeysOrder { | |
| sort[i] = v | |
| } | |
| } else { | |
| sort = []any{} | |
| } |
🤖 Prompt for AI Agents
internal/controller/dashboard/customformsoverride.go lines 40-43: code assumes
crd.Spec.Dashboard is non-nil when accessing KeysOrder which can panic; guard by
checking if crd.Spec != nil && crd.Spec.Dashboard != nil before iterating
KeysOrder and only allocate/assign sort when Dashboard and KeysOrder exist. Also
apply the same nil checks in internal/controller/dashboard/factory.go at lines
~30 and ~335 before using crd.Spec.Dashboard.KeysOrder. Finally add a unit or
e2e test creating a CRD without .spec.dashboard to ensure code returns
gracefully (no panic) and behaves as expected.
| "key": "WaitingdReason", | ||
| "value": "-", | ||
| }, | ||
| } |
There was a problem hiding this comment.
Typo in column keys: “WaitingdReason” → “WaitingReason”
Misspelling will break value mapping.
Apply this diff:
- "WaitingdReason",
+ "WaitingReason",(and the same fix in the init-containers block)
- "WaitingdReason",
+ "WaitingReason",Also applies to: 290-293
🤖 Prompt for AI Agents
In internal/controller/dashboard/static_helpers.go around lines 267-270 and
again around 290-293 (and the similar init-containers block), the column key is
misspelled as "WaitingdReason"; change it to "WaitingReason" so value mapping
works correctly — update both occurrences (and the mirror in the init-containers
section) to use the correct key name.
be9370d to
b82afa6
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/controller/dashboard/customcolumns.go (1)
40-43: Inconsistent placeholder: use {clusterName} (as elsewhere) instead of {2}.Sidebar and other links use {clusterName}/{namespace}. Make this consistent to avoid broken navigation.
Apply this diff:
- href := fmt.Sprintf("/openapi-ui/{2}/{reqsJsonPath[0]['.metadata.namespace']['-']}/factory/%s/{reqsJsonPath[0]['.metadata.name']['-']}", detailsSegment) + href := fmt.Sprintf("/openapi-ui/{clusterName}/{reqsJsonPath[0]['.metadata.namespace']['-']}/factory/%s/{reqsJsonPath[0]['.metadata.name']['-']}", detailsSegment)internal/controller/dashboard/sidebar.go (1)
101-106: Fix sort order: Weight should be descending (per comment), not ascending.Current code puts lower weights first.
Apply this diff:
- if categories[cat][i].Weight != categories[cat][j].Weight { - return categories[cat][i].Weight < categories[cat][j].Weight // lower weight first - } + if categories[cat][i].Weight != categories[cat][j].Weight { + return categories[cat][i].Weight > categories[cat][j].Weight // higher weight first + }
🧹 Nitpick comments (18)
internal/controller/dashboard/customcolumns.go (1)
148-163: Return the actual CreateOrUpdate result instead of OperationResultNone.Propagate the operation result for observability and tests.
Apply this diff:
- _, err := controllerutil.CreateOrUpdate(ctx, m.client, obj, func() error { + op, err := controllerutil.CreateOrUpdate(ctx, m.client, obj, func() error { if err := controllerutil.SetOwnerReference(crd, obj, m.scheme); err != nil { return err } // Add dashboard labels to dynamic resources m.addDashboardLabels(obj, crd, ResourceTypeDynamic) b, err := json.Marshal(desired["spec"]) if err != nil { return err } obj.Spec = dashv1alpha1.ArbitrarySpec{JSON: apiextv1.JSON{Raw: b}} return nil }) - // Return OperationResultCreated/Updated is not available here with unstructured; we can mimic Updated when no error. - return controllerutil.OperationResultNone, err + return op, errinternal/controller/dashboard/helpers.go (4)
3-13: Add missing import for sorting (deterministic spec generation).You’ll need sort for the next suggestions.
Apply this diff:
import ( "crypto/sha1" "encoding/hex" "encoding/json" "fmt" + "sort" "reflect" "regexp" "strings"
205-241: Deterministic traversal and avoid emitting nil values.
- Iterate properties in a stable order (sorted keys).
- Don’t emit entries with nil values even at top-level (unknown/unsupported types).
Apply this diff:
-func processSchemaProperties(props map[string]interface{}, path []string, values *[]interface{}, topLevel bool) { - for pname, raw := range props { +func processSchemaProperties(props map[string]interface{}, path []string, values *[]interface{}, topLevel bool) { + // stable order + keys := make([]string, 0, len(props)) + for k := range props { + keys = append(keys, k) + } + sort.Strings(keys) + for _, pname := range keys { + raw := props[pname] sub, _ := raw.(map[string]interface{}) if sub == nil { continue } @@ default: - // For primitive types, use default if present, otherwise zero value - val := defaultOrZero(sub) - // Only emit zero-value entries when at top level - if val != nil || topLevel { + val := defaultOrZero(sub) + // Only emit zero-values at top-level; skip nils entirely + if val != nil || (topLevel && val != nil) { entry := map[string]interface{}{ "path": toIfaceSlice(currentPath), "value": val, } *values = append(*values, entry) } } } }
243-260: Deterministic traversal for default objects.Sort keys for stable output.
Apply this diff:
func processDefaultObject(obj map[string]interface{}, path []string, values *[]interface{}) { - for key, value := range obj { + keys := make([]string, 0, len(obj)) + for k := range obj { + keys = append(keys, k) + } + sort.Strings(keys) + for _, key := range keys { + value := obj[key] currentPath := append(path, key) @@ } }
367-370: Humanize path by de-camel-casing and capitalizing segments.The current join doesn’t match the comment example.
Apply this diff:
func humanizePath(parts []string) string { - // "systemDisk.image" -> "System Disk / Image" - return strings.Join(parts, " / ") + // "systemDisk.image" -> "System Disk / Image" + out := make([]string, 0, len(parts)) + for _, p := range parts { + ws := splitCamel(p) + for i := range ws { + if ws[i] == "" { + continue + } + ws[i] = strings.ToUpper(ws[i][:1]) + ws[i][1:] + } + out = append(out, strings.Join(ws, " ")) + } + return strings.Join(out, " / ") }internal/controller/dashboard/ui_helpers.go (1)
140-160: Optional: auto-generate IDs for antdCol/antdColWithStyle for consistency.Other helpers auto-generate when id is empty.
Apply this diff:
func antdCol(id string, span float64, children []any) map[string]any { + if id == "" { + id = generateContainerID("auto", "col") + } return map[string]any{ @@ func antdColWithStyle(id string, style map[string]any, children []any) map[string]any { + if id == "" { + id = generateContainerID("auto", "col") + } return map[string]any{internal/controller/dashboard/static_processor.go (1)
32-34: Remove pre-mutation labeling; it’s overwritten by the GET in CreateOrUpdate.Set labels only inside the mutate function.
Apply this diff:
- // Add dashboard labels to static resources - m.addDashboardLabels(resource, nil, ResourceTypeStatic) - _, err := controllerutil.CreateOrUpdate(ctx, m.client, resource, func() error {internal/controller/dashboard/breadcrumb.go (1)
21-26: Confirm Breadcrumb id/name matches the target Factory page idBreadcrumb.Spec.id and the object name use “stock-project-factory-%s-details”, while Factory uses “%s-details”. If the UI expects these to match, the breadcrumb may not render on the details page.
Consider aligning the id to Factory’s naming:
- lowerKind := strings.ToLower(kind) - detailID := fmt.Sprintf("stock-project-factory-%s-details", lowerKind) + lowerKind := strings.ToLower(kind) + detailID := fmt.Sprintf("%s-details", lowerKind)Also applies to: 57-60
internal/controller/cozystackresource_controller.go (2)
78-84: Avoid running cleanup on every reconcileRunning CleanupOrphanedResources on each CRD reconcile can be expensive. Prefer a periodic Runnable (e.g., every few minutes) or rate-limit via a timestamp guard.
Apply a periodic manager Runnable that calls cleanup on a ticker; keep the reconcile path focused on the current CRD.
104-136: One-time static init: LGTM, consider sync.OnceThe mutex+bool works. Optionally replace with sync.Once for simpler semantics.
internal/controller/dashboard/customformsoverride.go (1)
33-38: Minor consistency nit: use []any uniformlyYou mix []any and []interface{} in the same slice construction. Prefer []any for consistency (Go 1.18+).
internal/controller/dashboard/factory.go (1)
49-57: Use a proper title instead of lowercased pluralLowercasing plural may degrade UX. Prefer titleFromKindPlural(kind, plural) or a provided Dashboard label.
Apply this diff:
- config := UnifiedResourceConfig{ + config := UnifiedResourceConfig{ Name: factoryName, ResourceType: "factory", Kind: kind, Plural: plural, - Title: strings.ToLower(plural), + Title: titleFromKindPlural(kind, plural), Size: BadgeSizeLarge, }internal/controller/dashboard/static_refactored.go (2)
556-560: Avoid nil tabs; prefer empty array.Some renderers treat null as invalid. Use [] instead of nil to avoid runtime UI issues.
- "items": nil, + "items": []any{},
1382-1383: Don’t ignore JSON marshal errors.Silently dropping errors risks producing invalid specs. Use a safe fallback.
- jsonData, _ := json.Marshal(newSpec) + jsonData, err := json.Marshal(newSpec) + if err != nil { + // Fallback: empty object to keep resource valid + jsonData = []byte("{}") + }internal/controller/dashboard/manager.go (1)
219-226: Harden resource kind detection for static resources.GroupVersionKind can be empty if not set; mirror the type-switch approach used in cleanupResourceType.
- for _, resource := range staticResources { - resourceType := resource.GetObjectKind().GroupVersionKind().Kind - if expected[resourceType] != nil { - expected[resourceType][resource.GetName()] = true - } - } + for _, resource := range staticResources { + var resourceKind string + switch resource.(type) { + case *dashv1alpha1.CustomColumnsOverride: + resourceKind = "CustomColumnsOverride" + case *dashv1alpha1.CustomFormsOverride: + resourceKind = "CustomFormsOverride" + case *dashv1alpha1.CustomFormsPrefill: + resourceKind = "CustomFormsPrefill" + case *dashv1alpha1.MarketplacePanel: + resourceKind = "MarketplacePanel" + case *dashv1alpha1.Sidebar: + resourceKind = "Sidebar" + case *dashv1alpha1.TableUriMapping: + resourceKind = "TableUriMapping" + case *dashv1alpha1.Breadcrumb: + resourceKind = "Breadcrumb" + case *dashv1alpha1.Factory: + resourceKind = "Factory" + } + if resourceKind != "" { + expected[resourceKind][resource.GetName()] = true + } + }internal/controller/dashboard/unified_helpers.go (1)
17-34: Reuse sanitizeForID in generateID.Reduces duplicated normalization logic and handles underscores consistently.
- // Join components with hyphens and convert to lowercase - id := strings.ToLower(strings.Join(components, "-")) - - // Remove any special characters that might cause issues - id = strings.ReplaceAll(id, ".", "-") - id = strings.ReplaceAll(id, "/", "-") - id = strings.ReplaceAll(id, " ", "-") - - // Remove multiple consecutive hyphens - for strings.Contains(id, "--") { - id = strings.ReplaceAll(id, "--", "-") - } - - // Remove leading/trailing hyphens - id = strings.Trim(id, "-") - - return id + raw := strings.Join(components, "-") + return sanitizeForID(raw)internal/controller/dashboard/static_helpers.go (2)
389-410: Deduplicate ingresses block.The “stock-namespace-networking.k8s.io.v1.ingresses” config appears twice with the same content. Keep one.
- if name == "stock-namespace-networking.k8s.io.v1.ingresses" { - data["additionalPrinterColumnsTrimLengths"] = []any{ - map[string]any{ - "key": "Name", - "value": float64(64), - }, - } - data["additionalPrinterColumnsUndefinedValues"] = []any{ - map[string]any{ - "key": "Hosts", - "value": "-", - }, - map[string]any{ - "key": "Address", - "value": "-", - }, - map[string]any{ - "key": "Port", - "value": "-", - }, - } - }Also applies to: 319-341
23-24: Handle JSON marshal errors with a safe fallback.Prevents silent failures when building specs.
Example for createBreadcrumb (apply similarly to others):
- jsonData, _ := json.Marshal(data) + jsonData, err := json.Marshal(data) + if err != nil { + jsonData = []byte("{}") + }Also applies to: 455-457, 476-478, 497-499
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (86)
internal/controller/cozystackresource_controller.go(5 hunks)internal/controller/dashboard/breadcrumb.go(1 hunks)internal/controller/dashboard/customcolumns.go(1 hunks)internal/controller/dashboard/customformsoverride.go(1 hunks)internal/controller/dashboard/customformsprefill.go(1 hunks)internal/controller/dashboard/factory.go(5 hunks)internal/controller/dashboard/helpers.go(1 hunks)internal/controller/dashboard/manager.go(2 hunks)internal/controller/dashboard/marketplacepanel.go(1 hunks)internal/controller/dashboard/sidebar.go(1 hunks)internal/controller/dashboard/static_helpers.go(1 hunks)internal/controller/dashboard/static_processor.go(1 hunks)internal/controller/dashboard/static_refactored.go(1 hunks)internal/controller/dashboard/tableurimapping.go(1 hunks)internal/controller/dashboard/ui_helpers.go(1 hunks)internal/controller/dashboard/unified_helpers.go(1 hunks)internal/controller/dashboard/webextras.go(0 hunks)packages/core/platform/bundles/paas-full.yaml(0 hunks)packages/core/platform/bundles/paas-hosted.yaml(0 hunks)packages/system/cozystack-controller/values.yaml(1 hunks)packages/system/dashboard-config/Chart.yaml(0 hunks)packages/system/dashboard-config/Makefile(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/configmap-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/namespace-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/node-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/pod-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/secret-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/service-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-api.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-builtin.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-project-api.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-project-builtin.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.apps.cozystack.io.workloadmonitors.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.core.cozystack.io.tenantsecretstables.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-ingress-details-rules.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-node-images.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-pod-details-list.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-status-conditions.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/hidden.metadata.tpl(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/icons.tpl(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/k8s.container.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1alpha1.cozystack.io.workloads.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/storage.k8s.io.v1.storageclasses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.namespaces.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumeclaims.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/cozy-marketplace.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/annotations.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/counters.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/labels.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/links.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/statuses.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/tables.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/taints.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/times.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/tolerations.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/namespace-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/node-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/pod-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/secret-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/service-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/workloadmonitor-details.yaml(0 hunks)packages/system/dashboard-config/templates/Navigation/navigaton.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/apps.cozystack.io.v1alpha1.virtualmachines.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/core.cozystack.io.v1alpha1.tenantnamespaces.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.namespaces.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/_helpers.tpl(0 hunks)packages/system/dashboard/values.yaml(1 hunks)
💤 Files with no reviewable changes (68)
- packages/system/dashboard-config/templates/Breadcrumb/factory/secret-details.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.namespaces.yaml
- packages/system/dashboard-config/templates/Factory/helpers/tables.tpl
- packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumes.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-ingress-details-rules.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.nodes.yaml
- packages/core/platform/bundles/paas-hosted.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.core.cozystack.io.tenantsecretstables.yaml
- packages/system/dashboard-config/templates/TableUriMapping/core.cozystack.io.v1alpha1.tenantnamespaces.yaml
- packages/core/platform/bundles/paas-full.yaml
- packages/system/dashboard-config/templates/Factory/helpers/taints.tpl
- packages/system/dashboard-config/templates/Factory/workloadmonitor-details.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/service-details.yaml
- packages/system/dashboard-config/templates/Factory/service-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-node-images.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-status-conditions.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.services.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-builtin.yaml
- packages/system/dashboard-config/templates/_helpers.tpl
- packages/system/dashboard-config/templates/TableUriMapping/v1.configmaps.yaml
- packages/system/dashboard-config/templates/Factory/helpers/links.tpl
- packages/system/dashboard-config/templates/Factory/node-details.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.nodes.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.pods.yaml
- packages/system/dashboard-config/templates/Factory/helpers/labels.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/icons.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/hidden.metadata.tpl
- packages/system/dashboard-config/templates/Breadcrumb/factory/namespace-details.yaml
- packages/system/dashboard-config/templates/Factory/pod-details.yaml
- packages/system/dashboard-config/templates/Navigation/navigaton.yaml
- packages/system/dashboard-config/templates/Factory/secret-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-pod-details-list.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.services.yaml
- packages/system/dashboard-config/templates/Factory/namespace-details.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.pods.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-api.yaml
- packages/system/dashboard-config/templates/TableUriMapping/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/Chart.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.secrets.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.secrets.yaml
- packages/system/dashboard-config/templates/Factory/helpers/annotations.tpl
- packages/system/dashboard-config/Makefile
- packages/system/dashboard-config/templates/TableUriMapping/v1.pods.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.secrets.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/pod-details.yaml
- packages/system/dashboard-config/templates/TableUriMapping/apps.cozystack.io.v1alpha1.virtualmachines.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.configmaps.yaml
- packages/system/dashboard-config/templates/Factory/helpers/statuses.tpl
- packages/system/dashboard-config/templates/Breadcrumb/factory/configmap-details.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-project-builtin.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.apps.cozystack.io.workloadmonitors.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/storage.k8s.io.v1.storageclasses.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumeclaims.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.namespaces.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.nodes.yaml
- packages/system/dashboard-config/templates/Factory/helpers/counters.tpl
- packages/system/dashboard-config/templates/Factory/cozy-marketplace.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/node-details.yaml
- packages/system/dashboard-config/templates/Factory/helpers/tolerations.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/k8s.container.yaml
- internal/controller/dashboard/webextras.go
- packages/system/dashboard-config/templates/Breadcrumb/stock-project-api.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.configmaps.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1.services.yaml
- packages/system/dashboard-config/templates/Factory/helpers/times.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1alpha1.cozystack.io.workloads.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/system/dashboard/values.yaml
- internal/controller/dashboard/tableurimapping.go
- internal/controller/dashboard/customformsprefill.go
- packages/system/cozystack-controller/values.yaml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-10T22:02:47.729Z
Learnt from: lllamnyp
PR: cozystack/cozystack#1380
File: internal/controller/kubeovnplunger/metrics.go:291-296
Timestamp: 2025-09-10T22:02:47.729Z
Learning: In the kubeovnplunger controller, r.lastLeader map is initialized in SetupWithManager method before any reconcile operations begin, making defensive nil checks unnecessary in the reconcile methods.
Applied to files:
internal/controller/cozystackresource_controller.go
📚 Learning: 2025-09-10T22:02:47.729Z
Learnt from: lllamnyp
PR: cozystack/cozystack#1380
File: internal/controller/kubeovnplunger/metrics.go:291-296
Timestamp: 2025-09-10T22:02:47.729Z
Learning: In the kubeovnplunger controller, r.lastLeader map is initialized in SetupWithManager method (line 151) with make(map[string]string) before any reconcile operations begin, making defensive nil checks unnecessary in the reconcile methods.
Applied to files:
internal/controller/cozystackresource_controller.go
🧬 Code graph analysis (13)
internal/controller/dashboard/sidebar.go (1)
internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(29-29)
internal/controller/dashboard/customformsoverride.go (2)
internal/controller/dashboard/manager.go (2)
Manager(39-43)ResourceTypeDynamic(29-29)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/marketplacepanel.go (2)
internal/controller/dashboard/manager.go (2)
Manager(39-43)ResourceTypeDynamic(29-29)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/cozystackresource_controller.go (2)
internal/controller/dashboard/manager.go (2)
NewManager(54-60)WithCRDListFunc(49-51)api/v1alpha1/cozystackresourcedefinitions_types.go (2)
CozystackResourceDefinition(27-32)CozystackResourceDefinitionList(37-41)
internal/controller/dashboard/helpers.go (1)
api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/factory.go (2)
internal/controller/dashboard/unified_helpers.go (3)
UnifiedResourceConfig(277-286)BadgeSizeLarge(96-96)BadgeSizeMedium(95-95)internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(29-29)
internal/controller/dashboard/breadcrumb.go (2)
internal/controller/dashboard/manager.go (2)
Manager(39-43)ResourceTypeDynamic(29-29)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/customcolumns.go (1)
internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(29-29)
internal/controller/dashboard/ui_helpers.go (1)
internal/controller/dashboard/unified_helpers.go (2)
BadgeSizeMedium(95-95)BadgeSizeLarge(96-96)
internal/controller/dashboard/static_processor.go (2)
internal/controller/dashboard/manager.go (2)
Manager(39-43)ResourceTypeStatic(28-28)internal/controller/dashboard/static_refactored.go (1)
CreateAllStaticResources(1683-1717)
internal/controller/dashboard/manager.go (2)
api/v1alpha1/cozystackresourcedefinitions_types.go (2)
CozystackResourceDefinition(27-32)CozystackResourceDefinitionList(37-41)internal/controller/dashboard/static_refactored.go (1)
CreateAllStaticResources(1683-1717)
internal/controller/dashboard/static_refactored.go (1)
internal/controller/dashboard/unified_helpers.go (2)
UnifiedResourceConfig(277-286)BadgeSizeLarge(96-96)
internal/controller/dashboard/static_helpers.go (1)
internal/controller/dashboard/unified_helpers.go (3)
BadgeSizeMedium(95-95)BadgeConfig(83-88)BadgeSizeLarge(96-96)
🔇 Additional comments (20)
internal/controller/dashboard/customcolumns.go (1)
153-154: Good addition: centralized dashboard labels on dynamic resources.This aligns with the new labeling/cleanup flow.
internal/controller/dashboard/sidebar.go (2)
225-227: Good: apply centralized labels on dynamic Sidebar resources.Consistent with new cleanup/selection approach.
73-87: pickGVK fallback group/version is intentional
CRD spec exposes only Application.Kind; no group/version overrides exist, so pickGVK correctly defaults to apps.cozystack.io/v1alpha1.internal/controller/dashboard/static_processor.go (1)
39-55: Spec copy across static resource types: LGTM.Covers all types returned by CreateAllStaticResources per current list; labels re-applied inside mutate.
If additional static types are added later (e.g., CustomFormsPrefill), extend the switch accordingly.
internal/controller/dashboard/ui_helpers.go (2)
201-209: Unified badge helpers are correctly referenced createUnifiedBadgeFromKind and BadgeSizeMedium/Large are defined in internal/controller/dashboard/unified_helpers.go.
26-31: generateTextID and generateContainerID are defined in internal/controller/dashboard/unified_helpers.go; no changes required.internal/controller/dashboard/breadcrumb.go (1)
39-43: Nil-safe dashboard name check: LGTMGood guard against nil and whitespace before dereferencing.
internal/controller/dashboard/marketplacepanel.go (3)
26-40: Deletion on absent dashboard: LGTMClean early-return deletion path with proper NotFound handling.
42-56: Skip when dashboard.name set: LGTMConsistent with the intended behavior; safe NotFound handling and idempotent delete.
72-83: Spec mapping looks consistentFields map cleanly from CRD to MarketplacePanel, including tags and icon. No issues.
internal/controller/cozystackresource_controller.go (1)
149-158: Startup init gated by manager start/leader: LGTMCorrectly moved to mgr.Add with Runnable; no background goroutine in Setup.
internal/controller/dashboard/factory.go (3)
113-117: Namespace link route: LGTMRoute updated to marketplace; placeholder usage matches surrounding conventions.
355-388: Order-by-keys implementation: LGTMStable fallback and prioritized ordering via orderMap are reasonable for the UI needs.
461-467: Array handling in schema traversal: LGTMSafely surfaces scalar array item types; avoids deep/complex structures.
internal/controller/dashboard/customformsoverride.go (1)
40-46: Default sort to [] instead of nullWhen KeysOrder is absent, sort marshals to null. If the UI expects an array, send [] instead.
Apply this diff:
- var sort []any - if crd.Spec.Dashboard != nil && len(crd.Spec.Dashboard.KeysOrder) > 0 { + var sort []any + if crd.Spec.Dashboard != nil && len(crd.Spec.Dashboard.KeysOrder) > 0 { sort = make([]any, len(crd.Spec.Dashboard.KeysOrder)) for i, v := range crd.Spec.Dashboard.KeysOrder { sort[i] = v } - } + } else { + sort = []any{} + }internal/controller/dashboard/static_refactored.go (2)
1304-1308: Inconsistent TableUriMapping keysToParse types.You mix array form (["metadata","name"]) and dotted-path form (".metadata.name"). Confirm the consumer accepts both; otherwise normalize.
Would you like me to normalize these to a single convention across the file?
Also applies to: 1311-1316, 1319-1324
1375-1380: Good: formItems now preserved in CustomFormsOverride.This addresses the earlier regression where formItems were discarded.
internal/controller/dashboard/manager.go (1)
250-283: Good fix: derive resourceKind explicitly in cleanupResourceType.Avoids empty GVK issues for zero-valued objects; orphan cleanup will execute.
internal/controller/dashboard/static_helpers.go (2)
262-271: LGTM: “WaitingReason” key corrected.Matches undefined-values config and avoids mapping issues.
Also applies to: 285-294
843-847: LGTM: “strategySuccess” key corrected in StatusText.Fixes success evaluation logic.
Also applies to: 905-907
b82afa6 to
a528085
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (13)
internal/controller/dashboard/tableurimapping.go (1)
9-13: No-op placeholder: either implement or document stronger intentThis method always returns nil. If TableUriMapping is now fully static, add a TODO or remove the method to avoid confusion; otherwise, wire in actual reconcile logic.
internal/controller/dashboard/ui_helpers.go (3)
11-19: Make id consistently a stringAll helpers treat id as string; accept string here for consistency and stable JSON types.
-func contentCardWithTitle(id any, title string, style map[string]any, children []any) map[string]any { +func contentCardWithTitle(id string, title string, style map[string]any, children []any) map[string]any {
140-149: Auto-generate missing IDs for antdColMatch other helpers’ behavior to avoid empty IDs.
func antdCol(id string, span float64, children []any) map[string]any { + // Auto-generate ID if not provided + if id == "" { + id = generateContainerID("auto", "col") + } return map[string]any{ "type": "antdCol", "data": map[string]any{ "id": id, "span": span, }, "children": children, } }
151-160: Auto-generate missing IDs for antdColWithStyleKeep ID semantics consistent.
func antdColWithStyle(id string, style map[string]any, children []any) map[string]any { + // Auto-generate ID if not provided + if id == "" { + id = generateContainerID("auto", "col") + } return map[string]any{ "type": "antdCol", "data": map[string]any{ "id": id, "style": style, }, "children": children, } }internal/controller/dashboard/customcolumns.go (1)
148-164: Return actual OperationResult from CreateOrUpdateYou’re using a typed resource; propagate op for better observability and call-site decisions.
- _, err := controllerutil.CreateOrUpdate(ctx, m.client, obj, func() error { + op, err := controllerutil.CreateOrUpdate(ctx, m.client, obj, func() error { @@ - // Return OperationResultCreated/Updated is not available here with unstructured; we can mimic Updated when no error. - return controllerutil.OperationResultNone, err + return op, errinternal/controller/dashboard/static_processor.go (1)
32-55: Remove duplicated label application (minor nit)Labels are added before and inside mutate; the pre-mutation call is redundant.
- // Add dashboard labels to static resources - m.addDashboardLabels(resource, nil, ResourceTypeStatic) + // Labels are applied inside the mutate functioninternal/controller/dashboard/marketplacepanel.go (1)
72-83: Avoid hardcoded apiGroup/apiVersion; derive via pickGVKPrevents drift if the resource’s G/V changes.
- specMap := map[string]any{ + g, v, kind := pickGVK(crd) + plural := pickPlural(kind, crd) + specMap := map[string]any{ "description": d.Description, "name": displayName, "type": "nonCrd", - "apiGroup": "apps.cozystack.io", - "apiVersion": "v1alpha1", - "typeName": app.Plural, // e.g., "buckets" + "apiGroup": g, + "apiVersion": v, + "typeName": plural, "disabled": false, "hidden": false, "tags": tags, "icon": d.Icon, }internal/controller/dashboard/breadcrumb.go (1)
30-32: Trim whitespace on Dashboard.Plural override.Prevents labels like " Foo " from leaking into UI.
Apply this diff:
- if crd != nil && crd.Spec.Dashboard != nil && crd.Spec.Dashboard.Plural != "" { - labelPlural = crd.Spec.Dashboard.Plural + if crd != nil && crd.Spec.Dashboard != nil && strings.TrimSpace(crd.Spec.Dashboard.Plural) != "" { + labelPlural = strings.TrimSpace(crd.Spec.Dashboard.Plural)internal/controller/dashboard/customformsoverride.go (1)
40-46: Avoid null in spec.sort; default to empty array.Nil makes JSON "sort": null; some consumers expect an array. Default to [].
Apply this diff:
- var sort []any - if crd.Spec.Dashboard != nil && len(crd.Spec.Dashboard.KeysOrder) > 0 { + var sort []any + if crd.Spec.Dashboard != nil && len(crd.Spec.Dashboard.KeysOrder) > 0 { sort = make([]any, len(crd.Spec.Dashboard.KeysOrder)) for i, v := range crd.Spec.Dashboard.KeysOrder { sort[i] = v } - } + } else { + sort = []any{} + }internal/controller/dashboard/factory.go (3)
49-56: Use titleFromKindPlural for a nicer Title.Lowercasing plural looks odd in headers; reuse existing helper for consistency.
Apply this diff:
config := UnifiedResourceConfig{ Name: factoryName, ResourceType: "factory", Kind: kind, Plural: plural, - Title: strings.ToLower(plural), + Title: titleFromKindPlural(kind, plural), Size: BadgeSizeLarge, }
372-390: Use a stable sort to honor “keep original order” intent.Comment says “stable”, but sort.Slice is not stable. Use sort.SliceStable.
Apply this diff:
- sort.Slice(fields, func(i, j int) bool { + sort.SliceStable(fields, func(i, j int) bool {
503-512: Comment contradicts implementation of defaults.The function sets all flags to true, but comment says defaults are false. Align the comment.
Apply this diff:
-// without breaking the controller. Defaults are false (hidden). +// without breaking the controller. Defaults are true (visible).internal/controller/dashboard/static_helpers.go (1)
116-123: Duplicate configmaps block—dedupe to reduce drift.The "stock-namespace-v1.configmaps" trim-length block appears twice. Keep one.
Apply this diff to remove the duplicate later block:
- if name == "stock-namespace-v1.configmaps" { - data["additionalPrinterColumnsTrimLengths"] = []any{ - map[string]any{ - "key": "Name", - "value": float64(64), - }, - } - }Also applies to: 412-419
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (86)
internal/controller/cozystackresource_controller.go(5 hunks)internal/controller/dashboard/breadcrumb.go(1 hunks)internal/controller/dashboard/customcolumns.go(1 hunks)internal/controller/dashboard/customformsoverride.go(1 hunks)internal/controller/dashboard/customformsprefill.go(1 hunks)internal/controller/dashboard/factory.go(5 hunks)internal/controller/dashboard/helpers.go(1 hunks)internal/controller/dashboard/manager.go(2 hunks)internal/controller/dashboard/marketplacepanel.go(1 hunks)internal/controller/dashboard/sidebar.go(1 hunks)internal/controller/dashboard/static_helpers.go(1 hunks)internal/controller/dashboard/static_processor.go(1 hunks)internal/controller/dashboard/static_refactored.go(1 hunks)internal/controller/dashboard/tableurimapping.go(1 hunks)internal/controller/dashboard/ui_helpers.go(1 hunks)internal/controller/dashboard/unified_helpers.go(1 hunks)internal/controller/dashboard/webextras.go(0 hunks)packages/core/platform/bundles/paas-full.yaml(0 hunks)packages/core/platform/bundles/paas-hosted.yaml(0 hunks)packages/system/cozystack-controller/values.yaml(1 hunks)packages/system/dashboard-config/Chart.yaml(0 hunks)packages/system/dashboard-config/Makefile(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/configmap-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/namespace-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/node-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/pod-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/secret-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/service-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-api.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-builtin.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-project-api.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-project-builtin.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.apps.cozystack.io.workloadmonitors.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.core.cozystack.io.tenantsecretstables.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-ingress-details-rules.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-node-images.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-pod-details-list.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-status-conditions.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/hidden.metadata.tpl(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/icons.tpl(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/k8s.container.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1alpha1.cozystack.io.workloads.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/storage.k8s.io.v1.storageclasses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.namespaces.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumeclaims.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/cozy-marketplace.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/annotations.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/counters.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/labels.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/links.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/statuses.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/tables.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/taints.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/times.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/tolerations.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/namespace-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/node-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/pod-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/secret-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/service-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/workloadmonitor-details.yaml(0 hunks)packages/system/dashboard-config/templates/Navigation/navigaton.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/apps.cozystack.io.v1alpha1.virtualmachines.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/core.cozystack.io.v1alpha1.tenantnamespaces.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.namespaces.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/_helpers.tpl(0 hunks)packages/system/dashboard/values.yaml(1 hunks)
💤 Files with no reviewable changes (68)
- packages/system/dashboard-config/templates/CustomFormOverride/v1.configmaps.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/pod-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1.services.yaml
- packages/system/dashboard-config/Makefile
- packages/system/dashboard-config/templates/Breadcrumb/factory/service-details.yaml
- packages/system/dashboard-config/templates/Factory/service-details.yaml
- packages/system/dashboard-config/templates/Factory/helpers/tolerations.tpl
- packages/system/dashboard-config/templates/Factory/helpers/labels.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-status-conditions.yaml
- packages/system/dashboard-config/templates/Factory/helpers/counters.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/k8s.container.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/namespace-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.pods.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumes.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/configmap-details.yaml
- packages/system/dashboard-config/templates/Factory/helpers/statuses.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/Chart.yaml
- packages/system/dashboard-config/templates/Factory/namespace-details.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.pods.yaml
- internal/controller/dashboard/webextras.go
- packages/core/platform/bundles/paas-full.yaml
- packages/system/dashboard-config/templates/Factory/helpers/annotations.tpl
- packages/system/dashboard-config/templates/Factory/helpers/links.tpl
- packages/system/dashboard-config/templates/_helpers.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1alpha1.cozystack.io.workloads.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/hidden.metadata.tpl
- packages/system/dashboard-config/templates/Factory/cozy-marketplace.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.core.cozystack.io.tenantsecretstables.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-api.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.configmaps.yaml
- packages/system/dashboard-config/templates/TableUriMapping/apps.cozystack.io.v1alpha1.virtualmachines.yaml
- packages/system/dashboard-config/templates/Factory/helpers/times.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.configmaps.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.secrets.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-ingress-details-rules.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-project-builtin.yaml
- packages/core/platform/bundles/paas-hosted.yaml
- packages/system/dashboard-config/templates/Factory/pod-details.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.secrets.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-builtin.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.namespaces.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.services.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.apps.cozystack.io.workloadmonitors.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.nodes.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.nodes.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.services.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/node-details.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.pods.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumeclaims.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-node-images.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.nodes.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/secret-details.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.namespaces.yaml
- packages/system/dashboard-config/templates/Navigation/navigaton.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-pod-details-list.yaml
- packages/system/dashboard-config/templates/Factory/helpers/tables.tpl
- packages/system/dashboard-config/templates/Factory/secret-details.yaml
- packages/system/dashboard-config/templates/Factory/node-details.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-project-api.yaml
- packages/system/dashboard-config/templates/TableUriMapping/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/TableUriMapping/core.cozystack.io.v1alpha1.tenantnamespaces.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/storage.k8s.io.v1.storageclasses.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.secrets.yaml
- packages/system/dashboard-config/templates/Factory/helpers/taints.tpl
- packages/system/dashboard-config/templates/Factory/workloadmonitor-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/icons.tpl
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/system/dashboard/values.yaml
- internal/controller/dashboard/unified_helpers.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-10T22:02:47.729Z
Learnt from: lllamnyp
PR: cozystack/cozystack#1380
File: internal/controller/kubeovnplunger/metrics.go:291-296
Timestamp: 2025-09-10T22:02:47.729Z
Learning: In the kubeovnplunger controller, r.lastLeader map is initialized in SetupWithManager method before any reconcile operations begin, making defensive nil checks unnecessary in the reconcile methods.
Applied to files:
internal/controller/cozystackresource_controller.go
📚 Learning: 2025-09-10T22:02:47.729Z
Learnt from: lllamnyp
PR: cozystack/cozystack#1380
File: internal/controller/kubeovnplunger/metrics.go:291-296
Timestamp: 2025-09-10T22:02:47.729Z
Learning: In the kubeovnplunger controller, r.lastLeader map is initialized in SetupWithManager method (line 151) with make(map[string]string) before any reconcile operations begin, making defensive nil checks unnecessary in the reconcile methods.
Applied to files:
internal/controller/cozystackresource_controller.go
🧬 Code graph analysis (15)
internal/controller/dashboard/customcolumns.go (1)
internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(30-30)
internal/controller/dashboard/customformsoverride.go (2)
internal/controller/dashboard/manager.go (2)
Manager(40-44)ResourceTypeDynamic(30-30)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/sidebar.go (1)
internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(30-30)
internal/controller/dashboard/tableurimapping.go (2)
internal/controller/dashboard/manager.go (1)
Manager(40-44)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/breadcrumb.go (2)
internal/controller/dashboard/manager.go (2)
Manager(40-44)ResourceTypeDynamic(30-30)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/helpers.go (1)
api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/customformsprefill.go (2)
internal/controller/dashboard/manager.go (2)
Manager(40-44)ResourceTypeDynamic(30-30)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/ui_helpers.go (1)
internal/controller/dashboard/unified_helpers.go (2)
BadgeSizeMedium(95-95)BadgeSizeLarge(96-96)
internal/controller/dashboard/static_refactored.go (1)
internal/controller/dashboard/unified_helpers.go (2)
UnifiedResourceConfig(277-286)BadgeSizeLarge(96-96)
internal/controller/dashboard/static_processor.go (2)
internal/controller/dashboard/manager.go (2)
Manager(40-44)ResourceTypeStatic(29-29)internal/controller/dashboard/static_refactored.go (1)
CreateAllStaticResources(1683-1717)
internal/controller/dashboard/manager.go (2)
api/v1alpha1/cozystackresourcedefinitions_types.go (2)
CozystackResourceDefinition(27-32)CozystackResourceDefinitionList(37-41)internal/controller/dashboard/static_refactored.go (1)
CreateAllStaticResources(1683-1717)
internal/controller/cozystackresource_controller.go (1)
internal/controller/dashboard/manager.go (2)
NewManager(55-61)WithCRDListFunc(50-52)
internal/controller/dashboard/factory.go (2)
internal/controller/dashboard/unified_helpers.go (3)
UnifiedResourceConfig(277-286)BadgeSizeLarge(96-96)BadgeSizeMedium(95-95)internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(30-30)
internal/controller/dashboard/marketplacepanel.go (2)
internal/controller/dashboard/manager.go (2)
Manager(40-44)ResourceTypeDynamic(30-30)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/static_helpers.go (1)
internal/controller/dashboard/unified_helpers.go (3)
BadgeSizeMedium(95-95)BadgeConfig(83-88)BadgeSizeLarge(96-96)
⏰ 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 (14)
packages/system/cozystack-controller/values.yaml (1)
2-2: Digest bump LGTM; confirm intent to keep tag=latest with digest pinUsing latest@sha256 is fine (digest pins immutably). If your image promotion or scanners key off tags, consider a versioned tag alongside digest. Also reflect this bump in release notes.
internal/controller/dashboard/customformsprefill.go (2)
22-28: Good: avoids hardcoded G/V and pluralUsing pickGVK + pickPlural for name/customizationID is correct and future-proof.
56-63: OwnerReference is valid Both CustomFormsPrefill and CozystackResourceDefinition are cluster-scoped, so using SetOwnerReference is safe. No changes needed.internal/controller/dashboard/sidebar.go (1)
225-227: LGTM: apply dashboard labels to dynamic Sidebar resourcesEnsures consistent labeling for selection/cleanup.
internal/controller/dashboard/customcolumns.go (1)
153-155: LGTM: label dynamic CustomColumnsOverrideConsistent with the new labeling model.
internal/controller/dashboard/marketplacepanel.go (1)
90-99: Add guard for ownerRef based on resource scopes
Only set ownerRef when the dependent is namespaced or the owner is cluster-scoped:- if err := controllerutil.SetOwnerReference(crd, mp, m.scheme); err != nil { - return err - } + if mp.GetNamespace() != "" || crd.GetNamespace() == "" { + if err := controllerutil.SetOwnerReference(crd, mp, m.scheme); err != nil { + return err + } + }Verify that the MarketplacePanel CRD is declared cluster-scoped (kubebuilder:resource scope=Cluster).
internal/controller/cozystackresource_controller.go (1)
149-158: Nice use of mgr.Add for one-time initMoving the static resource initialization behind a
manager.Runnablekeeps it leader-aware and guarantees caches are up before we touch the API. Thanks for wiring it this way.internal/controller/dashboard/helpers.go (1)
26-43: Populate group/version from CRD before constructing dashboard URLsRight now we never assign
grouporversionin this helper, so they stay empty and fall back toapps.cozystack.io/v1alpha1. Any CRD whose application lives in a different API group/version will have every dashboard resource (Factory, CustomFormsOverride, Breadcrumb, etc.) pointing at the wrong REST path and the UI calls will 404. Please hydrategroup/versionfrom the data we already carry on the CRD (e.g. anApplication.APIVersionstring or explicitGroup/Versionfields) before using the default.func pickGVK(crd *cozyv1alpha1.CozystackResourceDefinition) (group, version, kind string) { // Best guess based on your examples: if crd.Spec.Application.Kind != "" { kind = crd.Spec.Application.Kind } + + if av := crd.Spec.Application.APIVersion; av != "" { + if i := strings.IndexByte(av, '/'); i > 0 && i < len(av)-1 { + group, version = av[:i], av[i+1:] + } + } + if group == "" && crd.Spec.Application.Group != "" { + group = crd.Spec.Application.Group + } + if version == "" && crd.Spec.Application.Version != "" { + version = crd.Spec.Application.Version + } // Reasonable fallbacks if any are empty: if group == "" { group = "apps.cozystack.io" }internal/controller/dashboard/static_refactored.go (4)
207-226: Correct the misnamed WaitingReason column key.Line [213] and Line [224] are still spelled
WaitingdReason, so the override never matches the UI property. Please rename both occurrences toWaitingReason.- createStringColumn("WaitingdReason", ".state.waiting.reason"), + createStringColumn("WaitingReason", ".state.waiting.reason"), ... - createStringColumn("WaitingdReason", ".state.waiting.reason"), + createStringColumn("WaitingReason", ".state.waiting.reason"),
762-764: Fix JSONPath quoting for service-account annotation link.Line [762] still nests single quotes, which breaks JSONPath parsing and renders the link empty. Please use double quotes inside the bracket expression.
- "text": "{reqsJsonPath[0]['.metadata.annotations['kubernetes.io/service-account.name']']['-']}", - "href": "/openapi-ui/{2}/{3}/factory/serviceaccount-details/{reqsJsonPath[0]['.metadata.annotations['kubernetes.io/service-account.name']']['-']}", + "text": "{reqsJsonPath[0]['.metadata.annotations[\"kubernetes.io/service-account.name\"]']['-']}", + "href": "/openapi-ui/{2}/{3}/factory/serviceaccount-details/{reqsJsonPath[0]['.metadata.annotations[\"kubernetes.io/service-account.name\"]']['-']}",
931-939: Use Service selector path in LabelsToSearchParams.Line [934] still points to
.spec.template.metadata.labels; Services expose their selector at.spec.selector, so the current path always resolves empty.- "jsonPathToLabels": ".spec.template.metadata.labels", + "jsonPathToLabels": ".spec.selector",
1523-1531: Correct StatusText prop name strategySuccess.Line [1528] keeps the misspelling
stategySuccess, so the StatusText component never evaluates the success path. Please fix the key.- "stategySuccess": "every", + "strategySuccess": "every",internal/controller/dashboard/customformsoverride.go (1)
21-23: Confirm customizationId format.Is "default-/group/version/plural" the intended key (note trailing slash after default-)? Verify it matches consumers.
internal/controller/dashboard/factory.go (1)
33-34: Verify OpenAPISchema access is safe.Accesses crd.Spec.Application.OpenAPISchema without a nil guard. Ensure Application is always present or guard it.
| _, _, kind := pickGVK(crd) | ||
|
|
||
| lowerKind := strings.ToLower(kind) | ||
| detailID := fmt.Sprintf("stock-project-factory-%s-details", lowerKind) |
There was a problem hiding this comment.
Use CRD group/version in breadcrumb link (avoid hardcoded path).
Hardcoding apps.cozystack.io/v1alpha1 breaks links for CRDs in other groups/versions. Use g,v from pickGVK.
Apply this diff:
- _, _, kind := pickGVK(crd)
+ g, v, kind := pickGVK(crd)
@@
- link := fmt.Sprintf("/openapi-ui/{clusterName}/{namespace}/api-table/apps.cozystack.io/v1alpha1/%s", plural)
+ link := fmt.Sprintf("/openapi-ui/{clusterName}/{namespace}/api-table/%s/%s/%s", g, v, plural)Also applies to: 36-37
🤖 Prompt for AI Agents
internal/controller/dashboard/breadcrumb.go around lines 18-21 (and also 36-37):
pickGVK currently only extracts kind and hardcodes the breadcrumb path; change
the call to capture group and version as well (e.g. group, version, kind :=
pickGVK(crd)) and build the detailID using group and version instead of a
hardcoded apps.cozystack.io/v1alpha1, e.g. include group and version
(lowercased) in the fmt.Sprintf that creates the breadcrumb link (and mirror the
same change at lines 36-37).
a528085 to
0ba3995
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/controller/dashboard/factory.go (1)
931-939: Wrong JSONPath for Service Pod selector.Services use .spec.selector, not .spec.template.metadata.labels.
Apply this diff:
- "jsonPathToLabels": ".spec.template.metadata.labels", + "jsonPathToLabels": ".spec.selector",internal/controller/dashboard/sidebar.go (1)
100-107: Fix sort order: Weight should be descending per specComment says “Weight (desc)”, but code sorts ascending.
- if categories[cat][i].Weight != categories[cat][j].Weight { - return categories[cat][i].Weight < categories[cat][j].Weight // lower weight first - } + if categories[cat][i].Weight != categories[cat][j].Weight { + return categories[cat][i].Weight > categories[cat][j].Weight // higher weight first + }
🧹 Nitpick comments (16)
internal/controller/dashboard/static_processor.go (2)
32-34: Remove redundant pre-mutation labeling (set labels only inside CreateOrUpdate).Labels are applied again in the mutate closure; pre-setting here is unnecessary and can cause confusion.
- // Add dashboard labels to static resources - m.addDashboardLabels(resource, nil, ResourceTypeStatic) + // Labels are applied in the mutate closure below
39-52: Future-proof spec copy: guard unknown types or fail fast.If a new static type is added to CreateAllStaticResources and missed here, its Spec won’t be applied. Consider explicitly handling a default case (log or return error) to avoid silent drift.
switch o := obj.(type) { case *dashv1alpha1.CustomColumnsOverride: resource.(*dashv1alpha1.CustomColumnsOverride).Spec = o.Spec case *dashv1alpha1.Breadcrumb: resource.(*dashv1alpha1.Breadcrumb).Spec = o.Spec case *dashv1alpha1.CustomFormsOverride: resource.(*dashv1alpha1.CustomFormsOverride).Spec = o.Spec case *dashv1alpha1.Factory: resource.(*dashv1alpha1.Factory).Spec = o.Spec case *dashv1alpha1.Navigation: resource.(*dashv1alpha1.Navigation).Spec = o.Spec case *dashv1alpha1.TableUriMapping: resource.(*dashv1alpha1.TableUriMapping).Spec = o.Spec + default: + // Unknown static resource type — avoid silently skipping Spec sync + return fmt.Errorf("unsupported static resource type: %T", obj) }internal/controller/dashboard/marketplacepanel.go (1)
42-56: Deduplicate delete-on-skip logic.Both branches delete the MarketplacePanel the same way. Consider extracting a small helper to reduce duplication.
internal/controller/dashboard/customformsprefill.go (1)
51-55: Clarify comment about JSON map key ordering.encoding/json doesn’t guarantee map key order. The compareArbitrarySpecs guard is the real stabilizer; adjust the comment to avoid confusion.
- // Use json.Marshal with sorted keys to ensure consistent output + // Note: map key order is not guaranteed; rely on compareArbitrarySpecs for semantic equalityinternal/controller/dashboard/factory.go (1)
372-391: Use stable sort; keep original order when not specified by KeysOrder.sort.Slice is unstable and the i<j fallback is not a strict weak ordering. Use sort.SliceStable and return false when neither side is ordered.
Apply this diff:
- // Sort fields based on their position in keysOrder - sort.Slice(fields, func(i, j int) bool { + // Sort fields based on their position in keysOrder (stable) + sort.SliceStable(fields, func(i, j int) bool { posI, existsI := orderMap[fields[i].JSONPathSpec] posJ, existsJ := orderMap[fields[j].JSONPathSpec] // If both exist in orderMap, sort by position if existsI && existsJ { return posI < posJ } // If only one exists, prioritize the one that exists if existsI { return true } if existsJ { return false } - // If neither exists, maintain original order (stable sort) - return i < j + // If neither exists, keep original order (stable) + return false })internal/controller/dashboard/static_refactored.go (1)
1382-1382: Handle json.Marshal errors defensively (optional).Avoids silently producing empty/invalid payloads on unexpected encoding errors.
Apply this diff:
- jsonData, _ := json.Marshal(newSpec) + jsonData, err := json.Marshal(newSpec) + if err != nil { + // Fallback to minimal valid JSON to avoid panics; consider logging + jsonData = []byte("{}") + }And similarly in createNavigation:
- jsonData, _ := json.Marshal(spec) + jsonData, err := json.Marshal(spec) + if err != nil { + jsonData = []byte("{}") + }Also applies to: 1403-1403
internal/controller/dashboard/unified_helpers.go (1)
42-73: Optional: enforce RFC1123 validation for generated names.Consider k8s.io/apimachinery/pkg/util/validation for stricter name compliance if these ever become metadata.name values.
internal/controller/dashboard/helpers.go (6)
202-204: Ensure deterministic output ordering for values.Sort by path before returning to avoid map-iteration nondeterminism.
var values []interface{} processSchemaProperties(props, []string{"spec"}, &values, true) + // Ensure stable ordering for reproducible specs + if canSortArray(values) { + sortArray(values) + } return values, nil
209-216: Deterministic traversal of schema properties.Iterating Go maps is random; sort property names to produce stable results.
-func processSchemaProperties(props map[string]interface{}, path []string, values *[]interface{}, topLevel bool) { - for pname, raw := range props { +func processSchemaProperties(props map[string]interface{}, path []string, values *[]interface{}, topLevel bool) { + // Sort keys for deterministic traversal + keys := make([]string, 0, len(props)) + for k := range props { + keys = append(keys, k) + } + sort.Strings(keys) + for _, pname := range keys { + raw := props[pname] sub, _ := raw.(map[string]interface{}) if sub == nil { continue }
247-261: Deterministic traversal of default object fields.Same nondeterminism for map iteration; sort keys.
-func processDefaultObject(obj map[string]interface{}, path []string, values *[]interface{}) { - for key, value := range obj { +func processDefaultObject(obj map[string]interface{}, path []string, values *[]interface{}) { + keys := make([]string, 0, len(obj)) + for k := range obj { + keys = append(keys, k) + } + sort.Strings(keys) + for _, key := range keys { + value := obj[key] currentPath := append(path, key)
83-97: Normalize kind case before hashing for color stability.Different casing of the same kind yields different colors. Lowercase the input first.
-func hexColorForKind(kind string) string { +func hexColorForKind(kind string) string { + kind = strings.ToLower(kind) // Stable short hash (sha1 → bytes → hue) sum := sha1.Sum([]byte(kind))
495-519: Strengthen canSortArray: ensure all items have a "path" field.Current check only inspects the first element; mixed arrays can break sort assumptions.
func canSortArray(arr []interface{}) bool { if len(arr) == 0 { return false } // Check if all elements are objects for _, item := range arr { if _, ok := item.(map[string]interface{}); !ok { return false } } - // Check if objects have comparable fields (like "path" for CustomFormsPrefill values) - firstObj, ok := arr[0].(map[string]interface{}) - if !ok { - return false - } - - // Look for "path" field which is used in CustomFormsPrefill values - if _, hasPath := firstObj["path"]; hasPath { - return true - } - - return false + // Ensure every object has a "path" field + for _, item := range arr { + obj := item.(map[string]interface{}) + if _, hasPath := obj["path"]; !hasPath { + return false + } + } + return true }
521-544: Define explicit ordering for missing/unequal path types.Comparator returns false on missing paths, which can lead to non‑strict ordering. Prefer pushing missing/invalid paths last.
func sortArray(arr []interface{}) { sort.Slice(arr, func(i, j int) bool { objI, okI := arr[i].(map[string]interface{}) objJ, okJ := arr[j].(map[string]interface{}) - if !okI || !okJ { - return false - } + if !okI || !okJ { + return okI && !okJ + } pathI, hasPathI := objI["path"] pathJ, hasPathJ := objJ["path"] - if !hasPathI || !hasPathJ { - return false - } + if !hasPathI || !hasPathJ { + return hasPathI && !hasPathJ + } // Convert paths to strings for comparison pathIStr := fmt.Sprintf("%v", pathI) pathJStr := fmt.Sprintf("%v", pathJ) return pathIStr < pathJStr }) }internal/controller/dashboard/static_helpers.go (2)
23-23: Don’t ignore json.Marshal errorsSilently discarding marshal errors can hide bad data and make debugging harder.
- jsonData, _ := json.Marshal(data) + jsonData, err := json.Marshal(data) + if err != nil { + // Consider returning an error from the helper, or at least log/panic in this internal-only path + // For now, fail closed to avoid creating malformed resources + panic(err) + }Apply similarly at lines where spec is marshaled.
Also applies to: 455-455, 476-476, 497-497
971-976: Hardcoded plainTextValue in SecretBase64Plain"hello" looks like leftover demo text. If the component renders both, this may surface in UI.
- "id": "example-secretbase64", - "plainTextValue": "hello", - "base64Value": "{reqsJsonPath[0]['" + jsonPath + "']['-']}", + "id": "example-secretbase64", + "base64Value": "{reqsJsonPath[0]['" + jsonPath + "']['-']}",Confirm the component does not require plainTextValue when base64Value is provided.
cmd/cozystack-controller/main.go (1)
156-160: Make client QPS/Burst configurable via flagsHard-coding may overload small clusters. Expose flags with sensible defaults.
@@ - // Configure rate limiting for the Kubernetes client - config := ctrl.GetConfigOrDie() - config.QPS = 50.0 // Increased from default 5.0 - config.Burst = 100 // Increased from default 10 + // Configure rate limiting for the Kubernetes client + config := ctrl.GetConfigOrDie() + // Defaults keep current behavior; allow override via flags + config.QPS = float32(kubeClientQPS) + config.Burst = kubeClientBurstAdd variables and flags:
@@ - var tlsOpts []func(*tls.Config) + var tlsOpts []func(*tls.Config) + var kubeClientQPS float64 + var kubeClientBurst int @@ flag.StringVar(&cozystackVersion, "cozystack-version", "unknown", "Version of Cozystack") + flag.Float64Var(&kubeClientQPS, "kube-client-qps", 50.0, "QPS for the Kubernetes client") + flag.IntVar(&kubeClientBurst, "kube-client-burst", 100, "Burst for the Kubernetes client")Operational note: monitor apiserver throttling and adjust these per environment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (87)
cmd/cozystack-controller/main.go(1 hunks)internal/controller/cozystackresource_controller.go(6 hunks)internal/controller/dashboard/breadcrumb.go(1 hunks)internal/controller/dashboard/customcolumns.go(1 hunks)internal/controller/dashboard/customformsoverride.go(1 hunks)internal/controller/dashboard/customformsprefill.go(1 hunks)internal/controller/dashboard/factory.go(5 hunks)internal/controller/dashboard/helpers.go(1 hunks)internal/controller/dashboard/manager.go(2 hunks)internal/controller/dashboard/marketplacepanel.go(1 hunks)internal/controller/dashboard/sidebar.go(1 hunks)internal/controller/dashboard/static_helpers.go(1 hunks)internal/controller/dashboard/static_processor.go(1 hunks)internal/controller/dashboard/static_refactored.go(1 hunks)internal/controller/dashboard/tableurimapping.go(1 hunks)internal/controller/dashboard/ui_helpers.go(1 hunks)internal/controller/dashboard/unified_helpers.go(1 hunks)internal/controller/dashboard/webextras.go(0 hunks)packages/core/platform/bundles/paas-full.yaml(0 hunks)packages/core/platform/bundles/paas-hosted.yaml(0 hunks)packages/system/cozystack-controller/values.yaml(1 hunks)packages/system/dashboard-config/Chart.yaml(0 hunks)packages/system/dashboard-config/Makefile(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/configmap-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/namespace-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/node-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/pod-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/secret-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/service-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-api.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-builtin.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-project-api.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-project-builtin.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.apps.cozystack.io.workloadmonitors.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.core.cozystack.io.tenantsecretstables.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-ingress-details-rules.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-node-images.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-pod-details-list.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-status-conditions.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/hidden.metadata.tpl(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/icons.tpl(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/k8s.container.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1alpha1.cozystack.io.workloads.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/storage.k8s.io.v1.storageclasses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.namespaces.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumeclaims.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/cozy-marketplace.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/annotations.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/counters.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/labels.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/links.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/statuses.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/tables.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/taints.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/times.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/tolerations.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/namespace-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/node-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/pod-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/secret-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/service-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/workloadmonitor-details.yaml(0 hunks)packages/system/dashboard-config/templates/Navigation/navigaton.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/apps.cozystack.io.v1alpha1.virtualmachines.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/core.cozystack.io.v1alpha1.tenantnamespaces.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.namespaces.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/_helpers.tpl(0 hunks)packages/system/dashboard/values.yaml(1 hunks)
💤 Files with no reviewable changes (68)
- packages/system/dashboard-config/Chart.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/secret-details.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumes.yaml
- packages/system/dashboard-config/templates/Factory/namespace-details.yaml
- packages/system/dashboard-config/templates/_helpers.tpl
- packages/system/dashboard-config/templates/CustomFormOverride/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-pod-details-list.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.secrets.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.secrets.yaml
- internal/controller/dashboard/webextras.go
- packages/system/dashboard-config/templates/Factory/helpers/tolerations.tpl
- packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-api.yaml
- packages/system/dashboard-config/Makefile
- packages/system/dashboard-config/templates/Breadcrumb/stock-project-api.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/storage.k8s.io.v1.storageclasses.yaml
- packages/system/dashboard-config/templates/Factory/helpers/annotations.tpl
- packages/system/dashboard-config/templates/Factory/helpers/statuses.tpl
- packages/system/dashboard-config/templates/Factory/helpers/counters.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1alpha1.cozystack.io.workloads.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.pods.yaml
- packages/system/dashboard-config/templates/Factory/helpers/taints.tpl
- packages/system/dashboard-config/templates/CustomFormOverride/v1.nodes.yaml
- packages/system/dashboard-config/templates/Factory/cozy-marketplace.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.apps.cozystack.io.workloadmonitors.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1.services.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/hidden.metadata.tpl
- packages/system/dashboard-config/templates/CustomFormOverride/v1.pods.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.core.cozystack.io.tenantsecretstables.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.services.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.nodes.yaml
- packages/system/dashboard-config/templates/Factory/secret-details.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.namespaces.yaml
- packages/system/dashboard-config/templates/Factory/pod-details.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/node-details.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.secrets.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/namespace-details.yaml
- packages/core/platform/bundles/paas-hosted.yaml
- packages/system/dashboard-config/templates/Factory/helpers/labels.tpl
- packages/system/dashboard-config/templates/Breadcrumb/factory/service-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-node-images.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/pod-details.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-builtin.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.configmaps.yaml
- packages/system/dashboard-config/templates/Factory/helpers/times.tpl
- packages/core/platform/bundles/paas-full.yaml
- packages/system/dashboard-config/templates/TableUriMapping/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.namespaces.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-ingress-details-rules.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/configmap-details.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.nodes.yaml
- packages/system/dashboard-config/templates/Factory/helpers/tables.tpl
- packages/system/dashboard-config/templates/Factory/helpers/links.tpl
- packages/system/dashboard-config/templates/Factory/node-details.yaml
- packages/system/dashboard-config/templates/Factory/workloadmonitor-details.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-project-builtin.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.configmaps.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.pods.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumeclaims.yaml
- packages/system/dashboard-config/templates/Factory/service-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-status-conditions.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/icons.tpl
- packages/system/dashboard-config/templates/TableUriMapping/core.cozystack.io.v1alpha1.tenantnamespaces.yaml
- packages/system/dashboard-config/templates/Navigation/navigaton.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.configmaps.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.services.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/k8s.container.yaml
- packages/system/dashboard-config/templates/TableUriMapping/apps.cozystack.io.v1alpha1.virtualmachines.yaml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-10T22:02:47.729Z
Learnt from: lllamnyp
PR: cozystack/cozystack#1380
File: internal/controller/kubeovnplunger/metrics.go:291-296
Timestamp: 2025-09-10T22:02:47.729Z
Learning: In the kubeovnplunger controller, r.lastLeader map is initialized in SetupWithManager method before any reconcile operations begin, making defensive nil checks unnecessary in the reconcile methods.
Applied to files:
internal/controller/cozystackresource_controller.go
📚 Learning: 2025-09-10T22:02:47.729Z
Learnt from: lllamnyp
PR: cozystack/cozystack#1380
File: internal/controller/kubeovnplunger/metrics.go:291-296
Timestamp: 2025-09-10T22:02:47.729Z
Learning: In the kubeovnplunger controller, r.lastLeader map is initialized in SetupWithManager method (line 151) with make(map[string]string) before any reconcile operations begin, making defensive nil checks unnecessary in the reconcile methods.
Applied to files:
internal/controller/cozystackresource_controller.go
🧬 Code graph analysis (14)
internal/controller/dashboard/customcolumns.go (1)
internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(31-31)
internal/controller/dashboard/tableurimapping.go (2)
internal/controller/dashboard/manager.go (1)
Manager(41-45)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/breadcrumb.go (2)
internal/controller/dashboard/manager.go (2)
Manager(41-45)ResourceTypeDynamic(31-31)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/cozystackresource_controller.go (1)
internal/controller/dashboard/manager.go (2)
NewManager(56-62)WithCRDListFunc(51-53)
internal/controller/dashboard/factory.go (2)
internal/controller/dashboard/unified_helpers.go (3)
UnifiedResourceConfig(277-286)BadgeSizeLarge(96-96)BadgeSizeMedium(95-95)internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(31-31)
internal/controller/dashboard/static_processor.go (2)
internal/controller/dashboard/manager.go (2)
Manager(41-45)ResourceTypeStatic(30-30)internal/controller/dashboard/static_refactored.go (1)
CreateAllStaticResources(1683-1717)
internal/controller/dashboard/static_helpers.go (1)
internal/controller/dashboard/unified_helpers.go (3)
BadgeSizeMedium(95-95)BadgeConfig(83-88)BadgeSizeLarge(96-96)
internal/controller/dashboard/marketplacepanel.go (2)
internal/controller/dashboard/manager.go (2)
Manager(41-45)ResourceTypeDynamic(31-31)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/helpers.go (1)
api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/customformsoverride.go (2)
internal/controller/dashboard/manager.go (2)
Manager(41-45)ResourceTypeDynamic(31-31)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/customformsprefill.go (2)
internal/controller/dashboard/manager.go (2)
Manager(41-45)ResourceTypeDynamic(31-31)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/static_refactored.go (1)
internal/controller/dashboard/unified_helpers.go (2)
UnifiedResourceConfig(277-286)BadgeSizeLarge(96-96)
internal/controller/dashboard/manager.go (2)
api/v1alpha1/cozystackresourcedefinitions_types.go (2)
CozystackResourceDefinition(27-32)CozystackResourceDefinitionList(37-41)internal/controller/dashboard/static_refactored.go (1)
CreateAllStaticResources(1683-1717)
internal/controller/dashboard/sidebar.go (1)
internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(31-31)
🔇 Additional comments (26)
internal/controller/dashboard/customcolumns.go (1)
159-163: Spec comparison is inverted
compareArbitrarySpecsreturnstruewhen the specs are equal. Usingif !compareArbitrarySpecs(...)updates the object only when specs are identical, leaving stale specs untouched. Flip the condition so we assign when they differ.Apply this diff:
- if !compareArbitrarySpecs(obj.Spec, newSpec) { - obj.Spec = newSpec - } + if !compareArbitrarySpecs(obj.Spec, newSpec) { + obj.Spec = newSpec + }Likely an incorrect or invalid review comment.
internal/controller/dashboard/ui_helpers.go (1)
1-210: UI helper constructors look good.Consistent shapes, sensible auto-ID generation, and reuse of unified badge helpers.
Confirm generateTextID/generateContainerID/createUnifiedBadgeFromKind exist in this package and are covered by tests.
internal/controller/dashboard/breadcrumb.go (2)
18-21: Use CRD group/version in breadcrumb link (avoid hardcoded path).Capture group/version from pickGVK for link construction.
- _, _, kind := pickGVK(crd) + g, v, kind := pickGVK(crd)
34-43: Fix hardcoded link to use derived group/version.Hardcoding apps.cozystack.io/v1alpha1 breaks non-default GVs.
- link := fmt.Sprintf("/openapi-ui/{clusterName}/{namespace}/api-table/apps.cozystack.io/v1alpha1/%s", plural) + link := fmt.Sprintf("/openapi-ui/{clusterName}/{namespace}/api-table/%s/%s/%s", g, v, plural)Question: is the tenantmodules link intentionally fixed to core.cozystack.io/v1alpha1? If not, we should derive it similarly.
internal/controller/dashboard/customformsoverride.go (2)
40-46: Prefer empty array over nil for sort to avoid JSON null.Downstream JSON consumers often expect an array. Default to [] when no KeysOrder.
Apply this diff:
var sort []any if crd.Spec.Dashboard != nil && len(crd.Spec.Dashboard.KeysOrder) > 0 { sort = make([]any, len(crd.Spec.Dashboard.KeysOrder)) for i, v := range crd.Spec.Dashboard.KeysOrder { sort[i] = v } + } else { + sort = []any{} }
35-38: Compile-time type mismatch: use []any instead of []interface{} in prepend.append returns []interface{} here, which cannot be assigned to hidden (type []any).
Apply this diff:
- hidden = append([]interface{}{ - []any{"metadata"}, - }, hidden...) + hidden = append([]any{ + []any{"metadata"}, + }, hidden...)Likely an incorrect or invalid review comment.
internal/controller/dashboard/factory.go (1)
64-81: CreateOrUpdate pattern with JSON diff looks good.OwnerRef, labeling, marshaling, and no-op update guard are correct.
internal/controller/dashboard/static_refactored.go (5)
1375-1380: Good: caller-provided formItems preserved in CustomFormsOverride.Shallow merge keeps extensibility without regressing forms.
213-215: Fix typo: “WaitingdReason” → “WaitingReason”.Misspelling breaks column mapping.
Apply this diff:
- createStringColumn("WaitingdReason", ".state.waiting.reason"), + createStringColumn("WaitingReason", ".state.waiting.reason"),(Apply in both init and containers lists.)
Also applies to: 224-226
762-764: Fix JSONPath quoting for ServiceAccount annotation.Nested single quotes break parsing.
Apply this diff:
- "text": "{reqsJsonPath[0]['.metadata.annotations['kubernetes.io/service-account.name']']['-']}", - "href": "/openapi-ui/{2}/{3}/factory/serviceaccount-details/{reqsJsonPath[0]['.metadata.annotations['kubernetes.io/service-account.name']']['-']}", + "text": "{reqsJsonPath[0]['.metadata.annotations[\"kubernetes.io/service-account.name\"]']['-']}", + "href": "/openapi-ui/{2}/{3}/factory/serviceaccount-details/{reqsJsonPath[0]['.metadata.annotations[\"kubernetes.io/service-account.name\"]']['-']}",
931-939: Wrong JSONPath for Service Pod selector.Should be .spec.selector.
Apply this diff:
- "jsonPathToLabels": ".spec.template.metadata.labels", + "jsonPathToLabels": ".spec.selector",
1527-1531: Fix key: “stategySuccess” → “strategySuccess”.Prevents success evaluation from working.
Apply this diff:
- "stategySuccess": "every", + "strategySuccess": "every",internal/controller/dashboard/manager.go (2)
305-338: LGTM: robust kind detection in cleanupResourceType.Switch-based kind selection fixes empty GVK issue and ensures correct expected-set lookup.
239-246: Prevent create/delete thrash: expected-set skips CRDs with nil Dashboard but ensure still creates.*Either include CRDs with nil Dashboard in expected set, or skip ensure* for them. Including them is simplest here.
Apply this diff:
- if crd.Spec.Dashboard == nil { - continue - } -internal/controller/dashboard/unified_helpers.go (1)
288-347: Unified factory/header helpers look consistent and reusable.Auto badge, IDs, and tabs wiring are clean and align with the refactor.
internal/controller/dashboard/helpers.go (6)
264-298: Potential precision loss converting large uints to float64.If specs can contain large IDs/counters (> 2^53−1), float64 loses precision. Consider representing such numbers as strings or using json.Number.
Would large unsigned integers appear in these payloads (cluster IDs, timestamps as uint64, etc.)? If yes, I can draft a safe handling path (json.Decoder.UseNumber + preserving numbers as strings).
402-408: "appVersion" is nonstandard at top level; confirm intent.Kubernetes objects don’t have a top-level appVersion field. If this is a dashboard-only artifact, fine; otherwise consider removing.
19-23: Ignore removal suggestion for fieldInfo
fieldInfo is referenced in internal/controller/dashboard/factory.go (e.g., in sortFieldsByKeysOrder and collectOpenAPILeafFields), so it cannot be removed.Likely an incorrect or invalid review comment.
49-53: No changes needed:Application.Pluralis defined. ThePluralfield exists onCozystackResourceDefinitionApplication, so this code compiles as is.Likely an incorrect or invalid review comment.
29-46: Derive group/version from crd.APIVersion instead of hardcoding.Hardcoding "apps.cozystack.io/v1alpha1" will break for non‑apps resources. Parse crd.APIVersion ("group/version") first, then fall back.
func pickGVK(crd *cozyv1alpha1.CozystackResourceDefinition) (group, version, kind string) { // Best guess based on your examples: if crd.Spec.Application.Kind != "" { kind = crd.Spec.Application.Kind } + // Prefer deriving group/version from the top-level APIVersion ("group/version") + if av := crd.APIVersion; av != "" { + if i := strings.IndexByte(av, '/'); i > 0 && i < len(av)-1 { + group, version = av[:i], av[i+1:] + } + } + // Reasonable fallbacks if any are empty: if group == "" { group = "apps.cozystack.io" } if version == "" { version = "v1alpha1" } if kind == "" { kind = "Resource" } return }
218-241: Nested prefill still emits zero‑values; only explicit defaults should be emitted for nested fields.Condition
if val != nil || topLeveladds zero‑values at nested paths, duplicating/overriding defaults.- default: - // For primitive types, use default if present, otherwise zero value - val := defaultOrZero(sub) - // Only emit zero-value entries when at top level - if val != nil || topLevel { - entry := map[string]interface{}{ - "path": toIfaceSlice(currentPath), - "value": val, - } - *values = append(*values, entry) - } + default: + if topLevel { + // Top-level: emit default or zero-value + if val := defaultOrZero(sub); val != nil { + *values = append(*values, map[string]interface{}{ + "path": toIfaceSlice(currentPath), + "value": val, + }) + } + } else { + // Nested: emit only explicit defaults + if v, has := sub["default"]; has { + *values = append(*values, map[string]interface{}{ + "path": toIfaceSlice(currentPath), + "value": v, + }) + } + }packages/system/dashboard/values.yaml (1)
2-6: Image digest bumps look goodPinned by digest; no concerns from me.
Please confirm the referenced digests exist in GHCR and correspond to the intended builds.
packages/system/cozystack-controller/values.yaml (1)
2-2: Controller image digest updatedLooks good and consistent with the PR scope.
Please verify the new digest is published before merging to avoid rollout failures.
internal/controller/dashboard/static_helpers.go (2)
900-907: Previous typo fixed: strategySuccessThe earlier “stategySuccess” typo is corrected; thanks.
1075-1085: Verify front-end key name
The misspelled keysubstractHeightis used across Go code; nosubtractHeightexists—confirm the front-end consumes this exact name before renaming.internal/controller/dashboard/sidebar.go (1)
221-237: Good: idempotent update and consistent labelingAdding dashboard labels and skipping no-op spec updates reduces churn and improves reconcile performance.
| // Initialize static resources once during controller startup using manager.Runnable | ||
| if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error { | ||
| if err := r.initializeStaticResourcesOnce(ctx); err != nil { | ||
| log.FromContext(ctx).Error(err, "Failed to initialize static resources") | ||
| return err | ||
| } | ||
| return nil | ||
| })); err != nil { | ||
| return err |
There was a problem hiding this comment.
Avoid blocking manager start on static init
By returning the error from initializeStaticResourcesOnce inside mgr.Add, a transient API hiccup during startup prevents the manager from ever running. We should log and continue (letting the periodic reconciler attempt again) instead of aborting the process. Please swallow/log the error inside the runnable and return nil so the manager can keep serving.
🤖 Prompt for AI Agents
In internal/controller/cozystackresource_controller.go around lines 151 to 159,
the Runnable currently returns the error from initializeStaticResourcesOnce
which causes mgr.Add to fail and abort manager startup; change the Runnable so
that if initializeStaticResourcesOnce returns an error you log it (using
log.FromContext(ctx).Error(err, "...") or similar) but do not return the
error—always return nil from the Runnable so the manager continues starting and
the reconciler can retry later.
| // Build desired spec from CRD fields | ||
| d := crd.Spec.Dashboard | ||
| app := crd.Spec.Application | ||
|
|
||
| displayName := d.Singular | ||
| if displayName == "" { | ||
| displayName = app.Kind | ||
| } | ||
|
|
||
| tags := make([]any, len(d.Tags)) | ||
| for i, t := range d.Tags { | ||
| tags[i] = t | ||
| } | ||
|
|
||
| specMap := map[string]any{ | ||
| "description": d.Description, | ||
| "name": displayName, | ||
| "type": "nonCrd", | ||
| "apiGroup": "apps.cozystack.io", | ||
| "apiVersion": "v1alpha1", | ||
| "typeName": app.Plural, // e.g., "buckets" | ||
| "disabled": false, | ||
| "hidden": false, | ||
| "tags": tags, | ||
| "icon": d.Icon, | ||
| } |
There was a problem hiding this comment.
Avoid hardcoded API group/version; derive from CRD.
Using fixed "apps.cozystack.io/v1alpha1" and app.Plural breaks for other GVs or plural overrides. Use pickGVK/pickPlural.
- // Build desired spec from CRD fields
+ // Build desired spec from CRD fields
d := crd.Spec.Dashboard
app := crd.Spec.Application
displayName := d.Singular
if displayName == "" {
displayName = app.Kind
}
tags := make([]any, len(d.Tags))
for i, t := range d.Tags {
tags[i] = t
}
+ g, v, kind := pickGVK(crd)
+ plural := pickPlural(kind, crd)
+
specMap := map[string]any{
"description": d.Description,
"name": displayName,
- "type": "nonCrd",
- "apiGroup": "apps.cozystack.io",
- "apiVersion": "v1alpha1",
- "typeName": app.Plural, // e.g., "buckets"
+ "type": "nonCrd",
+ "apiGroup": g,
+ "apiVersion": v,
+ "typeName": plural, // e.g., "buckets"
"disabled": false,
"hidden": false,
"tags": tags,
"icon": d.Icon,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Build desired spec from CRD fields | |
| d := crd.Spec.Dashboard | |
| app := crd.Spec.Application | |
| displayName := d.Singular | |
| if displayName == "" { | |
| displayName = app.Kind | |
| } | |
| tags := make([]any, len(d.Tags)) | |
| for i, t := range d.Tags { | |
| tags[i] = t | |
| } | |
| specMap := map[string]any{ | |
| "description": d.Description, | |
| "name": displayName, | |
| "type": "nonCrd", | |
| "apiGroup": "apps.cozystack.io", | |
| "apiVersion": "v1alpha1", | |
| "typeName": app.Plural, // e.g., "buckets" | |
| "disabled": false, | |
| "hidden": false, | |
| "tags": tags, | |
| "icon": d.Icon, | |
| } | |
| // Build desired spec from CRD fields | |
| d := crd.Spec.Dashboard | |
| app := crd.Spec.Application | |
| displayName := d.Singular | |
| if displayName == "" { | |
| displayName = app.Kind | |
| } | |
| tags := make([]any, len(d.Tags)) | |
| for i, t := range d.Tags { | |
| tags[i] = t | |
| } | |
| g, v, kind := pickGVK(crd) | |
| plural := pickPlural(kind, crd) | |
| specMap := map[string]any{ | |
| "description": d.Description, | |
| "name": displayName, | |
| "type": "nonCrd", | |
| "apiGroup": g, | |
| "apiVersion": v, | |
| "typeName": plural, // e.g., "buckets" | |
| "disabled": false, | |
| "hidden": false, | |
| "tags": tags, | |
| "icon": d.Icon, | |
| } |
🤖 Prompt for AI Agents
internal/controller/dashboard/marketplacepanel.go around lines 58-83: the
specMap currently hardcodes apiGroup/apiVersion ("apps.cozystack.io"/"v1alpha1")
and uses app.Plural directly which breaks when the CRD has different
group/version or a custom plural; replace the hardcoded values by deriving the
group and version from the CRD using the existing pickGVK helper (or equivalent)
and derive the plural via pickPlural (or the CRD's stored plural), then set
"apiGroup" to the picked group, "apiVersion" to the picked version, and
"typeName" to the picked plural so the spec reflects the actual CRD GVK/plural
rather than fixed strings.
| if name == "stock-cluster-.v1.nodes" || | ||
| name == "stock-cluster-.v1.pods" || | ||
| name == "stock-namespace-.v1.pods" || | ||
| name == "factory-node-details-v1.pods" || | ||
| name == "factory-v1.pods" { | ||
| shouldHaveID = false | ||
| } | ||
|
|
There was a problem hiding this comment.
Suspicious names in shouldHaveID exceptions (-.v1.*)
The strings "stock-cluster-.v1.nodes|pods" and "stock-namespace-.v1.pods" look malformed (extra "-."). Either fix them or rely solely on the later Contains("pods"/"nodes") guard.
Minimal fix:
- if name == "stock-cluster-.v1.nodes" ||
- name == "stock-cluster-.v1.pods" ||
- name == "stock-namespace-.v1.pods" ||
+ if name == "stock-cluster-v1.nodes" ||
+ name == "stock-cluster-v1.pods" ||
+ name == "stock-namespace-v1.pods" ||
name == "factory-node-details-v1.pods" ||
name == "factory-v1.pods" {
shouldHaveID = false
}Alternatively, delete this whole block and keep the Contains("pods"/"nodes") rule below for simplicity.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if name == "stock-cluster-.v1.nodes" || | |
| name == "stock-cluster-.v1.pods" || | |
| name == "stock-namespace-.v1.pods" || | |
| name == "factory-node-details-v1.pods" || | |
| name == "factory-v1.pods" { | |
| shouldHaveID = false | |
| } | |
| if name == "stock-cluster-v1.nodes" || | |
| name == "stock-cluster-v1.pods" || | |
| name == "stock-namespace-v1.pods" || | |
| name == "factory-node-details-v1.pods" || | |
| name == "factory-v1.pods" { | |
| shouldHaveID = false | |
| } |
🤖 Prompt for AI Agents
In internal/controller/dashboard/static_helpers.go around lines 53 to 60, the
exception list contains malformed resource names with an extra "-." (e.g.
"stock-cluster-.v1.nodes" and "stock-namespace-.v1.pods"); either correct those
strings to the intended names (remove the spurious "-" so they read like
"stock-cluster.v1.nodes" / "stock-namespace.v1.pods") or delete this entire
conditional block and rely on the later generic
Contains("pods")/Contains("nodes") guard; update accordingly and run tests to
ensure behavior remains correct.
| // ensureTableUriMapping creates or updates a TableUriMapping resource for the given CRD | ||
| func (m *Manager) ensureTableUriMapping(ctx context.Context, crd *cozyv1alpha1.CozystackResourceDefinition) error { | ||
| // Links are fully managed by the CustomColumnsOverride. | ||
| return nil |
There was a problem hiding this comment.
Stubbed TableUriMapping breaks static resource sync
ensureTableUriMapping now unconditionally returns without creating or updating the mapping. The rest of the controller still expects the manager to surface these static TableUriMapping resources during initialization, so this turns the feature into a no-op. Please keep the previous create/update logic (or add the real implementation here) so the reconciler continues to publish the mappings.
0ba3995 to
c14212e
Compare
- Refactor code for dashboard resources creation - Move dashboard-config helm chart to dynamic dashboard controller - Move white-label configuration to separate configmap Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
c14212e to
9873011
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/controller/dashboard/sidebar.go (1)
101-106: Sorting order contradicts comment; use descending Weight.Comment says Weight (desc), but comparator sorts ascending. Flip the comparison.
Apply:
- if categories[cat][i].Weight != categories[cat][j].Weight { - return categories[cat][i].Weight < categories[cat][j].Weight // lower weight first - } + if categories[cat][i].Weight != categories[cat][j].Weight { + return categories[cat][i].Weight > categories[cat][j].Weight // higher weight first + }
🧹 Nitpick comments (22)
packages/system/dashboard/templates/web.yaml (2)
18-18: Add checksum annotation to trigger rollouts on ConfigMap changesWithout a pod‑template change, updates to incloud-web-dashboard-config won’t roll the Deployment. Add a checksum of the ConfigMap to annotations.
- annotations: null + annotations: + checksum/config: {{ include (print $.Template.BasePath "/configmap.yaml") . | sha256sum }}
134-152: LGTM; configMapKeyRef switch is good. Consider making SVG keys optionalText keys likely should be required; SVGs may be optional. Marking them optional avoids startup failure if SVGs are omitted.
- name: LOGO_SVG valueFrom: configMapKeyRef: name: incloud-web-dashboard-config key: LOGO_SVG + optional: true - name: ICON_SVG valueFrom: configMapKeyRef: name: incloud-web-dashboard-config key: ICON_SVG + optional: trueAlso applies to: 154-162
packages/system/dashboard/Makefile (2)
8-8: Ensure update-tenant-text runs before image builds; avoid parallel build raceAs written, prerequisites may run in parallel; TENANT_TEXT could be updated after images build. Put update-tenant-text first to enforce order.
-image: image-openapi-ui image-openapi-ui-k8s-bff image-token-proxy update-tenant-text +image: update-tenant-text image-openapi-ui image-openapi-ui-k8s-bff image-token-proxy
69-70: Avoid mutating template files at build time; drive TENANT_TEXT via Helm valuesChanging templates with yq dirties the repo and is brittle. Prefer templating TENANT_TEXT from values.yaml and update the value instead.
-update-tenant-text: - yq -i '.data.TENANT_TEXT = "$(TAG)"' ./templates/configmap.yaml +update-tenant-text: + yq -i '.dashboard.tenantText = strenv(TAG)' values.yamlAdd PHONY to silence checkmake and clarify intent:
.PHONY: image update-tenant-textpackages/system/dashboard/templates/configmap.yaml (1)
11-11: Template TENANT_TEXT from values instead of hardcodingAligns with the Makefile change to write values.yaml and avoids editing templates in CI/CD.
- TENANT_TEXT: "latest" + TENANT_TEXT: {{ .Values.dashboard.tenantText | default "latest" | quote }}internal/controller/dashboard/sidebar.go (1)
24-31: Doc vs implementation mismatch: “Tenant Info” location.Comments say “Tenant Info” under Marketplace, but code adds “Info” under Administration. Align comment or menu structure.
Also applies to: 150-171
internal/controller/dashboard/customcolumns.go (1)
148-168: Return the actual OperationResult from CreateOrUpdate.You’re discarding the OperationResult and always returning None, which hides useful state.
Apply:
- _, err := controllerutil.CreateOrUpdate(ctx, m.client, obj, func() error { + res, err := controllerutil.CreateOrUpdate(ctx, m.client, obj, func() error { @@ - return controllerutil.OperationResultNone, err + return res, errinternal/controller/cozystackresource_controller.go (3)
79-85: Orphan cleanup on every reconcile may be heavy; consider periodic or post-cache gating.If CleanupOrphanedResources lists and diffs many objects, run it on a leader-only periodic Runnable or with a backoff to reduce load.
If this may be expensive, measure call frequency and duration via logs/metrics before changing behavior. Do you want a small metrics wrapper to time this section?
106-138: Simplify one-time init with sync.Once.The bool+mutex works, but sync.Once is clearer and thread-safe.
Example:
var initOnce sync.Once func (r *CozystackResourceDefinitionReconciler) initializeStaticResourcesOnce(ctx context.Context) error { var initErr error initOnce.Do(func() { mgr := dashboard.NewManager(/*...*/) initErr = mgr.InitializeStaticResources(ctx) if initErr == nil { log.FromContext(ctx).Info("Static dashboard resources initialized successfully") } }) return initErr }
162-179: Redundant WithPredicates() with no predicates.Calling builder.WithPredicates() without args is a no-op; remove it for clarity or pass actual predicates.
- For(&cozyv1alpha1.CozystackResourceDefinition{}, builder.WithPredicates()). + For(&cozyv1alpha1.CozystackResourceDefinition{}).internal/controller/dashboard/breadcrumb.go (1)
38-43: TODO present: parameterizing “Tenant Modules”.If this needs to be configurable, add it as a field in Dashboard spec or derive via helper. I can draft it if you want.
internal/controller/dashboard/customformsprefill.go (1)
51-55: Comment is misleading: json.Marshal doesn't sort keysjson.Marshal doesn't guarantee map key order. Rely on compareArbitrarySpecs for change detection, or switch to a canonical JSON encoder if you need determinism in Raw.
Apply this diff:
- // Use json.Marshal with sorted keys to ensure consistent output + // Note: json.Marshal doesn't guarantee key order; compareArbitrarySpecs handles normalizationinternal/controller/dashboard/customformsoverride.go (1)
40-46: Prefer empty array over null for "sort"When KeysOrder is absent, emit an empty array instead of null to avoid consumer ambiguity.
Apply this diff:
var sort []any if crd.Spec.Dashboard != nil && len(crd.Spec.Dashboard.KeysOrder) > 0 { sort = make([]any, len(crd.Spec.Dashboard.KeysOrder)) for i, v := range crd.Spec.Dashboard.KeysOrder { sort[i] = v } + } else { + sort = []any{} }internal/controller/dashboard/factory.go (1)
372-391: Optional: simplify with SliceStableUse sort.SliceStable to keep original order when neither field is in orderMap; removes the i<j fallback.
Apply this diff:
- sort.Slice(fields, func(i, j int) bool { + sort.SliceStable(fields, func(i, j int) bool { posI, existsI := orderMap[fields[i].JSONPathSpec] posJ, existsJ := orderMap[fields[j].JSONPathSpec] // If both exist in orderMap, sort by position if existsI && existsJ { return posI < posJ } // If only one exists, prioritize the one that exists if existsI { return true } if existsJ { return false } - // If neither exists, maintain original order (stable sort) - return i < j + // If neither exists, keep original relative order + return false })internal/controller/dashboard/static_refactored.go (1)
311-314: Inconsistent hex format (#RRGGBBAA vs #RRGGBB) for colorsYou’re mixing 8‑digit and 6‑digit hex (e.g., "#a25792ff" vs "#6ca100"). If the UI/CSS consumer doesn’t support #RRGGBBAA, fallback will vary. Standardize to 6‑digit (or rgba()).
Also applies to: 640-644
internal/controller/dashboard/unified_helpers.go (2)
263-272: Remove unused parameter in createResourceWithAutoIDresourceType isn’t used; keep API tight to avoid confusion.
-func createResourceWithAutoID(resourceType, name string, spec map[string]any) map[string]any { +func createResourceWithAutoID(name string, spec map[string]any) map[string]any { // Generate spec.id from name specID := generateSpecID(name) // Add the spec.id to the spec spec["id"] = specID return spec }
42-73: Optional: enforce RFC1123 for metadata names where possiblegenerateMetadataName normalizes basic cases but doesn’t validate length/labels. Consider using k8s validation (e.g., apimachinery validation helpers) if these names are ever used as metadata.name. Not blocking if names are known-safe.
internal/controller/dashboard/static_helpers.go (5)
53-60: Remove brittle exceptions with malformed names (-.v1.*); rely on the generic guardThese names don’t match the generated metadata names and add confusion. The Contains("pods"/"nodes") guard already prevents setting id for those.
- shouldHaveID := true - if name == "stock-cluster-.v1.nodes" || - name == "stock-cluster-.v1.pods" || - name == "stock-namespace-.v1.pods" || - name == "factory-node-details-v1.pods" || - name == "factory-v1.pods" { - shouldHaveID = false - } + shouldHaveID := true
61-64: Consider setting id for pod/node overrides to avoid lookup ambiguityIf the UI resolves customizations by spec.id, keeping id unset for pods/nodes may break tables referencing "/v1/pods". Recommend setting id unconditionally or at least for known pod/node overrides.
Would you like a targeted patch to set id for:
- stock-cluster-/v1/pods
- stock-namespace-/v1/pods
- factory-node-details-v1.pods
- factory-v1.pods
…or to set id for all overrides?
389-410: Duplicate override config for stock-namespace-networking.k8s.io.v1.ingressesThis block duplicates settings already added at lines 319-340. Keep one to avoid drift.
- if name == "stock-namespace-networking.k8s.io.v1.ingresses" { - data["additionalPrinterColumnsTrimLengths"] = []any{ - map[string]any{ - "key": "Name", - "value": float64(64), - }, - } - data["additionalPrinterColumnsUndefinedValues"] = []any{ - map[string]any{ - "key": "Hosts", - "value": "-", - }, - map[string]any{ - "key": "Address", - "value": "-", - }, - map[string]any{ - "key": "Port", - "value": "-", - }, - } - }
1044-1045: CSS value type: use string with unit for marginTopElsewhere you use "-30px"; here it’s a number (-30). Use consistent CSS strings to avoid styling issues.
- "containerStyle": map[string]any{"marginTop": -30}, + "containerStyle": map[string]any{"marginTop": "-30px"},
533-566: Avoid fixed IDs in table cells to prevent DOM/id collisionsRepeated "header-badge" and "name-link" IDs across table rows can collide. Prefer generated IDs (e.g., generateBadgeID/generateLinkID) with a context suffix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (90)
cmd/cozystack-controller/main.go(1 hunks)internal/controller/cozystackresource_controller.go(6 hunks)internal/controller/dashboard/breadcrumb.go(1 hunks)internal/controller/dashboard/customcolumns.go(1 hunks)internal/controller/dashboard/customformsoverride.go(1 hunks)internal/controller/dashboard/customformsprefill.go(1 hunks)internal/controller/dashboard/factory.go(5 hunks)internal/controller/dashboard/helpers.go(1 hunks)internal/controller/dashboard/manager.go(2 hunks)internal/controller/dashboard/marketplacepanel.go(1 hunks)internal/controller/dashboard/sidebar.go(1 hunks)internal/controller/dashboard/static_helpers.go(1 hunks)internal/controller/dashboard/static_processor.go(1 hunks)internal/controller/dashboard/static_refactored.go(1 hunks)internal/controller/dashboard/tableurimapping.go(1 hunks)internal/controller/dashboard/ui_helpers.go(1 hunks)internal/controller/dashboard/unified_helpers.go(1 hunks)internal/controller/dashboard/webextras.go(0 hunks)packages/core/platform/bundles/paas-full.yaml(0 hunks)packages/core/platform/bundles/paas-hosted.yaml(0 hunks)packages/system/cozystack-controller/values.yaml(1 hunks)packages/system/dashboard-config/Chart.yaml(0 hunks)packages/system/dashboard-config/Makefile(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/configmap-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/namespace-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/node-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/pod-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/secret-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/factory/service-details.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-api.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-builtin.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-project-api.yaml(0 hunks)packages/system/dashboard-config/templates/Breadcrumb/stock-project-builtin.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.apps.cozystack.io.workloadmonitors.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.core.cozystack.io.tenantsecretstables.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-ingress-details-rules.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-node-images.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-pod-details-list.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/factory-status-conditions.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/hidden.metadata.tpl(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/icons.tpl(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/k8s.container.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/CustomColumnsOverride/v1alpha1.cozystack.io.workloads.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/storage.k8s.io.v1.storageclasses.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.namespaces.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumeclaims.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumes.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/CustomFormOverride/v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/cozy-marketplace.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/annotations.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/counters.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/labels.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/links.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/statuses.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/tables.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/taints.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/times.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/helpers/tolerations.tpl(0 hunks)packages/system/dashboard-config/templates/Factory/namespace-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/node-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/pod-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/secret-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/service-details.yaml(0 hunks)packages/system/dashboard-config/templates/Factory/workloadmonitor-details.yaml(0 hunks)packages/system/dashboard-config/templates/Navigation/navigaton.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/apps.cozystack.io.v1alpha1.virtualmachines.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/core.cozystack.io.v1alpha1.tenantnamespaces.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/networking.k8s.io.v1.ingresses.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.configmaps.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.namespaces.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.nodes.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.pods.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.secrets.yaml(0 hunks)packages/system/dashboard-config/templates/TableUriMapping/v1.services.yaml(0 hunks)packages/system/dashboard-config/templates/_helpers.tpl(0 hunks)packages/system/dashboard/Makefile(2 hunks)packages/system/dashboard/templates/configmap.yaml(1 hunks)packages/system/dashboard/templates/web.yaml(1 hunks)packages/system/dashboard/values.yaml(1 hunks)
💤 Files with no reviewable changes (68)
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1alpha1.cozystack.io.workloads.yaml
- packages/system/dashboard-config/Makefile
- packages/system/dashboard-config/templates/Navigation/navigaton.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.pods.yaml
- packages/system/dashboard-config/templates/Factory/helpers/counters.tpl
- packages/system/dashboard-config/templates/TableUriMapping/v1.services.yaml
- packages/system/dashboard-config/templates/TableUriMapping/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/node-details.yaml
- packages/core/platform/bundles/paas-hosted.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.pods.yaml
- packages/system/dashboard-config/templates/Factory/helpers/taints.tpl
- packages/system/dashboard-config/templates/Breadcrumb/factory/namespace-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.secrets.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/icons.tpl
- packages/system/dashboard-config/templates/CustomFormOverride/v1.configmaps.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.pods.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/Factory/helpers/labels.tpl
- packages/system/dashboard-config/templates/TableUriMapping/v1.namespaces.yaml
- packages/system/dashboard-config/templates/Factory/helpers/annotations.tpl
- packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumes.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/helpers/hidden.metadata.tpl
- packages/system/dashboard-config/templates/Factory/helpers/tolerations.tpl
- packages/system/dashboard-config/templates/Breadcrumb/factory/configmap-details.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/secret-details.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.secrets.yaml
- packages/system/dashboard-config/templates/TableUriMapping/v1.nodes.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-project-api.yaml
- packages/system/dashboard-config/templates/Factory/helpers/links.tpl
- packages/system/dashboard-config/templates/Factory/pod-details.yaml
- packages/system/dashboard-config/Chart.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-ingress-details-rules.yaml
- packages/system/dashboard-config/templates/Factory/helpers/tables.tpl
- packages/system/dashboard-config/templates/_helpers.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-node-images.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-api.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-pod-details-list.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.apps.cozystack.io.workloadmonitors.yaml
- packages/system/dashboard-config/templates/TableUriMapping/apps.cozystack.io.v1alpha1.virtualmachines.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/service-details.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/storage.k8s.io.v1.storageclasses.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.services.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-cluster-builtin.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.namespaces.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.nodes.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.nodes.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.secrets.yaml
- packages/system/dashboard-config/templates/TableUriMapping/core.cozystack.io.v1alpha1.tenantnamespaces.yaml
- packages/system/dashboard-config/templates/Factory/helpers/times.tpl
- packages/system/dashboard-config/templates/Factory/cozy-marketplace.yaml
- packages/system/dashboard-config/templates/Breadcrumb/factory/pod-details.yaml
- packages/system/dashboard-config/templates/Breadcrumb/stock-project-builtin.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/networking.k8s.io.v1.ingresses.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/k8s.container.yaml
- packages/system/dashboard-config/templates/CustomFormOverride/v1.persistentvolumeclaims.yaml
- packages/system/dashboard-config/templates/Factory/namespace-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/v1.configmaps.yaml
- internal/controller/dashboard/webextras.go
- packages/system/dashboard-config/templates/TableUriMapping/v1.configmaps.yaml
- packages/system/dashboard-config/templates/Factory/secret-details.yaml
- packages/system/dashboard-config/templates/Factory/service-details.yaml
- packages/core/platform/bundles/paas-full.yaml
- packages/system/dashboard-config/templates/Factory/helpers/statuses.tpl
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-status-conditions.yaml
- packages/system/dashboard-config/templates/Factory/node-details.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1alpha1.core.cozystack.io.tenantsecretstables.yaml
- packages/system/dashboard-config/templates/CustomColumnsOverride/factory-details-v1.services.yaml
- packages/system/dashboard-config/templates/Factory/workloadmonitor-details.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- internal/controller/dashboard/ui_helpers.go
- cmd/cozystack-controller/main.go
- internal/controller/dashboard/static_processor.go
- packages/system/dashboard/values.yaml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-10T22:02:47.729Z
Learnt from: lllamnyp
PR: cozystack/cozystack#1380
File: internal/controller/kubeovnplunger/metrics.go:291-296
Timestamp: 2025-09-10T22:02:47.729Z
Learning: In the kubeovnplunger controller, r.lastLeader map is initialized in SetupWithManager method before any reconcile operations begin, making defensive nil checks unnecessary in the reconcile methods.
Applied to files:
internal/controller/cozystackresource_controller.go
📚 Learning: 2025-09-10T22:02:47.729Z
Learnt from: lllamnyp
PR: cozystack/cozystack#1380
File: internal/controller/kubeovnplunger/metrics.go:291-296
Timestamp: 2025-09-10T22:02:47.729Z
Learning: In the kubeovnplunger controller, r.lastLeader map is initialized in SetupWithManager method (line 151) with make(map[string]string) before any reconcile operations begin, making defensive nil checks unnecessary in the reconcile methods.
Applied to files:
internal/controller/cozystackresource_controller.go
🧬 Code graph analysis (13)
internal/controller/dashboard/sidebar.go (1)
internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(31-31)
internal/controller/dashboard/customcolumns.go (1)
internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(31-31)
internal/controller/dashboard/helpers.go (1)
api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/customformsoverride.go (2)
internal/controller/dashboard/manager.go (2)
Manager(41-45)ResourceTypeDynamic(31-31)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/tableurimapping.go (2)
internal/controller/dashboard/manager.go (1)
Manager(41-45)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/customformsprefill.go (2)
internal/controller/dashboard/manager.go (2)
Manager(41-45)ResourceTypeDynamic(31-31)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/static_refactored.go (1)
internal/controller/dashboard/unified_helpers.go (2)
UnifiedResourceConfig(277-286)BadgeSizeLarge(96-96)
internal/controller/dashboard/factory.go (2)
internal/controller/dashboard/unified_helpers.go (3)
UnifiedResourceConfig(277-286)BadgeSizeLarge(96-96)BadgeSizeMedium(95-95)internal/controller/dashboard/manager.go (1)
ResourceTypeDynamic(31-31)
internal/controller/dashboard/manager.go (2)
api/v1alpha1/cozystackresourcedefinitions_types.go (2)
CozystackResourceDefinition(27-32)CozystackResourceDefinitionList(37-41)internal/controller/dashboard/static_refactored.go (1)
CreateAllStaticResources(1683-1717)
internal/controller/dashboard/static_helpers.go (1)
internal/controller/dashboard/unified_helpers.go (3)
BadgeSizeMedium(95-95)BadgeConfig(83-88)BadgeSizeLarge(96-96)
internal/controller/cozystackresource_controller.go (1)
internal/controller/dashboard/manager.go (2)
NewManager(56-62)WithCRDListFunc(51-53)
internal/controller/dashboard/marketplacepanel.go (2)
internal/controller/dashboard/manager.go (2)
Manager(41-45)ResourceTypeDynamic(31-31)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
internal/controller/dashboard/breadcrumb.go (2)
internal/controller/dashboard/manager.go (2)
Manager(41-45)ResourceTypeDynamic(31-31)api/v1alpha1/cozystackresourcedefinitions_types.go (1)
CozystackResourceDefinition(27-32)
🪛 checkmake (0.2.2)
packages/system/dashboard/Makefile
[warning] 8-8: Target "image" should be declared PHONY.
(phonydeclared)
⏰ 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 (10)
packages/system/cozystack-controller/values.yaml (1)
1-2: Digest bump looks good.The controller image digest update aligns with the refactored dashboard workflow.
internal/controller/dashboard/tableurimapping.go (1)
9-13: No-op breaks expected TableUriMapping behavior (same concern as earlier).This method returns without creating/updating any mapping; upstream logic likely still expects these resources to exist. Either implement the creation/update here or remove all call sites/cleanup logic that assumes mappings are present.
Run this to confirm whether TableUriMapping is still referenced or expected elsewhere:
#!/bin/bash # Calls to ensureTableUriMapping rg -n -C2 '\bensureTableUriMapping\s*\(' --type=go # Any usage of TableUriMapping resources/types rg -n -C3 '\bTableUriMapping\b' --type=go # Any references by name/label rg -n -i -C2 'tableuri'internal/controller/dashboard/sidebar.go (1)
225-227: LGTM: labels added and update guarded by spec diff.Adding dashboard labels and avoiding unnecessary updates is good.
Also applies to: 232-236
internal/controller/dashboard/customcolumns.go (1)
152-154: LGTM: labels added and spec updates are idempotent.Good addition of labels and change-detection before updating spec.
Also applies to: 159-163
internal/controller/dashboard/breadcrumb.go (1)
18-43: LGTM: now uses group/version in links and guards nil Dashboard.This addresses prior issues (hardcoded path and nil deref risk).
internal/controller/cozystackresource_controller.go (1)
151-160: Don’t block manager start on static init failures.Returning the error from the Runnable aborts manager startup on transient failures. Log and continue so reconcile can retry.
Apply:
- if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error { - if err := r.initializeStaticResourcesOnce(ctx); err != nil { - log.FromContext(ctx).Error(err, "Failed to initialize static resources") - return err - } - return nil - })); err != nil { + if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error { + if err := r.initializeStaticResourcesOnce(ctx); err != nil { + log.FromContext(ctx).Error(err, "Failed to initialize static resources") + // Do not block manager start; let reconciler retry later. + return nil + } + return nil + })); err != nil { return err }internal/controller/dashboard/manager.go (1)
316-343: LGTM: robust kind resolution in cleanupResourceTypeSwitch-based kind detection fixes the zero-GVK pitfall and aligns with expected-set keys.
internal/controller/dashboard/marketplacepanel.go (2)
26-56: LGTM: delete branches for nil/tenant dashboardsDefensive Get/Delete paths for missing dashboard and for tenant modules are correct and avoid dangling panels.
72-83: Derive apiGroup/apiVersion/typeName from CRD; avoid hardcoded valuesHardcoding "apps.cozystack.io/v1alpha1" and using app.Plural breaks for other GVs or plural overrides. Use pickGVK/pickPlural.
Apply this diff:
- specMap := map[string]any{ + // derive GV and plural from CRD + g, v, kind := pickGVK(crd) + plural := pickPlural(kind, crd) + specMap := map[string]any{ "description": d.Description, "name": displayName, "type": "nonCrd", - "apiGroup": "apps.cozystack.io", - "apiVersion": "v1alpha1", - "typeName": app.Plural, // e.g., "buckets" + "apiGroup": g, + "apiVersion": v, + "typeName": plural, // e.g., "buckets" "disabled": false, "hidden": false, "tags": tags, "icon": d.Icon, }internal/controller/dashboard/factory.go (1)
49-57: LGTM: unified factory spec + change-guard + dashboard labelsGood move to centralized config (UnifiedResourceConfig/createUnifiedFactory), guarded updates via compareArbitrarySpecs, and consistent dashboard labeling.
Also applies to: 75-79
| val := defaultOrZero(sub) | ||
| // Only emit zero-value entries when at top level | ||
| if val != nil || topLevel { | ||
| entry := map[string]interface{}{ | ||
| "path": toIfaceSlice(currentPath), | ||
| "value": val, | ||
| } | ||
| *values = append(*values, entry) | ||
| } |
There was a problem hiding this comment.
Nested defaults bug: zero-values are emitted for nested fields
Current condition appends entries for nested primitives even when no default is present (val != nil for zero-values). This overrides intended nested defaults.
Apply this fix to only emit zero-values at top-level; for nested, emit only explicit defaults:
- // For primitive types, use default if present, otherwise zero value
- val := defaultOrZero(sub)
- // Only emit zero-value entries when at top level
- if val != nil || topLevel {
- entry := map[string]interface{}{
- "path": toIfaceSlice(currentPath),
- "value": val,
- }
- *values = append(*values, entry)
- }
+ // For primitive/array types:
+ if topLevel {
+ if val := defaultOrZero(sub); val != nil {
+ entry := map[string]interface{}{
+ "path": toIfaceSlice(currentPath),
+ "value": val,
+ }
+ *values = append(*values, entry)
+ }
+ } else {
+ if v, has := sub["default"]; has {
+ entry := map[string]interface{}{
+ "path": toIfaceSlice(currentPath),
+ "value": v,
+ }
+ *values = append(*values, entry)
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val := defaultOrZero(sub) | |
| // Only emit zero-value entries when at top level | |
| if val != nil || topLevel { | |
| entry := map[string]interface{}{ | |
| "path": toIfaceSlice(currentPath), | |
| "value": val, | |
| } | |
| *values = append(*values, entry) | |
| } | |
| // For primitive/array types: | |
| if topLevel { | |
| if val := defaultOrZero(sub); val != nil { | |
| entry := map[string]interface{}{ | |
| "path": toIfaceSlice(currentPath), | |
| "value": val, | |
| } | |
| *values = append(*values, entry) | |
| } | |
| } else { | |
| if v, has := sub["default"]; has { | |
| entry := map[string]interface{}{ | |
| "path": toIfaceSlice(currentPath), | |
| "value": v, | |
| } | |
| *values = append(*values, entry) | |
| } | |
| } |
🤖 Prompt for AI Agents
internal/controller/dashboard/helpers.go lines 241-249: the current code appends
entries when defaultOrZero(sub) returns a non-nil zero-value, causing nested
primitives with no explicit default to be emitted; change the logic to
distinguish explicit defaults from implied zero-values (e.g., have defaultOrZero
return (value, explicit bool) or add an isExplicitDefault helper), then only
append the entry when explicit == true OR topLevel == true so nested fields emit
only explicit defaults while top-level still emits zero-values.
| // Early return if crd.Spec.Dashboard is nil to prevent oscillation | ||
| if crd.Spec.Dashboard == nil { | ||
| return reconcile.Result{}, nil | ||
| } | ||
|
|
There was a problem hiding this comment.
Prevent create/delete thrash for tenant modules (spec.dashboard.name set)
The expected set skips CRDs with non-empty Dashboard.Name, but ensure* still creates Prefill/Override/etc., which cleanup then deletes. Short-circuit ensures (except MarketplacePanel delete) when Name is set.
Apply this diff:
// Early return if crd.Spec.Dashboard is nil to prevent oscillation
if crd.Spec.Dashboard == nil {
return reconcile.Result{}, nil
}
+ // Tenant modules: skip all ensure* except MarketplacePanel (it handles delete)
+ if strings.TrimSpace(crd.Spec.Dashboard.Name) != "" {
+ return m.ensureMarketplacePanel(ctx, crd)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Early return if crd.Spec.Dashboard is nil to prevent oscillation | |
| if crd.Spec.Dashboard == nil { | |
| return reconcile.Result{}, nil | |
| } | |
| // Early return if crd.Spec.Dashboard is nil to prevent oscillation | |
| if crd.Spec.Dashboard == nil { | |
| return reconcile.Result{}, nil | |
| } | |
| // Tenant modules: skip all ensure* except MarketplacePanel (it handles delete) | |
| if strings.TrimSpace(crd.Spec.Dashboard.Name) != "" { | |
| return m.ensureMarketplacePanel(ctx, crd) | |
| } |
🤖 Prompt for AI Agents
In internal/controller/dashboard/manager.go around lines 76-80, the early-return
currently only checks for crd.Spec.Dashboard == nil but still runs ensure
functions when crd.Spec.Dashboard.Name is set, causing create/delete thrash;
change the early-return to also short-circuit when crd.Spec.Dashboard.Name is
non-empty (i.e., return if crd.Spec.Dashboard == nil || crd.Spec.Dashboard.Name
!= ""), so the ensure Prefill/Override/etc. calls are skipped for tenant-managed
dashboards while keeping the MarketplacePanel delete path outside this
short-circuit as before.
| staticResources := CreateAllStaticResources() | ||
| for _, resource := range staticResources { | ||
| resourceType := resource.GetObjectKind().GroupVersionKind().Kind | ||
| if expected[resourceType] != nil { | ||
| expected[resourceType][resource.GetName()] = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Static resources may be deleted: don't use zero-valued GVK to detect kind
GetObjectKind().GroupVersionKind().Kind is empty for zero-valued objects; static resources won't be added to the expected set and will be GC’d by cleanup. Derive the kind via type switch (as you did in cleanupResourceType).
Apply this diff:
- for _, resource := range staticResources {
- resourceType := resource.GetObjectKind().GroupVersionKind().Kind
- if expected[resourceType] != nil {
- expected[resourceType][resource.GetName()] = true
- }
- }
+ for _, resource := range staticResources {
+ var resourceType string
+ switch resource.(type) {
+ case *dashv1alpha1.CustomColumnsOverride:
+ resourceType = "CustomColumnsOverride"
+ case *dashv1alpha1.CustomFormsOverride:
+ resourceType = "CustomFormsOverride"
+ case *dashv1alpha1.CustomFormsPrefill:
+ resourceType = "CustomFormsPrefill"
+ case *dashv1alpha1.MarketplacePanel:
+ resourceType = "MarketplacePanel"
+ case *dashv1alpha1.Sidebar:
+ resourceType = "Sidebar"
+ case *dashv1alpha1.TableUriMapping:
+ resourceType = "TableUriMapping"
+ case *dashv1alpha1.Breadcrumb:
+ resourceType = "Breadcrumb"
+ case *dashv1alpha1.Factory:
+ resourceType = "Factory"
+ default:
+ continue
+ }
+ if expected[resourceType] != nil {
+ expected[resourceType][resource.GetName()] = true
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| staticResources := CreateAllStaticResources() | |
| for _, resource := range staticResources { | |
| resourceType := resource.GetObjectKind().GroupVersionKind().Kind | |
| if expected[resourceType] != nil { | |
| expected[resourceType][resource.GetName()] = true | |
| } | |
| } | |
| staticResources := CreateAllStaticResources() | |
| for _, resource := range staticResources { | |
| var resourceType string | |
| switch resource.(type) { | |
| case *dashv1alpha1.CustomColumnsOverride: | |
| resourceType = "CustomColumnsOverride" | |
| case *dashv1alpha1.CustomFormsOverride: | |
| resourceType = "CustomFormsOverride" | |
| case *dashv1alpha1.CustomFormsPrefill: | |
| resourceType = "CustomFormsPrefill" | |
| case *dashv1alpha1.MarketplacePanel: | |
| resourceType = "MarketplacePanel" | |
| case *dashv1alpha1.Sidebar: | |
| resourceType = "Sidebar" | |
| case *dashv1alpha1.TableUriMapping: | |
| resourceType = "TableUriMapping" | |
| case *dashv1alpha1.Breadcrumb: | |
| resourceType = "Breadcrumb" | |
| case *dashv1alpha1.Factory: | |
| resourceType = "Factory" | |
| default: | |
| continue | |
| } | |
| if expected[resourceType] != nil { | |
| expected[resourceType][resource.GetName()] = true | |
| } | |
| } |
🤖 Prompt for AI Agents
In internal/controller/dashboard/manager.go around lines 234 to 240, the loop
over staticResources uses resource.GetObjectKind().GroupVersionKind().Kind which
is empty for zero-valued objects so static resources are not marked expected and
get GC’d; replace that GVK-based kind detection with a type switch (the same
approach used in cleanupResourceType) to derive the resource kind reliably, then
use the determined kind to index into expected and set
expected[kind][resource.GetName()] = true.
| "key": "pods", | ||
| "label": "Pods", | ||
| "children": []any{ | ||
| map[string]any{ | ||
| "type": "VisibilityContainer", | ||
| "data": map[string]any{ | ||
| "id": "service-pod-serving-vis", | ||
| "value": "{reqsJsonPath[0]['.spec.selector']['-']}", | ||
| "style": map[string]any{ | ||
| "margin": 0, | ||
| "padding": 0, | ||
| }, | ||
| }, | ||
| "children": []any{ | ||
| map[string]any{ | ||
| "type": "EnrichedTable", | ||
| "data": map[string]any{ | ||
| "id": "pods-table", | ||
| "baseprefix": "/openapi-ui", | ||
| "clusterNamePartOfUrl": "{2}", | ||
| "customizationId": "factory-node-details-/v1/pods", | ||
| "fetchUrl": "/api/clusters/{2}/k8s/api/v1/namespaces/{3}/pods", | ||
| "labelsSelectorFull": map[string]any{ | ||
| "pathToLabels": ".spec.selector", | ||
| "reqIndex": 0, | ||
| }, | ||
| "pathToItems": ".items", | ||
| "withoutControls": false, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Potential mismatch: pods table customizationId may not map to any CustomColumnsOverride spec.id
Service “Pods” tab uses customizationId "factory-node-details-/v1/pods". In createCustomColumnsOverride(), we currently suppress spec.id for pod/node overrides. If the UI matches overrides by spec.id (not metadata.name), this table won’t pick up the intended columns.
Please confirm how the UI resolves customizations:
- If it matches by spec.id, set id for pod/node overrides (see related suggestion in static_helpers.go).
- If it matches by metadata.name, ensure the UI applies the same name transform ("/" → ".", "-." → "-") used by generateMetadataName.
Run to inspect all customizationId values and corresponding override builders:
🏁 Script executed:
#!/bin/bash
# List all customizationId usages
rg -nP --type=go -C2 '"customizationId"\s*:\s*"' internal/controller/dashboard
# List all createCustomColumnsOverride IDs
rg -nP --type=go -n 'createCustomColumnsOverride\("([^"]+)"' internal/controller/dashboard | sed -E 's/.*createCustomColumnsOverride\("([^"]+)".*/\1/'Length of output: 5616
Align pods customizationId with override spec.id
internal/controller/dashboard/static_refactored.go:1101
- "customizationId": "factory-node-details-/v1/pods"
- "customizationId": "factory-node-details-v1.pods"
🤖 Prompt for AI Agents
internal/controller/dashboard/static_refactored.go around lines 1081-1115: the
pods component uses the wrong customizationId value; change the string from
"factory-node-details-/v1/pods" to "factory-node-details-v1.pods" so it aligns
with the override spec.id format (remove the leading slash and replace the path
separator with a dot).
What this PR does
Release note
Summary by CodeRabbit
New Features
Improvements
Refactor
Chores