WIP: Ensure that NROP metrics are served securely

Signed-off-by: Swati Sehgal <[email protected]>
openshift-kni · Mar 27, 2024 · 150810d · 150810d
1 parent e6088c2
commit 150810d
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 4 deletions.
diff --git a/config/default/manager_auth_proxy_patch.yaml b/config/default/manager_auth_proxy_patch.yaml
@@ -13,16 +13,56 @@ spec:
         image: gcr.io/kubebuilder/kube-rbac-proxy:v0.8.0
         args:
         - "--secure-listen-address=0.0.0.0:8443"
-        - "--upstream=http://127.0.0.1:8080/"
+        - "--upstream=http://127.0.0.1:8081/"
+        - "--config-file=/etc/kube-rbac-proxy/config.yaml"
+        - "--tls-cert-file=/etc/tls/private/tls.crt"
+        - "--tls-private-key-file=/etc/tls/private/tls.key"
+        - "--client-ca-file=/etc/tls/client/client-ca-file"
+        - "--allow-paths=/metrics"
         - "--logtostderr=true"
         - "-v=10"
         ports:
         - containerPort: 8443
           protocol: TCP
           name: https
+        securityContext:
+          allowPrivilegeEscalation: false
+          capabilities:
+            drop:
+            - ALL
+        terminationMessagePolicy: FallbackToLogsOnError
+        volumeMounts:
+        - mountPath: /etc/kube-rbac-proxy
+          name: secret-kube-rbac-proxy-metric
+          readOnly: true
+        - mountPath: /etc/tls/private
+          name: secret-kube-rbac-proxy-tls
+          readOnly: true
+        - mountPath: /etc/tls/client
+          name: metrics-client-ca
+          readOnly: true
+      - volumes:
+        # Secret created by the service CA operator.
+        # We assume that the Kubernetes service exposing the application's pods has the
+        # "service.beta.openshift.io/serving-cert-secret-name: kube-rbac-proxy-tls"
+        # annotation.
+        - name: secret-kube-rbac-proxy-tls
+          secret:
+            secretName: kube-rbac-proxy-tls
+        # Secret containing the kube-rbac-proxy configuration (see below).
+        - name: secret-kube-rbac-proxy-metric
+          secret:
+            secretName: secret-kube-rbac-proxy-metric
+        # ConfigMap containing the CA used to verify the client certificate.
+        - name: metrics-client-ca
+          configMap:
+            name: metrics-client-ca
       - name: manager
         args:
         - "--platform=kubernetes"
         - "--health-probe-bind-address=:8081"
         - "--metrics-bind-address=127.0.0.1:8080"
+        - "--metrics-cacert-file=/etc/tls/client/client-ca-file"
+        - "--metrics-cert-file=/etc/tls/private/tls.crt"
+        - "--metrics-key-file=/etc/tls/private/tls.key"
         - "--leader-elect"
diff --git a/config/default/secret-kube-rbac-proxy.yaml b/config/default/secret-kube-rbac-proxy.yaml
@@ -0,0 +1,15 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: secret-kube-rbac-proxy-metric
+  namespace: system
+stringData:
+  config.yaml: |-
+    "authorization":
+      "static":
+      - "path": "/metrics"
+        "resourceRequest": false
+        "user":
+          "name": "system:serviceaccount:openshift-monitoring:prometheus-k8s"
+        "verb": "get"    
+type: Opaque
diff --git a/config/prometheus/monitor.yaml b/config/prometheus/monitor.yaml
@@ -12,9 +12,12 @@ spec:
     - path: /metrics
       port: https
       scheme: https
-      bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
+      bearerTokenFile: "/var/run/secrets/kubernetes.io/serviceaccount/token"
       tlsConfig:
-        insecureSkipVerify: true
+        caFile: /etc/tls/client/client-ca-file
+        certFile: /etc/tls/private/tls.crt
+        insecureSkipVerify: false
+        keyFile: /etc/tls/private/tls.key
   selector:
     matchLabels:
       control-plane: controller-manager
diff --git a/main.go b/main.go
@@ -22,6 +22,7 @@ import (
 	"flag"
 	"fmt"
 	"os"
+	"path/filepath"
 	"runtime"
 	"time"
 
@@ -73,6 +74,10 @@ const (
 	defaultProbeAddr   = ":8081"
 	defaultImage       = ""
 	defaultNamespace   = "numaresources-operator"
+	defaultCertsDir    = "/etc/secrets/nrop"
+	defaultTLSCert     = defaultCertsDir + "tls.crt"
+	defaultTLSKey      = defaultCertsDir + "tls.key"
+	caCert             = defaultCertsDir + "/ca.crt"
 )
 
 var (
@@ -100,6 +105,9 @@ type RenderParams struct {
 type Params struct {
 	webhookPort           int
 	metricsAddr           string
+	CACertFile            string
+	CertFile              string
+	KeyFile               string
 	enableLeaderElection  bool
 	probeAddr             string
 	platformName          string
@@ -139,6 +147,9 @@ func (pa *Params) FromFlags() {
 	flag.BoolVar(&pa.enableWebhooks, "enable-webhooks", pa.enableWebhooks, "enable conversion webhooks")
 	flag.IntVar(&pa.webhookPort, "webhook-port", defaultWebhookPort, "The port the operator webhook should listen to.")
 	flag.BoolVar(&pa.enableMetrics, "enable-metrics", pa.enableMetrics, "enable metrics server")
+	flag.StringVar(&pa.CACertFile, "metrics-cacert-file", pa.CACertFile, "CA certificate file path for TLS metrics serving ")
+	flag.StringVar(&pa.CertFile, "metrics-cert-file", pa.CertFile, "certificate file name for TLS metrics serving")
+	flag.StringVar(&pa.KeyFile, "metrics-key-file", pa.KeyFile, "key file name for TLS metrics serving")
 	flag.BoolVar(&pa.enableHTTP2, "enable-http2", pa.enableHTTP2, "If HTTP/2 should be enabled for the webhook servers.")
 	flag.BoolVar(&pa.enableMCPCondsForward, "enable-mcp-conds-fwd", pa.enableMCPCondsForward, "enable MCP Status Condition forwarding")
 
@@ -208,8 +219,11 @@ func main() {
 		Cache:  cache.Options{}, // TODO: restrict namespace here?
 		Scheme: scheme,
 		Metrics: metricsserver.Options{
-			// TODO: secureServing?
 			BindAddress: params.metricsAddr,
+			CertDir:     filepath.Dir(params.CACertFile),
+			CertName:    params.CertFile,
+			KeyName:     params.KeyFile,
+			// TODO: Figure out if we need to add TLSOpts here?
 		},
 		WebhookServer: webhook.NewServer(webhook.Options{
 			Port:    params.webhookPort,