Skip to content

feat: Configure Prometheus endpoint via Helm chart (#4562)#4643

Open
zyzzmohit wants to merge 1 commit intokubernetes-sigs:mainfrom
zyzzmohit:feature/prometheus-config-clean
Open

feat: Configure Prometheus endpoint via Helm chart (#4562)#4643
zyzzmohit wants to merge 1 commit intokubernetes-sigs:mainfrom
zyzzmohit:feature/prometheus-config-clean

Conversation

@zyzzmohit
Copy link
Contributor

  • 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

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 8, 2026
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zyzzmohit
Once this PR has been reviewed and has the lgtm label, please assign sniok for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 8, 2026
@illume illume requested a review from Copilot February 8, 2026 11:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-endpoint and returns it from the /config API.
  • 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-endpoint to 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,
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
PrometheusEndpoint: *conf.PrometheusEndpoint,

Copilot uses AI. Check for mistakes.
Comment on lines +1753 to 1754
clientConfig := clientConfig{c.getClusters(), c.EnableDynamicClusters, c.PrometheusEndpoint}

Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
# -- base url path at which headlamp should run
baseURL: ""
prometheus:
# -- Prometheus endpoint for the cluster
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
# -- 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.

Copilot uses AI. Check for mistakes.
Comment on lines 319 to 321
{{- with .Values.config.prometheus.endpoint }}
- "-prometheus-endpoint={{ . }}"
{{- end }}
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines 435 to 439
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")
}
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
}

clientConfig := clientConfig{contexts, c.EnableDynamicClusters}
clientConfig := clientConfig{contexts, c.EnableDynamicClusters, c.PrometheusEndpoint}
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
clientConfig := clientConfig{contexts, c.EnableDynamicClusters, c.PrometheusEndpoint}
clientConfig := clientConfig{contexts, c.EnableDynamicClusters, ""}

Copilot uses AI. Check for mistakes.
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 13, 2026
@zyzzmohit zyzzmohit force-pushed the feature/prometheus-config-clean branch from c4ac89e to 032985a Compare February 15, 2026 11:31
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 15, 2026
@zyzzmohit zyzzmohit force-pushed the feature/prometheus-config-clean branch 3 times, most recently from b73f0e7 to 3d27178 Compare February 15, 2026 12:22
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 15, 2026
@zyzzmohit zyzzmohit force-pushed the feature/prometheus-config-clean branch 2 times, most recently from d90b042 to 150c086 Compare February 15, 2026 12:23
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 15, 2026
)

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>
@zyzzmohit zyzzmohit force-pushed the feature/prometheus-config-clean branch from 150c086 to c3dab74 Compare February 15, 2026 13:06
@zyzzmohit
Copy link
Contributor Author

@illume @skoeva @vyncent-t
Hi I've rebased on main, resolved merge conflicts, and fixed all CI failures:

Helm Template Tests:

Added missing -session-ttl=86400 argument to all expected template files
Backend Lint (golangci-lint):

Fixed gofmt struct field alignment in
config.go
Reduced
Validate()
cognitive complexity (33 → below 30) by extracting
validatePrometheusEndpoint()
helper
Shortened
TestContext
(66 → 33 lines) by extracting
setupMockK8sServer()
helper
Fixed wsl cuddling violations in
kubeconfig_test.go
All CI checks should be passing now. Ready for review :)

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +148 to +156
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://")
}
}
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
}

// setupMockK8sServer creates a test HTTP server and a temporary kubeconfig pointing to it.
// Returns the server, temp kubeconfig path, and a cleanup function.
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
// Returns the server, temp kubeconfig path, and a cleanup function.
// It returns the server and the path to the temporary kubeconfig file.

Copilot uses AI. Check for mistakes.
- "-base-url={{ . }}"
{{- end }}
{{- with .Values.config.prometheus.endpoint }}
- "-prometheus-endpoint={{ . | quote }}"
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
- "-prometheus-endpoint={{ . | quote }}"
- "-prometheus-endpoint={{ . }}"

Copilot uses AI. Check for mistakes.
Comment on lines +182 to +201
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;
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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;
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants