feat: detect when ControlPlane's admission webhook is disabled and en…

…sure relevant resources do not exist (#326) * feat: detect when ControlPlane's admission webhook is disabled and ensure relevant resources do not exist * chore: use pointer to indicate AdmissionWebhookCertSecretName optionality * chore: use an argument for webhook being enabled * Revert "chore: use an argument for webhook being enabled" This reverts commit b4bec06.
Kong · Jun 12, 2024 · 96712ed · 96712ed
1 parent f883540
commit 96712ed
Show file tree

Hide file tree

Showing 23 changed files with 683 additions and 97 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,8 @@
 
 ## Unreleased
 
+> Release date: TBA
+
 ### Added
 
 - Add `ExternalTrafficPolicy` to `DataPlane`'s `ServiceOptions`
@@ -40,6 +42,10 @@
   [#246](https://github.com/Kong/gateway-operator/pull/246)
 - `Gateway`s' listeners now have their `attachedRoutes` count filled in in status.
   [#251](https://github.com/Kong/gateway-operator/pull/251)
+- Detect when `ControlPlane` has its admission webhook disabled via
+  `CONTROLLER_ADMISSION_WEBHOOK_LISTEN` environment variable and ensure that
+  relevant webhook resources are not created/deleted.
+  [#326](https://github.com/Kong/gateway-operator/pull/326)
 
 ### Fixes
 

diff --git a/config/samples/gateway-with-disabled-controlplane-admission-webhook.yaml b/config/samples/gateway-with-disabled-controlplane-admission-webhook.yaml
@@ -0,0 +1,111 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: echo
+spec:
+  ports:
+    - protocol: TCP
+      name: http
+      port: 80
+      targetPort: http
+  selector:
+    app: echo
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  labels:
+    app: echo
+  name: echo
+spec:
+  selector:
+    matchLabels:
+      app: echo
+  template:
+    metadata:
+      labels:
+        app: echo
+    spec:
+      containers:
+        - name: echo
+          image: registry.k8s.io/e2e-test-images/agnhost:2.40
+          command:
+            - /agnhost
+            - netexec
+            - --http-port=8080
+          ports:
+            - containerPort: 8080
+              name: http
+          env:
+            - name: NODE_NAME
+              valueFrom:
+                fieldRef:
+                  fieldPath: spec.nodeName
+            - name: POD_NAME
+              valueFrom:
+                fieldRef:
+                  fieldPath: metadata.name
+            - name: NAMESPACE
+              valueFrom:
+                fieldRef:
+                  fieldPath: metadata.namespace
+            - name: POD_IP
+              valueFrom:
+                fieldRef:
+                  fieldPath: status.podIP
+---
+kind: GatewayConfiguration
+apiVersion: gateway-operator.konghq.com/v1beta1
+metadata:
+  name: kong
+  namespace: default
+spec:
+  dataPlaneOptions:
+    deployment:
+      podTemplateSpec:
+        spec:
+          containers:
+          - name: proxy
+            # renovate: datasource=docker versioning=docker
+            image: kong/kong-gateway:3.7
+            readinessProbe:
+              initialDelaySeconds: 1
+              periodSeconds: 1
+  controlPlaneOptions:
+    deployment:
+      podTemplateSpec:
+        spec:
+          containers:
+          - name: controller
+            # renovate: datasource=docker versioning=docker
+            image: kong/kubernetes-ingress-controller:3.1.6
+            readinessProbe:
+              initialDelaySeconds: 1
+              periodSeconds: 1
+            env:
+            - name: CONTROLLER_ADMISSION_WEBHOOK_LISTEN
+              value: "off"
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: GatewayClass
+metadata:
+  name: kong
+spec:
+  controllerName: konghq.com/gateway-operator
+  parametersRef:
+    group: gateway-operator.konghq.com
+    kind: GatewayConfiguration
+    name: kong
+    namespace: default
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: Gateway
+metadata:
+  name: kong
+  namespace: default
+spec:
+  gatewayClassName: kong
+  listeners:
+  - name: http
+    protocol: HTTP
+    port: 80
diff --git a/controller/controlplane/controller.go b/controller/controlplane/controller.go
@@ -63,6 +63,9 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 	validatinWebhookConfigurationOwnerPredicate.UpdateFunc = func(e event.UpdateEvent) bool {
 		return r.validatingWebhookConfigurationHasControlPlaneOwner(e.ObjectOld)
 	}
+	validatinWebhookConfigurationOwnerPredicate.DeleteFunc = func(e event.DeleteEvent) bool {
+		return r.validatingWebhookConfigurationHasControlPlaneOwner(e.Object)
+	}
 
 	return ctrl.NewControllerManagedBy(mgr).
 		// watch ControlPlane objects
@@ -285,7 +288,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	}
 	changed := controlplane.SetDefaults(
 		&cp.Spec.ControlPlaneOptions,
-		nil,
 		defaultArgs)
 	if changed {
 		log.Debug(logger, "updating ControlPlane resource after defaults are set since resource has changed", cp)
@@ -361,43 +363,22 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, nil // requeue will be triggered by the creation or update of the owned object
 	}
 
-	log.Trace(logger, "creating admission webhook service", cp)
-	res, admissionWebhookService, err := r.ensureAdmissionWebhookService(ctx, r.Client, cp)
-	if err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to ensure admission webhook service: %w", err)
-	}
-	if res != op.Noop {
-		log.Debug(logger, "admission webhook service created/updated", cp)
-		return ctrl.Result{}, nil // requeue will be triggered by the creation or update of the owned object
-	}
-
-	log.Trace(logger, "creating admission webhook certificate", cp)
-	res, admissionWebhookCertificateSecret, err := r.ensureAdmissionWebhookCertificateSecret(ctx, cp, admissionWebhookService)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-	if res != op.Noop {
-		log.Debug(logger, "admission webhook certificate created/updated", cp)
-		return ctrl.Result{}, nil // requeue will be triggered by the creation or update of the owned object
+	deploymentParams := ensureDeploymentParams{
+		ControlPlane:            cp,
+		ServiceAccountName:      controlplaneServiceAccount.Name,
+		AdminMTLSCertSecretName: adminCertificate.Name,
 	}
 
-	log.Trace(logger, "creating admission webhook configuration", cp)
-	res, err = r.ensureValidatingWebhookConfiguration(ctx, cp, admissionWebhookCertificateSecret, admissionWebhookService.Name)
+	admissionWebhookCertificateSecretName, res, err := r.ensureWebhookResources(ctx, logger, cp)
 	if err != nil {
-		return ctrl.Result{}, err
-	}
-	if res != op.Noop {
-		log.Debug(logger, "ValidatingWebhookConfiguration created/updated", cp)
-		return ctrl.Result{}, nil // requeue will be triggered by the creation or update of the owned object
+		return ctrl.Result{}, fmt.Errorf("failed to ensure webhook resources: %w", err)
+	} else if res != op.Noop {
+		return ctrl.Result{Requeue: true, RequeueAfter: requeueWithoutBackoff}, nil
 	}
+	deploymentParams.AdmissionWebhookCertSecretName = admissionWebhookCertificateSecretName
 
 	log.Trace(logger, "looking for existing Deployments for ControlPlane resource", cp)
-	res, controlplaneDeployment, err := r.ensureDeployment(ctx, logger, ensureDeploymentParams{
-		ControlPlane:                   cp,
-		ServiceAccountName:             controlplaneServiceAccount.Name,
-		AdminMTLSCertSecretName:        adminCertificate.Name,
-		AdmissionWebhookCertSecretName: admissionWebhookCertificateSecret.Name,
-	})
+	res, controlplaneDeployment, err := r.ensureDeployment(ctx, logger, deploymentParams)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -489,3 +470,80 @@ func (r *Reconciler) patchStatus(ctx context.Context, logger logr.Logger, update
 
 	return ctrl.Result{}, nil
 }
+
+func (r *Reconciler) ensureWebhookResources(
+	ctx context.Context, logger logr.Logger, cp *operatorv1beta1.ControlPlane,
+) (string, op.Result, error) {
+	webhookEnabled := isAdmissionWebhookEnabled(ctx, r.Client, logger, cp)
+	if !webhookEnabled {
+		log.Debug(logger, "admission webhook disabled, ensuring admission webhook resources are not present", cp)
+	} else {
+		log.Debug(logger, "admission webhook enabled, enforcing admission webhook resources", cp)
+	}
+
+	log.Trace(logger, "ensuring admission webhook service", cp)
+	res, admissionWebhookService, err := r.ensureAdmissionWebhookService(ctx, logger, r.Client, cp)
+	if err != nil {
+		return "", res, fmt.Errorf("failed to ensure admission webhook service: %w", err)
+	}
+	if res != op.Noop {
+		if !webhookEnabled {
+			log.Debug(logger, "admission webhook service has been removed", cp)
+		} else {
+			log.Debug(logger, "admission webhook service has been created/updated", cp)
+		}
+		return "", res, nil // requeue will be triggered by the creation or update of the owned object
+	}
+
+	log.Trace(logger, "ensuring admission webhook certificate", cp)
+	res, admissionWebhookCertificateSecret, err := r.ensureAdmissionWebhookCertificateSecret(ctx, logger, cp, admissionWebhookService)
+	if err != nil {
+		return "", res, err
+	}
+	if res != op.Noop {
+		if !webhookEnabled {
+			log.Debug(logger, "admission webhook service certificate has been removed", cp)
+		} else {
+			log.Debug(logger, "admission webhook service certificate has been created/updated", cp)
+		}
+		return "", res, nil // requeue will be triggered by the creation or update of the owned object
+	}
+
+	log.Trace(logger, "ensuring admission webhook configuration", cp)
+	res, err = r.ensureValidatingWebhookConfiguration(ctx, cp, admissionWebhookCertificateSecret, admissionWebhookService)
+	if err != nil {
+		return "", res, err
+	}
+	if res != op.Noop {
+		if !webhookEnabled {
+			log.Debug(logger, "ValidatingWebhookConfiguration has been removed", cp)
+		} else {
+			log.Debug(logger, "ValidatingWebhookConfiguration has been created/updated", cp)
+		}
+	}
+	if webhookEnabled {
+		return admissionWebhookCertificateSecret.Name, res, nil
+	}
+	return "", res, nil
+}
+
+func isAdmissionWebhookEnabled(ctx context.Context, cl client.Client, logger logr.Logger, cp *operatorv1beta1.ControlPlane) bool {
+	if cp.Spec.Deployment.PodTemplateSpec == nil {
+		return false
+	}
+
+	container := k8sutils.GetPodContainerByName(&cp.Spec.Deployment.PodTemplateSpec.Spec, consts.ControlPlaneControllerContainerName)
+	if container == nil {
+		return false
+	}
+	admissionWebhookListen, ok, err := k8sutils.GetEnvValueFromContainer(ctx, container, cp.Namespace, "CONTROLLER_ADMISSION_WEBHOOK_LISTEN", cl)
+	if err != nil {
+		log.Debug(logger, "unable to get CONTROLLER_ADMISSION_WEBHOOK_LISTEN env var", cp, "error", err)
+		return false
+	}
+	if !ok {
+		return false
+	}
+	// We don't validate the value of the env var here, just that it is set.
+	return len(admissionWebhookListen) > 0 && admissionWebhookListen != "off"
+}