Add label validation on cluster group

Signed-off-by: wgrayson <[email protected]>
antrea-io · Feb 22, 2022 · bf0d5c7 · bf0d5c7
1 parent 81189cd
commit bf0d5c7
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 16 deletions.
diff --git a/pkg/controller/networkpolicy/validate.go b/pkg/controller/networkpolicy/validate.go
@@ -473,7 +473,7 @@ 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 {
+			if reason, allowed := checkSelectorsLabels([]*metav1.LabelSelector{eachAppliedTo.PodSelector, eachAppliedTo.NamespaceSelector, eachAppliedTo.ExternalEntitySelector}); !allowed {
 				return reason, allowed
 			}
 		}
@@ -515,7 +515,7 @@ 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 {
+			if reason, allowed := checkSelectorsLabels([]*metav1.LabelSelector{peer.PodSelector, peer.NamespaceSelector, peer.ExternalEntitySelector}); !allowed {
 				return reason, allowed
 			}
 		}
@@ -558,7 +558,7 @@ func numFieldsSetInPeer(peer crdv1alpha1.NetworkPolicyPeer) int {
 
 // checkPeerSelectorLabels validates labels used in PodSelector, NamespaceSelector
 // and ExternalEntitySelector in a NetworkPolicyPeer.
-func checkPeerSelectorLabels(peer crdv1alpha1.NetworkPolicyPeer) (string, bool) {
+func checkSelectorsLabels(selectors []*metav1.LabelSelector) (string, bool) {
 	validateLabels := func(labels map[string]string) (string, bool) {
 		for k, v := range labels {
 			err := validation.IsQualifiedName(k)
@@ -572,19 +572,11 @@ func checkPeerSelectorLabels(peer crdv1alpha1.NetworkPolicyPeer) (string, bool)
 		}
 		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
+	for _, selector := range selectors {
+		if selector != nil {
+			if reason, allowed := validateLabels(selector.MatchLabels); !allowed {
+				return reason, allowed
+			}
 		}
 	}
 	return "", true
@@ -747,6 +739,9 @@ func validateAntreaGroupSpec(s crdv1alpha2.GroupSpec) (string, bool) {
 	}
 	selector, serviceRef, ipBlock, ipBlocks, childGroups := 0, 0, 0, 0, 0
 	if s.NamespaceSelector != nil || s.ExternalEntitySelector != nil || s.PodSelector != nil {
+		if reason, allowed := checkSelectorsLabels([]*metav1.LabelSelector{s.PodSelector, s.NamespaceSelector, s.ExternalEntitySelector}); !allowed {
+			return reason, allowed
+		}
 		selector = 1
 	}
 	if s.IPBlock != nil {

diff --git a/pkg/controller/networkpolicy/validate_test.go b/pkg/controller/networkpolicy/validate_test.go
@@ -15,6 +15,7 @@
 package networkpolicy
 
 import (
+	crdv1alpha2 "antrea.io/antrea/pkg/apis/crd/v1alpha2"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -1033,3 +1034,53 @@ func TestValidateAntreaPolicy(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateClusterAntreaGroup(t *testing.T) {
+	tests := []struct {
+		name           string
+		group          *crdv1alpha2.ClusterGroup
+		expectedReason string
+	}{
+		{
+			name: "cg-invalid-label-key",
+			group: &crdv1alpha2.ClusterGroup{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "cg-invalid-label-key",
+				},
+				Spec: crdv1alpha2.GroupSpec{
+					PodSelector: &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: "cg-invalid-label-value",
+			group: &crdv1alpha2.ClusterGroup{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "cg-invalid-label-value",
+				},
+				Spec: crdv1alpha2.GroupSpec{
+					PodSelector: &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])?')",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, c := newController()
+			v := NewNetworkPolicyValidator(c.NetworkPolicyController)
+			actualReason, allowed := v.validateAntreaGroup(tt.group, nil, admv1.Create, authenticationv1.UserInfo{})
+			assert.Equal(t, tt.expectedReason, actualReason)
+			if tt.expectedReason == "" {
+				assert.True(t, allowed)
+			} else {
+				assert.False(t, allowed)
+			}
+		})
+	}
+}