-
Notifications
You must be signed in to change notification settings - Fork 17
feat: support cluster_domain in settings #3158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8 files reviewed, 1 comment
|
|
||
| const settingsSchema = z.object({ | ||
| use_meta_proxy: z.boolean().optional(), | ||
| cluster_domain: z.string().optional(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Field naming mismatch: issue #3157 specifies redirectDomain but implementation uses cluster_domain
| cluster_domain: z.string().optional(), | |
| redirect_domain: z.string().optional(), |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/store/reducers/cluster/parseFields.ts
Line: 72:72
Comment:
**logic:** Field naming mismatch: issue #3157 specifies `redirectDomain` but implementation uses `cluster_domain`
```suggestion
redirect_domain: z.string().optional(),
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for cluster-specific domain configuration through a new cluster_domain setting in the meta API. The feature allows clusters to specify external domains that will be used in generated cluster links.
Key changes:
- Adds
cluster_domainfield to cluster settings with validation - Introduces
useClusterHostfeature flag to control cluster domain usage - Updates routing to support host prefixes for external cluster domains
- Modifies cluster table links to open in new tabs when cluster domain is set
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/uiFactory/types.ts | Adds useClusterHost flag to gate cluster domain functionality |
| src/types/api/meta.ts | Adds cluster_domain field to MetaClusterSettings interface |
| src/store/reducers/clusters/types.ts | Adds clusterDomain field to PreparedCluster type |
| src/store/reducers/clusters/utils.ts | Extracts and prepares clusterDomain from settings when feature flag is enabled |
| src/store/reducers/cluster/parseFields.ts | Adds cluster_domain validation to settings Zod schema |
| src/routes.ts | Adds host parameter to createHref and getClusterPath functions for domain prefixing |
| src/containers/Clusters/utils.ts | Creates calculateClusterPath helper to centralize cluster path logic with domain support |
| src/containers/Clusters/columns.tsx | Refactors cluster name and version links to use new helper and open in new tab for external domains |
src/routes.ts
Outdated
| params?: Record<string, string | number | undefined>, | ||
| query: Query = {}, | ||
| options: CreateHrefOptions = {}, | ||
| host = '', |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new host parameter lacks documentation. Consider adding a JSDoc comment to the function or inline documentation explaining what the host parameter is for (e.g., "Optional host/domain prefix to prepend to the generated URL").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8 files reviewed, 1 comment
closes #3157
Greptile Overview
Greptile Summary
This PR implements support for
cluster_domainsettings to enable cluster page redirects. The implementation adds a new optional setting that allows configuring redirect domains for clusters, which when present, will prepend the domain to cluster URLs and open links in new tabs.Key Changes:
cluster_domainfield to cluster settings schema and typescalculateClusterPathutility to centralize path calculation logiccreateHrefto accept domain parameter for URL prefixingclusterDomainis presentuiFactory.useClusterDomainflagIssues Found:
clusterDomainis ignored whenuseEmbeddedUiis true, meaning redirect won't work for embedded UI clustersConfidence Score: 3/5
calculateClusterPathwhereclusterDomainis not applied to embedded UI paths means the redirect feature won't work for that use case. The rest of the code is clean and follows existing patterns.src/containers/Clusters/utils.ts- the redirect logic needs to handle embedded UI caseImportant Files Changed
File Analysis
calculateClusterPathhelper and conditionally opens links in new tab whenclusterDomainis presentdomainparameter support tocreateHrefandgetClusterPathfor domain prefixingSequence Diagram
sequenceDiagram participant User participant ClustersPage participant calculateClusterPath participant getClusterPath participant createHref participant Browser User->>ClustersPage: Click cluster link ClustersPage->>calculateClusterPath: row data with clusterDomain alt useEmbeddedUi && backend calculateClusterPath->>calculateClusterPath: Return embedded UI path calculateClusterPath-->>ClustersPage: backend/monitoring else Standard UI calculateClusterPath->>getClusterPath: params, query, options, clusterDomain getClusterPath->>createHref: route, params, query, options, domain alt withBasename && basename createHref->>createHref: Normalize path with domain and basename createHref-->>getClusterPath: domain + basename + route else No basename createHref->>createHref: Prepend domain to route createHref-->>getClusterPath: domain + route end getClusterPath-->>calculateClusterPath: Full URL with domain calculateClusterPath-->>ClustersPage: Full path end alt clusterDomain present ClustersPage->>Browser: Open in new tab (_blank) else No clusterDomain ClustersPage->>Browser: Open in same tab endCI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: ✅
Current: 62.33 MB | Main: 62.33 MB
Diff: +0.78 KB (0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information