Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for securing the operator metrics endpoint with RBAC and TLS #7567

Merged
merged 30 commits into from
Mar 7, 2024

Conversation

naemono
Copy link
Contributor

@naemono naemono commented Feb 15, 2024

What is this change?

  • This allows users to configure RBAC authentication/authorization on the exposed Prometheus metrics port.
  • This also allows users to use their own tls certificates when securing the metrics endpoint.
  • Adds detailed documentation for configuring the metrics endpoint.

TODO

  • Updating documentation
  • Adding the ability to bring your own TLS certificate

Note

The only thing missing here is allowing the removal of tlsConfig.insecureSkipVerify from the ServiceMonitor, as this would require creating secrets and configuration options for the prometheus installation as well, and this seems a bit outside the scope of this change. Maybe this is something we could test/document in the future? Do you all feel this is required in this PR?

Details

  • This is disabled by default, but we note the intent to enable by default in v2.14.0.
  • This adds a sidecar to the ECK Operator which runs gcr.io/kubebuilder/kube-rbac-proxy, which is exactly how kubebuilder templates this on new projects automatically in it's current version.
  • This adds a ServiceMonitor and a Service to the ECK Operator, which is required for this to function. (details here. tl;dr Only ServiceMonitor allows Bearer Token as an option, not PodMonitor)
  • This means that podMonitor.enabled: true will now fail when this option is enabled, as they are mutually exclusive.
  • This also means that createClusterScopedResources: false will now fail when this option is enabled, as kube-rbac-proxy requires cluster-scoped tokenreviews and subjectaccessreviews to function.
  • This should work with both createClusterScopedResources: true and false (testing in progress to verify; reason for draft state) (see above bullet point why this is crossed-out)
  • New metrics-host is added to the operator to allow securing the metrics endpoint from a network perspective. Documentation was added that shows how to use this option when using the manifests to secure the metrics endpoint. Documentation was also added showing describing how this is done using the Helm chart.

Local Testing

Tested both using the Helm chart, and the manifests with the provided instructions in all use cases.

Target configuration:
image
Metrics Data:
image

ServiceMonitor:

apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
  annotations:
    meta.helm.sh/release-name: elastic-operator
    meta.helm.sh/release-namespace: elastic-system
  creationTimestamp: "2024-02-20T20:52:06Z"
  generation: 1
  labels:
    app.kubernetes.io/instance: elastic-operator
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: elastic-operator
    app.kubernetes.io/version: 2.12.0-SNAPSHOT
    helm.sh/chart: eck-operator-2.12.0-SNAPSHOT
  name: elastic-operator
  namespace: elastic-system
  resourceVersion: "190053045"
  uid: 0b6f6edb-4673-4e96-829a-2439b7784bc4
spec:
  endpoints:
  - bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
    interval: 30s
    path: /metrics
    port: https
    scheme: https
    tlsConfig:
      insecureSkipVerify: true
  namespaceSelector:
    matchNames:
    - elastic-system
  selector:
    matchLabels:
      app.kubernetes.io/instance: elastic-operator
      app.kubernetes.io/name: elastic-operator-metrics-service

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono added >enhancement Enhancement of existing functionality v2.12.0 labels Feb 15, 2024
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono marked this pull request as ready for review February 20, 2024 21:12
Update for metricsBindAddress.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Remove debugging of kube-rbac-proxy

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

[source,sh]
----
kubectl get cm elastic-operator -n elastic-system -o yaml | sed "s|metrics-port: 0|metrics-port: 8081|" | kubectl apply -f -
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like this particularly. Should we simply have the command to create an updated full ConfigMap?

Copy link
Contributor

@kvalliyurnatt kvalliyurnatt Mar 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me having a comment to create an updated full ConfigMap sounds better

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've actually added a full configmap apply here instead, which just seems cleaner.

Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a quick look. I need to come back to this later.

deploy/eck-operator/values.yaml Outdated Show resolved Hide resolved
deploy/eck-operator/templates/serviceMonitor.yaml Outdated Show resolved Hide resolved
deploy/eck-operator/templates/configmap.yaml Outdated Show resolved Hide resolved
docs/advanced-topics/secure-metrics.asciidoc Outdated Show resolved Hide resolved
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono changed the title Add RBAC authentication option for the ECK metrics endpoint. WIP: Add RBAC authentication option for the ECK metrics endpoint. Feb 26, 2024
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Fix having nil pod/servicemonitor causing crash.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono changed the title WIP: Add RBAC authentication option for the ECK metrics endpoint. Add RBAC authentication option for the ECK metrics endpoint. Feb 27, 2024
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Add configure operator metrics link.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
cmd/manager/main.go Outdated Show resolved Hide resolved
docs/operating-eck/configure-operator-metrics.asciidoc Outdated Show resolved Hide resolved
docs/operating-eck/configure-operator-metrics.asciidoc Outdated Show resolved Hide resolved
…ificate.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono requested a review from pebrc March 1, 2024 17:11
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
docs/operating-eck/configure-operator-metrics.asciidoc Outdated Show resolved Hide resolved
docs/operating-eck/configure-operator-metrics.asciidoc Outdated Show resolved Hide resolved
docs/operating-eck/configure-operator-metrics.asciidoc Outdated Show resolved Hide resolved
docs/operating-eck/configure-operator-metrics.asciidoc Outdated Show resolved Hide resolved
docs/operating-eck/operating-eck.asciidoc Outdated Show resolved Hide resolved
docs/operating-eck/configure-operator-metrics.asciidoc Outdated Show resolved Hide resolved
docs/operating-eck/configure-operator-metrics.asciidoc Outdated Show resolved Hide resolved
docs/operating-eck/configure-operator-metrics.asciidoc Outdated Show resolved Hide resolved
docs/operating-eck/configure-operator-metrics.asciidoc Outdated Show resolved Hide resolved
docs/operating-eck/configure-operator-metrics.asciidoc Outdated Show resolved Hide resolved
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two last small adjustements to write chart and operator in lowercase and also adding "If using the Prometheus operator" before "add a ServiceMonitor" and it is good to me.

diff
diff --git a/docs/operating-eck/configure-operator-metrics.asciidoc b/docs/operating-eck/configure-operator-metrics.asciidoc
index c7a8050c3..6a699152d 100644
--- a/docs/operating-eck/configure-operator-metrics.asciidoc
+++ b/docs/operating-eck/configure-operator-metrics.asciidoc
@@ -21,7 +21,7 @@ NOTE: The ECK operator metrics endpoint will be secured by default beginning in

 The metrics endpoint is not enabled by default. To enable the metrics endpoint, follow the instructions in the next sections depending on whether you installed ECK through the Helm chart or the manifests.

-=== Using the operator Helm Chart
+=== Using the operator Helm chart

 If you installed ECK through the Helm chart commands listed in <<{p}-install-helm>>, you can now set  `config.metrics.port` to a value greater than 0 in your values file and the metrics endpoint will be enabled.

@@ -95,7 +95,7 @@ EOF
 kubectl delete pod -n elastic-system elastic-operator-0
 ----

-* If using the Prometheus Operator, install a `PodMonitor` to allow scraping of the metrics endpoint by Prometheus.
+* If using the Prometheus operator, install a `PodMonitor` to allow scraping of the metrics endpoint by Prometheus.

 [source,shell,subs="attributes,+macros"]
 ----
@@ -134,7 +134,7 @@ The ECK operator provides a metrics endpoint that can be used to monitor the ope

 If you installed ECK through the Helm chart commands listed in <<{p}-install-helm>>, you can now set `config.metrics.secureMode.enabled` to `true` and both RBAC and TLS/HTTPs will be enabled for the metrics endpoint.

-==== Using your own TLS certificate for the metrics endpoint when using the Helm Chart
+==== Using your own TLS certificate for the metrics endpoint when using the Helm chart

 By default a self-signed certificate will be generated for use by the metrics endpoint. If you want to use your own TLS certificate for the metrics endpoint you can provide the `config.metrics.secureMode.tls.certificateSecret` to the Helm chart. The `certificateSecret` should be the name of an existing Kubernetes `Secret` that contains both the TLS certificate and the TLS private key. The following keys are supported within the secret:

@@ -155,7 +155,7 @@ Providing this secret is sufficient to use your own certificate if it is from a
 * Provide the Certificate Authority to Prometheus.
     ** Set `config.metrics.secureMode.tls.insecureSkipVerify` to `false` to enable TLS validation.
     ** Set `config.metrics.secureMode.tls.caSecret` to the name of an existing Kubernetes secret within the Prometheus namespace that contains the CA in PEM format.
-    ** Set the `spec.secrets` field of the `Prometheus` custom resource such that the CA secret is mounted into the Prometheus pod at `config.metrics.secureMode.tls.caMountDirectory` (assuming you are using the Prometheus operator). See the link:{eck_github}/tree/{eck_release_branch}/deploy/eck-operator/values.yaml[ECK Helm Chart values file] for more information.
+    ** Set the `spec.secrets` field of the `Prometheus` custom resource such that the CA secret is mounted into the Prometheus pod at `config.metrics.secureMode.tls.caMountDirectory` (assuming you are using the Prometheus operator). See the link:{eck_github}/tree/{eck_release_branch}/deploy/eck-operator/values.yaml[ECK Helm chart values file] for more information.

 See the <<{p}-prometheus-requirements,prometheus requirements section>> for more information on creating the CA secret.

@@ -299,7 +299,7 @@ spec:
 EOF
 ----

-* Add a `ServiceMonitor` to allow scraping of the metrics endpoint by Prometheus.
+* If using the Prometheus operator, add a `ServiceMonitor` to allow scraping of the metrics endpoint by Prometheus.

 [source,shell,subs="attributes,+macros"]
 ----
@@ -440,9 +440,9 @@ rules:
   - get
 ----

-=== Optional Prometheus Operator Helm settings to allow reading PodMonitor and ServiceMonitor across namespaces
+=== Optional Prometheus operator Helm settings to allow reading PodMonitor and ServiceMonitor across namespaces

-* If using the Prometheus Operator and your Prometheus instance is not in the same namespace as the ECK operator you will need the Prometheus Operator configured with the following Helm values:
+* If using the Prometheus operator and your Prometheus instance is not in the same namespace as the ECK operator you will need the Prometheus operator configured with the following Helm values:

 [source,yaml,subs="attributes"]
 ----
@@ -469,4 +469,4 @@ kubectl create secret generic eck-metrics-tls-ca -n monitoring --from-file=ca.cr

 * Ensure that the CA secret is mounted within the Prometheus Pod.

-This will vary between Prometheus installations, but if using the Prometheus Operator you can set the `spec.secrets` field of the `Prometheus` custom resource to the name of the previously created Kubernetes Secret. See the link:{eck_github}/tree/{eck_release_branch}/deploy/eck-operator/values.yaml[ECK Helm Chart values file] for more information.
+This will vary between Prometheus installations, but if using the Prometheus operator you can set the `spec.secrets` field of the `Prometheus` custom resource to the name of the previously created Kubernetes Secret. See the link:{eck_github}/tree/{eck_release_branch}/deploy/eck-operator/values.yaml[ECK Helm chart values file] for more information.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono merged commit 81df441 into elastic:main Mar 7, 2024
5 checks passed
@naemono naemono deleted the 480-add-metrics-auth branch March 7, 2024 14:44
thbkrkr pushed a commit that referenced this pull request Mar 8, 2024
* Add RBAC+TLS options to the ECK metrics endpoint.
* Add documentation for RBAC/TLS ECK metrics features.

---------

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
thbkrkr added a commit that referenced this pull request Mar 8, 2024
…7605)

* Add RBAC+TLS options to the ECK metrics endpoint.
* Add documentation for RBAC/TLS ECK metrics features.

---------
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@thbkrkr thbkrkr changed the title Add RBAC authentication option for the ECK metrics endpoint. Add support for securing the operator metrics endpoint with RBAC and TLS Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality v2.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants