feat(scalityui): add ExtraUIApps field for direct UI app injection#77
feat(scalityui): add ExtraUIApps field for direct UI app injection#77ZiyangLinScality merged 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an ExtraUIApps field to the ScalityUI custom resource, enabling direct injection of UI application configurations into the deployed-ui-apps ConfigMap without requiring ScalityUIComponent and ScalityUIComponentExposer resources. This provides a simpler way to add external or non-component-based UI applications to the shell.
Changes:
- Introduced
DeployedUIApptype in the API package with kubebuilder validation for required fields - Added
ExtraUIAppsfield to ScalityUISpec allowing users to specify additional UI apps directly - Updated the deployed-apps-configmap controller logic to merge ExtraUIApps with exposer-based apps
- Added comprehensive integration tests covering both standalone ExtraUIApps and merging scenarios
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| api/v1alpha1/scalityui_types.go | Added DeployedUIApp type definition and ExtraUIApps field to ScalityUISpec with validation |
| api/v1alpha1/zz_generated.deepcopy.go | Auto-generated DeepCopy methods for the new DeployedUIApp type |
| config/crd/bases/ui.scality.com_scalityuis.yaml | Generated CRD schema including extraUIApps array with validation rules |
| internal/controller/scalityui/controller.go | Removed duplicate DeployedUIApp type definition (moved to API package) |
| internal/controller/scalityui/deployed_apps_configmap.go | Updated to use API type, merge ExtraUIApps with exposer-based apps, and enhanced logging |
| internal/controller/scalityui/controller_test.go | Added tests for ExtraUIApps functionality including standalone and merge scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Append ExtraUIApps directly from ScalityUI spec | ||
| deployedApps = append(deployedApps, cr.Spec.ExtraUIApps...) |
There was a problem hiding this comment.
Potential issue: No deduplication logic for app names between ExtraUIApps and exposer-based apps. If an app in ExtraUIApps has the same name as an exposer-based app, both will be included in the deployed apps list, which could cause conflicts or unexpected behavior in the UI. Consider adding logic to detect and handle duplicate app names, either by warning the user, preventing duplicates, or documenting the expected behavior when duplicates occur.
Summary
Add
ExtraUIAppsfield to ScalityUI CR spec, allowing direct injection of UI application configurations into thedeployed-ui-appsConfigMap without requiring ScalityUIComponent and ScalityUIComponentExposer resources.Changes
DeployedUIApptype toapi/v1alpha1/scalityui_types.gowith kubebuilder validationExtraUIApps []DeployedUIAppfield toScalityUISpecdeployed_apps_configmap.goto merge ExtraUIApps into the deployed apps listUsage