Refactor Admission Webhook templates and values (#1832)

* Refactor Admission Webhook templates and values Co-Authored-By: DanielZhangQD <36026334+DanielZhangQD@users.noreply.github.com>
pingcap · Mar 2, 2020 · 494e09a · 494e09a
1 parent aed1835
commit 494e09a
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 80 deletions.
diff --git a/charts/tidb-operator/templates/admission/admission-webhook-registration.yaml b/charts/tidb-operator/templates/admission/admission-webhook-registration.yaml
@@ -23,26 +23,26 @@ spec:
     namespace: {{ .Release.Namespace }}
   version: v1alpha1
 ---
-{{- if .Values.admissionWebhook.hooksEnabled.pods }}
+{{- if .Values.admissionWebhook.validation.pods }}
 apiVersion: admissionregistration.k8s.io/v1beta1
 kind: ValidatingWebhookConfiguration
 metadata:
-  name: validation-delete-tidb-admission-webhook-cfg
+  name: validation-tidb-pod-webhook-cfg
   labels:
     app.kubernetes.io/name: {{ template "chart.name" . }}
     app.kubernetes.io/managed-by: {{ .Release.Service }}
     app.kubernetes.io/instance: {{ .Release.Name }}
     app.kubernetes.io/component: admission-webhook
     helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
 webhooks:
-  - name: delete.podadmission.tidb.pingcap.com
+  - name: podadmission.tidb.pingcap.com
     {{- if semverCompare ">=1.15-0" .Capabilities.KubeVersion.GitVersion }}
     objectSelector:
       matchLabels:
         "app.kubernetes.io/managed-by": "tidb-operator"
         "app.kubernetes.io/name": "tidb-cluster"
     {{- end }}
-    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.deletePod | default "Fail" }}
+    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.validation | default "Fail" }}
     clientConfig:
       service:
         name: kubernetes
@@ -54,61 +54,32 @@ webhooks:
       caBundle: null
       {{- end }}
     rules:
-      - operations: ["DELETE"]
+      - operations: ["DELETE","CREATE"]
         apiGroups: [""]
         apiVersions: ["v1"]
         resources: ["pods"]
+{{- end }}
 ---
+{{- if .Values.admissionWebhook.validation.statefulSets }}
 apiVersion: admissionregistration.k8s.io/v1beta1
 kind: ValidatingWebhookConfiguration
 metadata:
-  name: validation-create-tidb-admission-webhook-cfg
+  name: validation-tidb-statefulset-webhook-cfg
   labels:
     app.kubernetes.io/name: {{ template "chart.name" . }}
     app.kubernetes.io/managed-by: {{ .Release.Service }}
     app.kubernetes.io/instance: {{ .Release.Name }}
     app.kubernetes.io/component: admission-webhook
     helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
 webhooks:
-  - name: create.podadmission.tidb.pingcap.com
+  - name: stsadmission.tidb.pingcap.com
     {{- if semverCompare ">=1.15-0" .Capabilities.KubeVersion.GitVersion }}
     objectSelector:
       matchLabels:
         "app.kubernetes.io/managed-by": "tidb-operator"
         "app.kubernetes.io/name": "tidb-cluster"
     {{- end }}
-    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.createPod | default "Ignore" }}
-    clientConfig:
-      service:
-        name: kubernetes
-        namespace: default
-        path: "/apis/admission.tidb.pingcap.com/v1alpha1/admissionreviews"
-      {{- if .Values.admissionWebhook.cabundle }}
-      caBundle: {{ .Values.admissionWebhook.cabundle | b64enc }}
-      {{- else }}
-      caBundle: null
-      {{- end }}
-    rules:
-      - operations: ["CREATE"]
-        apiGroups: [""]
-        apiVersions: ["v1"]
-        resources: ["pods"]
-{{- end }}
----
-{{- if .Values.admissionWebhook.hooksEnabled.statefulSets }}
-apiVersion: admissionregistration.k8s.io/v1beta1
-kind: ValidatingWebhookConfiguration
-metadata:
-  name: validation-update-tidb-admission-webhook-cfg
-  labels:
-    app.kubernetes.io/name: {{ template "chart.name" . }}
-    app.kubernetes.io/managed-by: {{ .Release.Service }}
-    app.kubernetes.io/instance: {{ .Release.Name }}
-    app.kubernetes.io/component: admission-webhook
-    helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
-webhooks:
-  - name: update.stsadmission.tidb.pingcap.com
-    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.updateStatefulSet | default "Ignore" }}
+    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.validation | default "Ignore" }}
     clientConfig:
       service:
         name: kubernetes
