feat: require TLS client certificate for /metrics

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

cmd/operator/main.go

@@ -120,13 +120,14 @@ func main() {
 	op, err := operator.New(
 		ctx,
 		operator.NewOperatorConfiguration(
+			operator.WithNamespace(namespace),
 			operator.WithMetricsAddr(metricsAddr),
 			operator.WithHealthProbeAddr(healthProbeAddr),
 			operator.WithPrometheusImage(imgMap["prometheus"]),
 			operator.WithAlertmanagerImage(imgMap["alertmanager"]),
 			operator.WithThanosSidecarImage(imgMap["thanos"]),
 			operator.WithThanosQuerierImage(imgMap["thanos"]),
-			operator.WithUIPlugins(namespace, imgMap),
+			operator.WithUIPluginImages(imgMap),
 			operator.WithFeatureGates(operator.FeatureGates{
 				OpenShift: operator.OpenShiftFeatureGates{
 					Enabled: openShiftEnabled,

pkg/operator/operator.go

@@ -8,10 +8,13 @@ import (
 	"path/filepath"
 	"time"
 
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/server/dynamiccertificates"
 	"k8s.io/client-go/kubernetes"
+	typedv1 "k8s.io/client-go/kubernetes/typed/core/v1"
+	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
@@ -40,6 +43,7 @@ const (
 type Operator struct {
 	manager               manager.Manager
 	servingCertController *dynamiccertificates.DynamicServingCertificateController
+	clientCAController    *dynamiccertificates.ConfigMapCAController
 }
 
 type OpenShiftFeatureGates struct {
@@ -51,6 +55,7 @@ type FeatureGates struct {
 }
 
 type OperatorConfiguration struct {
+	Namespace       string
 	MetricsAddr     string
 	HealthProbeAddr string
 	Prometheus      stackctrl.PrometheusConfiguration
@@ -61,6 +66,13 @@ type OperatorConfiguration struct {
 	FeatureGates    FeatureGates
 }
 
+func WithNamespace(ns string) func(*OperatorConfiguration) {
+	return func(oc *OperatorConfiguration) {
+		oc.Namespace = ns
+		oc.UIPlugins.ResourcesNamespace = ns
+	}
+}
+
 func WithPrometheusImage(image string) func(*OperatorConfiguration) {
 	return func(oc *OperatorConfiguration) {
 		oc.Prometheus.Image = image
@@ -97,10 +109,9 @@ func WithHealthProbeAddr(addr string) func(*OperatorConfiguration) {
 	}
 }
 
-func WithUIPlugins(namespace string, images map[string]string) func(*OperatorConfiguration) {
+func WithUIPluginImages(images map[string]string) func(*OperatorConfiguration) {
 	return func(oc *OperatorConfiguration) {
 		oc.UIPlugins.Images = images
-		oc.UIPlugins.ResourcesNamespace = namespace
 	}
 }
 
@@ -120,12 +131,16 @@ func NewOperatorConfiguration(opts ...func(*OperatorConfiguration)) *OperatorCon
 
 func New(ctx context.Context, cfg *OperatorConfiguration) (*Operator, error) {
 	restConfig := ctrl.GetConfigOrDie()
+	scheme := NewScheme(cfg)
 
 	metricsOpts := metricsserver.Options{
 		BindAddress: cfg.MetricsAddr,
 	}
 
-	var servingCertController *dynamiccertificates.DynamicServingCertificateController
+	var (
+		clientCAController    *dynamiccertificates.ConfigMapCAController
+		servingCertController *dynamiccertificates.DynamicServingCertificateController
+	)
 	if cfg.FeatureGates.OpenShift.Enabled {
 		// When running in OpenShift, the server uses HTTPS thanks to the
 		// service CA operator.
@@ -162,8 +177,7 @@ func New(ctx context.Context, cfg *OperatorConfiguration) (*Operator, error) {
 			return nil, err
 		}
 
-		// ConfigMapCAController automatically reloads the client CA.
-		clientCAProvider, err := dynamiccertificates.NewDynamicCAFromConfigMapController(
+		clientCAController, err = dynamiccertificates.NewDynamicCAFromConfigMapController(
 			"client-ca",
 			metav1.NamespaceSystem,
 			"extension-apiserver-authentication",
@@ -174,19 +188,29 @@ func New(ctx context.Context, cfg *OperatorConfiguration) (*Operator, error) {
 			return nil, fmt.Errorf("failed to initialize client CA controller: %w", err)
 		}
 
+		eventBroadcaster := record.NewBroadcaster()
+		eventBroadcaster.StartStructuredLogging(0)
+		eventBroadcaster.StartRecordingToSink(&typedv1.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")})
+		eventRecorder := record.NewEventRecorderAdapter(
+			eventBroadcaster.NewRecorder(scheme, v1.EventSource{Component: "cluster-observability-operator"}),
+		)
+
 		servingCertController = dynamiccertificates.NewDynamicServingCertificateController(
 			&tls.Config{
-				ClientAuth: tls.NoClientCert,
+				ClientAuth: tls.RequireAndVerifyClientCert,
 			},
-			clientCAProvider,
+			clientCAController,
 			certKeyProvider,
 			nil,
-			nil,
+			eventRecorder,
 		)
 		if err := servingCertController.RunOnce(); err != nil {
 			return nil, fmt.Errorf("failed to initialize serving certificate controller: %w", err)
 		}
 
+		clientCAController.AddListener(servingCertController)
+		certKeyProvider.AddListener(servingCertController)
+
 		metricsOpts.SecureServing = true
 		metricsOpts.TLSOpts = []func(*tls.Config){
 			func(c *tls.Config) {
@@ -198,7 +222,7 @@ func New(ctx context.Context, cfg *OperatorConfiguration) (*Operator, error) {
 	mgr, err := ctrl.NewManager(
 		restConfig,
 		ctrl.Options{
-			Scheme:                 NewScheme(cfg),
+			Scheme:                 scheme,
 			Metrics:                metricsOpts,
 			HealthProbeBindAddress: cfg.HealthProbeAddr,
 		})
@@ -235,10 +259,15 @@ func New(ctx context.Context, cfg *OperatorConfiguration) (*Operator, error) {
 	return &Operator{
 		manager:               mgr,
 		servingCertController: servingCertController,
+		clientCAController:    clientCAController,
 	}, nil
 }
 
 func (o *Operator) Start(ctx context.Context) error {
+	if o.clientCAController != nil {
+		go o.clientCAController.Run(ctx, 1)
+	}
+
 	if o.servingCertController != nil {
 		go o.servingCertController.Run(1, ctx.Done())
 	}

test/e2e/framework/assertions.go

@@ -345,14 +345,22 @@ func (f *Framework) getPodMetrics(ctx context.Context, pod *v1.Pod, opts ...func
 	tr.TLSClientConfig = &tls.Config{
 		ServerName: fmt.Sprintf("observability-operator.%s.svc", pod.Namespace),
 		RootCAs:    f.RootCA,
+		GetClientCertificate: func(*tls.CertificateRequestInfo) (*tls.Certificate, error) {
+			fmt.Printf("client cert: %#v\n", f.MetricsClientCert)
+			return f.MetricsClientCert, nil
+		},
 	}
 
 	resp, err := (&http.Client{Transport: tr}).Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get a response from /metrics: %w", err)
+		return nil, fmt.Errorf("failed to get a response from %q: %w", req.URL.String(), err)
 	}
 	defer resp.Body.Close()
 
+	if resp.StatusCode != 200 {
+		return nil, fmt.Errorf("invalid status code from %q: got %d", req.URL.String(), resp.StatusCode)
+	}
+
 	return io.ReadAll(resp.Body)
 }
 

test/e2e/framework/framework.go

@@ -3,6 +3,7 @@ package framework
 import (
 	"bytes"
 	"context"
+	"crypto/tls"
 	"crypto/x509"
 	"fmt"
 	"net/http"
@@ -34,6 +35,7 @@ type Framework struct {
 	Retain             bool
 	IsOpenshiftCluster bool
 	RootCA             *x509.CertPool
+	MetricsClientCert  *tls.Certificate
 	OperatorNamespace  string
 }
 
@@ -74,6 +76,32 @@ func (f *Framework) Setup() error {
 	}
 	f.RootCA = rootCA
 
+	// Load the prometheus-k8s TLS client certificate.
+	var s v1.Secret
+	key = client.ObjectKey{
+		Namespace: "openshift-monitoring",
+		Name:      "metrics-client-certs",
+	}
+	if err := f.K8sClient.Get(context.Background(), key, &s); err != nil {
+		return err
+	}
+
+	cert, found := s.Data["tls.crt"]
+	if !found {
+		return errors.New("failed to find TLS client certificate")
+	}
+
+	k, found := s.Data["tls.key"]
+	if !found {
+		return errors.New("failed to find TLS client key")
+	}
+
+	c, err := tls.X509KeyPair(cert, k)
+	if err != nil {
+		return err
+	}
+	f.MetricsClientCert = &c
+
 	return nil
 }