Merge pull request #883 from fabriziosestito/fix/cluster-admission-na…

…mespace-selector-getter

fix: cluster admission policy and policygroup namspaceSelector getter

Makefile

@@ -200,7 +200,7 @@ KUSTOMIZE_VERSION ?= v5.4.1
 CONTROLLER_TOOLS_VERSION ?= v0.16.1
 ENVTEST_VERSION ?= release-0.18
 GOLANGCI_LINT_VERSION ?= v1.60.3
-GINGKO_VERSION ?= v2.20.1
+GINGKO_VERSION ?= v2.20.2
 
 .PHONY: kustomize
 kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary.

api/policies/v1/admissionpolicy_types.go

@@ -143,7 +143,7 @@ func (r *AdmissionPolicy) GetMatchConditions() []admissionregistrationv1.MatchCo
 }
 
 // GetNamespaceSelector returns the namespace of the AdmissionPolicy since it is the only namespace we want the policy to be applied to.
-func (r *AdmissionPolicy) GetUpdatedNamespaceSelector(string) *metav1.LabelSelector {
+func (r *AdmissionPolicy) GetNamespaceSelector() *metav1.LabelSelector {
 	return &metav1.LabelSelector{
 		MatchLabels: map[string]string{"kubernetes.io/metadata.name": r.ObjectMeta.Namespace},
 	}

api/policies/v1/admissionpolicygroup_types.go

@@ -150,7 +150,7 @@ func (r *AdmissionPolicyGroup) GetMatchConditions() []admissionregistrationv1.Ma
 }
 
 // GetNamespaceSelector returns the namespace of the AdmissionPolicyGroup since it is the only namespace we want the policy to be applied to.
-func (r *AdmissionPolicyGroup) GetUpdatedNamespaceSelector(string) *metav1.LabelSelector {
+func (r *AdmissionPolicyGroup) GetNamespaceSelector() *metav1.LabelSelector {
 	return &metav1.LabelSelector{
 		MatchLabels: map[string]string{"kubernetes.io/metadata.name": r.ObjectMeta.Namespace},
 	}

api/policies/v1/clusteradmissionpolicy_types.go

@@ -196,24 +196,7 @@ func (r *ClusterAdmissionPolicy) GetMatchConditions() []admissionregistrationv1.
 	return r.Spec.MatchConditions
 }
 
-func (r *ClusterAdmissionPolicy) GetUpdatedNamespaceSelector(deploymentNamespace string) *metav1.LabelSelector {
-	// exclude namespace where kubewarden was deployed
-	if r.Spec.NamespaceSelector != nil {
-		r.Spec.NamespaceSelector.MatchExpressions = append(r.Spec.NamespaceSelector.MatchExpressions, metav1.LabelSelectorRequirement{
-			Key:      "kubernetes.io/metadata.name",
-			Operator: "NotIn",
-			Values:   []string{deploymentNamespace},
-		})
-	} else {
-		r.Spec.NamespaceSelector = &metav1.LabelSelector{
-			MatchExpressions: []metav1.LabelSelectorRequirement{{
-				Key:      "kubernetes.io/metadata.name",
-				Operator: "NotIn",
-				Values:   []string{deploymentNamespace},
-			}},
-		}
-	}
-
+func (r *ClusterAdmissionPolicy) GetNamespaceSelector() *metav1.LabelSelector {
 	return r.Spec.NamespaceSelector
 }
 

api/policies/v1/clusteradmissionpolicy_types_test.go

@@ -2,59 +2,8 @@ package v1
 
 import (
 	"testing"
-
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const kubewardenNamespace = "kubewarden"
-
-func TestGetNamespaceSelectorWithEmptyNamespaceSelector(t *testing.T) {
-	c := ClusterAdmissionPolicy{}
-	nsSelector := c.GetUpdatedNamespaceSelector(kubewardenNamespace)
-	isKubewardenNsFound := isNamespaceFoundInSelector(nsSelector, kubewardenNamespace)
-
-	if !isKubewardenNsFound {
-		t.Errorf("Kubewarden namespace not added to namespace selector")
-	}
-}
-
-func TestGetNamespaceSelectorWithExistingMatchExpressions(t *testing.T) {
-	policy := ClusterAdmissionPolicy{
-		Spec: ClusterAdmissionPolicySpec{
-			NamespaceSelector: &v1.LabelSelector{
-				MatchExpressions: []v1.LabelSelectorRequirement{
-					{
-						Key:      "In",
-						Operator: "kubernetes.io/metadata.name",
-						Values:   []string{"foo"},
-					},
-				},
-			},
-		},
-	}
-	nsSelector := policy.GetUpdatedNamespaceSelector(kubewardenNamespace)
-	isKubewardenNsFound := isNamespaceFoundInSelector(nsSelector, kubewardenNamespace)
-
-	if !isKubewardenNsFound {
-		t.Errorf("Kubewarden namespace not added to namespace selector")
-	}
-}
-
-func isNamespaceFoundInSelector(selector *v1.LabelSelector, namespace string) bool {
-	isKubewardenNsFound := false
-
-	for _, matchExpression := range selector.MatchExpressions {
-		if len(matchExpression.Values) == 1 &&
-			matchExpression.Values[0] == namespace &&
-			matchExpression.Key == "kubernetes.io/metadata.name" &&
-			matchExpression.Operator == "NotIn" {
-			isKubewardenNsFound = true
-		}
-	}
-
-	return isKubewardenNsFound
-}
-
 func TestClusterAdmissionPolicyGetContextAwareResources(t *testing.T) {
 	policy := ClusterAdmissionPolicy{
 		Spec: ClusterAdmissionPolicySpec{

api/policies/v1/clusteradmissionpolicygroup_types.go

@@ -188,24 +188,7 @@ func (r *ClusterAdmissionPolicyGroup) GetMatchConditions() []admissionregistrati
 	return r.Spec.MatchConditions
 }
 
-func (r *ClusterAdmissionPolicyGroup) GetUpdatedNamespaceSelector(deploymentNamespace string) *metav1.LabelSelector {
-	// exclude namespace where kubewarden was deployed
-	if r.Spec.NamespaceSelector != nil {
-		r.Spec.NamespaceSelector.MatchExpressions = append(r.Spec.NamespaceSelector.MatchExpressions, metav1.LabelSelectorRequirement{
-			Key:      "kubernetes.io/metadata.name",
-			Operator: "NotIn",
-			Values:   []string{deploymentNamespace},
-		})
-	} else {
-		r.Spec.NamespaceSelector = &metav1.LabelSelector{
-			MatchExpressions: []metav1.LabelSelectorRequirement{{
-				Key:      "kubernetes.io/metadata.name",
-				Operator: "NotIn",
-				Values:   []string{deploymentNamespace},
-			}},
-		}
-	}
-
+func (r *ClusterAdmissionPolicyGroup) GetNamespaceSelector() *metav1.LabelSelector {
 	return r.Spec.NamespaceSelector
 }
 

api/policies/v1/policy.go

@@ -99,7 +99,7 @@ type Policy interface {
 	GetFailurePolicy() *admissionregistrationv1.FailurePolicyType
 	GetMatchPolicy() *admissionregistrationv1.MatchPolicyType
 	GetMatchConditions() []admissionregistrationv1.MatchCondition
-	GetUpdatedNamespaceSelector(deploymentNamespace string) *metav1.LabelSelector
+	GetNamespaceSelector() *metav1.LabelSelector
 	GetObjectSelector() *metav1.LabelSelector
 	GetTimeoutSeconds() *int32
 	GetObjectMeta() *metav1.ObjectMeta

internal/controller/clusteradmissionpolicy_controller_test.go

@@ -21,6 +21,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega/gstruct"
 
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 
@@ -73,6 +74,12 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(BeEmpty())
 				Expect(validatingWebhookConfiguration.Webhooks).To(HaveLen(1))
+				Expect(validatingWebhookConfiguration.Webhooks[0].NamespaceSelector.MatchExpressions).To(ContainElement(MatchFields(IgnoreExtras,
+					Fields{
+						"Key":      Equal("kubernetes.io/metadata.name"),
+						"Operator": BeEquivalentTo("NotIn"),
+						"Values":   ConsistOf(deploymentsNamespace),
+					})))
 				Expect(validatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name).To(Equal("policy-server-" + policyServerName))
 				Expect(validatingWebhookConfiguration.Webhooks[0].MatchConditions).To(HaveLen(1))
 
@@ -162,6 +169,12 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun
 				Expect(mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
 				Expect(mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(BeEmpty())
 				Expect(mutatingWebhookConfiguration.Webhooks).To(HaveLen(1))
+				Expect(mutatingWebhookConfiguration.Webhooks[0].NamespaceSelector.MatchExpressions).To(ContainElement(MatchFields(IgnoreExtras,
+					Fields{
+						"Key":      Equal("kubernetes.io/metadata.name"),
+						"Operator": BeEquivalentTo("NotIn"),
+						"Values":   ConsistOf(deploymentsNamespace),
+					})))
 				Expect(mutatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name).To(Equal("policy-server-" + policyServerName))
 				Expect(mutatingWebhookConfiguration.Webhooks[0].MatchConditions).To(HaveLen(1))
 

internal/controller/clusteradmissionpolicygroup_controller_test.go

@@ -21,6 +21,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega/gstruct"
 
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 
@@ -73,6 +74,12 @@ var _ = Describe("ClusterAdmissionPolicyGroup controller", Label("real-cluster")
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
 				Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(BeEmpty())
 				Expect(validatingWebhookConfiguration.Webhooks).To(HaveLen(1))
+				Expect(validatingWebhookConfiguration.Webhooks[0].NamespaceSelector.MatchExpressions).To(ContainElement(MatchFields(IgnoreExtras,
+					Fields{
+						"Key":      Equal("kubernetes.io/metadata.name"),
+						"Operator": BeEquivalentTo("NotIn"),
+						"Values":   ConsistOf(deploymentsNamespace),
+					})))
 				Expect(validatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name).To(Equal("policy-server-" + policyServerName))
 				Expect(validatingWebhookConfiguration.Webhooks[0].MatchConditions).To(HaveLen(1))
 

internal/controller/policy_subreconciler_webhook.go

@@ -70,7 +70,7 @@ func (r *policySubReconciler) reconcileValidatingWebhookConfiguration(
 				Rules:                   policy.GetRules(),
 				FailurePolicy:           policy.GetFailurePolicy(),
 				MatchPolicy:             policy.GetMatchPolicy(),
-				NamespaceSelector:       policy.GetUpdatedNamespaceSelector(r.deploymentsNamespace),
+				NamespaceSelector:       r.namespaceSelector(policy),
 				ObjectSelector:          policy.GetObjectSelector(),
 				SideEffects:             sideEffects,
 				TimeoutSeconds:          policy.GetTimeoutSeconds(),
@@ -161,7 +161,7 @@ func (r *policySubReconciler) reconcileMutatingWebhookConfiguration(
 				Rules:                   policy.GetRules(),
 				FailurePolicy:           policy.GetFailurePolicy(),
 				MatchPolicy:             policy.GetMatchPolicy(),
-				NamespaceSelector:       policy.GetUpdatedNamespaceSelector(r.deploymentsNamespace),
+				NamespaceSelector:       r.namespaceSelector(policy),
 				ObjectSelector:          policy.GetObjectSelector(),
 				SideEffects:             sideEffects,
 				TimeoutSeconds:          policy.GetTimeoutSeconds(),
@@ -196,3 +196,27 @@ func (r *policySubReconciler) reconcileMutatingWebhookConfigurationDeletion(ctx
 
 	return nil
 }
+
+func (r *policySubReconciler) namespaceSelector(policy policiesv1.Policy) *metav1.LabelSelector {
+	switch policy.(type) {
+	case *policiesv1.ClusterAdmissionPolicyGroup, *policiesv1.ClusterAdmissionPolicy:
+		namespaceSelector := &metav1.LabelSelector{
+			MatchExpressions: []metav1.LabelSelectorRequirement{
+				{
+					Key:      "kubernetes.io/metadata.name",
+					Operator: "NotIn",
+					Values:   []string{r.deploymentsNamespace},
+				},
+			},
+		}
+
+		if policy.GetNamespaceSelector() != nil {
+			namespaceSelector.MatchExpressions = append(namespaceSelector.MatchExpressions, policy.GetNamespaceSelector().MatchExpressions...)
+		}
+
+		return namespaceSelector
+
+	default:
+		return policy.GetNamespaceSelector()
+	}
+}