From 17008336013e50cb23aee168173969b00ab68432 Mon Sep 17 00:00:00 2001
From: James Munnelly <james@munnelly.eu>
Date: Wed, 25 Mar 2020 14:11:33 +0000
Subject: [PATCH] Default webhook to use the dynamic secret source

Signed-off-by: James Munnelly <james@munnelly.eu>
---
 deploy/charts/cert-manager/README.template.md |  2 +-
 .../crds/certificaterequests.yaml             |  2 +-
 .../cert-manager/crds/certificates.yaml       |  2 +-
 .../charts/cert-manager/crds/challenges.yaml  |  2 +-
 .../cert-manager/crds/clusterissuers.yaml     |  2 +-
 deploy/charts/cert-manager/crds/issuers.yaml  |  2 +-
 deploy/charts/cert-manager/crds/orders.yaml   |  2 +-
 .../cert-manager/templates/_helpers.tpl       | 20 --------
 .../cert-manager/templates/deployment.yaml    |  4 --
 .../templates/webhook-deployment.yaml         | 16 +++----
 .../templates/webhook-mutating-webhook.yaml   |  2 +-
 .../cert-manager/templates/webhook-rbac.yaml  | 47 +++++++++++++++++++
 .../templates/webhook-validating-webhook.yaml |  2 +-
 deploy/charts/cert-manager/values.yaml        | 17 +++----
 deploy/manifests/00-crds.yaml                 | 12 ++---
 15 files changed, 78 insertions(+), 56 deletions(-)
 create mode 100644 deploy/charts/cert-manager/templates/webhook-rbac.yaml

diff --git a/deploy/charts/cert-manager/README.template.md b/deploy/charts/cert-manager/README.template.md
index 005bd4cdd..ac485838f 100644
--- a/deploy/charts/cert-manager/README.template.md
+++ b/deploy/charts/cert-manager/README.template.md
@@ -130,6 +130,7 @@ The following table lists the configurable parameters of the cert-manager chart
 | `no_proxy` | Value of the `NO_PROXY` environment variable in the cert-manager pod | |
 | `webhook.replicaCount` | Number of cert-manager webhook replicas | `1` |
 | `webhook.serviceName` | The name of the Service resource deployed for the webhook pod | `cert-manager-webhook` |
+| `webhook.rootCASecretName` | The name of the Secret resource that will be used to store the webhook's root CA certificate | `cert-manager-webhook-ca` |
 | `webhook.podAnnotations` | Annotations to add to the webhook pods | `{}` |
 | `webhook.deploymentAnnotations` | Annotations to add to the webhook deployment | `{}` |
 | `webhook.extraArgs` | Optional flags for cert-manager webhook component | `[]` |
@@ -140,7 +141,6 @@ The following table lists the configurable parameters of the cert-manager chart
 | `webhook.image.repository` | Webhook image repository | `quay.io/jetstack/cert-manager-webhook` |
 | `webhook.image.tag` | Webhook image tag | `{{RELEASE_VERSION}}` |
 | `webhook.image.pullPolicy` | Webhook image pull policy | `IfNotPresent` |
-| `webhook.injectAPIServerCA` | if true, the apiserver's CABundle will be automatically injected into the ValidatingWebhookConfiguration resource | `true` |
 | `webhook.securePort` | The port that the webhook should listen on for requests. | `10250` |
 | `webhook.securityContext` | Security context for webhook pod assignment | `{}` |
 | `cainjector.enabled` | Toggles whether the cainjector component should be installed (required for the webhook component to work) | `true` |
diff --git a/deploy/charts/cert-manager/crds/certificaterequests.yaml b/deploy/charts/cert-manager/crds/certificaterequests.yaml
index 930639005..cc087d3d2 100644
--- a/deploy/charts/cert-manager/crds/certificaterequests.yaml
+++ b/deploy/charts/cert-manager/crds/certificaterequests.yaml
@@ -3,7 +3,7 @@ kind: CustomResourceDefinition
 metadata:
   name: certificaterequests.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.conditions[?(@.type=="Ready")].status
diff --git a/deploy/charts/cert-manager/crds/certificates.yaml b/deploy/charts/cert-manager/crds/certificates.yaml
index b1a5390bc..72024266f 100644
--- a/deploy/charts/cert-manager/crds/certificates.yaml
+++ b/deploy/charts/cert-manager/crds/certificates.yaml
@@ -3,7 +3,7 @@ kind: CustomResourceDefinition
 metadata:
   name: certificates.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.conditions[?(@.type=="Ready")].status
diff --git a/deploy/charts/cert-manager/crds/challenges.yaml b/deploy/charts/cert-manager/crds/challenges.yaml
index ac5a69294..535e057d8 100644
--- a/deploy/charts/cert-manager/crds/challenges.yaml
+++ b/deploy/charts/cert-manager/crds/challenges.yaml
@@ -3,7 +3,7 @@ kind: CustomResourceDefinition
 metadata:
   name: challenges.acme.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.state
diff --git a/deploy/charts/cert-manager/crds/clusterissuers.yaml b/deploy/charts/cert-manager/crds/clusterissuers.yaml
index 7b94886bf..cce71e3e9 100644
--- a/deploy/charts/cert-manager/crds/clusterissuers.yaml
+++ b/deploy/charts/cert-manager/crds/clusterissuers.yaml
@@ -3,7 +3,7 @@ kind: CustomResourceDefinition
 metadata:
   name: clusterissuers.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.conditions[?(@.type=="Ready")].status
diff --git a/deploy/charts/cert-manager/crds/issuers.yaml b/deploy/charts/cert-manager/crds/issuers.yaml
index c35ad4861..b8f5ea08f 100644
--- a/deploy/charts/cert-manager/crds/issuers.yaml
+++ b/deploy/charts/cert-manager/crds/issuers.yaml
@@ -3,7 +3,7 @@ kind: CustomResourceDefinition
 metadata:
   name: issuers.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.conditions[?(@.type=="Ready")].status
diff --git a/deploy/charts/cert-manager/crds/orders.yaml b/deploy/charts/cert-manager/crds/orders.yaml
index a9a05b126..9e8f8edbe 100644
--- a/deploy/charts/cert-manager/crds/orders.yaml
+++ b/deploy/charts/cert-manager/crds/orders.yaml
@@ -3,7 +3,7 @@ kind: CustomResourceDefinition
 metadata:
   name: orders.acme.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.state
diff --git a/deploy/charts/cert-manager/templates/_helpers.tpl b/deploy/charts/cert-manager/templates/_helpers.tpl
index 4522116c0..9abfd0c5a 100644
--- a/deploy/charts/cert-manager/templates/_helpers.tpl
+++ b/deploy/charts/cert-manager/templates/_helpers.tpl
@@ -71,26 +71,6 @@ Create chart name and version as used by the chart label.
 {{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" -}}
 {{- end -}}
 
-{{- define "webhook.selfSignedIssuer" -}}
-{{- $trimmedName := printf "%s" (include "cert-manager.fullname" .) | trunc 46 | trimSuffix "-" -}}
-{{ printf "%s-webhook-selfsign" $trimmedName }}
-{{- end -}}
-
-{{- define "webhook.rootCAIssuer" -}}
-{{- $trimmedName := printf "%s" (include "cert-manager.fullname" .) | trunc 52 | trimSuffix "-" -}}
-{{ printf "%s-webhook-ca" $trimmedName }}
-{{- end -}}
-
-{{- define "webhook.rootCACertificate" -}}
-{{- $trimmedName := printf "%s" (include "cert-manager.fullname" .) | trunc 52 | trimSuffix "-" -}}
-{{ printf "%s-webhook-ca" $trimmedName }}
-{{- end -}}
-
-{{- define "webhook.servingCertificate" -}}
-{{- $trimmedName := printf "%s" (include "cert-manager.fullname" .) | trunc 51 | trimSuffix "-" -}}
-{{ printf "%s-webhook-tls" $trimmedName }}
-{{- end -}}
-
 {{/*
 cainjector templates
 */}}
diff --git a/deploy/charts/cert-manager/templates/deployment.yaml b/deploy/charts/cert-manager/templates/deployment.yaml
index c25d132a3..696d229b2 100644
--- a/deploy/charts/cert-manager/templates/deployment.yaml
+++ b/deploy/charts/cert-manager/templates/deployment.yaml
@@ -99,10 +99,6 @@ spec:
           - --default-issuer-group={{ .defaultIssuerGroup }}
           {{- end }}
           {{- end }}
-          - --webhook-namespace=$(POD_NAMESPACE)
-          - --webhook-ca-secret={{ include "webhook.rootCACertificate" . }}
-          - --webhook-serving-secret={{ include "webhook.servingCertificate" . }}
-          - --webhook-dns-names={{ .Values.webhook.serviceName }},{{ .Values.webhook.serviceName }}.{{ .Release.Namespace }},{{ .Values.webhook.serviceName }}.{{ .Release.Namespace }}.svc
           ports:
           - containerPort: 9402
             protocol: TCP
diff --git a/deploy/charts/cert-manager/templates/webhook-deployment.yaml b/deploy/charts/cert-manager/templates/webhook-deployment.yaml
index 3a4688e70..4f4e01f71 100644
--- a/deploy/charts/cert-manager/templates/webhook-deployment.yaml
+++ b/deploy/charts/cert-manager/templates/webhook-deployment.yaml
@@ -56,8 +56,9 @@ spec:
           - --v={{ .Values.global.logLevel }}
           {{- end }}
           - --secure-port={{ .Values.webhook.securePort }}
-          - --tls-cert-file=/certs/tls.crt
-          - --tls-private-key-file=/certs/tls.key
+          - --dynamic-serving-ca-secret-namespace={{ .Release.Namespace }}
+          - --dynamic-serving-ca-secret-name={{ .Values.webhook.rootCASecretName }}
+          - --dynamic-serving-dns-names={{ .Values.webhook.serviceName }},{{ .Values.webhook.serviceName }}.{{ .Release.Namespace }},{{ .Values.webhook.serviceName }}.{{ .Release.Namespace }}.svc
         {{- if .Values.webhook.extraArgs }}
 {{ toYaml .Values.webhook.extraArgs | indent 10 }}
         {{- end }}
@@ -66,11 +67,15 @@ spec:
               path: /livez
               port: 6080
               scheme: HTTP
+            initialDelaySeconds: 60
+            periodSeconds: 10
           readinessProbe:
             httpGet:
               path: /healthz
               port: 6080
               scheme: HTTP
+            initialDelaySeconds: 5
+            periodSeconds: 5
           env:
           - name: POD_NAMESPACE
             valueFrom:
@@ -78,13 +83,6 @@ spec:
                 fieldPath: metadata.namespace
           resources:
 {{ toYaml .Values.webhook.resources | indent 12 }}
-          volumeMounts:
-          - name: certs
-            mountPath: /certs
-      volumes:
-      - name: certs
-        secret:
-          secretName: {{ include "webhook.servingCertificate" . }}
     {{- with .Values.webhook.nodeSelector }}
       nodeSelector:
 {{ toYaml . | indent 8 }}
diff --git a/deploy/charts/cert-manager/templates/webhook-mutating-webhook.yaml b/deploy/charts/cert-manager/templates/webhook-mutating-webhook.yaml
index 3435f93d0..ad165d5b8 100644
--- a/deploy/charts/cert-manager/templates/webhook-mutating-webhook.yaml
+++ b/deploy/charts/cert-manager/templates/webhook-mutating-webhook.yaml
@@ -10,7 +10,7 @@ metadata:
     app.kubernetes.io/component: "webhook"
     helm.sh/chart: {{ include "webhook.chart" . }}
   annotations:
-    cert-manager.io/inject-ca-from-secret: "{{ .Release.Namespace }}/{{ include "webhook.servingCertificate" . }}"
+    cert-manager.io/inject-ca-from-secret: "{{ .Release.Namespace }}/{{ .Values.webhook.rootCASecretName }}"
 webhooks:
   - name: webhook.cert-manager.io
     rules:
diff --git a/deploy/charts/cert-manager/templates/webhook-rbac.yaml b/deploy/charts/cert-manager/templates/webhook-rbac.yaml
new file mode 100644
index 000000000..4ba148975
--- /dev/null
+++ b/deploy/charts/cert-manager/templates/webhook-rbac.yaml
@@ -0,0 +1,47 @@
+{{- if .Values.global.rbac.create -}}
+
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: Role
+metadata:
+  name: {{ template "webhook.fullname" . }}:dynamic-serving
+  labels:
+    app: {{ include "webhook.name" . }}
+    app.kubernetes.io/name: {{ include "webhook.name" . }}
+    app.kubernetes.io/instance: {{ .Release.Name }}
+    app.kubernetes.io/managed-by: {{ .Release.Service }}
+    app.kubernetes.io/component: "webhook"
+    helm.sh/chart: {{ include "webhook.chart" . }}
+rules:
+- apiGroups: [""]
+  resources: ["secrets"]
+  resourceNames:
+  - '{{ .Values.webhook.rootCASecretName }}'
+  verbs: ["get", "list", "watch", "update"]
+# It's not possible to grant CREATE permission on a single resourceName.
+- apiGroups: [""]
+  resources: ["secrets"]
+  verbs: ["create"]
+---
+
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: RoleBinding
+metadata:
+  name: {{ template "webhook.fullname" . }}:dynamic-serving
+  labels:
+    app: {{ include "webhook.name" . }}
+    app.kubernetes.io/name: {{ include "webhook.name" . }}
+    app.kubernetes.io/instance: {{ .Release.Name }}
+    app.kubernetes.io/managed-by: {{ .Release.Service }}
+    app.kubernetes.io/component: "webhook"
+    helm.sh/chart: {{ include "webhook.chart" . }}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: {{ template "webhook.fullname" . }}:dynamic-serving
+subjects:
+- apiGroup: ""
+  kind: ServiceAccount
+  name: {{ include "webhook.fullname" . }}
+  namespace: {{ .Release.Namespace }}
+
+{{- end -}}
diff --git a/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml b/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml
index fa42a73d8..2e6bf83e9 100644
--- a/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml
+++ b/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml
@@ -10,7 +10,7 @@ metadata:
     app.kubernetes.io/component: "webhook"
     helm.sh/chart: {{ include "webhook.chart" . }}
   annotations:
-    cert-manager.io/inject-ca-from-secret: "{{ .Release.Namespace }}/{{ include "webhook.servingCertificate" . }}"
+    cert-manager.io/inject-ca-from-secret: "{{ .Release.Namespace }}/{{ .Values.webhook.rootCASecretName }}"
 webhooks:
   - name: webhook.cert-manager.io
     namespaceSelector:
diff --git a/deploy/charts/cert-manager/values.yaml b/deploy/charts/cert-manager/values.yaml
index 2297ade3a..87951d3e6 100644
--- a/deploy/charts/cert-manager/values.yaml
+++ b/deploy/charts/cert-manager/values.yaml
@@ -159,8 +159,16 @@ webhook:
   # conversion webhook configuration.
   # If you change this value, you must also manually update the
   # `spec.conversion.webhookClientConfig.service.name` field on all
-  # cert-manager CRD resources as well.
+  # cert-manager CRD resources.
   serviceName: cert-manager-webhook
+  # The name of the Secret resource that will be created and used to store the
+  # root CA used to sign serving certificates for the webhook.
+  # This will be automatically injected into the Mutating, Validating and CRD
+  # resources by the cainjector component.
+  # If you change this value, you must also manually update the
+  # `cert-manager.io/inject-ca-from-secret` value on all cert-manager CRD
+  # resources.
+  rootCASecretName: cert-manager-webhook-ca
 
   strategy: {}
     # type: RollingUpdate
@@ -195,13 +203,6 @@ webhook:
     # tag: canary
     pullPolicy: IfNotPresent
 
-  # If true, the apiserver's cabundle will be automatically injected into the
-  # webhook's ValidatingWebhookConfiguration resource by the CA injector.
-  # in future this will default to false, as the apiserver can use the loopback
-  # configuration caBundle to talk to itself in kubernetes 1.11+
-  # see https://github.com/kubernetes/kubernetes/pull/62649
-  injectAPIServerCA: true
-
   # The port that the webhook should listen on for requests.
   # In GKE private clusters, by default kubernetes apiservers are allowed to
   # talk to the cluster nodes only on 443 and 10250. so configuring
diff --git a/deploy/manifests/00-crds.yaml b/deploy/manifests/00-crds.yaml
index 9b23ca1e9..7d61f7f6d 100644
--- a/deploy/manifests/00-crds.yaml
+++ b/deploy/manifests/00-crds.yaml
@@ -3,7 +3,7 @@ kind: CustomResourceDefinition
 metadata:
   name: certificaterequests.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.conditions[?(@.type=="Ready")].status
@@ -209,7 +209,7 @@ kind: CustomResourceDefinition
 metadata:
   name: certificates.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.conditions[?(@.type=="Ready")].status
@@ -758,7 +758,7 @@ kind: CustomResourceDefinition
 metadata:
   name: challenges.acme.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.state
@@ -2193,7 +2193,7 @@ kind: CustomResourceDefinition
 metadata:
   name: clusterissuers.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.conditions[?(@.type=="Ready")].status
@@ -3992,7 +3992,7 @@ kind: CustomResourceDefinition
 metadata:
   name: issuers.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.conditions[?(@.type=="Ready")].status
@@ -5791,7 +5791,7 @@ kind: CustomResourceDefinition
 metadata:
   name: orders.acme.cert-manager.io
   annotations:
-    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-tls
+    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
 spec:
   additionalPrinterColumns:
   - JSONPath: .status.state