Dynamic webhook lifecycle management

Signed-off-by: Damien Dassieu <dassieu.damien@gmail.com>

Makefile

@@ -27,6 +27,9 @@ SHELL = /usr/bin/env bash -o pipefail
 # WEBHOOK_PATH is the path to the webhook directory.
 WEBHOOK_PATH ?= config/webhook
 
+# DYNAMIC_WEBHOOK_NAME is the name of the webhook that handle the interception logic of RemoteSyncers
+DYNAMIC_WEBHOOK_NAME ?= dynamic-remotesyncer-webhook
+
 .PHONY: all
 all: build
 
@@ -52,24 +55,29 @@ pre-commit-check: manifests generate test lint ## Run all the tests and linters.
 
 ##@ Dev environment
 
-.PHONY: run
-run: manifests generate fmt vet ## Run a controller from your host. No resources are deleted when killed (meant to be run often).
+# DEV_WEBHOOK_HOST is a IP:PORT combination. The static & dynamic webhooks will be served on this host.
+DEV_WEBHOOK_HOST ?= "172.17.0.1:9443" # 172.17.0.1 is the default docker0 bridge IP.
+# DEV_WEBHOOK_CERT is the path to the certificate that will be used by the webhook server.
+DEV_WEBHOOK_CERT ?= "/tmp/k8s-webhook-server/serving-certs/tls.crt"
+
+.PHONY: run-fast
+run-fast: manifests generate fmt vet ## Run a controller from your host. No resources are installed. No resources are deleted when killed (meant to be run often).
 	cd $(WEBHOOK_PATH) && ./gen-certs-serv-cli.sh 1 >/dev/null
-	export MANAGER_NAMESPACE=syngit DYNAMIC_WEBHOOK_NAME=remotesyncer.syngit.io DEV_MODE="true" DEV_WEBHOOK_HOST="172.17.0.1:9443" DEV_WEBHOOK_CERT="/tmp/k8s-webhook-server/serving-certs/tls.crt" && go run cmd/main.go
+	export MANAGER_NAMESPACE=syngit DYNAMIC_WEBHOOK_NAME=$(DYNAMIC_WEBHOOK_NAME) DEV_MODE="true" DEV_WEBHOOK_HOST=$(DEV_WEBHOOK_HOST) DEV_WEBHOOK_CERT=$(DEV_WEBHOOK_CERT) && go run cmd/main.go
 
-.PHONY: run-onetime
-run-onetime: manifests generate fmt vet install-crds install-dev-webhooks ## Install CRDs, webhooks & run a controller from your host. All resources are deleted when killed.
+.PHONY: run
+run: manifests generate fmt vet install-crds install-dev-webhooks ## Install CRDs, webhooks & run a controller from your host. All resources are deleted when killed.
 	cd $(WEBHOOK_PATH) && ./gen-certs-serv-cli.sh 1 >/dev/null
-	export MANAGER_NAMESPACE=syngit DYNAMIC_WEBHOOK_NAME=remotesyncer.syngit.io DEV_MODE="true" DEV_WEBHOOK_HOST="172.17.0.1:9443" DEV_WEBHOOK_CERT="/tmp/k8s-webhook-server/serving-certs/tls.crt" && \
+	export MANAGER_NAMESPACE=syngit DYNAMIC_WEBHOOK_NAME=$(DYNAMIC_WEBHOOK_NAME) DEV_MODE="true" DEV_WEBHOOK_HOST=$(DEV_WEBHOOK_HOST) DEV_WEBHOOK_CERT=$(DEV_WEBHOOK_CERT) && \
 	{ \
 		trap 'echo "Cleanup resources"; make uninstall-crds && make uninstall-dev-webhooks; exit' SIGINT; \
 		go run cmd/main.go; \
 	}
 
-.PHONY: install-run
-install-run: manifests generate fmt vet install-crds install-dev-webhooks ## Install CRDs, webhooks & run a controller from your host. No resources are deleted when killed (meant to be run often).
+.PHONY: run-full
+run-full: manifests generate fmt vet install-crds install-dev-webhooks ## Install CRDs, webhooks & run a controller from your host. No resources are deleted when killed (meant to be run often).
 	cd $(WEBHOOK_PATH) && ./gen-certs-serv-cli.sh 1 >/dev/null
-	export MANAGER_NAMESPACE=syngit DYNAMIC_WEBHOOK_NAME=remotesyncer.syngit.io DEV_MODE="true" DEV_WEBHOOK_HOST="172.17.0.1:9443" DEV_WEBHOOK_CERT="/tmp/k8s-webhook-server/serving-certs/tls.crt" && go run cmd/main.go
+	export MANAGER_NAMESPACE=syngit DYNAMIC_WEBHOOK_NAME=$(DYNAMIC_WEBHOOK_NAME) DEV_MODE="true" DEV_WEBHOOK_HOST=$(DEV_WEBHOOK_HOST) DEV_WEBHOOK_CERT=$(DEV_WEBHOOK_CERT) && go run cmd/main.go
 
 .PHONY: cleanup-run
 cleanup-run: uninstall-crds uninstall-dev-webhooks ## Cleanup the resources created by make run.
@@ -196,18 +204,20 @@ install-crds: manifests kustomize ## Install CRDs into the K8s cluster specified
 	$(KUSTOMIZE) build config/crd | $(KUBECTL) apply -f -
 
 .PHONY: install-dev-webhooks
-install-dev-webhooks: manifests kustomize ## Deploy dev webhooks (host: 172.17.0.1) into the K8s cluster specified in ~/.kube/config.
+install-dev-webhooks: manifests kustomize ## Deploy dev webhooks using the docker bridge host into the K8s cluster specified in ~/.kube/config.
 	cd $(WEBHOOK_PATH) && ./cert-injector.sh .
 	cat $(WEBHOOK_PATH)/dev-webhook.yaml | $(KUBECTL) apply -f -
+	cat $(WEBHOOK_PATH)/dynamic-webhook.yaml | $(KUBECTL) apply -f -
 
 .PHONY: uninstall-crds
 uninstall-crds: manifests kustomize ## Uninstall CRDs from the K8s cluster specified in ~/.kube/config.
 	$(KUSTOMIZE) build config/crd | $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f -
 
 .PHONY: uninstall-dev-webhooks
-uninstall-dev-webhooks: manifests kustomize ## Undeploy dev webhooks (host: 172.17.0.1) into the K8s cluster specified in ~/.kube/config.
+uninstall-dev-webhooks: manifests kustomize ## Undeploy dev webhooks using the docker bridge host into the K8s cluster specified in ~/.kube/config.
 	cd $(WEBHOOK_PATH) && ./cleanup-injector.sh . || true
 	cat $(WEBHOOK_PATH)/dev-webhook.yaml | $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f -
+	cat $(WEBHOOK_PATH)/dynamic-webhook.yaml | $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f -
 
 .PHONY: deploy
 deploy: manifests kustomize ## Deploy syngit to the K8s cluster specified in ~/.kube/config.

cmd/main.go

@@ -17,7 +17,6 @@ limitations under the License.
 package main
 
 import (
-	"context"
 	"crypto/tls"
 	"flag"
 	"os"
@@ -27,8 +26,6 @@ import (
 	// to ensure that exec-entrypoint and run can make use of them.
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 
-	admissionv1 "k8s.io/api/admissionregistration/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -231,7 +228,6 @@ func main() {
 	}
 
 	setupLog.Info("starting manager")
-	k8sClient := mgr.GetClient()
 
 	signalCh := ctrl.SetupSignalHandler()
 	var wg sync.WaitGroup
@@ -243,20 +239,6 @@ func main() {
 			setupLog.Error(err, "problem running manager")
 			os.Exit(1)
 		}
-
-		webhookName := os.Getenv("DYNAMIC_WEBHOOK_NAME")
-		// Delete the dynamic webhooks (can be re-created when the manager is running again and if RemoteSyncers are not deleted)
-		webhookConfig := &admissionv1.ValidatingWebhookConfiguration{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: webhookName,
-			},
-		}
-		err := k8sClient.Delete(context.Background(), webhookConfig)
-		if err != nil {
-			setupLog.Error(err, "failed to delete ValidatingWebhookConfiguration", "name", webhookName)
-		} else {
-			setupLog.Info("Successfully deleted ValidatingWebhookConfiguration", "name", webhookName)
-		}
 	}()
 
 	// Block the main goroutine until the signal is received and the manager shuts down

config/crd/kustomization.yaml

@@ -26,6 +26,4 @@ patches:
 # the following config is for teaching kustomize how to do kustomization for CRDs.
 
 configurations:
-- kustomizeconfig.yaml
-
-namespace: syngit
+- kustomizeconfig.yaml

config/manager/manager.yaml

@@ -77,7 +77,7 @@ spec:
             fieldRef:
               fieldPath: metadata.namespace
         - name: DYNAMIC_WEBHOOK_NAME
-          value: remotesyncer.syngit.io
+          value: syngit-dynamic-remotesyncer-webhook
         name: manager
         securityContext:
           allowPrivilegeEscalation: false

config/webhook/dynamic-webhook.yaml

@@ -0,0 +1,6 @@
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: dynamic-remotesyncer-webhook
+webhooks: []

config/webhook/kustomization.yaml

@@ -1,4 +1,5 @@
 resources:
+- dynamic-webhook.yaml
 - manifests.yaml
 - service.yaml
 - secret.yaml

internal/controller/remotesyncer_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"slices"
 
 	. "github.com/syngit-org/syngit/internal/interceptor"
 	syngit "github.com/syngit-org/syngit/pkg/api/v1beta2"
@@ -30,23 +31,30 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
-const WebhookServiceName = "syngit-webhook-service"
+const (
+	WebhookServiceName = "syngit-webhook-service"
+	certificateName    = "operator-webhook-cert"
+)
 
 // RemoteSyncerReconciler reconciles a RemoteSyncer object
 type RemoteSyncerReconciler struct {
 	client.Client
-	Scheme         *runtime.Scheme
-	webhookServer  WebhookInterceptsAll
-	Namespace      string
-	devMode        bool
-	devWebhookHost string
-	devWebhookCert string
-	Recorder       record.EventRecorder
+	Scheme             *runtime.Scheme
+	webhookServer      WebhookInterceptsAll
+	dynamicWebhookName string
+	Namespace          string
+	devMode            bool
+	devWebhookHost     string
+	devWebhookCert     string
+	Recorder           record.EventRecorder
 }
 
 //+kubebuilder:rbac:groups=syngit.io,resources=remotesyncers,verbs=get;list;watch;create;update;patch;delete
@@ -118,19 +126,19 @@ func (r *RemoteSyncerReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		CABundle: caCert,
 	}
 
+	annotations := make(map[string]string)
 	if r.devMode {
 		url := "https://" + r.devWebhookHost + "/" + webhookPath
 		clientConfig = admissionv1.WebhookClientConfig{
 			URL:      &url,
 			CABundle: caCert,
 		}
+	} else {
+		annotations["cert-manager.io/inject-ca-from"] = fmt.Sprintf("%s:%s", r.Namespace, certificateName)
 	}
 
-	annotations := make(map[string]string)
-	annotations["cert-manager.io/inject-ca-from"] = "operator-webhook-cert"
-
 	// Create the webhook specs for this specific RI
-	webhookObjectName := os.Getenv("DYNAMIC_WEBHOOK_NAME")
+	webhookObjectName := r.dynamicWebhookName
 	var sideEffectsNone = admissionv1.SideEffectClassNone
 	webhookSpecificName := rSName + "." + rSNamespace + ".syngit.io"
 
@@ -169,18 +177,27 @@ func (r *RemoteSyncerReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	}
 
 	if err == nil {
-		// Search for the webhook spec associated to this RI
+		isExactlyTheSame := false
+
+		// Search for the webhook spec associated to this RSy
 		var currentWebhookCopy []admissionv1.ValidatingWebhook
-		for _, riWebhook := range found.Webhooks {
-			if riWebhook.Name != webhookSpecificName {
-				currentWebhookCopy = append(currentWebhookCopy, riWebhook)
+		for _, rsyWebhook := range found.Webhooks {
+			if rsyWebhook.Name != webhookSpecificName {
+				currentWebhookCopy = append(currentWebhookCopy, rsyWebhook)
+			} else {
+				isExactlyTheSame = slices.EqualFunc(rsyWebhook.Rules, webhook.Rules, rulesAreEqual)
 			}
 		}
 		if !isDeleted {
 			currentWebhookCopy = append(currentWebhookCopy, *webhook)
 		}
 
-		// If not found, then just add the new webhook spec for this RI
+		// The webhook already exists and is exactly the same -> do not update
+		if isExactlyTheSame {
+			return reconcile.Result{}, err
+		}
+
+		// If not found, then just add the new webhook spec for this RSy
 		found.Webhooks = currentWebhookCopy
 
 		err = r.Update(ctx, found)
@@ -194,7 +211,7 @@ func (r *RemoteSyncerReconciler) Reconcile(ctx context.Context, req ctrl.Request
 			return reconcile.Result{}, err
 		}
 	} else {
-		// Create a new webhook if not found -> if it is the first RI to be created
+		// Create a new webhook if not found -> if it is the first RSy to be created
 		err := r.Create(ctx, webhookConf)
 		if err != nil {
 			r.Recorder.Event(&remoteSyncer, "Warning", "WebhookNotCreated", "The webhook does not exists and has not been created")
@@ -215,6 +232,22 @@ func (r *RemoteSyncerReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	return ctrl.Result{}, nil
 }
 
+func rulesAreEqual(r1, r2 admissionv1.RuleWithOperations) bool {
+	if !slices.Equal(r1.APIGroups, r2.APIGroups) {
+		return false
+	}
+	if !slices.Equal(r1.APIVersions, r2.APIVersions) {
+		return false
+	}
+	if !slices.Equal(r1.Resources, r2.Resources) {
+		return false
+	}
+	if !slices.Equal(r1.Operations, r2.Operations) {
+		return false
+	}
+	return true
+}
+
 func (r *RemoteSyncerReconciler) updateStatus(ctx context.Context, remoteSyncer *syngit.RemoteSyncer, condition v1.Condition) error {
 	conditions := utils.TypeBasedConditionUpdater(remoteSyncer.Status.DeepCopy().Conditions, condition)
 
@@ -225,17 +258,41 @@ func (r *RemoteSyncerReconciler) updateStatus(ctx context.Context, remoteSyncer
 	return nil
 }
 
+func (r *RemoteSyncerReconciler) findObjectsForDynamicWebhook(ctx context.Context, webhook client.Object) []reconcile.Request {
+	attachedRemoteSyncers := &syngit.RemoteSyncerList{}
+	listOps := &client.ListOptions{
+		Namespace: "",
+	}
+	// List all the RemoteSyncers of the cluster
+	err := r.List(ctx, attachedRemoteSyncers, listOps)
+	if err != nil {
+		return []reconcile.Request{}
+	}
+
+	// Returns back all the RemoteSyncer of the cluster
+	requests := make([]reconcile.Request, len(attachedRemoteSyncers.Items))
+	for i, item := range attachedRemoteSyncers.Items {
+		requests[i] = reconcile.Request{
+			NamespacedName: types.NamespacedName{
+				Name:      item.GetName(),
+				Namespace: item.GetNamespace(),
+			},
+		}
+	}
+	return requests
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *RemoteSyncerReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 	recorder := mgr.GetEventRecorderFor("remotesyncer-controller")
 	r.Recorder = recorder
 
-	managerNamespace := os.Getenv("MANAGER_NAMESPACE")
 	r.devMode = os.Getenv("DEV_MODE") == "true"
 	r.devWebhookHost = os.Getenv("DEV_WEBHOOK_HOST")
 	r.devWebhookCert = os.Getenv("DEV_WEBHOOK_CERT")
-	r.Namespace = managerNamespace
+	r.Namespace = os.Getenv("MANAGER_NAMESPACE")
+	r.dynamicWebhookName = os.Getenv("DYNAMIC_WEBHOOK_NAME")
 
 	// Initialize the webhookServer
 	r.webhookServer = WebhookInterceptsAll{
@@ -246,5 +303,10 @@ func (r *RemoteSyncerReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&syngit.RemoteSyncer{}).
+		Watches(
+			&admissionv1.ValidatingWebhookConfiguration{},
+			handler.EnqueueRequestsFromMapFunc(r.findObjectsForDynamicWebhook),
+			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
+		).
 		Complete(r)
 }