MON-3380: re-add kube-rbac-proxy for /metrics and /federate

Until brancz/kube-rbac-proxy#146 is implemented, we need a dedicated kube-rbac-proxy for static authz. Otherwise it will always do a SubjectAccessReview for requests authenticated using bearer tokens. Signed-off-by: Simon Pasquier <[email protected]>
simonpasquier · Feb 15, 2024 · 78fe72a · 78fe72a
1 parent 63d6426
commit 78fe72a
Show file tree

Hide file tree

Showing 12 changed files with 115 additions and 89 deletions.
diff --git a/Documentation/resources.adoc b/Documentation/resources.adoc
@@ -109,6 +109,7 @@ Expose the `prometheus-adapter` web server on port 443. This port is for interna
 
 Expose the Prometheus web server within the cluster on the following ports:
 * Port 9091 provides access to all the Prometheus endpoints. Granting access requires binding a user to the `cluster-monitoring-view` cluster role.
+* Port 9092 provides access the `/metrics` endpoint only. This port is for internal use, and no other usage is guaranteed.
 
 === openshift-user-workload-monitoring/prometheus-operator
 

diff --git a/Documentation/resources.md b/Documentation/resources.md
@@ -90,6 +90,7 @@ Expose the `prometheus-adapter` web server on port 443. This port is for interna
 
 Expose the Prometheus web server within the cluster on the following ports:
 * Port 9091 provides access to all the Prometheus endpoints. Granting access requires binding a user to the `cluster-monitoring-view` cluster role.
+* Port 9092 provides access the `/metrics` endpoint only. This port is for internal use, and no other usage is guaranteed.
 
 ### openshift-user-workload-monitoring/prometheus-operator
 

diff --git a/assets/cluster-monitoring-operator/cluster-role-view.yaml b/assets/cluster-monitoring-operator/cluster-role-view.yaml
@@ -22,7 +22,3 @@ rules:
   - get
   - create
   - update
-- nonResourceURLs:
-  - /federate
-  verbs:
-  - get
diff --git a/assets/prometheus-k8s/kube-rbac-proxy-secret.yaml b/assets/prometheus-k8s/kube-rbac-proxy-secret.yaml
@@ -16,4 +16,14 @@ stringData:
         "user":
           "name": "system:serviceaccount:openshift-monitoring:prometheus-k8s"
         "verb": "get"
+      - "path": "/federate"
+        "resourceRequest": false
+        "user":
+          "name": "system:serviceaccount:openshift-monitoring:prometheus-k8s"
+        "verb": "get"
+      - "path": "/federate"
+        "resourceRequest": false
+        "user":
+          "name": "system:serviceaccount:openshift-monitoring:telemeter-client"
+        "verb": "get"
 type: Opaque
diff --git a/assets/prometheus-k8s/kube-rbac-proxy-web-secret.yaml b/assets/prometheus-k8s/kube-rbac-proxy-web-secret.yaml
@@ -16,20 +16,4 @@ stringData:
         "namespace": "openshift-monitoring"
         "resource": "prometheuses"
         "subresource": "api"
-      "static":
-      - "path": "/metrics"
-        "resourceRequest": false
-        "user":
-          "name": "system:serviceaccount:openshift-monitoring:prometheus-k8s"
-        "verb": "get"
-      - "path": "/federate"
-        "resourceRequest": false
-        "user":
-          "name": "system:serviceaccount:openshift-monitoring:prometheus-k8s"
-        "verb": "get"
-      - "path": "/federate"
-        "resourceRequest": false
-        "user":
-          "name": "system:serviceaccount:openshift-monitoring:telemeter-client"
-        "verb": "get"
 type: Opaque
diff --git a/assets/prometheus-k8s/prometheus.yaml b/assets/prometheus-k8s/prometheus.yaml
@@ -52,7 +52,6 @@ spec:
     - --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.crt
     - --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305
     - --ignore-paths=/-/healthy,/-/ready
     image: quay.io/brancz/kube-rbac-proxy:v0.15.0
@@ -71,13 +70,37 @@ spec:
         - ALL
     terminationMessagePolicy: FallbackToLogsOnError
     volumeMounts:
+    - mountPath: /etc/tls/private
+      name: secret-prometheus-k8s-tls
+    - mountPath: /etc/kube-rbac-proxy
+      name: secret-prometheus-k8s-kube-rbac-proxy-web
+  - args:
+    - --secure-listen-address=0.0.0.0:9092
+    - --upstream=http://127.0.0.1:9090
+    - --allow-paths=/metrics,/federate
+    - --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.crt
+    - --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305
+    image: quay.io/brancz/kube-rbac-proxy:v0.15.0
+    name: kube-rbac-proxy
+    ports:
+    - containerPort: 9092
+      name: metrics
+    resources:
+      requests:
+        cpu: 1m
+        memory: 15Mi
+    terminationMessagePolicy: FallbackToLogsOnError
+    volumeMounts:
     - mountPath: /etc/tls/private
       name: secret-prometheus-k8s-tls
     - mountPath: /etc/tls/client
       name: configmap-metrics-client-ca
       readOnly: true
     - mountPath: /etc/kube-rbac-proxy
-      name: secret-prometheus-k8s-kube-rbac-proxy-web
+      name: secret-kube-rbac-proxy
   - args:
     - --secure-listen-address=[$(POD_IP)]:10903
     - --upstream=http://127.0.0.1:10902
@@ -106,8 +129,13 @@ spec:
     volumeMounts:
     - mountPath: /etc/tls/private
       name: secret-prometheus-k8s-thanos-sidecar-tls
+      readOnly: true
     - mountPath: /etc/kube-rbac-proxy
       name: secret-kube-rbac-proxy
+      readOnly: true
+    - mountPath: /etc/tls/client
+      name: configmap-metrics-client-ca
+      readOnly: true
   - args:
     - sidecar
     - --prometheus.url=http://localhost:9090/

diff --git a/assets/prometheus-k8s/service-monitor.yaml b/assets/prometheus-k8s/service-monitor.yaml
@@ -14,7 +14,7 @@ spec:
   endpoints:
   - bearerTokenFile: ""
     interval: 30s
-    port: web
+    port: metrics
     scheme: https
     tlsConfig:
       caFile: /etc/prometheus/configmaps/serving-certs-ca-bundle/service-ca.crt

diff --git a/assets/prometheus-k8s/service.yaml b/assets/prometheus-k8s/service.yaml
@@ -5,6 +5,7 @@ metadata:
     openshift.io/description: |-
       Expose the Prometheus web server within the cluster on the following ports:
       * Port 9091 provides access to all the Prometheus endpoints. Granting access requires binding a user to the `cluster-monitoring-view` cluster role.
+      * Port 9092 provides access the `/metrics` endpoint only. This port is for internal use, and no other usage is guaranteed.
     service.beta.openshift.io/serving-cert-secret-name: prometheus-k8s-tls
   labels:
     app.kubernetes.io/component: prometheus
@@ -20,6 +21,9 @@ spec:
   - name: web
     port: 9091
     targetPort: web
+  - name: metrics
+    port: 9092
+    targetPort: metrics
   selector:
     app.kubernetes.io/component: prometheus
     app.kubernetes.io/instance: k8s

diff --git a/jsonnet/components/cluster-monitoring-operator.libsonnet b/jsonnet/components/cluster-monitoring-operator.libsonnet
@@ -359,10 +359,6 @@ function(params) {
         resourceNames: ['k8s'],
         verbs: ['get', 'create', 'update'],
       },
-      {
-        nonResourceURLs: ['/federate'],
-        verbs: ['get'],
-      },
     ],
   },
 

diff --git a/jsonnet/components/prometheus.libsonnet b/jsonnet/components/prometheus.libsonnet
@@ -103,9 +103,11 @@ function(params)
           |||
             Expose the Prometheus web server within the cluster on the following ports:
             * Port %d provides access to all the Prometheus endpoints. %s
+            * Port %d provides access the `/metrics` endpoint only. This port is for internal use, and no other usage is guaranteed.
           ||| % [
             $.service.spec.ports[0].port,
             requiredClusterRoles(['cluster-monitoring-view'], true),
+            $.service.spec.ports[1].port,
           ],
         ),
       },
@@ -116,6 +118,11 @@ function(params)
             port: 9091,
             targetPort: 'web',
           },
+          {
+            name: 'metrics',
+            port: 9092,
+            targetPort: 'metrics',
+          },
         ],
         type: 'ClusterIP',
       },
@@ -208,28 +215,16 @@ function(params)
       }],
     },
 
+    // Eventually this container could be merged with the 'kube-rbac-proxy-web'
+    // container once https://github.com/brancz/kube-rbac-proxy/issues/146 is
+    // implemented.
     kubeRbacProxySecret: generateSecret.staticAuthSecret(
       cfg.namespace,
       cfg.commonLabels,
       'kube-rbac-proxy',
-    ),
-
-    kubeRbacProxyWebSecret: generateSecret.kubeRBACSecretForMonitoringAPI(
-      'prometheus-k8s-kube-rbac-proxy-web',
-      cfg.commonLabels,
       {
         authorization+: {
-          static: [
-            // The prometheus-k8s service account is allowed to access the /metrics endpoint.
-            {
-              user: {
-                name: 'system:serviceaccount:openshift-monitoring:prometheus-k8s',
-              },
-              verb: 'get',
-              path: '/metrics',
-              resourceRequest: false,
-            },
-            // telemeter-client using the prometheus-k8s TLS client certificate is allowed to access the /federate endpoint.
+          static+: [
             {
               user: {
                 name: 'system:serviceaccount:openshift-monitoring:prometheus-k8s',
@@ -238,7 +233,6 @@ function(params)
               path: '/federate',
               resourceRequest: false,
             },
-            // The telemeter-client service account is allowed to access the /federate endpoint.
             {
               user: {
                 name: 'system:serviceaccount:openshift-monitoring:telemeter-client',
@@ -249,7 +243,12 @@ function(params)
             },
           ],
         },
-      }
+      },
+    ),
+
+    kubeRbacProxyWebSecret: generateSecret.kubeRBACSecretForMonitoringAPI(
+      'prometheus-k8s-kube-rbac-proxy-web',
+      cfg.commonLabels,
     ),
 
     // Secret holding the token to authenticate against the Telemetry server when using native remote-write.
@@ -272,7 +271,7 @@ function(params)
       spec+: {
         endpoints: [
           {
-            port: 'web',
+            port: 'metrics',
             interval: '30s',
             scheme: 'https',
             tlsConfig: {
@@ -414,7 +413,6 @@ function(params)
               '--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.crt',
               '--tls-cipher-suites=' + cfg.tlsCipherSuites,
               // Liveness and readiness endpoints are always allowed.
               '--ignore-paths=' + std.join(',', ['/-/healthy', '/-/ready']),
@@ -424,11 +422,6 @@ function(params)
                 mountPath: '/etc/tls/private',
                 name: 'secret-' + prometheusTLSSecret,
               },
-              {
-                mountPath: '/etc/tls/client',
-                name: 'configmap-metrics-client-ca',
-                readOnly: true,
-              },
               {
                 mountPath: '/etc/kube-rbac-proxy',
                 name: 'secret-' + $.kubeRbacProxyWebSecret.metadata.name,
@@ -441,6 +434,47 @@ function(params)
               },
             },
           },
+          {
+            name: 'kube-rbac-proxy',
+            image: cfg.kubeRbacProxyImage,
+            resources: {
+              requests: {
+                memory: '15Mi',
+                cpu: '1m',
+              },
+            },
+            ports: [
+              {
+                containerPort: 9092,
+                name: 'metrics',
+              },
+            ],
+            args: [
+              '--secure-listen-address=0.0.0.0:9092',
+              '--upstream=http://127.0.0.1:9090',
+              '--allow-paths=/metrics,/federate',
+              '--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.crt',
+              '--tls-cipher-suites=' + cfg.tlsCipherSuites,
+            ],
+            volumeMounts: [
+              {
+                mountPath: '/etc/tls/private',
+                name: 'secret-prometheus-k8s-tls',
+              },
+              {
+                mountPath: '/etc/tls/client',
+                name: 'configmap-metrics-client-ca',
+                readOnly: true,
+              },
+              {
+                mountPath: '/etc/kube-rbac-proxy',
+                name: 'secret-' + $.kubeRbacProxySecret.metadata.name,
+              },
+            ],
+          },
           {
             name: 'kube-rbac-proxy-thanos',
             image: cfg.kubeRbacProxyImage,
@@ -479,10 +513,17 @@ function(params)
               {
                 mountPath: '/etc/tls/private',
                 name: 'secret-' + thanosSidecarTLSSecret,
+                readOnly: true,
               },
               {
                 mountPath: '/etc/kube-rbac-proxy',
                 name: 'secret-' + $.kubeRbacProxySecret.metadata.name,
+                readOnly: true,
+              },
+              {
+                mountPath: '/etc/tls/client',
+                name: 'configmap-metrics-client-ca',
+                readOnly: true,
               },
             ],
           },

diff --git a/manifests/0000_50_cluster-monitoring-operator_02-role.yaml b/manifests/0000_50_cluster-monitoring-operator_02-role.yaml
@@ -211,10 +211,6 @@ rules:
   - get
   - create
   - update
-- nonResourceURLs:
-  - /federate
-  verbs:
-  - get
 - apiGroups:
   - ""
   resourceNames:

diff --git a/pkg/manifests/manifests.go b/pkg/manifests/manifests.go
@@ -58,8 +58,6 @@ const (
 	configManagedNamespace = "openshift-config-managed"
 	sharedConfigMap        = "monitoring-shared-config"
 
-	clientCAArg = "--client-ca-file=/etc/tls/client/client-ca.crt"
-
 	tmpClusterIDLabelName = "__tmp_openshift_cluster_id__"
 
 	nodeSelectorMaster = "node-role.kubernetes.io/master"
@@ -1467,41 +1465,12 @@ func (f *Factory) PrometheusK8s(grpcTLS *v1.Secret, trustedCABundleCM *v1.Config
 
 	for i, container := range p.Spec.Containers {
 		switch container.Name {
-		case "kube-rbac-proxy-web":
-			p.Spec.Containers[i].Image = f.config.Images.KubeRbacProxy
-			p.Spec.Containers[i].Args = f.setTLSSecurityConfiguration(container.Args, KubeRbacProxyTLSCipherSuitesFlag, KubeRbacProxyMinTLSVersionFlag)
-		case "kube-rbac-proxy-thanos":
+		case "kube-rbac-proxy", "kube-rbac-proxy-web", "kube-rbac-proxy-thanos":
 			p.Spec.Containers[i].Image = f.config.Images.KubeRbacProxy
-
 			p.Spec.Containers[i].Args = f.setTLSSecurityConfiguration(container.Args, KubeRbacProxyTLSCipherSuitesFlag, KubeRbacProxyMinTLSVersionFlag)
-			p.Spec.Containers[i].Args = append(
-				p.Spec.Containers[i].Args,
-				clientCAArg,
-			)
-
-			p.Spec.Containers[i].VolumeMounts = append(
-				p.Spec.Containers[i].VolumeMounts,
-				v1.VolumeMount{
-					Name:      "metrics-client-ca",
-					MountPath: "/etc/tls/client",
-					ReadOnly:  true,
-				},
-			)
-
-			p.Spec.Volumes = append(
-				p.Spec.Volumes,
-				v1.Volume{
-					Name: "metrics-client-ca",
-					VolumeSource: v1.VolumeSource{
-						ConfigMap: &v1.ConfigMapVolumeSource{
-							LocalObjectReference: v1.LocalObjectReference{
-								Name: "metrics-client-ca",
-							},
-						},
-					},
-				})
 		}
 	}
+
 	p.Spec.Volumes = append(p.Spec.Volumes, v1.Volume{
 		Name: "secret-grpc-tls",
 		VolumeSource: v1.VolumeSource{
-Original file line number
+Diff line change
@@ Expand Up / @@ -22,7 +22,3 @@ rules: @@
       - get
       - create
       - update
-    - nonResourceURLs:
-      - /federate
-      verbs:
-      - get