Remove kube-rbac-proxy (#8302)

This removes the kube-auth-proxy from the ECK Helm charts and the ECK documentation. Instead if follows the recommendation from controller-runtime to use the built-in FilterProvider filters.WithAuthenticationAndAuthorization.

This pulls in a bunch of k8s API server dependencies increasing the binary size by about 12MB IIRC.

I have also tried to address some issues with our current Helm templating of the metrics server:

- allow enabling the secure mode with TLS+auth while not forcing users to have Promotheus installed e.g. when using Elastic Agent (!) (service monitor is still generated by default for bwc)
-    mixing configuration properties for the service monitor with the configuration properties of the metrics server (I moved them to serviceMonitor.* while implementing a form of bwc layer in the template

---------

Co-authored-by: Michael Montgomery <mmontg1@gmail.com>
Co-authored-by: Michael Morello <michael.morello@elastic.co>

cmd/manager/main.go

@@ -37,6 +37,7 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
+	"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	crwebhook "sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -292,6 +293,17 @@ func Command() *cobra.Command {
 		"0.0.0.0",
 		fmt.Sprintf("The host to which the operator should bind to serve metrics in the Prometheus format. Will be combined with %s.", operator.MetricsPortFlag),
 	)
+	cmd.Flags().Bool(
+		operator.MetricsSecureFlag,
+		false,
+		fmt.Sprintf("Enables TLS for the metrics server. Only effective combined with %s", operator.MetricsPortFlag),
+	)
+	cmd.Flags().String(
+		operator.MetricsCertDirFlag,
+		// this is controller-runtime's own default, copied here for making the default explicit when using `--help`
+		filepath.Join(os.TempDir(), "k8s-metrics-server", "serving-certs"),
+		fmt.Sprintf("Location of TLS certs for the metrics server. Directory needs to contain tls.key and tls.crt. If empty self-signed certificates are used. Only effective when combined with %s and %s", operator.MetricsPortFlag, operator.MetricsSecureFlag),
+	)
 	cmd.Flags().StringSlice(
 		operator.NamespacesFlag,
 		nil,
@@ -588,6 +600,14 @@ func startOperator(ctx context.Context) error {
 	opts.Metrics = metricsserver.Options{
 		BindAddress: fmt.Sprintf("%s:%d", metricsHost, metricsPort), // 0 to disable
 	}
+	if viper.GetBool(operator.MetricsSecureFlag) {
+		opts.Metrics.FilterProvider = filters.WithAuthenticationAndAuthorization
+		opts.Metrics.SecureServing = true
+	}
+
+	if metricsCertDir := viper.GetString(operator.MetricsCertDirFlag); len(metricsCertDir) > 0 {
+		opts.Metrics.CertDir = metricsCertDir
+	}
 
 	webhookPort := viper.GetInt(operator.WebhookPortFlag)
 	webhookCertDir := viper.GetString(operator.WebhookCertDirFlag)

config/eck.yaml

@@ -1,5 +1,6 @@
 log-verbosity: 0
 metrics-port: 0
+metrics-secure: false
 container-registry: docker.elastic.co
 max-concurrent-reconciles: 3
 ca-cert-validity: 8760h

deploy/eck-operator/templates/cluster-roles.yaml

@@ -103,7 +103,7 @@ kind: ClusterRole
 metadata:
   labels:
     {{- include "eck-operator.labels" . | nindent 4 }}
-  name: "{{ include "eck-operator.fullname" . }}-proxy-role"
+  name: "{{ include "eck-operator.fullname" . }}-metrics-auth-role"
 rules:
 - apiGroups:
   - authentication.k8s.io

deploy/eck-operator/templates/configmap.yaml

@@ -13,11 +13,8 @@ data:
     {{- if and .Values.config.metrics.secureMode.enabled (eq $metricsPort 0) }}
     {{- fail "config.metrics.port must be greater than 0 when config.metrics.secureMode.enabled is true" }}
     {{- end }}
-    {{- if .Values.config.metrics.secureMode.enabled }}
-    metrics-port: {{ add $metricsPort 1 }}
-    {{- else }}
     metrics-port: {{ $metricsPort }}
-    {{- end }}
+    metrics-secure: {{ .Values.config.metrics.secureMode.enabled }}
     container-registry: {{ .Values.config.containerRegistry }}
     {{- with .Values.config.containerSuffix }}
     container-suffix: {{ . }}

deploy/eck-operator/templates/role-bindings.yaml

@@ -85,11 +85,11 @@ kind: ClusterRoleBinding
 metadata:
   labels:
     {{- include "eck-operator.labels" $ | nindent 4 }}
-  name: "{{ include "eck-operator.fullname" . }}-proxy-rolebinding"
+  name: "{{ include "eck-operator.fullname" . }}-metrics-auth-rolebinding"
 roleRef:
   apiGroup: rbac.authorization.k8s.io
   kind: ClusterRole
-  name: "{{ include "eck-operator.fullname" . }}-proxy-role"
+  name: "{{ include "eck-operator.fullname" . }}-metrics-auth-role"
 subjects:
 - kind: ServiceAccount
   name: {{ $svcAccount }}

deploy/eck-operator/templates/serviceMonitor.yaml → deploy/eck-operator/templates/service-monitor.yaml

@@ -1,4 +1,4 @@
-{{- if .Values.config.metrics.secureMode.enabled }}
+{{- if and .Values.config.metrics.secureMode.enabled .Values.serviceMonitor.enabled }}
 apiVersion: monitoring.coreos.com/v1
 kind: ServiceMonitor
 metadata:
@@ -19,10 +19,13 @@ spec:
     scheme: https
     interval: 30s
     tlsConfig:
-      insecureSkipVerify: {{ .Values.config.metrics.secureMode.tls.insecureSkipVerify | default false }}
-      {{- if (not .Values.config.metrics.secureMode.tls.insecureSkipVerify) }}
-      {{- $leading_path := trimSuffix "/" .Values.config.metrics.secureMode.tls.caMountDirectory }}
-      {{- with .Values.config.metrics.secureMode.tls.caSecret }}
+      {{- $insecureSkipVerify := (ternary .Values.config.metrics.secureMode.tls.insecureSkipVerify .Values.serviceMonitor.insecureSkipVerify (hasKey .Values.config.metrics.secureMode.tls "insecureSkipVerify")) }}
+      insecureSkipVerify: {{ $insecureSkipVerify }}
+      {{- if (not $insecureSkipVerify) }}
+      {{- $caMountDirectory := or (.Values.config.metrics.secureMode.tls.caMountDirectory) (.Values.serviceMonitor.caMountDirectory) -}}
+      {{- $leading_path := trimSuffix "/" $caMountDirectory }}
+      {{- $caSecret := or (.Values.config.metrics.secureMode.tls.caSecret) (.Values.serviceMonitor.caSecret) -}}
+      {{- with $caSecret }}
       caFile: "{{ $leading_path }}/{{ . }}/ca.crt"
       {{- end }}
       serverName: "{{ include "eck-operator.fullname" . }}-metrics.{{ .Release.Namespace }}.svc"

deploy/eck-operator/templates/statefulset.yaml

@@ -89,7 +89,7 @@ spec:
           {{- end }}
           {{- if or .Values.webhook.enabled (gt $metricsPort 0) }}
           ports:
-            {{- if and (gt $metricsPort 0) (not .Values.config.metrics.secureMode.enabled) }}
+            {{- if (gt $metricsPort 0) }}
             - containerPort: {{ $metricsPort }}
               name: metrics
               protocol: TCP
@@ -109,49 +109,14 @@ spec:
               name: cert
               readOnly: true
             {{- end }}
-            {{- with .Values.volumeMounts }}
-              {{- toYaml . | nindent 12 }}
-            {{- end }}
-        {{- if .Values.config.metrics.secureMode.enabled }}
-        - 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:{{ $metricsPort }}"
-          - "--upstream=http://127.0.0.1:{{ add $metricsPort 1 }}/"
-          - "--logtostderr=true"
-          - "--v=0"
-          {{- if .Values.config.metrics.secureMode.tls.certificateSecret }}
-          - "--tls-cert-file=/tls/tls.crt"
-          - "--tls-private-key-file=/tls/tls.key"
-          {{- end }}
-          {{- if or .Values.config.metrics.secureMode.tls.certificateSecret .Values.config.metrics.secureMode.volumeMounts }}
-          volumeMounts:
-            {{- with .Values.config.metrics.secureMode.volumeMounts }}
-              {{- toYaml . | nindent 12 }}
-            {{- end }}
             {{- if .Values.config.metrics.secureMode.tls.certificateSecret }}
-            - mountPath: "/tls"
+            - mountPath: "/tmp/k8s-metrics-server/serving-certs"
               name: tls-certificate
               readOnly: true
+            {{- end }} 
+            {{- with .Values.volumeMounts }}
+              {{- toYaml . | nindent 12 }}
             {{- end }}
-          {{- end }}
-          ports:
-          - containerPort: {{ $metricsPort }}
-            protocol: TCP
-            name: metrics
-          resources:
-            limits:
-              cpu: 500m
-              memory: 128Mi
-            requests:
-              cpu: 5m
-              memory: 64Mi
-        {{- end }}
       volumes:
         - name: conf
           configMap:

deploy/eck-operator/templates/tests/service-monitor_test.yaml

@@ -0,0 +1,79 @@
+# yaml-language-server: $schema=https://raw.githubusercontent.com/helm-unittest/helm-unittest/main/schema/helm-testsuite.json
+suite: test operator service monitor
+templates:
+  - service-monitor.yaml
+tests:
+  - it: default service monitor 
+    set:
+      config:
+        metrics:
+          secureMode:
+            enabled: true
+    asserts:
+      - template: service-monitor.yaml
+        equal:
+          path: spec
+          value:
+            endpoints:
+            - bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
+              interval: 30s
+              path: /metrics
+              port: https
+              scheme: https
+              tlsConfig:
+                insecureSkipVerify: true
+            namespaceSelector:
+              matchNames:
+              - NAMESPACE
+            selector:
+              matchLabels:
+                app.kubernetes.io/instance: RELEASE-NAME
+                app.kubernetes.io/name: elastic-operator-metrics-service 
+  - it: legacy values 
+    set:
+      config:
+        metrics:
+          secureMode:
+            enabled: true
+            tls:
+              insecureSkipVerify: false
+              caSecret: metrics-ca
+              caMountDirectory: /etc/custom-ca/
+    asserts:
+      - template: service-monitor.yaml
+        equal:
+          path: spec.endpoints[0].tlsConfig
+          value:
+            caFile: /etc/custom-ca/metrics-ca/ca.crt
+            insecureSkipVerify: false
+            serverName: elastic-operator-metrics.NAMESPACE.svc
+  - it: serviceMonitor values 
+    set:
+      config:
+        metrics:
+          secureMode: 
+            enabled: true
+      serviceMonitor:
+        insecureSkipVerify: false
+        caSecret: metrics-ca
+        caMountDirectory: /etc/custom-ca/
+    asserts:
+      - template: service-monitor.yaml
+        equal:
+          path: spec.endpoints[0].tlsConfig
+          value:
+            caFile: /etc/custom-ca/metrics-ca/ca.crt
+            insecureSkipVerify: false
+            serverName: elastic-operator-metrics.NAMESPACE.svc
+  - it: secure mode without service monitor 
+    set:
+      serviceMonitor:
+        enabled: false
+      config:
+        metrics:
+          secureMode: 
+            enabled: true
+    asserts:
+      - template: service-monitor.yaml
+        hasDocuments:
+          count: 0

deploy/eck-operator/values.yaml

@@ -182,29 +182,12 @@ config:
     port: "0"
     # secureMode contains the options for enabling and configuring RBAC and TLS/HTTPs for the metrics endpoint.
     secureMode:
-      # secureMode.enabled specifies whether to enable RBAC and TLS/HTTPs for the metrics endpoint. (Will be enabled by default in v2.14.0)
-      # * This option requires using a ServiceMonitor to scrape the metrics and as such is mutually exclusive with the podMonitor.enabled option.
+      # secureMode.enabled specifies whether to enable RBAC and TLS/HTTPs for the metrics endpoint.
+      # * This option makes most sense when using a ServiceMonitor to scrape the metrics and is therefore mutually exclusive with the podMonitor.enabled option.
       # * This option also requires using cluster scoped resources (ClusterRole, ClusterRoleBinding) to
       #   grant access to the /metrics endpoint. (createClusterScopedResources: true is required)
       #
-      # This option requires the following settings within Prometheus to function:
-      # 1. RBAC settings for the Prometheus instance to access the metrics endpoint.
-      #
-      # - nonResourceURLs:
-      #   - /metrics
-      #   verbs:
-      #   - get
-      #
-      # 2. If using the Prometheus Operator and your Prometheus instance is not in the same namespace as the operator you will need
-      #    the Prometheus Operator configured with the following Helm values:
-      #
-      #   prometheus:
-      #     prometheusSpec:
-      #       serviceMonitorNamespaceSelector: {}
-      #       serviceMonitorSelectorNilUsesHelmValues: false
       enabled: false
-      # additional volume mounts for the kube-rbac-proxy container.
-      volumeMounts: []
       tls:
         # certificateSecret is the name of the tls secret containing the custom TLS certificate and key for the secure metrics endpoint.
         #
@@ -216,27 +199,6 @@ config:
         # example: kubectl create secret tls eck-metrics-tls-certificate -n elastic-system \
         #            --cert=/path/to/tls.crt --key=/path/to/tls.key
         certificateSecret: ""
-        # caSecret is the name of the secret containing the custom CA certificate used to generate the custom TLS certificate for the secure metrics endpoint.
-        #
-        # * This *must* be the name of the secret containing the CA certificate used to sign the custom TLS certificate.
-        # * This secret *must* be in the same namespace as the Prometheus instance that will scrape the metrics.
-        # * If using the Prometheus operator this secret must be within the `spec.secrets` field of the `Prometheus` custom resource such that it is mounted into the Prometheus pod at `caMountDirectory`, which defaults to /etc/prometheus/secrets/{secret-name}.
-        # * This is an optional setting and is only required if you are using a custom TLS certificate.
-        # * Key must be named ca.crt.
-        #
-        # example: kubectl create secret generic eck-metrics-tls-ca -n monitoring \
-        #            --from-file=ca.crt=/path/to/ca.pem
-        caSecret: ""
-        # caMountDirectory is the directory at which the CA certificate is mounted within the Prometheus pod.
-        #
-        # * You should only need to adjust this if you are *not* using the Prometheus operator.
-        caMountDirectory: "/etc/prometheus/secrets/"
-        # insecureSkipVerify specifies whether to skip verification of the TLS certificate for the secure metrics endpoint.
-        #
-        # * If this setting is set to false, then the following settings are required:
-        #   - certificateSecret
-        #   - caSecret
-        insecureSkipVerify: true
 
   # containerRegistry to use for pulling Elasticsearch and other application container images.
   containerRegistry: docker.elastic.co
@@ -337,11 +299,49 @@ podMonitor:
 # Prometheus ServiceMonitor configuration
 # Only used when config.enableSecureMetrics is true
 # Reference: https://github.com/prometheus-operator/prometheus-operator/blob/master/Documentation/api.md#servicemonitor
-serviceMonitor: {}
-
+serviceMonitor:
+  # This option requires the following settings within Prometheus to function:
+  # 1. RBAC settings for the Prometheus instance to access the metrics endpoint.
+  #
+  # - nonResourceURLs:
+  #   - /metrics
+  #   verbs:
+  #   - get
+  #
+  # 2. If using the Prometheus Operator and your Prometheus instance is not in the same namespace as the operator you will need
+  #    the Prometheus Operator configured with the following Helm values:
+  #
+  #   prometheus:
+  #     prometheusSpec:
+  #       serviceMonitorNamespaceSelector: {}
+  #       serviceMonitorSelectorNilUsesHelmValues: false
+  #
+  # allows to disable the serviceMonitor, enabled by default for backwards compatibility
+  enabled: true
   # namespace determines in which namespace the serviceMonitor will be deployed.
   # If not set the serviceMonitor will be created in the namespace where the Helm release is installed into
   # namespace: monitoring
+  # caSecret is the name of the secret containing the custom CA certificate used to generate the custom TLS certificate for the secure metrics endpoint.
+  #
+  # * This *must* be the name of the secret containing the CA certificate used to sign the custom TLS certificate for the metrics endpoint.
+  # * This secret *must* be in the same namespace as the Prometheus instance that will scrape the metrics.
+  # * If using the Prometheus operator this secret must be within the `spec.secrets` field of the `Prometheus` custom resource such that it is mounted into the Prometheus pod at `caMountDirectory`, which defaults to /etc/prometheus/secrets/{secret-name}.
+  # * This is an optional setting and is only required if you are using a custom TLS certificate.
+  # * Key must be named ca.crt.
+  #
+  # example: kubectl create secret generic eck-metrics-tls-ca -n monitoring \
+  #            --from-file=ca.crt=/path/to/ca.pem
+  caSecret: ""
+  # caMountDirectory is the directory at which the CA certificate is mounted within the Prometheus pod.
+  #
+  # * You should only need to adjust this if you are *not* using the Prometheus operator.
+  caMountDirectory: "/etc/prometheus/secrets/"
+  # insecureSkipVerify specifies whether to skip verification of the TLS certificate for the secure metrics endpoint.
+  #
+  # * If this setting is set to false, then the following settings are required:
+  #   - certificateSecret
+  #   - caSecret
+  insecureSkipVerify: true
 
 # Globals meant for internal use only
 global: