From 86b05bc157712820d8a4d1895d3a5ad752789679 Mon Sep 17 00:00:00 2001 From: Marco Kilchhofer Date: Fri, 14 Jul 2023 14:07:28 +0200 Subject: [PATCH] feat(helm chart): Add metrics-server hardening options --- .github/workflows/lint-test-chart.yaml | 34 ++++++- charts/metrics-server/CHANGELOG.md | 4 + charts/metrics-server/README.md | 96 +++++++++++++++++++ .../ci/tls-certManager-values.yaml | 8 ++ .../ci/tls-existingSecret-values.yaml | 12 +++ charts/metrics-server/ci/tls-helm-values.yaml | 8 ++ .../metrics-server/templates/apiservice.yaml | 58 +++++++++-- .../metrics-server/templates/certificate.yaml | 47 +++++++++ .../metrics-server/templates/deployment.yaml | 18 ++++ charts/metrics-server/values.yaml | 44 +++++++++ 10 files changed, 322 insertions(+), 7 deletions(-) create mode 100644 charts/metrics-server/ci/tls-certManager-values.yaml create mode 100644 charts/metrics-server/ci/tls-existingSecret-values.yaml create mode 100644 charts/metrics-server/ci/tls-helm-values.yaml create mode 100644 charts/metrics-server/templates/certificate.yaml diff --git a/.github/workflows/lint-test-chart.yaml b/.github/workflows/lint-test-chart.yaml index f573ccb3e..741353340 100644 --- a/.github/workflows/lint-test-chart.yaml +++ b/.github/workflows/lint-test-chart.yaml @@ -87,6 +87,38 @@ jobs: with: wait: 120s + - name: Install cert-manager dependency + if: steps.changes.outputs.changed == 'true' + run: | + helm repo add jetstack https://charts.jetstack.io + helm install cert-manager jetstack/cert-manager \ + --namespace cert-manager \ + --create-namespace \ + --wait \ + --set installCRDs=true \ + --set extraArgs='{--enable-certificate-owner-ref}' + + - name: Prepare existing secret test scenario + if: steps.changes.outputs.changed == 'true' + run: | + openssl req -x509 -newkey rsa:2048 -sha256 -days 365 \ + -nodes -keyout ${{ runner.temp }}/tls.key -out ${{ runner.temp }}/tls.crt \ + -subj "/CN=metrics-server" \ + -addext "subjectAltName=DNS:metrics-server,DNS:metrics-server.kube-system.svc" + + kubectl -n kube-system create secret generic metrics-server-existing \ + --from-file=${{ runner.temp }}/tls.key \ + --from-file=${{ runner.temp }}/tls.crt + + cat <> charts/metrics-server/ci/tls-existingSecret-values.yaml + apiService: + insecureSkipTLSVerify: false + caBundle: | + $(cat ${{ runner.temp }}/tls.crt | sed -e "s/^/ /g") + EOF + + rm ${{ runner.temp }}/tls.key ${{ runner.temp }}/tls.crt + - name: Run chart-testing install if: steps.changes.outputs.changed == 'true' - run: ct install + run: ct install --namespace kube-system diff --git a/charts/metrics-server/CHANGELOG.md b/charts/metrics-server/CHANGELOG.md index 481fb57eb..0d71b0da1 100644 --- a/charts/metrics-server/CHANGELOG.md +++ b/charts/metrics-server/CHANGELOG.md @@ -14,6 +14,10 @@ ## [UNRELEASED] +### Added + +- Add options on howto secure the connection between metrics-server and the kube-apiserver. ([#1288](https://github.com/kubernetes-sigs/metrics-server/pull/1288)) _@mkilchhofer_ + ## [3.12.2] - TBC ### Added diff --git a/charts/metrics-server/README.md b/charts/metrics-server/README.md index 4b6ce652b..e4e846285 100644 --- a/charts/metrics-server/README.md +++ b/charts/metrics-server/README.md @@ -90,3 +90,99 @@ The following table lists the configurable parameters of the _Metrics Server_ ch | `schedulerName` | scheduler to set to the deployment. | `""` | | `dnsConfig` | Set the dns configuration options for the deployment. | `{}` | | `tmpVolume` | Volume to be mounted in Pods for temporary files. | `{"emptyDir":{}}` | +| `tls.type` | TLS option to use. Either use `metrics-server` for self-signed certificates, `helm`, `cert-manager` or `existingSecret`. | `"metrics-server"` | +| `tls.clusterDomain` | Kubernetes cluster domain. Used to configure Subject Alt Names for the certificate when using `tls.type` `helm` or `cert-manager`. | `"cluster.local"` | +| `tls.certManager.addInjectorAnnotations` | Automatically add the cert-manager.io/inject-ca-from annotation to the APIService resource. | `true` | +| `tls.certManager.existingIssuer.enabled` | Use an existing cert-manager issuer | `false` | +| `tls.certManager.existingIssuer.kind` | Kind of the existing cert-manager issuer | `"Issuer"` | +| `tls.certManager.existingIssuer.name` | Name of the existing cert-manager issuer | `"my-issuer"` | +| `tls.certManager.duration` | Set the requested duration (i.e. lifetime) of the Certificate. | `""` | +| `tls.certManager.renewBefore` | How long before the currently issued certificate’s expiry cert-manager should renew the certificate. | `""` | +| `tls.certManager.annotations` | Add extra annotations to the Certificate resource | `{}` | +| `tls.certManager.labels` | Add extra labels to the Certificate resource | `{}` | +| `tls.helm.certDurationDays` | Cert validity duration in days | `365` | +| `tls.helm.lookup` | Use helm lookup function to reuse Secret created in previous helm install | `true` | +| `tls.existingSecret.lookup` | Use helm lookup function to provision `apiService.caBundle` | `true` | +| `tls.existingSecret.name` | Name of the existing Secret to use for TLS | `""` | + +## Hardening metrics-server + +By default, metrics-server is using a self-signed certificate which is generated during startup. The `APIservice` resource is registered with `.spec.insecureSkipTLSVerify` set to `true` as you can see here: + +```yaml +apiVersion: apiregistration.k8s.io/v1 +kind: APIService +metadata: + name: v1beta1.metrics.k8s.io +spec: + #.. + insecureSkipTLSVerify: true # <-- see here + service: + name: metrics-server + #.. +``` + +To harden metrics-server, you have these options described in the following section. + +### Option 1: Let helm generate a self-signed certificate + +This option is probably the easiest solution for you. We delegate the process to generate a self-signed certificate to helm. +As helm generates them during deploy time, helm can also inject the `apiService.caBundle` for you. + +**The only disadvantage of using this method is that it is not GitOps friendly** (e.g. Argo CD). If you are using one of these +GitOps tools with drift detection, it will always detect changes. However if you are deploying the helm chart via Terraform +for example (or maybe even Flux), this method is perfectly fine. + +To use this method, please setup your values file like this: + +```yaml +apiService: + insecureSkipTLSVerify: false +tls: + type: helm +``` + +### Option 2: Use cert-manager + +> **Requirement:** cert-manager needs to be installed before you install metrics-server + +To use this method, please setup your values file like this: + +```yaml +apiService: + insecureSkipTLSVerify: false +tls: + type: cert-manager +``` + +There are other optional parameters, if you want to customize the behavior of the certificate even more. + +### Option 3: Use existing Secret + +This option allows you to reuse an existing Secret. This Secrets can have an arbitrary origin, e.g. + +- Created via kubectl / Terraform / etc. +- Synced from a secret management solution like AWS Secrets Manager, HashiCorp Vault, etc. + +When using this type of TLS option, the keys `tls.key` and the `tls.crt` key must be provided in the data field of the +existing Secret. + +You need to pass the certificate of the issuing CA (or the certificate itself) via `apiService.caBundle` to ensure +proper configuration of the `APIservice` resource. Otherwise you cannot set `apiService.insecureSkipTLSVerify` to +`false`. + +To use this method, please setup your values file like this: + +```yaml +apiService: + insecureSkipTLSVerify: false + caBundle: | + -----BEGIN CERTIFICATE----- + ... + -----END CERTIFICATE----- + +tls: + type: existingSecret + existingSecret: + name: metrics-server-existing +``` diff --git a/charts/metrics-server/ci/tls-certManager-values.yaml b/charts/metrics-server/ci/tls-certManager-values.yaml new file mode 100644 index 000000000..3dcfd0ea6 --- /dev/null +++ b/charts/metrics-server/ci/tls-certManager-values.yaml @@ -0,0 +1,8 @@ +args: + - --kubelet-insecure-tls + +apiService: + insecureSkipTLSVerify: false + +tls: + type: cert-manager diff --git a/charts/metrics-server/ci/tls-existingSecret-values.yaml b/charts/metrics-server/ci/tls-existingSecret-values.yaml new file mode 100644 index 000000000..8daa8483c --- /dev/null +++ b/charts/metrics-server/ci/tls-existingSecret-values.yaml @@ -0,0 +1,12 @@ +args: + - --kubelet-insecure-tls + +## Set via GH action (step "Prepare existing secret test scenario") +# apiService: +# insecureSkipTLSVerify: false +# caBundle: | + +tls: + type: existingSecret + existingSecret: + name: metrics-server-existing diff --git a/charts/metrics-server/ci/tls-helm-values.yaml b/charts/metrics-server/ci/tls-helm-values.yaml new file mode 100644 index 000000000..dc8342865 --- /dev/null +++ b/charts/metrics-server/ci/tls-helm-values.yaml @@ -0,0 +1,8 @@ +args: + - --kubelet-insecure-tls + +apiService: + insecureSkipTLSVerify: false + +tls: + type: helm diff --git a/charts/metrics-server/templates/apiservice.yaml b/charts/metrics-server/templates/apiservice.yaml index f58931d9e..d1f437042 100644 --- a/charts/metrics-server/templates/apiservice.yaml +++ b/charts/metrics-server/templates/apiservice.yaml @@ -1,17 +1,63 @@ -{{- if .Values.apiService.create -}} +{{- $altNames := list }} +{{- $certs := dict }} +{{- $previous := dict }} + +{{- if eq .Values.tls.type "helm" }} +{{- $previous = lookup "v1" "Secret" .Release.Namespace (include "metrics-server.fullname" .) }} +{{- $commonName := include "metrics-server.fullname" . }} +{{- $ns := .Release.Namespace }} +{{- $altNames = append $altNames (printf "%s.%s" $commonName $ns) }} +{{- $altNames = append $altNames (printf "%s.%s.svc" $commonName $ns) }} +{{- $altNames = append $altNames (printf "%s.%s.svc.%s" $commonName $ns .Values.tls.clusterDomain) }} +{{- $certs = genSelfSignedCert $commonName nil $altNames (int .Values.tls.helm.certDurationDays) }} +apiVersion: v1 +kind: Secret +metadata: + name: {{ include "metrics-server.fullname" . }} + labels: + {{- include "metrics-server.labels" . | nindent 4 }} +type: Opaque +data: + {{- if and $previous .Values.tls.helm.lookup }} + tls.crt: {{ index $previous.data "tls.crt" }} + tls.key: {{ index $previous.data "tls.key" }} + {{- else }} + tls.crt: {{ $certs.Cert| b64enc | quote }} + tls.key: {{ $certs.Key | b64enc | quote }} + {{- end }} +{{- end }} +--- +{{- $existing := dict }} +{{- if .Values.apiService.create }} +{{- if and (eq .Values.tls.type "existingSecret") .Values.tls.existingSecret.lookup }} +{{- $existing := lookup "v1" "Secret" .Release.Namespace .Values.tls.existingSecret.name }} +{{- end }} apiVersion: apiregistration.k8s.io/v1 kind: APIService metadata: name: v1beta1.metrics.k8s.io labels: {{- include "metrics-server.labels" . | nindent 4 }} - {{- with .Values.apiService.annotations }} + {{- if or .Values.apiService.annotations .Values.tls.certManager.addInjectorAnnotations }} annotations: - {{- toYaml . | nindent 4 }} + {{- if and (eq .Values.tls.type "cert-manager") .Values.tls.certManager.addInjectorAnnotations }} + cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ include "metrics-server.fullname" . }} + {{- end }} + {{- with .Values.apiService.annotations }} + {{- toYaml . | nindent 4 }} + {{- end }} {{- end }} spec: - {{- with .Values.apiService.caBundle }} - caBundle: {{ b64enc . }} + {{- if eq .Values.tls.type "helm" }} + {{- if and $previous .Values.tls.helm.lookup }} + caBundle: {{ index $previous.data "tls.crt" }} + {{- else }} + caBundle: {{ $certs.Cert | b64enc }} + {{- end }} + {{- else if $existing }} + caBundle: {{ index $existing.data "tls.crt" }} + {{- else if and .Values.apiService.caBundle (ne .Values.tls.type "cert-manager") }} + caBundle: {{ .Values.apiService.caBundle | b64enc }} {{- end }} group: metrics.k8s.io groupPriorityMinimum: 100 @@ -22,4 +68,4 @@ spec: port: {{ .Values.service.port }} version: v1beta1 versionPriority: 100 -{{- end -}} +{{- end }} diff --git a/charts/metrics-server/templates/certificate.yaml b/charts/metrics-server/templates/certificate.yaml new file mode 100644 index 000000000..c4de300f8 --- /dev/null +++ b/charts/metrics-server/templates/certificate.yaml @@ -0,0 +1,47 @@ +{{- if eq .Values.tls.type "cert-manager" }} +{{- if not .Values.tls.certManager.existingIssuer.enabled }} +apiVersion: cert-manager.io/v1 +kind: Issuer +metadata: + annotations: + {{- toYaml .Values.additionalAnnotations | nindent 4 }} + name: {{ include "metrics-server.fullname" . }}-issuer + namespace: {{ .Release.Namespace }} +spec: + selfSigned: {} +{{- end }} +--- +apiVersion: cert-manager.io/v1 +kind: Certificate +metadata: + name: {{ include "metrics-server.fullname" . }} + namespace: {{ .Release.Namespace }} +spec: + commonName: {{ include "metrics-server.fullname" . }} + dnsNames: + - {{ include "metrics-server.fullname" . }}.{{ .Release.Namespace }} + - {{ include "metrics-server.fullname" . }}.{{ .Release.Namespace }}.svc + - {{ include "metrics-server.fullname" . }}.{{ .Release.Namespace }}.svc.{{ .Values.tls.clusterDomain }} + secretName: {{ include "metrics-server.fullname" . }} + usages: + - server auth + - client auth + privateKey: + algorithm: RSA + size: 2048 + {{- with .Values.tls.certManager.duration }} + duration: {{ . }} + {{- end }} + {{- with .Values.tls.certManager.renewBefore }} + renewBefore: {{ . }} + {{- end }} + issuerRef: + {{- if .Values.tls.certManager.existingIssuer.enabled }} + name: {{ .Values.tls.certManager.existingIssuer.name }} + kind: {{ .Values.tls.certManager.existingIssuer.kind }} + {{- else }} + name: {{ include "metrics-server.fullname" . }}-issuer + kind: Issuer + {{- end }} + group: cert-manager.io +{{- end }} diff --git a/charts/metrics-server/templates/deployment.yaml b/charts/metrics-server/templates/deployment.yaml index 37e7f953b..d7535b876 100644 --- a/charts/metrics-server/templates/deployment.yaml +++ b/charts/metrics-server/templates/deployment.yaml @@ -71,6 +71,10 @@ spec: {{- if .Values.metrics.enabled }} - --authorization-always-allow-paths=/metrics {{- end }} + {{- if ne .Values.tls.type "metrics-server" }} + - --tls-cert-file=/tmp/tls-certs/tls.crt + - --tls-private-key-file=/tmp/tls-certs/tls.key + {{- end }} {{- range .Values.args }} - {{ . }} {{- end }} @@ -89,6 +93,11 @@ spec: volumeMounts: - name: tmp mountPath: /tmp + {{- if ne .Values.tls.type "metrics-server" }} + - mountPath: /tmp/tls-certs + name: certs + readOnly: true + {{- end }} {{- with .Values.extraVolumeMounts }} {{- toYaml . | nindent 12 }} {{- end }} @@ -138,6 +147,15 @@ spec: configMap: name: {{ include "metrics-server.addonResizer.configMap" . }} {{- end }} + {{- if ne .Values.tls.type "metrics-server" }} + - name: certs + secret: + {{- if and (eq .Values.tls.type "existingSecret") .Values.tls.existingSecret.name }} + secretName: {{ .Values.tls.existingSecret.name }} + {{- else }} + secretName: {{ include "metrics-server.fullname" . }} + {{- end }} + {{- end }} {{- with .Values.extraVolumes }} {{- toYaml . | nindent 8 }} {{- end }} diff --git a/charts/metrics-server/values.yaml b/charts/metrics-server/values.yaml index be843db41..79eaf9397 100644 --- a/charts/metrics-server/values.yaml +++ b/charts/metrics-server/values.yaml @@ -198,3 +198,47 @@ schedulerName: "" tmpVolume: emptyDir: {} + +tls: + # Set the TLS method to use. Supported values: + # - `metrics-server` : Metrics-server will generate a self-signed certificate + # - `helm` : Helm will generate a self-signed certificate + # - `cert-manager` : Use cert-manager.io to create and maintain the certificate + # - `existingSecret` : Reuse an existing secret. No new secret will be created + type: "metrics-server" + # Kubernetes cluster domain. Used to configure Subject Alt Names for the certificate + clusterDomain: cluster.local + + certManager: + # Automatically add the cert-manager.io/inject-ca-from annotation to the APIService resource. + # See https://cert-manager.io/docs/concepts/ca-injector + addInjectorAnnotations: true + existingIssuer: + # Use an existing cert-manager issuer + enabled: false + # Kind of the existing cert-manager issuer + kind: "Issuer" + # Name of the existing cert-manager issuer + name: "my-issuer" + # Set the requested duration (i.e. lifetime) of the Certificate. + # See https://cert-manager.io/docs/reference/api-docs/#cert-manager.io/v1.CertificateSpec + duration: "" + # How long before the currently issued certificate’s expiry cert-manager should renew the certificate. + # See https://cert-manager.io/docs/reference/api-docs/#cert-manager.io/v1.CertificateSpec + renewBefore: "" + # Add extra annotations to the Certificate resource + annotations: {} + # Add extra labels to the Certificate resource + labels: {} + + helm: + # Use helm lookup function to reuse Secret created in previous helm install + lookup: true + # Cert validity duration in days + certDurationDays: 365 + + existingSecret: + # Name of the existing Secret to use for TLS + name: "" + # Use helm lookup function to provision `apiService.caBundle` + lookup: true