@@ -130,11 +101,11 @@ webhooks:
         resources: ["statefulsets"]
 {{- end }}
 ---
-{{- if .Values.admissionWebhook.hooksEnabled.validating }}
+{{- if .Values.admissionWebhook.validation.pingcapResources }}
 apiVersion: admissionregistration.k8s.io/v1beta1
 kind: ValidatingWebhookConfiguration
 metadata:
-  name: pingcap-resources-validating
+  name: pingcap-tidb-resources-validating
   labels:
     app.kubernetes.io/name: {{ template "chart.name" . }}
     app.kubernetes.io/managed-by: {{ .Release.Service }}
@@ -143,7 +114,7 @@ metadata:
     helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
 webhooks:
   - name: validating.admission.tidb.pingcap.com
-    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.validating | default "Ignore" }}
+    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.validation | default "Ignore" }}
     clientConfig:
       service:
         name: kubernetes
@@ -161,20 +132,20 @@ webhooks:
         resources: ["tidbclusters"]
 {{- end }}
 ---
-{{- if .Values.admissionWebhook.hooksEnabled.defaulting }}
+{{- if .Values.admissionWebhook.mutation.pingcapResources }}
 apiVersion: admissionregistration.k8s.io/v1beta1
 kind: MutatingWebhookConfiguration
 metadata:
-  name: pingcap-resources-defaulitng
+  name: pingcap-tidb-resources-defaulitng
   labels:
     app.kubernetes.io/name: {{ template "chart.name" . }}
     app.kubernetes.io/managed-by: {{ .Release.Service }}
     app.kubernetes.io/instance: {{ .Release.Name }}
     app.kubernetes.io/component: admission-webhook
     helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
 webhooks:
-  - name: defaulting.dmission.tidb.pingcap.com
-    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.defaulting | default "Ignore" }}
+  - name: defaulting.admission.tidb.pingcap.com
+    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.mutation | default "Ignore" }}
     clientConfig:
       service:
         name: kubernetes

diff --git a/charts/tidb-operator/templates/admission/pre-delete-job.yaml b/charts/tidb-operator/templates/admission/pre-delete-job.yaml
@@ -1,4 +1,4 @@
-{{- if and ( .Values.admissionWebhook.create ) ( .Values.admissionWebhook.hooksEnabled.pods ) }}
+{{- if and ( .Values.admissionWebhook.create ) ( .Values.admissionWebhook.validation.pods ) }}
 apiVersion: v1
 kind: ServiceAccount
 metadata:
@@ -96,5 +96,5 @@ spec:
             - "-c"
             - |
               set -e
-              kubectl delete validatingWebhookConfigurations.admissionregistration.k8s.io validation-delete-tidb-admission-webhook-cfg || true
+              kubectl delete validatingWebhookConfigurations.admissionregistration.k8s.io validation-tidb-pod-webhook-cfg || true
 {{- end }}
diff --git a/charts/tidb-operator/templates/controller-manager-deployment.yaml b/charts/tidb-operator/templates/controller-manager-deployment.yaml
@@ -49,7 +49,7 @@ spec:
           {{- if .Values.features }}
           - -features={{ join "," .Values.features }}
           {{- end }}
-          {{- if and ( .Values.admissionWebhook.create ) ( .Values.admissionWebhook.hooksEnabled.pods ) }}
+          {{- if and ( .Values.admissionWebhook.create ) ( .Values.admissionWebhook.validation.pods ) }}
           - -pod-webhook-enabled=true
           {{- end }}
         env:

diff --git a/charts/tidb-operator/values.yaml b/charts/tidb-operator/values.yaml
@@ -182,33 +182,30 @@ admissionWebhook:
   rbac:
     create: true
   ## jobImage is to indicate the image used in `pre-delete-job.yaml`
-  ## if admissionWebhook.create and admissionWebhook.hooksEnabled.pods are both enabled,
+  ## if admissionWebhook.create and admissionWebhook.validation.pods are both enabled,
   ## The pre-delete-job would delete the validationWebhookConfiguration using this image
   jobImage: "bitnami/kubectl:latest"
-  hooksEnabled:
+  ## validation webhook would check the given request for the specific resource and operation
+  validation:
     ## statefulsets hook would check requests for updating tidbcluster's statefulsets
     ## If enabled it, the statefulsets of tidbcluseter would update in partition by tidbcluster's annotation
     statefulSets: false
     ## pods hook would check requests for creating and deleting tidbcluster's pods
     ## if enabled it, the pods of tidbcluster would safely created or deleted by webhook instead of controller
     pods: true
     ## validating hook validates the correctness of the resources under pingcap.com group
-    validating: false
+    pingcapResources: false
+  ## mutation webhook would mutate the given request for the specific resource and operation
+  mutation:
     ## defaulting hook set default values for the the resources under pingcap.com group
-    defaulting: true
+    pingcapResources: true
   ## failurePolicy are applied to ValidatingWebhookConfiguration which affect tidb-admission-webhook
   ## refer to https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#failure-policy
   failurePolicy:
-    ## deletePod Webhook would check the deleting request of tidbcluster pod and the failurePolicy is recommended as Fail
-    deletePod: Fail
-    ## createPod Webhook would check the creating request of tidbcluster pod and the failurePolicy is recommended as Ignore
-    createPod: Ignore
-    ## updateStatefulSet Webhook would check the updating request of tidbcluster statefulset and the failurePolicy is recommended as Ignore
-    updateStatefulSet: Ignore
-    ## validation hook validates the correctness of the resources under pingcap.com group
-    validating: Ignore
-    ## defaulting hook set default values for the the resources under pingcap.com group
-    defaulting: Ignore
+    ## the validation webhook would check the request of the given resources, and the failurePolicy is recommended as Ignore
+    validation: Ignore
+    ## the mutation webhook would mutate the request of the given resources, and the failurePolicy is recommended as Ignore
+    mutation: Ignore
   ## tidb-admission-webhook deployed as kubernetes apiservice server
   ## refer to https://github.com/openshift/generic-admission-server
   apiservice:

diff --git a/ci/deploy_tidb_operator_staging.groovy b/ci/deploy_tidb_operator_staging.groovy
@@ -23,12 +23,12 @@ scheduler:
   replicas: 2
 admissionWebhook:
   create: true
-  hooksEnabled:
+  validation:
     statefulSets: true
     pods: true
-    # TODO: enable validating and defaulting after we ease the constrain
-    validating: false
-    defaulting: true
+    pingcapResources: false
+  mutation:
+    pingcapResources: true
 features:
   - AutoScaling=true
 '''

diff --git a/tests/actions.go b/tests/actions.go
@@ -408,19 +408,19 @@ func (tc *TidbClusterConfig) TidbClusterHelmSetString(m map[string]string) strin
 
 func (oi *OperatorConfig) OperatorHelmSetString(m map[string]string) string {
 	set := map[string]string{
-		"operatorImage":                              oi.Image,
-		"controllerManager.autoFailover":             "true",
-		"scheduler.kubeSchedulerImageName":           oi.SchedulerImage,
-		"controllerManager.logLevel":                 oi.LogLevel,
-		"scheduler.logLevel":                         "4",
-		"imagePullPolicy":                            string(oi.ImagePullPolicy),
-		"testMode":                                   strconv.FormatBool(oi.TestMode),
-		"admissionWebhook.cabundle":                  oi.Cabundle,
-		"admissionWebhook.create":                    strconv.FormatBool(oi.WebhookEnabled),
-		"admissionWebhook.hooksEnabled.pods":         strconv.FormatBool(oi.PodWebhookEnabled),
-		"admissionWebhook.hooksEnabled.statefulSets": strconv.FormatBool(oi.StsWebhookEnabled),
-		"admissionWebhook.hooksEnabled.defaulting":   strconv.FormatBool(oi.DefaultingEnabled),
-		"admissionWebhook.hooksEnabled.validating":   strconv.FormatBool(oi.ValidatingEnabled),
+		"operatorImage":                                oi.Image,
+		"controllerManager.autoFailover":               "true",
+		"scheduler.kubeSchedulerImageName":             oi.SchedulerImage,
+		"controllerManager.logLevel":                   oi.LogLevel,
+		"scheduler.logLevel":                           "4",
+		"imagePullPolicy":                              string(oi.ImagePullPolicy),
+		"testMode":                                     strconv.FormatBool(oi.TestMode),
+		"admissionWebhook.cabundle":                    oi.Cabundle,
+		"admissionWebhook.create":                      strconv.FormatBool(oi.WebhookEnabled),
+		"admissionWebhook.validation.pods":             strconv.FormatBool(oi.PodWebhookEnabled),
+		"admissionWebhook.validation.statefulSets":     strconv.FormatBool(oi.StsWebhookEnabled),
+		"admissionWebhook.mutation.pingcapResources":   strconv.FormatBool(oi.DefaultingEnabled),
+		"admissionWebhook.validation.pingcapResources": strconv.FormatBool(oi.ValidatingEnabled),
 	}
 	if oi.ControllerManagerReplicas != nil {
 		set["controllerManager.replicas"] = strconv.Itoa(*oi.ControllerManagerReplicas)