Skip to content

refactor: include scheme in base URL for metrics #1243

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.

Sign up for GitHub

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

Merged
xcoulon merged 8 commits into from
Jan 7, 2026
Merged

refactor: include scheme in base URL for metrics #1243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e987764
refactor: include scheme in base URL for metrics
xcoulon Jan 5, 2026
33a830a
also, update vuln exclusions
xcoulon Jan 5, 2026
71680ad
the year is now 2026
xcoulon Jan 5, 2026
165f7df
more vulns
xcoulon Jan 5, 2026
7868358
Merge branch 'master' into metrics_getvalue_http
xcoulon Jan 6, 2026
8bf33ec
Merge branch 'master' into metrics_getvalue_http
rsoaresd Jan 6, 2026
f0e1220
Merge branch 'master' into metrics_getvalue_http
xcoulon Jan 7, 2026
54b8935
Merge branch 'master' into metrics_getvalue_http
xcoulon Jan 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions .govulncheck.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,52 @@ ignored-vulnerabilities:
# Fixed in: crypto/x509@go1.24.8
- id: GO-2025-4013
info: https://pkg.go.dev/vuln/GO-2025-4013
silence-until: 2025-12-03
silence-until: 2026-02-05
# Lack of limit when parsing cookies can cause memory exhaustion in net/http
# Found in: net/http@go1.23.12
# Fixed in: net/http@go1.24.8
- id: GO-2025-4012
info: https://pkg.go.dev/vuln/GO-2025-4012
silence-until: 2025-12-03
silence-until: 2026-02-05
# Parsing DER payload can cause memory exhaustion in encoding/asn1
# Found in: encoding/asn1@go1.23.12
# Fixed in: encoding/asn1@go1.24.8
- id: GO-2025-4011
info: https://pkg.go.dev/vuln/GO-2025-4011
silence-until: 2025-12-03
silence-until: 2026-02-05
# Insufficient validation of bracketed IPv6 hostnames in net/url
# Found in: net/url@go1.23.12
# Fixed in: net/url@go1.24.8
- id: GO-2025-4010
info: https://pkg.go.dev/vuln/GO-2025-4010
silence-until: 2025-12-03
silence-until: 2026-02-05
# Quadratic complexity when parsing some invalid inputs in encoding/pem
# Found in: encoding/pem@go1.23.12
# Fixed in: encoding/pem@go1.24.8
- id: GO-2025-4009
info: https://pkg.go.dev/vuln/GO-2025-4009
silence-until: 2025-12-03
silence-until: 2026-02-05
# ALPN negotiation error contains attacker controlled information in crypto/tls
# Found in: crypto/tls@go1.23.12
# Fixed in: crypto/tls@go1.24.8
- id: GO-2025-4008
info: https://pkg.go.dev/vuln/GO-2025-4008
silence-until: 2025-12-03
silence-until: 2026-02-05
# Quadratic complexity when checking name constraints in crypto/x509
# Found in: crypto/x509@go1.23.12
# Fixed in: crypto/x509@go1.24.9
- id: GO-2025-4007
info: https://pkg.go.dev/vuln/GO-2025-4007
silence-until: 2025-12-03
silence-until: 2026-02-05
# Excessive resource consumption when printing error string for host certificate validation in crypto/x509
# Found in: crypto/x509@go1.23.12
# Fixed in: crypto/x509@go1.24.11
- id: GO-2025-4155
info: https://pkg.go.dev/vuln/GO-2025-4155
silence-until: 2026-02-04
# Improper application of excluded DNS name constraints when verifying wildcard names in crypto/x509
# Found in: crypto/x509@go1.23.12
# Fixed in: crypto/x509@go1.24.11
- id: GO-2025-4175
info: https://pkg.go.dev/vuln/GO-2025-4175
silence-until: 2026-02-04
Comment on lines +44 to +55
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

GO-2025-4155 crypto/x509 vulnerability

💡 Result:

