diff --git a/plugin/metrics/metrics.go b/plugin/metrics/metrics.go index d8c16de26..640495f55 100644 --- a/plugin/metrics/metrics.go +++ b/plugin/metrics/metrics.go @@ -40,15 +40,17 @@ type Metrics struct { plugins map[string]struct{} // all available plugins, used to determine which plugin made the client write tlsConfigPath string + serveTLS func(net.Listener, *http.Server, *web.FlagConfig, *slog.Logger) error } // New returns a new instance of Metrics with the given address. func New(addr string) *Metrics { met := &Metrics{ - Addr: addr, - Reg: prometheus.DefaultRegisterer.(*prometheus.Registry), - zoneMap: make(map[string]struct{}), - plugins: pluginList(caddy.ListPlugins()), + Addr: addr, + Reg: prometheus.DefaultRegisterer.(*prometheus.Registry), + zoneMap: make(map[string]struct{}), + plugins: pluginList(caddy.ListPlugins()), + serveTLS: web.Serve, } return met @@ -117,6 +119,13 @@ func (sl *startupListener) Ready() <-chan struct{} { // OnStartup sets up the metrics on startup. func (m *Metrics) OnStartup() error { + if m.tlsConfigPath != "" { + if err := web.Validate(m.tlsConfigPath); err != nil { + log.Errorf("Invalid TLS config: %s", err) + return err + } + } + ln, err := reuseport.Listen("tcp", m.Addr) if err != nil { log.Errorf("Failed to start metrics handler: %s", err) @@ -151,12 +160,6 @@ func (m *Metrics) OnStartup() error { return nil } - // Check TLS config file existence - if _, err := os.Stat(m.tlsConfigPath); os.IsNotExist(err) { - log.Errorf("TLS config file does not exist: %s", m.tlsConfigPath) - return err - } - // Create web config for ListenAndServe webConfig := &web.FlagConfig{ WebListenAddresses: &[]string{m.Addr}, @@ -173,7 +176,7 @@ func (m *Metrics) OnStartup() error { // web.Serve() never returns nil, it always returns a non-nil error and // it doesn't retun anything if server starts successfully. // startupListener handles capturing succesful startup. - err := web.Serve(m.ln, server, webConfig, logger) + err := m.serveTLS(m.ln, server, webConfig, logger) if err != nil && err != http.ErrServerClosed { log.Errorf("Failed to start HTTPS metrics server: %v", err) startUpErr <- err @@ -183,6 +186,10 @@ func (m *Metrics) OnStartup() error { // Wait for startup errors select { case err := <-startUpErr: + if closeErr := ln.Close(); closeErr != nil { + log.Errorf("Failed to close metrics listener after startup error: %s", closeErr) + } + m.lnSetup = false return err case <-startupListener.Ready(): log.Infof("Server is ready and accepting connections") diff --git a/plugin/metrics/metrics_test.go b/plugin/metrics/metrics_test.go index d6fd1e767..137ad2684 100644 --- a/plugin/metrics/metrics_test.go +++ b/plugin/metrics/metrics_test.go @@ -8,8 +8,10 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "errors" "fmt" "io" + "log/slog" "math/big" "net" "net/http" @@ -24,6 +26,7 @@ import ( "github.com/miekg/dns" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prometheus/exporter-toolkit/web" ) const ( @@ -408,6 +411,63 @@ func TestMetricsTLS(t *testing.T) { } } +func TestMetricsTLSValidationErrorDoesNotClaimAddress(t *testing.T) { + probe, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("failed to reserve test address: %v", err) + } + addr := probe.Addr().String() + if err := probe.Close(); err != nil { + t.Fatalf("failed to release test address: %v", err) + } + + met := New(addr) + met.tlsConfigPath = "test_data/configs/junk.yml" + if err := met.OnStartup(); err == nil { + t.Fatal("expected invalid TLS config to fail startup") + } + if met.lnSetup { + t.Fatal("listener marked as set up after failed startup") + } + + listener, err := net.Listen("tcp", addr) + if err != nil { + t.Fatalf("failed startup kept metrics address %s: %v", addr, err) + } + listener.Close() +} + +func TestMetricsTLSServeErrorReleasesAddress(t *testing.T) { + probe, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("failed to reserve test address: %v", err) + } + addr := probe.Addr().String() + if err := probe.Close(); err != nil { + t.Fatalf("failed to release test address: %v", err) + } + + serveErr := errors.New("injected TLS serve failure") + met := New(addr) + met.tlsConfigPath = "test_data/configs/empty.yml" + met.serveTLS = func(net.Listener, *http.Server, *web.FlagConfig, *slog.Logger) error { + return serveErr + } + + if err := met.OnStartup(); !errors.Is(err, serveErr) { + t.Fatalf("expected injected TLS serve error, got %v", err) + } + if met.lnSetup { + t.Fatal("listener marked as set up after TLS serve failure") + } + + listener, err := net.Listen("tcp", addr) + if err != nil { + t.Fatalf("TLS serve failure kept metrics address %s: %v", addr, err) + } + listener.Close() +} + func TestMetrics(t *testing.T) { met := New("localhost:0") if err := met.OnStartup(); err != nil {