feat: Configure Prometheus endpoint via Helm chart (#4562)#4643
feat: Configure Prometheus endpoint via Helm chart (#4562)#4643zyzzmohit wants to merge 1 commit intokubernetes-sigs:mainfrom
Conversation
zyzzmohit
commented
Feb 8, 2026
- Add PrometheusEndpoint to backend config and API\n- Update frontend to use configured endpoint for Prometheus plugin\n- Add config.prometheus.endpoint to Helm chart values\n- Verify fix by deploying with configured endpoint
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zyzzmohit The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
Adds support for configuring the Prometheus endpoint centrally (via backend config + Helm values) and wiring that value into the frontend so the Prometheus plugin can use it.
Changes:
- Introduces a new backend config flag/field for
prometheus-endpointand returns it from the/configAPI. - Updates the frontend to apply the configured endpoint into the Prometheus plugin’s stored config per cluster.
- Extends the Helm chart values and deployment args to pass
-prometheus-endpointto the backend.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/redux/configSlice.ts | Adds prometheusEndpoint to Redux config state and carries it via setConfig. |
| frontend/src/components/App/Layout.tsx | On config fetch, writes the configured endpoint into the Prometheus plugin config (per cluster). |
| charts/headlamp/values.yaml | Adds config.prometheus.endpoint chart value (plus some YAML formatting normalization). |
| charts/headlamp/templates/deployment.yaml | Passes -prometheus-endpoint arg when the Helm value is set. |
| backend/pkg/config/config.go | Adds PrometheusEndpoint to parsed config + registers --prometheus-endpoint flag. |
| backend/cmd/stateless.go | Includes PrometheusEndpoint in the stateless /parseKubeConfig response payload. |
| backend/cmd/server.go | Maps parsed config into the server config struct (attempts to include PrometheusEndpoint). |
| backend/cmd/headlamp.go | Includes PrometheusEndpoint in the /config response payload. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ProxyURLs: strings.Split(conf.ProxyURLs, ","), | ||
| TLSCertPath: conf.TLSCertPath, | ||
| TLSKeyPath: conf.TLSKeyPath, | ||
| PrometheusEndpoint: *conf.PrometheusEndpoint, |
There was a problem hiding this comment.
PrometheusEndpoint is being set on headlampconfig.HeadlampCFG, but that struct currently does not define this field (backend/pkg/headlampconfig/headlampConfig.go). As-is, this change will not compile; add the field to HeadlampCFG (and wire it through wherever HeadlampCFG is constructed) or remove this assignment.
| PrometheusEndpoint: *conf.PrometheusEndpoint, |
| clientConfig := clientConfig{c.getClusters(), c.EnableDynamicClusters, c.PrometheusEndpoint} | ||
|
|
There was a problem hiding this comment.
This code references c.PrometheusEndpoint, but HeadlampConfig embeds headlampconfig.HeadlampConfig/HeadlampCFG, which currently do not include a PrometheusEndpoint field. This will fail to compile until the config structs are updated to carry this value (or the code is changed to retrieve it from the correct place).
| # -- base url path at which headlamp should run | ||
| baseURL: "" | ||
| prometheus: | ||
| # -- Prometheus endpoint for the cluster |
There was a problem hiding this comment.
A new chart value (config.prometheus.endpoint) was added, but the Helm chart documentation/typing should be kept in sync. Update charts/headlamp/README.md configuration tables and charts/headlamp/values.schema.json to include this value (and any constraints/description).
| # -- Prometheus endpoint for the cluster | |
| # -- Prometheus HTTP(S) endpoint URL used by Headlamp to query metrics for the cluster. Leave empty to disable Prometheus integration. |
| {{- with .Values.config.prometheus.endpoint }} | ||
| - "-prometheus-endpoint={{ . }}" | ||
| {{- end }} |
There was a problem hiding this comment.
Helm values are injected directly into a double-quoted YAML string here. If the endpoint ever contains characters that need escaping (e.g., quotes), it can render invalid YAML. Prefer using Helm quoting/printf patterns that safely escape the value when building the arg string.
| f.Uint("port", defaultPort, "Port to listen from") | ||
| f.String("proxy-urls", "", "Allow proxy requests to specified URLs") | ||
| f.Bool("enable-helm", false, "Enable Helm operations") | ||
| f.String("prometheus-endpoint", "", "Prometheus endpoint for the cluster") | ||
| } |
There was a problem hiding this comment.
The new --prometheus-endpoint flag/env var is not covered by the existing config parsing tests (backend/pkg/config/config_test.go). Add a ParseFlags/ParseWithEnv test to ensure the value is correctly read and propagated (including the empty/unset case).
| } | ||
|
|
||
| clientConfig := clientConfig{contexts, c.EnableDynamicClusters} | ||
| clientConfig := clientConfig{contexts, c.EnableDynamicClusters, c.PrometheusEndpoint} |
There was a problem hiding this comment.
This code references c.PrometheusEndpoint, but the embedded backend config structs don’t currently define this field. Until the config structs are updated to include PrometheusEndpoint, this change will not compile.
| clientConfig := clientConfig{contexts, c.EnableDynamicClusters, c.PrometheusEndpoint} | |
| clientConfig := clientConfig{contexts, c.EnableDynamicClusters, ""} |
c4ac89e to
032985a
Compare
b73f0e7 to
3d27178
Compare
d90b042 to
150c086
Compare
) Add PrometheusEndpoint to backend config and API - Update frontend to use configured endpoint for Prometheus plugin - Add config.prometheus.endpoint to Helm chart values - Use safe quoting for Helm value injection - Add backend validation for Prometheus URL Signed-off-by: zyzzmohit <mohitray949@gmail.com>
150c086 to
c3dab74
Compare
|
@illume @skoeva @vyncent-t Helm Template Tests: Added missing -session-ttl=86400 argument to all expected template files Fixed gofmt struct field alignment in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return c.validatePrometheusEndpoint() | ||
| } | ||
|
|
||
| func (c *Config) validatePrometheusEndpoint() error { | ||
| if c.PrometheusEndpoint != nil && *c.PrometheusEndpoint != "" { | ||
| if !strings.HasPrefix(*c.PrometheusEndpoint, "http://") && !strings.HasPrefix(*c.PrometheusEndpoint, "https://") { | ||
| return errors.New("prometheus-endpoint must start with http:// or https://") | ||
| } | ||
| } |
There was a problem hiding this comment.
New validation logic for prometheus-endpoint is added, but there are no config parsing/validation tests covering valid/invalid values (e.g. http://, https://, missing scheme). Adding tests alongside existing TestParseErrors cases would prevent regressions.
| } | ||
|
|
||
| // setupMockK8sServer creates a test HTTP server and a temporary kubeconfig pointing to it. | ||
| // Returns the server, temp kubeconfig path, and a cleanup function. |
There was a problem hiding this comment.
The helper comment says it returns a cleanup function, but the function only returns the server and temp kubeconfig path. Either update the comment to match what is returned, or return a cleanup func (e.g. to close the server and remove the temp file) to keep the test setup self-contained.
| // Returns the server, temp kubeconfig path, and a cleanup function. | |
| // It returns the server and the path to the temporary kubeconfig file. |
| - "-base-url={{ . }}" | ||
| {{- end }} | ||
| {{- with .Values.config.prometheus.endpoint }} | ||
| - "-prometheus-endpoint={{ . | quote }}" |
There was a problem hiding this comment.
The Helm template wraps the endpoint with quote inside an already-quoted arg string, which will render as -prometheus-endpoint="http://..." (literal quotes included). This will cause the backend flag value to include quotes and fail the http(s):// prefix validation. Remove the | quote filter here, or alternatively remove the outer YAML quotes and rely on quote for YAML safety.
| - "-prometheus-endpoint={{ . | quote }}" | |
| - "-prometheus-endpoint={{ . }}" |
| if (config?.prometheusEndpoint) { | ||
| const currentPluginConfigs: { [key: string]: any } = store.getState().pluginConfigs || {}; | ||
| const prometheusConfig = currentPluginConfigs['prometheus'] || {}; | ||
| const newPrometheusConfig: { [key: string]: any } = { ...prometheusConfig }; | ||
| let hasChanges = false; | ||
|
|
||
| Object.keys(clustersToConfig).forEach(clusterName => { | ||
| const currentClusterConfig = prometheusConfig[clusterName] || {}; | ||
| if ( | ||
| currentClusterConfig.address !== config.prometheusEndpoint || | ||
| !currentClusterConfig.isMetricsEnabled || | ||
| currentClusterConfig.autoDetect !== false | ||
| ) { | ||
| newPrometheusConfig[clusterName] = { | ||
| ...currentClusterConfig, | ||
| isMetricsEnabled: true, | ||
| address: config.prometheusEndpoint, | ||
| autoDetect: false, | ||
| }; | ||
| hasChanges = true; |
There was a problem hiding this comment.
This block only runs when config.prometheusEndpoint is truthy. If the endpoint was previously configured and later removed/emptied (backend returns ""), the plugin config will never be reverted and will keep forcing the old address from localStorage. Consider explicitly handling the empty-string case by restoring Prometheus plugin settings to autodetect/disabled, or clearing the stored address.
| if (config?.prometheusEndpoint) { | |
| const currentPluginConfigs: { [key: string]: any } = store.getState().pluginConfigs || {}; | |
| const prometheusConfig = currentPluginConfigs['prometheus'] || {}; | |
| const newPrometheusConfig: { [key: string]: any } = { ...prometheusConfig }; | |
| let hasChanges = false; | |
| Object.keys(clustersToConfig).forEach(clusterName => { | |
| const currentClusterConfig = prometheusConfig[clusterName] || {}; | |
| if ( | |
| currentClusterConfig.address !== config.prometheusEndpoint || | |
| !currentClusterConfig.isMetricsEnabled || | |
| currentClusterConfig.autoDetect !== false | |
| ) { | |
| newPrometheusConfig[clusterName] = { | |
| ...currentClusterConfig, | |
| isMetricsEnabled: true, | |
| address: config.prometheusEndpoint, | |
| autoDetect: false, | |
| }; | |
| hasChanges = true; | |
| if (config && 'prometheusEndpoint' in config) { | |
| const currentPluginConfigs: { [key: string]: any } = store.getState().pluginConfigs || {}; | |
| const prometheusConfig = currentPluginConfigs['prometheus'] || {}; | |
| const newPrometheusConfig: { [key: string]: any } = { ...prometheusConfig }; | |
| let hasChanges = false; | |
| const endpoint = | |
| typeof config.prometheusEndpoint === 'string' | |
| ? config.prometheusEndpoint.trim() | |
| : config.prometheusEndpoint; | |
| Object.keys(clustersToConfig).forEach(clusterName => { | |
| const currentClusterConfig = prometheusConfig[clusterName] || {}; | |
| if (endpoint) { | |
| // Endpoint configured: ensure metrics are enabled and autodetect is disabled for this endpoint. | |
| if ( | |
| currentClusterConfig.address !== endpoint || | |
| !currentClusterConfig.isMetricsEnabled || | |
| currentClusterConfig.autoDetect !== false | |
| ) { | |
| newPrometheusConfig[clusterName] = { | |
| ...currentClusterConfig, | |
| isMetricsEnabled: true, | |
| address: endpoint, | |
| autoDetect: false, | |
| }; | |
| hasChanges = true; | |
| } | |
| } else { | |
| // Endpoint cleared/empty: restore autodetect/disabled and clear any forced address. | |
| const hasForcedSettings = | |
| currentClusterConfig.isMetricsEnabled || | |
| currentClusterConfig.autoDetect === false || | |
| (typeof currentClusterConfig.address === 'string' && | |
| currentClusterConfig.address.trim() !== ''); | |
| if (hasForcedSettings) { | |
| const updatedClusterConfig: { [key: string]: any } = { | |
| ...currentClusterConfig, | |
| isMetricsEnabled: false, | |
| autoDetect: true, | |
| }; | |
| delete updatedClusterConfig.address; | |
| newPrometheusConfig[clusterName] = updatedClusterConfig; | |
| hasChanges = true; | |
| } |