Validate labels in CRDs

Fixes #3285. This PR added some additional validations to ANP and ACNP in validating webhook. Antrea will validate labels used in ANP and ACNP under PodSelector, NamespaceSelector and ExternalEntitySelector. Signed-off-by: wgrayson <wgrayson@vmware.com>
antrea-io · Feb 17, 2022 · 989f958 · 989f958
1 parent 78e3583
commit 989f958
Show file tree

Hide file tree

Showing 2 changed files with 107 additions and 0 deletions.
diff --git a/pkg/controller/networkpolicy/validate.go b/pkg/controller/networkpolicy/validate.go
@@ -27,6 +27,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apiserver/pkg/authentication/serviceaccount"
 	"k8s.io/klog/v2"
 
@@ -472,6 +473,9 @@ func (v *antreaPolicyValidator) validateAppliedTo(ingress, egress []crdv1alpha1.
 			if eachAppliedTo.ServiceAccount != nil && appliedToFieldsNum > 1 {
 				return "serviceAccount cannot be set with other peers in appliedTo", false
 			}
+			if reason, allowed := checkPeerSelectorLabels(eachAppliedTo); !allowed {
+				return reason, allowed
+			}
 		}
 		return "", true
 	}
@@ -511,6 +515,9 @@ func (v *antreaPolicyValidator) validatePeers(ingress, egress []crdv1alpha1.Rule
 			if peer.ServiceAccount != nil && peerFieldsNum > 1 {
 				return "serviceAccount cannot be set with other peers in rules", false
 			}
+			if reason, allowed := checkPeerSelectorLabels(peer); !allowed {
+				return reason, allowed
+			}
 		}
 		return "", true
 	}
@@ -549,6 +556,40 @@ func numFieldsSetInPeer(peer crdv1alpha1.NetworkPolicyPeer) int {
 	return num
 }
 
+// checkPeerSelectorLabels validates labels used in PodSelector, NamespaceSelector
+// and ExternalEntitySelector in a NetworkPolicyPeer.
+func checkPeerSelectorLabels(peer crdv1alpha1.NetworkPolicyPeer) (string, bool) {
+	validateLabels := func(labels map[string]string) (string, bool) {
+		for k, v := range labels {
+			err := validation.IsQualifiedName(k)
+			if err != nil {
+				return fmt.Sprintf("Invalid label key: %s: %s", k, strings.Join(err, "; ")), false
+			}
+			err = validation.IsValidLabelValue(v)
+			if err != nil {
+				return fmt.Sprintf("Invalid label value: %s: %s", v, strings.Join(err, "; ")), false
+			}
+		}
+		return "", true
+	}
+	if peer.PodSelector != nil {
+		if reason, allowed := validateLabels(peer.PodSelector.MatchLabels); !allowed {
+			return reason, allowed
+		}
+	}
+	if peer.NamespaceSelector != nil {
+		if reason, allowed := validateLabels(peer.NamespaceSelector.MatchLabels); !allowed {
+			return reason, allowed
+		}
+	}
+	if peer.ExternalEntitySelector != nil {
+		if reason, allowed := validateLabels(peer.ExternalEntitySelector.MatchLabels); !allowed {
+			return reason, allowed
+		}
+	}
+	return "", true
+}
+
 // validateTierForPolicy validates whether a referenced Tier exists.
 func (v *antreaPolicyValidator) validateTierForPolicy(tier string) (string, bool) {
 	// "tier" must exist before referencing

diff --git a/pkg/controller/networkpolicy/validate_test.go b/pkg/controller/networkpolicy/validate_test.go
@@ -940,6 +940,72 @@ func TestValidateAntreaPolicy(t *testing.T) {
 			},
 			expectedReason: "",
 		},
+		{
+			name: "acnp-invalid-label-key-applied-to",
+			policy: &crdv1alpha1.ClusterNetworkPolicy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "acnp-invalid-label-key-applied-to",
+				},
+				Spec: crdv1alpha1.ClusterNetworkPolicySpec{
+					AppliedTo: []crdv1alpha1.NetworkPolicyPeer{
+						{
+							NamespaceSelector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"foo=": "bar"},
+							},
+						},
+					},
+				},
+			},
+			expectedReason: "Invalid label key: foo=: name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')",
+		},
+		{
+			name: "acnp-invalid-label-value-applied-to",
+			policy: &crdv1alpha1.ClusterNetworkPolicy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "acnp-invalid-label-value-applied-to",
+				},
+				Spec: crdv1alpha1.ClusterNetworkPolicySpec{
+					AppliedTo: []crdv1alpha1.NetworkPolicyPeer{
+						{
+							NamespaceSelector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"foo": "bar="},
+							},
+						},
+					},
+				},
+			},
+			expectedReason: "Invalid label value: bar=: a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')",
+		},
+		{
+			name: "acnp-invalid-label-key-rule",
+			policy: &crdv1alpha1.ClusterNetworkPolicy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "acnp-invalid-label-key-rule",
+				},
+				Spec: crdv1alpha1.ClusterNetworkPolicySpec{
+					AppliedTo: []crdv1alpha1.NetworkPolicyPeer{
+						{
+							NamespaceSelector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"foo": "bar"},
+							},
+						},
+					},
+					Ingress: []crdv1alpha1.Rule{
+						{
+							Action: &allowAction,
+							From: []crdv1alpha1.NetworkPolicyPeer{
+								{
+									PodSelector: &metav1.LabelSelector{
+										MatchLabels: map[string]string{"foo=": "bar1"},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedReason: "Invalid label key: foo=: name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {