From f0dc04c7a8ce4cd5a6cb9b8c67e4641d2a4422fc Mon Sep 17 00:00:00 2001
From: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com>
Date: Tue, 26 Nov 2024 12:49:45 +0000
Subject: [PATCH] Replace kube-rbac-proxy to ensure the same level of
 protection with controller-runtime feature

Utilise Controller-Runtime's WithAuthenticationAndAuthorization feature to protect the metrics endpoint. This approach provides access control, similar to the functionality of kube-rbac-proxy. kube-rbac-proxy image from gcr.io/kubebuilder/kube-rbac-proxy is deprecated and should no longer be used

More info: https://github.com/kubernetes-sigs/kubebuilder/discussions/3907
---
 cmd/manager/main.go                           | 58 ++++++++++++++++++-
 config/base/manager/manager.yaml              | 23 +-------
 config/base/rbac/kustomization.yaml           | 10 +++-
 .../coverage/manager_e2e_coverage_patch.yaml  |  1 -
 .../manager_e2e_registries_conf_patch.yaml    |  1 -
 .../tls/patches/manager_deployment_cert.yaml  |  8 ++-
 6 files changed, 71 insertions(+), 30 deletions(-)

diff --git a/cmd/manager/main.go b/cmd/manager/main.go
index 38d7534f3..a8d2d0d21 100644
--- a/cmd/manager/main.go
+++ b/cmd/manager/main.go
@@ -18,8 +18,10 @@ package main
 
 import (
 	"context"
+	"crypto/tls"
 	"flag"
 	"fmt"
+	"log"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -41,9 +43,11 @@ import (
 	"k8s.io/klog/v2/textlogger"
 	ctrl "sigs.k8s.io/controller-runtime"
 	crcache "sigs.k8s.io/controller-runtime/pkg/cache"
+	"sigs.k8s.io/controller-runtime/pkg/certwatcher"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
+	"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
 	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
 	catalogd "github.com/operator-framework/catalogd/api/v1"
@@ -70,6 +74,7 @@ import (
 var (
 	setupLog               = ctrl.Log.WithName("setup")
 	defaultSystemNamespace = "olmv1-system"
+	certWatcher            *certwatcher.CertWatcher
 )
 
 const authFilePrefix = "operator-controller-global-pull-secrets"
@@ -89,7 +94,10 @@ func podNamespace() string {
 func main() {
 	var (
 		metricsAddr               string
+		certFile                  string
+		keyFile                   string
 		enableLeaderElection      bool
+		disableMetricsTLS         bool
 		probeAddr                 string
 		cachePath                 string
 		operatorControllerVersion bool
@@ -97,9 +105,14 @@ func main() {
 		caCertDir                 string
 		globalPullSecret          string
 	)
-	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
+	flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+
+		"Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
 	flag.StringVar(&caCertDir, "ca-certs-dir", "", "The directory of TLS certificate to use for verifying HTTPS connections to the Catalogd and docker-registry web servers.")
+	flag.BoolVar(&disableMetricsTLS, "disable-metrics-tls", false,
+		"Disables TLS for the metrics endpoint, making it insecure. Use with caution. Defaults to secure (false).")
+	flag.StringVar(&certFile, "tls-cert", "", "The certificate file used for serving metrics contents over HTTPS. Requires tls-key.")
+	flag.StringVar(&keyFile, "tls-key", "", "The key file used for serving metrics contents over HTTPS. Requires tls-cert.")
 	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
@@ -119,6 +132,11 @@ func main() {
 		os.Exit(0)
 	}
 
+	if (certFile != "" && keyFile == "") || (certFile == "" && keyFile != "") {
+		setupLog.Error(nil, "unable to configure TLS certificates: tls-cert and tls-key flags must be used together")
+		os.Exit(1)
+	}
+
 	ctrl.SetLogger(textlogger.NewLogger(textlogger.NewConfig()))
 
 	setupLog.Info("starting up the controller", "version info", version.String())
@@ -161,9 +179,37 @@ func main() {
 			},
 		}
 	}
+
+	metricsServerOptions := server.Options{
+		BindAddress:   metricsAddr,
+		SecureServing: !disableMetricsTLS,
+	}
+
+	if !disableMetricsTLS {
+		// FilterProvider is used to protect the metrics endpoint with authn/authz.
+		// These configurations ensure that only authorized users and service accounts
+		// can access the metrics endpoint.
+		metricsServerOptions.FilterProvider = filters.WithAuthenticationAndAuthorization
+
+		// If the certificate files are provided, the metrics server will use them to serve the metrics endpoint.
+		// Otherwise, the metrics server will use the default certificate provided by the controller-runtime which
+		// is not recommended for production use.
+		if len(certFile) > 0 && len(keyFile) > 0 {
+			// If the certificate files change, the watcher will reload them.
+			var err error
+			certWatcher, err = certwatcher.New(certFile, keyFile)
+			if err != nil {
+				log.Fatalf("Failed to initialize certificate watcher: %v", err)
+			}
+			metricsServerOptions.TLSOpts = append(metricsServerOptions.TLSOpts, func(config *tls.Config) {
+				config.GetCertificate = certWatcher.GetCertificate
+			})
+		}
+	}
+
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme:                 scheme.Scheme,
-		Metrics:                server.Options{BindAddress: metricsAddr},
+		Metrics:                metricsServerOptions,
 		HealthProbeBindAddress: probeAddr,
 		LeaderElection:         enableLeaderElection,
 		LeaderElectionID:       "9c4404e7.operatorframework.io",
@@ -337,6 +383,14 @@ func main() {
 
 	//+kubebuilder:scaffold:builder
 
+	if secureMetrics && certWatcher != nil {
+		setupLog.Info("Adding certificate watcher to manager")
+		if err := mgr.Add(certWatcher); err != nil {
+			setupLog.Error(err, "unable to add certificate watcher to manager")
+			os.Exit(1)
+		}
+	}
+
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
 		setupLog.Error(err, "unable to set up health check")
 		os.Exit(1)
diff --git a/config/base/manager/manager.yaml b/config/base/manager/manager.yaml
index e261c5c3e..9c44b9eff 100644
--- a/config/base/manager/manager.yaml
+++ b/config/base/manager/manager.yaml
@@ -52,7 +52,7 @@ spec:
         - /manager
         args:
         - "--health-probe-bind-address=:8081"
-        - "--metrics-bind-address=127.0.0.1:8080"
+        - "--metrics-bind-address=0.0.0.0:8443"
         - "--leader-elect"
         image: controller:latest
         imagePullPolicy: IfNotPresent
@@ -84,27 +84,6 @@ spec:
             cpu: 10m
             memory: 64Mi
         terminationMessagePolicy: FallbackToLogsOnError
-      - name: kube-rbac-proxy
-        securityContext:
-          allowPrivilegeEscalation: false
-          capabilities:
-            drop:
-              - "ALL"
-        image: gcr.io/kubebuilder/kube-rbac-proxy:v0.15.0
-        args:
-        - --secure-listen-address=0.0.0.0:8443
-        - --http2-disable
-        - --upstream=http://127.0.0.1:8080/
-        - --logtostderr=true
-        ports:
-        - containerPort: 8443
-          protocol: TCP
-          name: https
-        resources:
-          requests:
-            cpu: 5m
-            memory: 64Mi
-        terminationMessagePolicy: FallbackToLogsOnError
       serviceAccountName: operator-controller-controller-manager
       terminationGracePeriodSeconds: 10
       volumes:
diff --git a/config/base/rbac/kustomization.yaml b/config/base/rbac/kustomization.yaml
index 33b8765d5..1736277f3 100644
--- a/config/base/rbac/kustomization.yaml
+++ b/config/base/rbac/kustomization.yaml
@@ -17,10 +17,14 @@ resources:
 - extension_editor_role.yaml
 - extension_viewer_role.yaml
 
-# Comment the following 4 lines if you want to disable
-# the auth proxy (https://github.com/brancz/kube-rbac-proxy)
-# which protects your /metrics endpoint.
+# The following RBAC configurations are used to protect
+# the metrics endpoint with authn/authz. These configurations
+# ensure that only authorized users and service accounts
+# can access the metrics endpoint. Comment the following
+# permissions if you want to disable this protection.
+# More info: https://book.kubebuilder.io/reference/metrics.html
 - auth_proxy_service.yaml
 - auth_proxy_role.yaml
 - auth_proxy_role_binding.yaml
 - auth_proxy_client_clusterrole.yaml
+
diff --git a/config/components/coverage/manager_e2e_coverage_patch.yaml b/config/components/coverage/manager_e2e_coverage_patch.yaml
index bda011daf..f2be3a19a 100644
--- a/config/components/coverage/manager_e2e_coverage_patch.yaml
+++ b/config/components/coverage/manager_e2e_coverage_patch.yaml
@@ -7,7 +7,6 @@ spec:
   template:
     spec:
       containers:
-      - name: kube-rbac-proxy
       - name: manager
         env:
         - name: GOCOVERDIR
diff --git a/config/components/registries-conf/manager_e2e_registries_conf_patch.yaml b/config/components/registries-conf/manager_e2e_registries_conf_patch.yaml
index 7530f9b08..42012d697 100644
--- a/config/components/registries-conf/manager_e2e_registries_conf_patch.yaml
+++ b/config/components/registries-conf/manager_e2e_registries_conf_patch.yaml
@@ -7,7 +7,6 @@ spec:
   template:
     spec:
       containers:
-      - name: kube-rbac-proxy
       - name: manager
         volumeMounts:
         - name: e2e-registries-conf
diff --git a/config/components/tls/patches/manager_deployment_cert.yaml b/config/components/tls/patches/manager_deployment_cert.yaml
index 747979321..18afac59d 100644
--- a/config/components/tls/patches/manager_deployment_cert.yaml
+++ b/config/components/tls/patches/manager_deployment_cert.yaml
@@ -1,9 +1,15 @@
 - op: add
   path: /spec/template/spec/volumes/-
-  value: {"name":"olmv1-certificate", "secret":{"secretName":"olmv1-cert", "optional": false, "items": [{"key": "ca.crt", "path": "olm-ca.crt"}]}}
+  value: {"name":"olmv1-certificate", "secret":{"secretName":"olmv1-cert", "optional": false, "items": [{"key": "ca.crt", "path": "olm-ca.crt"}, {"key": "tls.crt", "path": "tls.cert"}, {"key": "tls.key", "path": "tls.key"}]}}
 - op: add
   path: /spec/template/spec/containers/0/volumeMounts/-
   value: {"name":"olmv1-certificate", "readOnly": true, "mountPath":"/var/certs/"}
 - op: add
   path: /spec/template/spec/containers/0/args/-
   value: "--ca-certs-dir=/var/certs"
+- op: add
+  path: /spec/template/spec/containers/0/args/-
+  value: "--tls-cert=/var/certs/tls.cert"
+- op: add
+  path: /spec/template/spec/containers/0/args/-
+  value: "--tls-key=/var/certs/tls.key"