feat(helm chart): Add metrics-server hardening options

.github/workflows/lint-test-chart.yaml

@@ -68,6 +68,38 @@ jobs:
         with:
           wait: 120s
 
+      - name: Install cert-manager dependency
+        if: fromJSON(steps.changes.outputs.changed)
+        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: fromJSON(steps.changes.outputs.changed)
+        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 <<EOF >> 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: fromJSON(steps.changes.outputs.changed)
-        run: ct install
+        run: ct install --namespace kube-system

charts/metrics-server/README.md

@@ -86,3 +86,99 @@ The following table lists the configurable parameters of the _Metrics Server_ ch
 | `topologySpreadConstraints`          | Pod Topology Spread Constraints.                                                                                                                                                                                                                                 | `[]`                                                                           |
 | `deploymentAnnotations`              | Annotations to add to the deployment.                                                                                                                                                                                                                            | `{}`                                                                           |
 | `schedulerName`                      | scheduler to set to the deployment.                                                                                                                                                                                                                              | `""`                                                                           |
+| `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                                                                                                                                                                                                                               | `90`                                            |
+| `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
+```

charts/metrics-server/ci/tls-certManager-values.yaml

@@ -0,0 +1,8 @@
+args:
+  - --kubelet-insecure-tls
+
+apiService:
+  insecureSkipTLSVerify: false
+
+tls:
+  type: cert-manager

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

charts/metrics-server/ci/tls-helm-values.yaml

@@ -0,0 +1,8 @@
+args:
+  - --kubelet-insecure-tls
+
+apiService:
+  insecureSkipTLSVerify: false
+
+tls:
+  type: helm

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 }}

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 }}

charts/metrics-server/templates/deployment.yaml

@@ -62,6 +62,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 }}
@@ -80,6 +84,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 }}
@@ -125,6 +134,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 }}

charts/metrics-server/values.yaml

@@ -180,3 +180,47 @@ topologySpreadConstraints: []
 deploymentAnnotations: {}
 
 schedulerName: ""
+
+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: 90
+
+  existingSecret:
+    # Name of the existing Secret to use for TLS
+    name: ""
+    # Use helm lookup function to provision `apiService.caBundle`
+    lookup: true