Merge pull request #894 from fabriziosestito/refactor/simplify-webhoo…

…k-configuation-controller-handlers

refactor: simplify wehbook configuration controller handlers

internal/constants/constants.go

@@ -49,11 +49,8 @@ const (
 	OptelInjectAnnotation = "sidecar.opentelemetry.io/inject"
 
 	// Webhook Configurations.
-	WebhookConfigurationPolicyScopeLabelKey          = "kubewardenPolicyScope"
 	WebhookConfigurationPolicyNameAnnotationKey      = "kubewardenPolicyName"
 	WebhookConfigurationPolicyNamespaceAnnotationKey = "kubewardenPolicyNamespace"
-	WebhookConfigurationPolicyGroupAnnotationKey     = "kubewardenPolicyGroup"
-	True                                             = "true"
 
 	// Scope.
 	NamespacePolicyScope = "namespace"

internal/controller/admissionpolicy_controller.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	policiesv1 "github.com/kubewarden/kubewarden-controller/api/policies/v1"
+	"github.com/kubewarden/kubewarden-controller/internal/constants"
 )
 
 // Warning: this controller is deployed by a helm chart which has its own
@@ -102,5 +103,26 @@ func (r *AdmissionPolicyReconciler) findAdmissionPoliciesForPod(ctx context.Cont
 }
 
 func (r *AdmissionPolicyReconciler) findAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
-	return findPolicyForWebhookConfiguration(webhookConfiguration, false, r.Log)
+	if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
+		return []reconcile.Request{}
+	}
+
+	policyName := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
+	if policyName == "" {
+		return []reconcile.Request{}
+	}
+
+	policyNamespace := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]
+	if policyNamespace == "" {
+		return []reconcile.Request{}
+	}
+
+	return []reconcile.Request{
+		{
+			NamespacedName: client.ObjectKey{
+				Name:      policyName,
+				Namespace: policyNamespace,
+			},
+		},
+	}
 }

internal/controller/admissionpolicy_controller_test.go

@@ -84,7 +84,6 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() {
 				}
 
 				Expect(validatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
-				Expect(validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.NamespacePolicyScope))
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(Equal(policyNamespace))
 				Expect(validatingWebhookConfiguration.Webhooks).To(HaveLen(1))
@@ -114,7 +113,6 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() {
 
 			By("changing the ValidatingWebhookConfiguration")
 			delete(validatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
-			validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
 			delete(validatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
 			validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
 			validatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")
@@ -210,7 +208,6 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() {
 				}
 
 				Expect(mutatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
-				Expect(mutatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.NamespacePolicyScope))
 				Expect(mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
 				Expect(mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(Equal(policyNamespace))
 				Expect(mutatingWebhookConfiguration.Webhooks).To(HaveLen(1))
@@ -240,7 +237,6 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() {
 
 			By("changing the MutatingWebhookConfiguration")
 			delete(mutatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
-			mutatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
 			delete(mutatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
 			mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
 			mutatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")

internal/controller/admissionpolicygroup_controller.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	policiesv1 "github.com/kubewarden/kubewarden-controller/api/policies/v1"
+	"github.com/kubewarden/kubewarden-controller/internal/constants"
 )
 
 // Warning: this controller is deployed by a helm chart which has its own
@@ -98,5 +99,26 @@ func (r *AdmissionPolicyGroupReconciler) findAdmissionPoliciesForPod(ctx context
 }
 
 func (r *AdmissionPolicyGroupReconciler) findAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
-	return findPolicyForWebhookConfiguration(webhookConfiguration, true, r.Log)
+	if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
+		return []reconcile.Request{}
+	}
+
+	policyName := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
+	if policyName == "" {
+		return []reconcile.Request{}
+	}
+
+	policyNamespace := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]
+	if policyNamespace == "" {
+		return []reconcile.Request{}
+	}
+
+	return []reconcile.Request{
+		{
+			NamespacedName: client.ObjectKey{
+				Name:      policyName,
+				Namespace: policyNamespace,
+			},
+		},
+	}
 }

internal/controller/admissionpolicygroup_controller_test.go

@@ -83,8 +83,6 @@ var _ = Describe("AdmissionPolicyGroup controller", Label("real-cluster"), func(
 				}
 
 				Expect(validatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
-				Expect(validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.NamespacePolicyScope))
-				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyGroupAnnotationKey]).To(Equal(constants.True))
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(Equal(policyNamespace))
 				Expect(validatingWebhookConfiguration.Webhooks).To(HaveLen(1))
@@ -114,8 +112,6 @@ var _ = Describe("AdmissionPolicyGroup controller", Label("real-cluster"), func(
 
 			By("changing the ValidatingWebhookConfiguration")
 			delete(validatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
-			validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
-			validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyGroupAnnotationKey] = "false"
 			delete(validatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
 			validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
 			validatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")

internal/controller/clusteradmissionpolicy_controller.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	policiesv1 "github.com/kubewarden/kubewarden-controller/api/policies/v1"
+	"github.com/kubewarden/kubewarden-controller/internal/constants"
 )
 
 // Warning: this controller is deployed by a helm chart which has its own
@@ -101,5 +102,20 @@ func (r *ClusterAdmissionPolicyReconciler) findClusterAdmissionPoliciesForPod(ct
 }
 
 func (r *ClusterAdmissionPolicyReconciler) findClusterAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
-	return findClusterPolicyForWebhookConfiguration(webhookConfiguration, false, r.Log)
+	if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
+		return []reconcile.Request{}
+	}
+
+	policyName := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
+	if policyName == "" {
+		return []reconcile.Request{}
+	}
+
+	return []reconcile.Request{
+		{
+			NamespacedName: client.ObjectKey{
+				Name: policyName,
+			},
+		},
+	}
 }

internal/controller/clusteradmissionpolicy_controller_test.go

@@ -71,7 +71,6 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun
 				}
 
 				Expect(validatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
-				Expect(validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.ClusterPolicyScope))
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(BeEmpty())
 				Expect(validatingWebhookConfiguration.Webhooks).To(HaveLen(1))
@@ -107,7 +106,6 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun
 			}, timeout, pollInterval).Should(Succeed())
 
 			delete(validatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
-			validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
 			delete(validatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
 			validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
 			validatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")
@@ -180,7 +178,6 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun
 					return err
 				}
 				Expect(mutatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
-				Expect(mutatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.ClusterPolicyScope))
 				Expect(mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
 				Expect(mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(BeEmpty())
 				Expect(mutatingWebhookConfiguration.Webhooks).To(HaveLen(1))
@@ -216,7 +213,6 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun
 			By("changing the MutatingWebhookConfiguration")
 
 			delete(mutatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
-			mutatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
 			delete(mutatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
 			mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
 			mutatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")

internal/controller/clusteradmissionpolicygroup_controller.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	policiesv1 "github.com/kubewarden/kubewarden-controller/api/policies/v1"
+	"github.com/kubewarden/kubewarden-controller/internal/constants"
 )
 
 // Warning: this controller is deployed by a helm chart which has its own
@@ -97,5 +98,20 @@ func (r *ClusterAdmissionPolicyGroupReconciler) findClusterAdmissionPoliciesForP
 }
 
 func (r *ClusterAdmissionPolicyGroupReconciler) findClusterAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
-	return findClusterPolicyForWebhookConfiguration(webhookConfiguration, true, r.Log)
+	if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
+		return []reconcile.Request{}
+	}
+
+	policyName := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
+	if policyName == "" {
+		return []reconcile.Request{}
+	}
+
+	return []reconcile.Request{
+		{
+			NamespacedName: client.ObjectKey{
+				Name: policyName,
+			},
+		},
+	}
 }

internal/controller/clusteradmissionpolicygroup_controller_test.go

@@ -71,8 +71,6 @@ var _ = Describe("ClusterAdmissionPolicyGroup controller", Label("real-cluster")
 				}
 
 				Expect(validatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
-				Expect(validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.ClusterPolicyScope))
-				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyGroupAnnotationKey]).To(Equal(constants.True))
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(BeEmpty())
 				Expect(validatingWebhookConfiguration.Webhooks).To(HaveLen(1))
@@ -108,8 +106,6 @@ var _ = Describe("ClusterAdmissionPolicyGroup controller", Label("real-cluster")
 			}, timeout, pollInterval).Should(Succeed())
 
 			delete(validatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
-			validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
-			validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyGroupAnnotationKey] = "false"
 			delete(validatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
 			validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
 			validatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")

internal/controller/policy_subreconciler.go

@@ -349,96 +349,14 @@ func findClusterPoliciesForPod(ctx context.Context, k8sClient client.Client, obj
 	return findClusterPoliciesForConfigMap(&configMap)
 }
 
-func findClusterPolicyForWebhookConfiguration(webhookConfiguration client.Object, isGroup bool, log logr.Logger) []reconcile.Request {
-	if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
-		return []reconcile.Request{}
-	}
-
-	if isGroup && !hasGroupAnnotation(webhookConfiguration.GetAnnotations()) {
-		return []reconcile.Request{}
-	}
-
-	policyScope, found := webhookConfiguration.GetLabels()[constants.WebhookConfigurationPolicyScopeLabelKey]
-	if !found {
-		log.Info("Found a webhook configuration without a scope label, reconciling it",
-			"name", webhookConfiguration.GetName())
-		return []reconcile.Request{}
-	}
-
-	// Filter out AdmissionPolicies
-	if policyScope != constants.ClusterPolicyScope {
-		return []reconcile.Request{}
-	}
-
-	policyName, found := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
-	if !found {
-		log.Info("Found a webhook configuration without a policy name annotation, reconciling it",
-			"name", webhookConfiguration.GetName())
-		return []reconcile.Request{}
-	}
-
-	return []reconcile.Request{
-		{
-			NamespacedName: client.ObjectKey{
-				Name: policyName,
-			},
-		},
-	}
-}
-
-func findPolicyForWebhookConfiguration(webhookConfiguration client.Object, isGroup bool, log logr.Logger) []reconcile.Request {
-	if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
-		return []reconcile.Request{}
-	}
-
-	if isGroup && !hasGroupAnnotation(webhookConfiguration.GetAnnotations()) {
-		return []reconcile.Request{}
-	}
-
-	policyScope, found := webhookConfiguration.GetLabels()[constants.WebhookConfigurationPolicyScopeLabelKey]
-	if !found {
-		log.Info("Found a webhook configuration without a scope label, reconciling it", "name", webhookConfiguration.GetName())
-		return []reconcile.Request{}
-	}
-
-	// Filter out ClusterAdmissionPolicies
-	if policyScope != constants.NamespacePolicyScope {
-		return []reconcile.Request{}
-	}
-
-	policyNamespace, found := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]
-	if !found {
-		log.Info("Found a webhook configuration without a namespace annotation, reconciling it", "name", webhookConfiguration.GetName())
-		return []reconcile.Request{}
-	}
-
-	policyName, found := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
-	if !found {
-		log.Info("Found a webhook configuration without a policy name annotation, reconciling it", "name", webhookConfiguration.GetName())
-		return []reconcile.Request{}
-	}
-
-	return []reconcile.Request{
-		{
-			NamespacedName: client.ObjectKey{
-				Name:      policyName,
-				Namespace: policyNamespace,
-			},
-		},
-	}
-}
-
+//nolint:goconst // we don't want to use a constant for "true"
 func hasKubewardenLabel(labels map[string]string) bool {
 	// Pre v1.16.0
 	kubewardenLabel := labels["kubewarden"]
 	// From v1.16.0 on we are using the recommended label "app.kubernetes.io/part-of"
 	partOfLabel := labels[constants.PartOfLabelKey]
 
-	return kubewardenLabel == constants.True || partOfLabel == constants.PartOfLabelValue
-}
-
-func hasGroupAnnotation(annotations map[string]string) bool {
-	return annotations[constants.WebhookConfigurationPolicyGroupAnnotationKey] == constants.True
+	return kubewardenLabel == "true" || partOfLabel == constants.PartOfLabelValue
 }
 
 func getPolicyMapFromConfigMap(configMap *corev1.ConfigMap) (policyConfigEntryMap, error) {