Skip to content

Commit

Permalink
Remove kube-rbac-proxy (elastic#8302)
Browse files Browse the repository at this point in the history
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>
(cherry picked from commit 032bff5)

# Conflicts:
#	go.sum
  • Loading branch information
pebrc committed Dec 11, 2024
1 parent 8ac97db commit 8c1cc0e
Show file tree
Hide file tree
Showing 17 changed files with 3,579 additions and 1,010 deletions.
4,146 changes: 3,320 additions & 826 deletions NOTICE.txt

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions config/eck.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion deploy/eck-operator/templates/cluster-roles.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions deploy/eck-operator/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: {{ . }}
Expand Down
4 changes: 2 additions & 2 deletions deploy/eck-operator/templates/role-bindings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -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"
Expand Down
45 changes: 5 additions & 40 deletions deploy/eck-operator/templates/statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
79 changes: 79 additions & 0 deletions deploy/eck-operator/templates/tests/service-monitor_test.yaml
Original file line number Diff line number Diff line change
@@ -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
84 changes: 42 additions & 42 deletions deploy/eck-operator/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down
Loading

0 comments on commit 8c1cc0e

Please sign in to comment.