feat(sidecar+coordinator): add support for HTTPS /metrics

cmd/coordinator/main.go

@@ -18,11 +18,13 @@ package main
 
 import (
 	"context"
+	"crypto/tls"
 	"errors"
 	"fmt"
 	"net/http"
 	"os"
 	"os/signal"
+	"path/filepath"
 	"syscall"
 	"time"
 
@@ -32,6 +34,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
 
+	"github.com/llm-d/llm-d-router/pkg/common"
 	logutil "github.com/llm-d/llm-d-router/pkg/common/observability/logging"
 	"github.com/llm-d/llm-d-router/version"
 
@@ -47,9 +50,12 @@ import (
 // Scrapes are short; a small budget is enough and keeps process exit prompt.
 const metricsShutdownTimeout = 5 * time.Second
 
+var errMetricsTLS = errors.New("metrics TLS")
+
 func main() {
 	configPath := pflag.String("config", "config/coordinator/coordinator.yaml", "path to configuration file")
 	metricsPort := pflag.Int("metrics-port", 0, "port for the Prometheus /metrics endpoint. Non-positive disables the endpoint. Overrides server.metrics_port (default 9090).")
+	metricsCertDir := pflag.String("metrics-cert-dir", "", "directory with tls.crt and tls.key for the metrics endpoint. Empty serves metrics over HTTP. Overrides server.metrics_cert_dir.")
 
 	logOpts := logutil.NewOptions()
 	logOpts.AddFlags(pflag.CommandLine)
@@ -75,6 +81,10 @@ func main() {
 	if f := pflag.CommandLine.Lookup("metrics-port"); f != nil && f.Changed {
 		cfg.Server.MetricsPort = *metricsPort
 	}
+	// CLI --metrics-cert-dir wins over server.metrics_cert_dir.
+	if f := pflag.CommandLine.Lookup("metrics-cert-dir"); f != nil && f.Changed {
+		cfg.Server.MetricsCertDir = *metricsCertDir
+	}
 	if err := logOpts.Validate(); err != nil {
 		log.Error(err, "invalid logging options")
 		os.Exit(1)
@@ -116,7 +126,10 @@ func main() {
 		os.Exit(1)
 	}
 
-	log.Info("starting coordinator", "addr", cfg.Server.ListenAddr, "metrics_port", cfg.Server.MetricsPort)
+	log.Info("starting coordinator",
+		"addr", cfg.Server.ListenAddr,
+		"metrics_port", cfg.Server.MetricsPort,
+		"metrics_tls", cfg.Server.MetricsCertDir != "")
 	if cfg.Server.MetricsPort <= 0 {
 		log.Info("metrics endpoint disabled", "reason", "server.metrics_port <= 0")
 	}
@@ -162,28 +175,36 @@ func run(ctx context.Context, srv *server.Server, cfg config.ServerConfig) error
 
 	if cfg.MetricsPort > 0 {
 		g.Go(func() error {
-			return serveMetrics(gctx, cfg.MetricsPort)
+			return serveMetrics(gctx, cfg.MetricsPort, cfg.MetricsCertDir)
 		})
 	}
 
 	return g.Wait()
 }
 
-// serveMetrics stands up a Prometheus /metrics HTTP server on port and blocks
+// serveMetrics stands up a Prometheus /metrics server on port and blocks
 // until ctx is cancelled or the underlying ListenAndServe returns
 // unexpectedly. On ctx cancellation the server is drained via Shutdown
 // bounded by metricsShutdownTimeout. Uses the shared controller-runtime
 // registry so every package that registers against it (this coordinator's
 // metrics, controller-runtime's process collectors) is exposed on the same
-// endpoint.
-func serveMetrics(ctx context.Context, port int) error {
+// endpoint. A non-empty certDir enables TLS with tls.crt and tls.key.
+func serveMetrics(ctx context.Context, port int, certDir string) error {
 	mux := http.NewServeMux()
 	mux.Handle("/metrics", promhttp.HandlerFor(ctrlmetrics.Registry, promhttp.HandlerOpts{EnableOpenMetrics: true}))
 	srv := &http.Server{
 		Addr:              fmt.Sprintf(":%d", port),
 		Handler:           mux,
 		ReadHeaderTimeout: 5 * time.Second,
 	}
+	serveTLS := certDir != ""
+	if serveTLS {
+		tlsConfig, err := metricsTLSConfig(ctx, certDir)
+		if err != nil {
+			return err
+		}
+		srv.TLSConfig = tlsConfig
+	}
 
 	// Shutdown fires when ctx cancels (normal path) or when the local
 	// cancel below is invoked after ListenAndServe returns (bind failure).
@@ -199,7 +220,12 @@ func serveMetrics(ctx context.Context, port int) error {
 		_ = srv.Shutdown(graceCtx)
 	}()
 
-	err := srv.ListenAndServe()
+	var err error
+	if serveTLS {
+		err = srv.ListenAndServeTLS("", "")
+	} else {
+		err = srv.ListenAndServe()
+	}
 	cancel()
 	<-shutdownDone
 
@@ -208,3 +234,25 @@ func serveMetrics(ctx context.Context, port int) error {
 	}
 	return nil
 }
+
+// metricsTLSConfig loads and reloads the certificate used by the metrics server.
+func metricsTLSConfig(ctx context.Context, certDir string) (*tls.Config, error) {
+	certFile := filepath.Join(certDir, "tls.crt")
+	keyFile := filepath.Join(certDir, "tls.key")
+	cert, err := tls.LoadX509KeyPair(certFile, keyFile)
+	if err != nil {
+		return nil, fmt.Errorf("%w: load key pair from cert %q and key %q: %w", errMetricsTLS, certFile, keyFile, err)
+	}
+
+	reloader, err := common.NewCertReloader(ctx, certDir, &cert)
+	if err != nil {
+		return nil, fmt.Errorf("%w: start certificate reloader: %w", errMetricsTLS, err)
+	}
+
+	return &tls.Config{
+		MinVersion: tls.VersionTLS12,
+		GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) {
+			return reloader.Get(), nil
+		},
+	}, nil
+}

cmd/coordinator/main_test.go

@@ -18,13 +18,21 @@
 
 import (
 	"context"
+	"crypto/tls"
+	"crypto/x509"
+	"encoding/pem"
 	"net"
+	"net/http"
+	"os"
+	"path/filepath"
 	"strconv"
 	"testing"
 	"time"
 
+	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/require"
 
+	tlsutil "github.com/llm-d/llm-d-router/internal/tls"
 	"github.com/llm-d/llm-d-router/pkg/coordinator/config"
 	"github.com/llm-d/llm-d-router/pkg/coordinator/gateway"
 	"github.com/llm-d/llm-d-router/pkg/coordinator/pipeline"
@@ -58,6 +66,101 @@
 	t.Fatalf("no listener came up on %s within %s", addr, timeout)
 }
 
+func writeMetricsCertificate(t *testing.T, dir string) {
+	t.Helper()
+
+	cert, err := tlsutil.CreateSelfSignedTLSCertificate(logr.Discard())
+	require.NoError(t, err)
+
+	keyDER, err := x509.MarshalPKCS8PrivateKey(cert.PrivateKey)
+	require.NoError(t, err)
+
+	certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert.Certificate[0]})
+	keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyDER})
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "tls.crt"), certPEM, 0o600))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "tls.key"), keyPEM, 0o600))
+}
+
+func TestServeMetricsHTTP(t *testing.T) {
+	port, err := fwknet.GetFreePort()
+	require.NoError(t, err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	errCh := make(chan error, 1)
+	go func() { errCh <- serveMetrics(ctx, port, "") }()
+
+	client := &http.Client{Timeout: 2 * time.Second}
+	require.Eventually(t, func() bool {
+		resp, err := client.Get("http://127.0.0.1:" + strconv.Itoa(port) + "/metrics")
+		if err != nil {
+			return false
+		}
+		defer resp.Body.Close()
+		return resp.StatusCode == http.StatusOK
+	}, 2*time.Second, 20*time.Millisecond)
+
+	cancel()
+	require.NoError(t, <-errCh)
+}
+
+func TestServeMetricsHTTPS(t *testing.T) {
+	certDir := t.TempDir()
+	writeMetricsCertificate(t, certDir)
+	port, err := fwknet.GetFreePort()
+	require.NoError(t, err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	errCh := make(chan error, 1)
+	go func() { errCh <- serveMetrics(ctx, port, certDir) }()
+
+	client := &http.Client{
+		Timeout:   2 * time.Second,
+		Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}, //nolint:gosec // test certificate
+	}
+	require.Eventually(t, func() bool {
+		resp, err := client.Get("https://127.0.0.1:" + strconv.Itoa(port) + "/metrics")
+		if err != nil {
+			return false
+		}
+		defer resp.Body.Close()
+		return resp.StatusCode == http.StatusOK
+	}, 2*time.Second, 20*time.Millisecond)
+
+	resp, err := (&http.Client{Timeout: 500 * time.Millisecond}).Get("http://127.0.0.1:" + strconv.Itoa(port) + "/metrics")
+	require.NoError(t, err)
+	require.NoError(t, resp.Body.Close())
+	require.Equal(t, http.StatusBadRequest, resp.StatusCode)
+
+	cancel()
+	require.NoError(t, <-errCh)
+}
+
+func TestServeMetricsInvalidTLSFiles(t *testing.T) {
+	tests := []struct {
+		name  string
+		write bool
+	}{
+		{name: "missing"},
+		{name: "invalid", write: true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			certDir := t.TempDir()
+			if tt.write {
+				require.NoError(t, os.WriteFile(filepath.Join(certDir, "tls.crt"), []byte("invalid"), 0o600))
+				require.NoError(t, os.WriteFile(filepath.Join(certDir, "tls.key"), []byte("invalid"), 0o600))
+			}
+			port, err := fwknet.GetFreePort()
+			require.NoError(t, err)
+
+			err = serveMetrics(context.Background(), port, certDir)
+			require.ErrorIs(t, err, errMetricsTLS)
+		})
+	}
+}
+
 // With MetricsPort <= 0 no metrics goroutine joins the errgroup, so the only
 // exit path is context cancellation. run must return nil once the
 // coordinator server drains.