Summary: GO-2025-4155 (CVE-2025-61729) is a crypto/x509 vulnerability where HostnameError.Error() builds an unbounded, repeatedly-concatenated host list when constructing its error string. A maliciously-crafted certificate can cause excessive CPU/memory (quadratic) usage during hostname verification (Certificate.Verify / VerifyHostname). Fixes released in go1.24.11 and go1.25.5 — upgrade to those or later. [1][2][3]

Sources:

  • Go vulnerability entry (GO-2025-4155 / CVE-2025-61729). [1]
  • OSV summary and affected ranges (fix: 1.24.11, 1.25.5). [2]
  • Vendor/security advisory listing fixes in go1.25.5 (summary of related CVEs). [3]

🌐 Web query:

GO-2025-4175 crypto/x509 vulnerability golang

💡 Result:

Summary — GO-2025-4175 (CVE-2025-61727)

  • What it is: an incorrect enforcement of excluded DNS name constraints in crypto/x509 that allows a leaf certificate with a wildcard SAN (e.g. *.example.com) to bypass an excluded subdomain constraint (e.g. exclude test.example.com). [1][2]
  • Affected symbols/flows: crypto/x509 Certificate.Verify (certificate verification). [1]
  • Affected Go versions: all releases before go1.24.11, and go1.25.0 through before go1.25.5. [1][2]
  • Fixed in: go1.24.11 and go1.25.5. Upgrade to one of these or later. [1][3]
  • References: Go vulnerability entry and change/issue tracking. [1][2][3]

Sources:
[1] Go vuln DB: GO-2025-4175 / CVE-2025-61727 (pkg.go.dev).
[2] OSV / OSV JSON entry for GO-2025-4175.
[3] Release/security notes referencing go1.25.5 (SUSE advisory).

Confirm remediation plan exists for crypto/x509 vulnerabilities before silence-until deadline.

Both vulnerabilities are documented in the official Go vulnerability database:

  • GO-2025-4155: Unbounded string concatenation in HostnameError.Error() causes excessive CPU/memory usage during certificate verification
  • GO-2025-4175: Wildcard certificates can bypass excluded DNS name constraints

Since these affect certificate validation (critical security component), ensure:

  1. Risk assessment has been completed for this codebase
  2. An upgrade plan to go1.24.11 exists before 2026-02-04
🤖 Prompt for AI Agents 
In @.govulncheck.yaml around lines 44-55, The vulnerability entries GO-2025-4155
and GO-2025-4175 in the .govulncheck configuration currently silence until
2026-02-04 without documented remediation; add a short remediation plan
referencing upgrade to crypto/x509 fixed in go1.24.11, confirm a completed risk
assessment entry, and create an actionable upgrade timeline (owner, target date
before 2026-02-04, and rollback plan) to be linked to these IDs and the
silence-until key so the silence is only allowed if the upgrade plan and risk
assessment are recorded.

4 changes: 2 additions & 2 deletions testsupport/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,12 @@ func WaitForDeployments(t *testing.T) wait.Awaitilities {
// setup host metrics route for metrics verification in tests
hostMetricsRoute, err := initHostAwait.SetupRouteForService(t, "host-operator-metrics-service", "/metrics")
require.NoError(t, err)
initHostAwait.MetricsURL = hostMetricsRoute.Status.Ingress[0].Host
initHostAwait.MetricsURL = "https://" + hostMetricsRoute.Status.Ingress[0].Host
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Hardcoded "https://" scheme contradicts the PR objective.

The PR aims to "include the URL scheme (rather than assuming 'https') when constructing the base URL for metrics, allowing testing against an 'http' endpoint." However, these lines still hardcode "https://" for the metrics URLs.

Follow the pattern established for the registration service URL (lines 100-104), which conditionally checks route.Spec.TLS to determine the appropriate scheme.

🔎 Proposed fix to determine scheme based on TLS configuration

For line 210:

-	initHostAwait.MetricsURL = "https://" + hostMetricsRoute.Status.Ingress[0].Host
+	hostMetricsURL := "http://" + hostMetricsRoute.Status.Ingress[0].Host
+	if hostMetricsRoute.Spec.TLS != nil {
+		hostMetricsURL = "https://" + hostMetricsRoute.Status.Ingress[0].Host
+	}
+	initHostAwait.MetricsURL = hostMetricsURL

For line 215:

-	initMemberAwait.MetricsURL = "https://" + memberMetricsRoute.Status.Ingress[0].Host
+	memberMetricsURL := "http://" + memberMetricsRoute.Status.Ingress[0].Host
+	if memberMetricsRoute.Spec.TLS != nil {
+		memberMetricsURL = "https://" + memberMetricsRoute.Status.Ingress[0].Host
+	}
+	initMemberAwait.MetricsURL = memberMetricsURL

Also applies to: 215-215

🤖 Prompt for AI Agents 
In @testsupport/init.go around line 210, The code hardcodes "https://" when
setting initHostAwait.MetricsURL (using hostMetricsRoute.Status.Ingress[0].Host)
which contradicts the PR goal to derive the scheme from the route TLS config;
change the assignment to choose "https://" if hostMetricsRoute.Spec.TLS is
non-nil/has entries and "http://" otherwise, following the same pattern used for
the registration URL (the conditional logic around route.Spec.TLS in the earlier
block), and apply the same fix for the similar assignment at the other
occurrence (line 215) so both metrics URLs use the correct scheme based on
hostMetricsRoute.Spec.TLS.

Comment thread
fbm3307 marked this conversation as resolved.

// setup member metrics route for metrics verification in tests
memberMetricsRoute, err := initMemberAwait.SetupRouteForService(t, "member-operator-metrics-service", "/metrics")
require.NoError(t, err, "failed while setting up or waiting for the route to the 'member-operator-metrics' service to be available")
initMemberAwait.MetricsURL = memberMetricsRoute.Status.Ingress[0].Host
initMemberAwait.MetricsURL = "https://" + memberMetricsRoute.Status.Ingress[0].Host

// Wait for the webhooks in Member 1 only because we do not deploy webhooks for Member 2
// (we can't deploy the same webhook multiple times on the same cluster)
Expand Down
16 changes: 8 additions & 8 deletions testsupport/metrics/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,27 @@ import (
"k8s.io/client-go/rest"
)

func GetMetricValue(restConfig *rest.Config, url string, family string, expectedLabels []string) (float64, error) {
value, err := getMetricValue(restConfig, url, family, expectedLabels, getValue)
func GetMetricValue(restConfig *rest.Config, baseURL string, family string, expectedLabels []string) (float64, error) {
value, err := getMetricValue(restConfig, baseURL, family, expectedLabels, getValue)
if value == nil {
return -1, err
}
return *value, err
}

func GetHistogramBuckets(restConfig *rest.Config, url string, family string, expectedLabels []string) ([]*dto.Bucket, error) {
value, err := getMetricValue(restConfig, url, family, expectedLabels, getBuckets)
func GetHistogramBuckets(restConfig *rest.Config, baseURL string, family string, expectedLabels []string) ([]*dto.Bucket, error) {
value, err := getMetricValue(restConfig, baseURL, family, expectedLabels, getBuckets)
if value == nil {
return nil, err
}
return *value, err
}

func getMetricValue[T any](restConfig *rest.Config, url string, family string, expectedLabels []string, getValue func(dto.MetricType, *dto.Metric) (*T, error)) (*T, error) {
func getMetricValue[T any](restConfig *rest.Config, baseURL string, family string, expectedLabels []string, getValue func(dto.MetricType, *dto.Metric) (*T, error)) (*T, error) {
if len(expectedLabels)%2 != 0 {
return nil, fmt.Errorf("received odd number of label arguments, labels must be key-value pairs")
}
uri := fmt.Sprintf("https://%s/metrics", url)
uri := baseURL + "/metrics"
var metrics []byte

client := http.Client{
Expand Down Expand Up @@ -128,8 +128,8 @@ func getBuckets(t dto.MetricType, m *dto.Metric) (*[]*dto.Bucket, error) {
}

// GetMetricLabels return all labels (indexed by key) for all metrics of the given `family`
func GetMetricLabels(restConfig *rest.Config, url string, family string) ([]map[string]string, error) {
uri := fmt.Sprintf("https://%s/metrics", url)
func GetMetricLabels(restConfig *rest.Config, baseURL string, family string) ([]map[string]string, error) {
uri := baseURL + "/metrics"
var metrics []byte

client := http.Client{
Expand Down
17 changes: 7 additions & 10 deletions testsupport/metrics/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -62,36 +61,34 @@ func TestGetMetricValue(t *testing.T) {
BearerToken: "1a2b3bc",
}

url := strings.TrimPrefix(ts.URL, "https://")
Comment thread
rsoaresd marked this conversation as resolved.

t.Run("valid metrics", func(t *testing.T) {
t.Run("counter with no labels", func(t *testing.T) {
// when
result, err := GetMetricValue(config, url, "sandbox_user_signups_total", []string{})
result, err := GetMetricValue(config, ts.URL, "sandbox_user_signups_total", []string{})
// then
require.NoError(t, err)
assert.InDelta(t, float64(7), result, 0.1)
})

t.Run("counter with single label", func(t *testing.T) {
// when
result, err := GetMetricValue(config, url, "workqueue_depth", []string{"name", "masteruserrecord-controller"})
result, err := GetMetricValue(config, ts.URL, "workqueue_depth", []string{"name", "masteruserrecord-controller"})
// then
require.NoError(t, err)
assert.InDelta(t, float64(0), result, 0.1)
})

t.Run("counter with two labels", func(t *testing.T) {
// when
result, err := GetMetricValue(config, url, "controller_runtime_reconcile_total", []string{"controller", "usersignup-controller", "result", "success"})
result, err := GetMetricValue(config, ts.URL, "controller_runtime_reconcile_total", []string{"controller", "usersignup-controller", "result", "success"})
// then
require.NoError(t, err)
assert.InDelta(t, float64(10), result, 0.1)
})

t.Run("gauge with no labels", func(t *testing.T) {
// when
result, err := GetMetricValue(config, url, "sandbox_master_user_record_current", []string{})
result, err := GetMetricValue(config, ts.URL, "sandbox_master_user_record_current", []string{})
// then
require.NoError(t, err)
assert.InDelta(t, float64(7), result, 0.01)
Expand All @@ -101,7 +98,7 @@ func TestGetMetricValue(t *testing.T) {
t.Run("failures", func(t *testing.T) {
t.Run("metric does not exist", func(t *testing.T) {
// when
result, err := GetMetricValue(config, url, "non_existent_counter", []string{})
result, err := GetMetricValue(config, ts.URL, "non_existent_counter", []string{})
// then
require.Error(t, err)
require.EqualError(t, err, "metric 'non_existent_counter{[]}' not found")
Expand All @@ -110,7 +107,7 @@ func TestGetMetricValue(t *testing.T) {

t.Run("metric family exists but labels do not match", func(t *testing.T) {
// when
result, err := GetMetricValue(config, url, "workqueue_depth", []string{"name", "non-existent-controller"})
result, err := GetMetricValue(config, ts.URL, "workqueue_depth", []string{"name", "non-existent-controller"})
// then
require.Error(t, err)
require.EqualError(t, err, "metric 'workqueue_depth{[name non-existent-controller]}' not found")
Expand All @@ -119,7 +116,7 @@ func TestGetMetricValue(t *testing.T) {

t.Run("odd number of label parameters", func(t *testing.T) {
// when
result, err := GetMetricValue(config, url, "workqueue_depth", []string{"name"})
result, err := GetMetricValue(config, ts.URL, "workqueue_depth", []string{"name"})
// then
require.Error(t, err)
require.EqualError(t, err, "received odd number of label arguments, labels must be key-value pairs")
Expand Down
Loading