Merge pull request #566 from viccuad/feat-webhooks-reconciled

feat: Always enforce `{Validating,Mutating}WebhookConfiguration` state

controllers/admissionpolicy_controller.go

@@ -26,6 +26,7 @@ import (
 	"github.com/kubewarden/kubewarden-controller/internal/pkg/constants"
 	"github.com/kubewarden/kubewarden-controller/internal/pkg/naming"
 	policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1"
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -80,16 +81,17 @@ func (r *AdmissionPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
 			&corev1.Pod{},
 			handler.EnqueueRequestsFromMapFunc(r.findAdmissionPoliciesForPod),
 		).
-		// Despite this policy server watch is not strictly necessary, we
-		// include it for the integration tests, so that we identify
-		// policy server creations even when the controller-manager is not
-		// present (so no pods end up being created)
 		Watches(
+			// Despite this PolicyServer watch not being strictly necessary, we
+			// include it for the integration tests, so that we identify
+			// PolicyServer creations even when the controller-manager is not
+			// present (so no pods end up being created)
 			&policiesv1.PolicyServer{},
 			handler.EnqueueRequestsFromMapFunc(r.findAdmissionPoliciesForPolicyServer),
 		).
+		Owns(&admissionregistrationv1.ValidatingWebhookConfiguration{}).
+		Owns(&admissionregistrationv1.MutatingWebhookConfiguration{}).
 		Complete(r)
-
 	if err != nil {
 		return errors.Join(errors.New("failed enrolling controller with manager"), err)
 	}

controllers/clusteradmissionpolicy_controller.go

@@ -26,6 +26,7 @@ import (
 	"github.com/kubewarden/kubewarden-controller/internal/pkg/constants"
 	"github.com/kubewarden/kubewarden-controller/internal/pkg/naming"
 	policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1"
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -80,16 +81,17 @@ func (r *ClusterAdmissionPolicyReconciler) SetupWithManager(mgr ctrl.Manager) er
 			&corev1.Pod{},
 			handler.EnqueueRequestsFromMapFunc(r.findClusterAdmissionPoliciesForPod),
 		).
-		// Despite this policy server watch is not strictly necessary, we
-		// include it for the integration tests, so that we identify
-		// policy server creations even when the controller-manager is not
-		// present (so no pods end up being created)
 		Watches(
+			// Despite this PolicyServer watch not being strictly necessary, we
+			// include it for the integration tests, so that we identify
+			// PolicyServer creations even when the controller-manager is not
+			// present (so no pods end up being created)
 			&policiesv1.PolicyServer{},
 			handler.EnqueueRequestsFromMapFunc(r.findClusterAdmissionPoliciesForPolicyServer),
 		).
+		Owns(&admissionregistrationv1.ValidatingWebhookConfiguration{}).
+		Owns(&admissionregistrationv1.MutatingWebhookConfiguration{}).
 		Complete(r)
-
 	if err != nil {
 		return errors.Join(errors.New("failed enrolling controller with manager"), err)
 	}

controllers/policyserver_controller.go

@@ -155,7 +155,7 @@ func (r *PolicyServerReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Watches(&policiesv1.AdmissionPolicy{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
 			// The watch will trigger twice per object change; once with the old
 			// object, and once the new object. We need to be mindful when doing
-			// Updates since they will invalidate the newever versions of the
+			// Updates since they will invalidate the newer versions of the
 			// object.
 			policy, ok := object.(*policiesv1.AdmissionPolicy)
 			if !ok {
@@ -174,7 +174,7 @@ func (r *PolicyServerReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Watches(&policiesv1.ClusterAdmissionPolicy{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
 			// The watch will trigger twice per object change; once with the old
 			// object, and once the new object. We need to be mindful when doing
-			// Updates since they will invalidate the newever versions of the
+			// Updates since they will invalidate the newer versions of the
 			// object.
 			policy, ok := object.(*policiesv1.ClusterAdmissionPolicy)
 			if !ok {

internal/pkg/admission/mutating-webhook.go

@@ -7,12 +7,14 @@ import (
 	"reflect"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 
 	"github.com/kubewarden/kubewarden-controller/internal/pkg/constants"
 )
@@ -23,9 +25,13 @@ func (r *Reconciler) ReconcileMutatingWebhookConfiguration(
 	ctx context.Context,
 	policy policiesv1.Policy,
 	admissionSecret *corev1.Secret,
-	policyServerNameWithPrefix string) error {
-	webhook := r.mutatingWebhookConfiguration(policy, admissionSecret, policyServerNameWithPrefix)
-	err := r.Client.Create(ctx, webhook)
+	policyServerNameWithPrefix string,
+) error {
+	webhook, err := r.mutatingWebhookConfiguration(policy, admissionSecret, policyServerNameWithPrefix)
+	if err != nil {
+		return err
+	}
+	err = r.Client.Create(ctx, webhook)
 	if err == nil {
 		return nil
 	}
@@ -37,7 +43,8 @@ func (r *Reconciler) ReconcileMutatingWebhookConfiguration(
 
 func (r *Reconciler) updateMutatingWebhook(ctx context.Context,
 	policy policiesv1.Policy,
-	newWebhook *admissionregistrationv1.MutatingWebhookConfiguration) error {
+	newWebhook *admissionregistrationv1.MutatingWebhookConfiguration,
+) error {
 	var originalWebhook admissionregistrationv1.MutatingWebhookConfiguration
 
 	err := r.Client.Get(ctx, client.ObjectKey{
@@ -62,7 +69,8 @@ func (r *Reconciler) updateMutatingWebhook(ctx context.Context,
 func (r *Reconciler) mutatingWebhookConfiguration(
 	policy policiesv1.Policy,
 	admissionSecret *corev1.Secret,
-	policyServerName string) *admissionregistrationv1.MutatingWebhookConfiguration {
+	policyServerName string,
+) (*admissionregistrationv1.MutatingWebhookConfiguration, error) {
 	admissionPath := filepath.Join("/validate", policy.GetUniqueName())
 	admissionPort := int32(constants.PolicyServerPort)
 
@@ -78,7 +86,7 @@ func (r *Reconciler) mutatingWebhookConfiguration(
 		noneSideEffects := admissionregistrationv1.SideEffectClassNone
 		sideEffects = &noneSideEffects
 	}
-	return &admissionregistrationv1.MutatingWebhookConfiguration{
+	webhook := admissionregistrationv1.MutatingWebhookConfiguration{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: policy.GetUniqueName(),
 			Labels: map[string]string{
@@ -103,4 +111,15 @@ func (r *Reconciler) mutatingWebhookConfiguration(
 			},
 		},
 	}
+
+	// set policy as the owner of the webhook
+	scheme := runtime.NewScheme()
+	if err := policiesv1.AddToScheme(scheme); err != nil {
+		return nil, fmt.Errorf("cannot add policies.kubewarden.io/v1 to scheme: %w", err)
+	}
+	if err := controllerutil.SetControllerReference(policy, &webhook, scheme); err != nil {
+		return nil, fmt.Errorf("cannot set OwnerReference on WebhookConfiguration: %w", err)
+	}
+
+	return &webhook, nil
 }

internal/pkg/admission/validating-webhook.go

@@ -8,7 +8,9 @@ import (
 
 	policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -23,9 +25,13 @@ func (r *Reconciler) ReconcileValidatingWebhookConfiguration(
 	ctx context.Context,
 	policy policiesv1.Policy,
 	admissionSecret *corev1.Secret,
-	policyServerNameWithPrefix string) error {
-	webhook := r.validatingWebhookConfiguration(policy, admissionSecret, policyServerNameWithPrefix)
-	err := r.Client.Create(ctx, webhook)
+	policyServerNameWithPrefix string,
+) error {
+	webhook, err := r.validatingWebhookConfiguration(policy, admissionSecret, policyServerNameWithPrefix)
+	if err != nil {
+		return err
+	}
+	err = r.Client.Create(ctx, webhook)
 	if err == nil {
 		return nil
 	}
@@ -37,14 +43,15 @@ func (r *Reconciler) ReconcileValidatingWebhookConfiguration(
 
 func (r *Reconciler) updateValidatingWebhook(ctx context.Context,
 	policy policiesv1.Policy,
-	newWebhook *admissionregistrationv1.ValidatingWebhookConfiguration) error {
+	newWebhook *admissionregistrationv1.ValidatingWebhookConfiguration,
+) error {
 	var originalWebhook admissionregistrationv1.ValidatingWebhookConfiguration
 
 	err := r.Client.Get(ctx, client.ObjectKey{
 		Name: policy.GetUniqueName(),
 	}, &originalWebhook)
 	if err != nil && apierrors.IsNotFound(err) {
-		return fmt.Errorf("cannot retrieve mutating webhook: %w", err)
+		return fmt.Errorf("cannot retrieve validating webhook: %w", err)
 	}
 
 	if !reflect.DeepEqual(originalWebhook.Webhooks, newWebhook.Webhooks) {
@@ -62,7 +69,8 @@ func (r *Reconciler) updateValidatingWebhook(ctx context.Context,
 func (r *Reconciler) validatingWebhookConfiguration(
 	policy policiesv1.Policy,
 	admissionSecret *corev1.Secret,
-	policyServerName string) *admissionregistrationv1.ValidatingWebhookConfiguration {
+	policyServerName string,
+) (*admissionregistrationv1.ValidatingWebhookConfiguration, error) {
 	admissionPath := filepath.Join("/validate", policy.GetUniqueName())
 	admissionPort := int32(constants.PolicyServerPort)
 
@@ -78,7 +86,8 @@ func (r *Reconciler) validatingWebhookConfiguration(
 		noneSideEffects := admissionregistrationv1.SideEffectClassNone
 		sideEffects = &noneSideEffects
 	}
-	return &admissionregistrationv1.ValidatingWebhookConfiguration{
+
+	webhook := admissionregistrationv1.ValidatingWebhookConfiguration{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: policy.GetUniqueName(),
 			Labels: map[string]string{
@@ -103,4 +112,15 @@ func (r *Reconciler) validatingWebhookConfiguration(
 			},
 		},
 	}
+
+	// set policy as the owner of the webhook
+	scheme := runtime.NewScheme()
+	if err := policiesv1.AddToScheme(scheme); err != nil {
+		return nil, fmt.Errorf("cannot add policies.kubewarden.io/v1 to scheme: %w", err)
+	}
+	if err := controllerutil.SetControllerReference(policy, &webhook, scheme); err != nil {
+		return nil, fmt.Errorf("cannot set OwnerReference on WebhookConfiguration: %w", err)
+	}
+
+	return &webhook, nil
 }