@@ -99,7 +202,7 @@
 	// Bind the wildcard the same way serveMetrics does so the collision is
 	// guaranteed on macOS as well as Linux. fwknet.ReserveListener binds only
 	// 127.0.0.1, which does not shadow [::]:<port> on macOS.
 	blocker, err := net.Listen("tcp", ":0")
 	require.NoError(t, err)
 	t.Cleanup(func() { _ = blocker.Close() })
 	blockedPort := blocker.Addr().(*net.TCPAddr).Port
@@ -137,3 +240,39 @@
 		t.Fatalf("coordinator server at %s still accepts connections after run returned", listenAddr)
 	}
 }
+
+func TestRun_InvalidMetricsTLSDrainsCoordinatorServer(t *testing.T) {
+	metricsPort, err := fwknet.GetFreePort()
+	require.NoError(t, err)
+	inferencePort, err := fwknet.GetFreePort()
+	require.NoError(t, err)
+	listenAddr := "127.0.0.1:" + strconv.Itoa(inferencePort)
+
+	cfg := config.ServerConfig{
+		ListenAddr:      listenAddr,
+		ShutdownTimeout: time.Second,
+		ReadTimeout:     time.Second,
+		WriteTimeout:    time.Second,
+		MetricsPort:     metricsPort,
+		MetricsCertDir:  t.TempDir(),
+	}
+	srv := newTestServer(t, listenAddr)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	done := make(chan error, 1)
+	go func() { done <- run(ctx, srv, cfg) }()
+
+	select {
+	case err := <-done:
+		require.ErrorIs(t, err, errMetricsTLS)
+	case <-time.After(5 * time.Second):
+		t.Fatal("run did not return within 5s")
+	}
+
+	conn, dialErr := net.DialTimeout("tcp", listenAddr, 100*time.Millisecond)
+	if dialErr == nil {
+		_ = conn.Close()
+		t.Fatalf("coordinator server at %s still accepts connections after run returned", listenAddr)
+	}
+}

config/coordinator/coordinator.yaml

@@ -30,6 +30,13 @@ server:
   # env: COORDINATOR_SERVER_METRICS_PORT
   # metrics_port: 9090
 
+  # metrics_cert_dir is the directory containing tls.crt and tls.key for the
+  # metrics endpoint. Empty serves metrics over HTTP. If set, missing or invalid
+  # files stop the coordinator. The metrics listener does not fall back to HTTP.
+  # The --metrics-cert-dir CLI flag overrides this field.
+  # env: COORDINATOR_SERVER_METRICS_CERT_DIR
+  # metrics_cert_dir: ""
+
   # read_timeout caps how long the server waits for the full request body.
   # Long enough for HD images inlined as data URLs (~hundreds of KB).
   read_timeout: 30s

docs/metrics.coord.md

@@ -28,6 +28,11 @@ process on the metrics port (default 9090, configurable with `--metrics-port`),
 carrying the inference paths and `/healthz` and `/readyz`. A non-positive port disables the
 endpoint.
 
+The endpoint serves HTTP when no certificate directory is set. Set `--metrics-cert-dir` or
+`server.metrics_cert_dir` to a directory containing `tls.crt` and `tls.key` to serve HTTPS.
+If a certificate directory is set, missing or invalid files stop the coordinator. The metrics listener does not fall back to HTTP.
+Valid certificate changes take effect without restarting the coordinator.
+
 The endpoint serves the shared controller-runtime registry, so controller-runtime's process
 collectors appear alongside the coordinator metrics. It is unauthenticated. Authenticating it costs
 RBAC for TokenReview and SubjectAccessReview on the coordinator's ServiceAccount.

docs/metrics.md

@@ -53,10 +53,10 @@ metrics, not EPP metrics. The raw engine metrics remain available at the model s
 
 ### P/D sidecar: MoRI-IO metrics
 
-The P/D sidecar currently exposes only the `moriio_dns_*` MoRI-IO metrics through an HTTP `/metrics`
-endpoint. It exposes them only when `--metrics-port` or the backward-compatible
-`MORIIO_METRICS_ADDR` environment variable is set. The P/D sidecar metrics server does not configure
-TLS. `SecureServing` applies to the sidecar data-plane listener.
+The P/D sidecar currently exposes only the `moriio_dns_*` MoRI-IO metrics, and only when
+`--metrics-port` or the backward-compatible `MORIIO_METRICS_ADDR` environment variable is set. The
+endpoint serves plain HTTP unless `--metrics-cert-dir` is set; see
+[MoRI-IO DNS re-resolution](#mori-io-dns-re-resolution) for the enablement and TLS settings.
 
 This endpoint belongs to the sidecar process;
 its controller-runtime registry is separate from the router pod's EPP registry.
@@ -547,6 +547,12 @@ expose these counters at `/metrics` on that port; `0` (the default) disables it.
 The `MORIIO_METRICS_ADDR` env var (e.g. `:9090`) is a backward-compatible
 fallback, consulted only when `--metrics-port` is unset.
 
+The endpoint serves plain HTTP by default. Pass `--metrics-cert-dir` with a
+directory containing `tls.crt` and `tls.key` to serve it over TLS instead.
+Missing or invalid files stop the sidecar; the metrics listener does not fall
+back to HTTP. The metrics TLS setting is independent of `--secure-proxy` and
+`--cert-path`, which apply to the sidecar data-plane listener.
+
 | Full metric name | Type | Labels | Notes |
 |---|---|---|---|
 | `moriio_dns_reresolve_total` | Counter | - | Successful request-path re-resolutions of a peer DNS name (counted per actual lookup; concurrent lookups coalesced by singleflight count once). |

pkg/coordinator/config/config.go

@@ -42,6 +42,7 @@ const DefaultMaxRequestBodySize = 64 // 64 MB
 type ServerConfig struct {
 	ListenAddr         string        `mapstructure:"listen_addr"`
 	MetricsPort        int           `mapstructure:"metrics_port"` // default 9090; non-positive disables the endpoint
+	MetricsCertDir     string        `mapstructure:"metrics_cert_dir"`
 	ReadTimeout        time.Duration `mapstructure:"read_timeout"`
 	WriteTimeout       time.Duration `mapstructure:"write_timeout"`
 	ShutdownTimeout    time.Duration `mapstructure:"shutdown_timeout"`
@@ -77,6 +78,7 @@ func Load(path string) (*Config, error) {
 	v.SetDefault("log_level", 2)
 	v.SetDefault("server.listen_addr", ":8080")
 	v.SetDefault("server.metrics_port", 9090)
+	v.SetDefault("server.metrics_cert_dir", "")
 	v.SetDefault("server.read_timeout", 30*time.Second)
 	v.SetDefault("server.write_timeout", 120*time.Second)
 	v.SetDefault("server.shutdown_timeout", 25*time.Second)