Merge pull request #48 from cybozu-go/add-excludeLabelExpressions

Add excludeLabelExpressions
cybozu-go · Jan 8, 2025 · 23c8eb7 · 23c8eb7
2 parents 77ed1f5 + 69a35f5
commit 23c8eb7
Show file tree

Hide file tree

Showing 10 changed files with 146 additions and 11 deletions.
diff --git a/api/v1beta2/networkpolicyadmissionrule_types.go b/api/v1beta2/networkpolicyadmissionrule_types.go
@@ -48,6 +48,9 @@ type NetworkPolicyAdmissionRuleSpec struct {
 type NetworkPolicyAdmissionRuleNamespaceSelector struct {
 	// ExcludeLabels defines labels through which a namespace should be excluded.
 	ExcludeLabels map[string]string `json:"excludeLabels,omitempty"`
+
+	// ExcludeLabelExpressions defines labels through which a namespace should be excluded by some expressions.
+	ExcludeLabelExpressions []metav1.LabelSelectorRequirement `json:"excludeLabelExpressions,omitempty"`
 }
 
 // NetworkPolicyAdmissionRuleForbiddenIPRanges defines forbidden IP ranges.

diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
diff --git a/charts/tenet/templates/generated/crds/tenet.cybozu.io_crds.yaml b/charts/tenet/templates/generated/crds/tenet.cybozu.io_crds.yaml
@@ -105,6 +105,37 @@ spec:
                 description: NamespaceSelector qualifies which namespaces the rules
                   should apply to.
                 properties:
+                  excludeLabelExpressions:
+                    description: ExcludeLabelExpressions defines labels through which
+                      a namespace should be excluded by some expressions.
+                    items:
+                      description: |-
+                        A label selector requirement is a selector that contains values, a key, and an operator that
+                        relates the key and values.
+                      properties:
+                        key:
+                          description: key is the label key that the selector applies
+                            to.
+                          type: string
+                        operator:
+                          description: |-
+                            operator represents a key's relationship to a set of values.
+                            Valid operators are In, NotIn, Exists and DoesNotExist.
+                          type: string
+                        values:
+                          description: |-
+                            values is an array of string values. If the operator is In or NotIn,
+                            the values array must be non-empty. If the operator is Exists or DoesNotExist,
+                            the values array must be empty. This array is replaced during a strategic
+                            merge patch.
+                          items:
+                            type: string
+                          type: array
+                      required:
+                      - key
+                      - operator
+                      type: object
+                    type: array
                   excludeLabels:
                     additionalProperties:
                       type: string

diff --git a/config/crd/bases/tenet.cybozu.io_networkpolicyadmissionrules.yaml b/config/crd/bases/tenet.cybozu.io_networkpolicyadmissionrules.yaml
@@ -89,6 +89,37 @@ spec:
                 description: NamespaceSelector qualifies which namespaces the rules
                   should apply to.
                 properties:
+                  excludeLabelExpressions:
+                    description: ExcludeLabelExpressions defines labels through which
+                      a namespace should be excluded by some expressions.
+                    items:
+                      description: |-
+                        A label selector requirement is a selector that contains values, a key, and an operator that
+                        relates the key and values.
+                      properties:
+                        key:
+                          description: key is the label key that the selector applies
+                            to.
+                          type: string
+                        operator:
+                          description: |-
+                            operator represents a key's relationship to a set of values.
+                            Valid operators are In, NotIn, Exists and DoesNotExist.
+                          type: string
+                        values:
+                          description: |-
+                            values is an array of string values. If the operator is In or NotIn,
+                            the values array must be non-empty. If the operator is Exists or DoesNotExist,
+                            the values array must be empty. This array is replaced during a strategic
+                            merge patch.
+                          items:
+                            type: string
+                          type: array
+                      required:
+                      - key
+                      - operator
+                      type: object
+                    type: array
                   excludeLabels:
                     additionalProperties:
                       type: string

diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go
@@ -35,6 +35,9 @@ var (
 	//go:embed t/node-entity-allow-cnp.yaml
 	nodeEntityAllowCiliumNetworkPolicy []byte
 
+	//go:embed t/world-entity-allow-cnp.yaml
+	worldEntityAllowCiliumNetworkPolicy []byte
+
 	//go:embed t/legal-cnp.yaml
 	legalCiliumNetworkPolicy []byte
 )
@@ -317,6 +320,26 @@ var _ = Describe("NetworkPolicyAdmissionRule", func() {
 		}).Should(Succeed())
 	})
 
+	It("should not accept CiliumNetworkPolicy with forbidden rules except for excluded namespaces using expressions", func() {
+		By("setting up namespace")
+		ns := uuid.NewString()
+		kubectlSafe(nil, "create", "ns", ns)
+		necoNS := uuid.NewString()
+		kubectlSafe(nil, "create", "ns", necoNS)
+		kubectlSafe(nil, "label", "ns", necoNS, "team=neco")
+		tenantNS := uuid.NewString()
+		kubectlSafe(nil, "create", "ns", tenantNS)
+		kubectlSafe(nil, "label", "ns", tenantNS, "team=tenant")
+
+		By("applying world entity CiliumNetworkPolicy which is forbidden in namespaces except for neco")
+		_, err := kubectl(worldEntityAllowCiliumNetworkPolicy, "apply", "-n", ns, "-f", "-")
+		Expect(err).To(HaveOccurred())
+		_, err = kubectl(worldEntityAllowCiliumNetworkPolicy, "apply", "-n", necoNS, "-f", "-")
+		Expect(err).NotTo(HaveOccurred())
+		_, err = kubectl(worldEntityAllowCiliumNetworkPolicy, "apply", "-n", tenantNS, "-f", "-")
+		Expect(err).NotTo(HaveOccurred())
+	})
+
 	It("should not reject a legal CiliumNetworkPolicy", func() {
 		By("setting up namespace")
 		nsName := uuid.NewString()

diff --git a/e2e/suite_test.go b/e2e/suite_test.go
@@ -27,6 +27,9 @@ var (
 
 	//go:embed t/exclude-only-npar.yaml
 	excludeOnlyNetworkPolicyAdmissionRule []byte
+
+	//go:embed t/exclude-expressions-npar.yaml
+	excludeExpressionsNetworkPolicyAdmissionRule []byte
 )
 
 func TestE2E(t *testing.T) {
@@ -51,4 +54,5 @@ var _ = BeforeSuite(func() {
 	kubectlSafe(bmcDenyNetworkPolicyAdmissionRule, "apply", "-f", "-")
 	kubectlSafe(nodeDenyNetworkPolicyAdmissionRule, "apply", "-f", "-")
 	kubectlSafe(excludeOnlyNetworkPolicyAdmissionRule, "apply", "-f", "-")
+	kubectlSafe(excludeExpressionsNetworkPolicyAdmissionRule, "apply", "-f", "-")
 })
diff --git a/e2e/t/exclude-expressions-npar.yaml b/e2e/t/exclude-expressions-npar.yaml
@@ -0,0 +1,15 @@
+apiVersion: tenet.cybozu.io/v1beta2
+kind: NetworkPolicyAdmissionRule
+metadata:
+    name: exclude-only-npar
+spec:
+    namespaceSelector:
+      excludeLabelExpressions:
+      - key: team
+        operator: In
+        values:
+        - neco
+        - tenant
+    forbiddenEntities:
+    - entity: world
+      type: all
diff --git a/e2e/t/world-entity-allow-cnp.yaml b/e2e/t/world-entity-allow-cnp.yaml
@@ -0,0 +1,9 @@
+apiVersion: cilium.io/v2
+kind: CiliumNetworkPolicy
+metadata:
+    name: dummy
+spec:
+  endpointSelector: {}
+  egress:
+  - toEntities:
+    - world
diff --git a/hooks/ciliumnetworkpolicy.go b/hooks/ciliumnetworkpolicy.go
@@ -6,7 +6,9 @@ import (
 
 	admissionv1 "k8s.io/api/admission/v1"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/labels"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -113,7 +115,10 @@ func (v *ciliumNetworkPolicyValidator) validateEntity(nparl tenetv1beta2.Network
 	if err != nil {
 		return admission.Errored(http.StatusBadRequest, err)
 	}
-	egressFilters, ingressFilters := v.gatherEntityFilters(&nparl, ls)
+	egressFilters, ingressFilters, err := v.gatherEntityFilters(&nparl, ls)
+	if err != nil {
+		return admission.Errored(http.StatusBadRequest, err)
+	}
 	for _, egressPolicy := range egressPolicies {
 		for _, egressFilter := range egressFilters {
 			if egressPolicy == egressFilter {
@@ -131,13 +136,15 @@ func (v *ciliumNetworkPolicyValidator) validateEntity(nparl tenetv1beta2.Network
 	return admission.Allowed("")
 }
 
-func (v *ciliumNetworkPolicyValidator) shouldValidate(npar *tenetv1beta2.NetworkPolicyAdmissionRule, ls map[string]string) bool {
-	for k, v := range npar.Spec.NamespaceSelector.ExcludeLabels {
-		if ls[k] == v {
-			return false
-		}
+func (v *ciliumNetworkPolicyValidator) shouldExclude(npar *tenetv1beta2.NetworkPolicyAdmissionRule, ls map[string]string) (bool, error) {
+	s, err := v1.LabelSelectorAsSelector(&v1.LabelSelector{
+		MatchLabels:      npar.Spec.NamespaceSelector.ExcludeLabels,
+		MatchExpressions: npar.Spec.NamespaceSelector.ExcludeLabelExpressions,
+	})
+	if err != nil {
+		return false, err
 	}
-	return true
+	return s.Matches(labels.Set(ls)), nil
 }
 
 func SetupCiliumNetworkPolicyWebhook(mgr manager.Manager, dec admission.Decoder, sa string) {

diff --git a/hooks/ciliumnetworkpolicy_util.go b/hooks/ciliumnetworkpolicy_util.go
@@ -57,7 +57,9 @@ func (v *ciliumNetworkPolicyValidator) toIPNetSlice(raw []string) ([]*net.IPNet,
 func (v *ciliumNetworkPolicyValidator) gatherIPFilters(nparl *tenetv1beta2.NetworkPolicyAdmissionRuleList, ls map[string]string) ([]*net.IPNet, []*net.IPNet, error) {
 	var egressFilters, ingressFilters []*net.IPNet
 	for _, npar := range nparl.Items {
-		if !v.shouldValidate(&npar, ls) {
+		if matched, err := v.shouldExclude(&npar, ls); err != nil {
+			return nil, nil, err
+		} else if matched {
 			continue
 		}
 
@@ -84,10 +86,12 @@ func (v *ciliumNetworkPolicyValidator) gatherEntityPolicies(cnp *unstructured.Un
 	return v.gatherPolicies(cnp, cilium.EntityRuleKey, v.gatherPoliciesFromStringRule)
 }
 
-func (v *ciliumNetworkPolicyValidator) gatherEntityFilters(nparl *tenetv1beta2.NetworkPolicyAdmissionRuleList, ls map[string]string) ([]string, []string) {
+func (v *ciliumNetworkPolicyValidator) gatherEntityFilters(nparl *tenetv1beta2.NetworkPolicyAdmissionRuleList, ls map[string]string) ([]string, []string, error) {
 	var egressFilters, ingressFilters []string
 	for _, npar := range nparl.Items {
-		if !v.shouldValidate(&npar, ls) {
+		if matched, err := v.shouldExclude(&npar, ls); err != nil {
+			return nil, nil, err
+		} else if matched {
 			continue
 		}
 
@@ -103,7 +107,7 @@ func (v *ciliumNetworkPolicyValidator) gatherEntityFilters(nparl *tenetv1beta2.N
 			}
 		}
 	}
-	return egressFilters, ingressFilters
+	return egressFilters, ingressFilters, nil
 }
 
 func (v *ciliumNetworkPolicyValidator) intersectIP(cidr1, cidr2 *net.IPNet) bool {