diff --git a/cmd/prometheus/main_test.go b/cmd/prometheus/main_test.go index fa99000cd04..7d97f0c5f6c 100644 --- a/cmd/prometheus/main_test.go +++ b/cmd/prometheus/main_test.go @@ -16,6 +16,7 @@ package main import ( "bytes" "context" + "crypto/tls" "errors" "fmt" "io" @@ -40,6 +41,7 @@ import ( "github.com/prometheus/common/model" "github.com/prometheus/common/promslog" "github.com/stretchr/testify/require" + "go.uber.org/atomic" "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/model/labels" @@ -818,3 +820,95 @@ scrape_configs: t.Fatal("Timeout waiting for target to be scraped") } } + +func cpFile(t *testing.T, dst, src string) { + t.Helper() + data, err := os.ReadFile(src) + require.NoError(t, err) + require.NoError(t, os.WriteFile(dst, data, 0o644)) +} + +// TestClientTLSCertRotationWithoutCAFile checks that scrapes keep working after +// a client cert rotation when no ca_file is configured (cert_file + key_file only). +// Regression test for: https://github.com/prometheus/prometheus/issues/16622. +func TestClientTLSCertRotationWithoutCAFile(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + + // The target starts serving an extra metric after client cert rotation. + // If the scrape recovers, the new series should get ingested. + var clientCertRotated atomic.Bool + target := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprintln(w, "test_up 1") + if clientCertRotated.Load() { + fmt.Fprintln(w, "test_after_rotation 1") + } + })) + target.TLS = &tls.Config{ + ClientAuth: tls.RequireAnyClientCert, + } + target.StartTLS() + defer target.Close() + + clientCertFile := filepath.Join(tmpDir, "client.cer") + clientKeyFile := filepath.Join(tmpDir, "client.key") + cpFile(t, clientCertFile, filepath.Join("..", "..", "scrape", "testdata", "client.cer")) + cpFile(t, clientKeyFile, filepath.Join("..", "..", "scrape", "testdata", "client.key")) + + port := testutil.RandomUnprivilegedPort(t) + configFile := filepath.Join(tmpDir, "prometheus.yml") + require.NoError(t, os.WriteFile(configFile, fmt.Appendf(nil, ` +scrape_configs: + - job_name: target + scrape_interval: 1s + scheme: https + static_configs: + - targets: ['%s'] + tls_config: + cert_file: %s + key_file: %s + insecure_skip_verify: true +`, target.Listener.Addr().String(), clientCertFile, clientKeyFile), 0o644)) + + prom := prometheusCommandWithLogging(t, configFile, port, + "--storage.tsdb.path="+filepath.Join(tmpDir, "data")) + require.NoError(t, prom.Start()) + + promURL := "http://localhost:" + strconv.Itoa(port) + const headSeriesCreated = "prometheus_tsdb_head_series_created_total" + + getHeadSeriesCreated := func() (float64, error) { + r, err := http.Get(promURL + "/metrics") + if err != nil { + return 0, err + } + defer r.Body.Close() + b, _ := io.ReadAll(r.Body) + return getMetricValue(t, bytes.NewReader(b), model.MetricTypeCounter, headSeriesCreated) + } + + // Wait for at least one scrape to ingest series. + var seriesCreatedBeforeRotation float64 + require.Eventually(t, func() bool { + v, err := getHeadSeriesCreated() + if err != nil { + return false + } + seriesCreatedBeforeRotation = v + return seriesCreatedBeforeRotation > 0 + }, startupTime, 500*time.Millisecond) + + // Overwrite client cert files (content doesn't matter) to trigger a + // transport rebuild. + cpFile(t, clientCertFile, filepath.Join("..", "..", "scrape", "testdata", "server.cer")) + cpFile(t, clientKeyFile, filepath.Join("..", "..", "scrape", "testdata", "server.key")) + clientCertRotated.Store(true) + + require.Eventually(t, func() bool { + v, err := getHeadSeriesCreated() + if err != nil { + return false + } + return v > seriesCreatedBeforeRotation + }, 5*time.Second, 1*time.Second, "scrape should keep working after cert rotation") +} diff --git a/go.mod b/go.mod index 272f633f304..65f8df75f53 100644 --- a/go.mod +++ b/go.mod @@ -221,3 +221,6 @@ exclude ( replace github.com/fsnotify/fsnotify v1.8.0 => github.com/fsnotify/fsnotify v1.7.0 replace google.golang.org/grpc => github.com/openshift-sustaining/grpc-go v1.75.1-sec.1 + +// Carry fix for https://redhat.atlassian.net/browse/OCPBUGS-62118: TLS cert rotation failing without ca_file +replace github.com/prometheus/common v0.62.0 => github.com/machine424/prometheus-common v0.0.0-20260519225033-cca8fa415d7e diff --git a/go.sum b/go.sum index 06146c6e23c..3f4286f29da 100644 --- a/go.sum +++ b/go.sum @@ -1367,6 +1367,8 @@ github.com/lyft/protoc-gen-star v0.6.1/go.mod h1:TGAoBVkt8w7MPG72TrKIu85MIdXwDuz github.com/lyft/protoc-gen-star/v2 v2.0.1/go.mod h1:RcCdONR2ScXaYnQC5tUzxzlpA3WVYF7/opLeUgcQs/o= github.com/lyft/protoc-gen-star/v2 v2.0.3/go.mod h1:amey7yeodaJhXSbf/TlLvWiqQfLOSpEk//mLlc+axEk= github.com/lyft/protoc-gen-star/v2 v2.0.4-0.20230330145011-496ad1ac90a4/go.mod h1:amey7yeodaJhXSbf/TlLvWiqQfLOSpEk//mLlc+axEk= +github.com/machine424/prometheus-common v0.0.0-20260519225033-cca8fa415d7e h1:W/VHB1RFlU9TkBKXDKI+px5ItAJClZ0pEHDeXRl5sGI= +github.com/machine424/prometheus-common v0.0.0-20260519225033-cca8fa415d7e/go.mod h1:vyBcEuLSvWos9B1+CyL7JZ2up+uFzXhkqml0W5zIY1I= github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= @@ -1544,8 +1546,6 @@ github.com/prometheus/common v0.2.0/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y8 github.com/prometheus/common v0.4.1/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4= github.com/prometheus/common v0.7.0/go.mod h1:DjGbpBbp5NYNiECxcL/VnbXCCaQpKd3tt26CguLLsqA= github.com/prometheus/common v0.9.1/go.mod h1:yhUN8i9wzaXS3w1O07YhxHEBxD+W35wd8bs7vj7HSQ4= -github.com/prometheus/common v0.62.0 h1:xasJaQlnWAeyHdUBeGjXmutelfJHWMRr+Fg4QszZ2Io= -github.com/prometheus/common v0.62.0/go.mod h1:vyBcEuLSvWos9B1+CyL7JZ2up+uFzXhkqml0W5zIY1I= github.com/prometheus/common/assets v0.2.0 h1:0P5OrzoHrYBOSM1OigWL3mY8ZvV2N4zIE/5AahrSrfM= github.com/prometheus/common/assets v0.2.0/go.mod h1:D17UVUE12bHbim7HzwUvtqm6gwBEaDQ0F+hIGbFbccI= github.com/prometheus/exporter-toolkit v0.13.2 h1:Z02fYtbqTMy2i/f+xZ+UK5jy/bl1Ex3ndzh06T/Q9DQ= diff --git a/vendor/github.com/prometheus/common/config/http_config.go b/vendor/github.com/prometheus/common/config/http_config.go index 63809083aca..68ed977d0a7 100644 --- a/vendor/github.com/prometheus/common/config/http_config.go +++ b/vendor/github.com/prometheus/common/config/http_config.go @@ -1368,7 +1368,7 @@ func (t *tlsRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { rt := t.rt t.mtx.RUnlock() if equal { - // The CA cert hasn't changed, use the existing RoundTripper. + // The TLS materials (CA, cert, key) haven't changed, use the existing RoundTripper. return rt.RoundTrip(req) } @@ -1376,7 +1376,7 @@ func (t *tlsRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { // The cert and key files are read separately by the client // using GetClientCertificate. tlsConfig := t.tlsConfig.Clone() - if !updateRootCA(tlsConfig, caData) { + if t.settings.CA != nil && !updateRootCA(tlsConfig, caData) { return nil, fmt.Errorf("unable to use specified CA cert %s", t.settings.CA.Description()) } rt, err = t.newRT(tlsConfig) diff --git a/vendor/modules.txt b/vendor/modules.txt index 1740f87efc5..a944919aafa 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -691,7 +691,7 @@ github.com/prometheus/client_golang/prometheus/testutil/promlint/validations # github.com/prometheus/client_model v0.6.1 ## explicit; go 1.19 github.com/prometheus/client_model/go -# github.com/prometheus/common v0.62.0 +# github.com/prometheus/common v0.62.0 => github.com/machine424/prometheus-common v0.0.0-20260519225033-cca8fa415d7e ## explicit; go 1.21 github.com/prometheus/common/config github.com/prometheus/common/expfmt