Add extra value validation for matchExpression field in LabelSelector

Kubernetes-commit: 0843c4dfcab93dd242044e51d6a2a21dc73d233e

Author: Zheaoli

pkg/apis/meta/v1/validation/validation.go

@@ -28,19 +28,25 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
-func ValidateLabelSelector(ps *metav1.LabelSelector, fldPath *field.Path) field.ErrorList {
+// LabelSelectorValidationOptions is a struct that can be passed to ValidateLabelSelector to record the validate options
+type LabelSelectorValidationOptions struct {
+	// Allow invalid label value in selector
+	AllowInvalidLabelValueInSelector bool
+}
+
+func ValidateLabelSelector(ps *metav1.LabelSelector, opts LabelSelectorValidationOptions, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 	if ps == nil {
 		return allErrs
 	}
 	allErrs = append(allErrs, ValidateLabels(ps.MatchLabels, fldPath.Child("matchLabels"))...)
 	for i, expr := range ps.MatchExpressions {
-		allErrs = append(allErrs, ValidateLabelSelectorRequirement(expr, fldPath.Child("matchExpressions").Index(i))...)
+		allErrs = append(allErrs, ValidateLabelSelectorRequirement(expr, opts, fldPath.Child("matchExpressions").Index(i))...)
 	}
 	return allErrs
 }
 
-func ValidateLabelSelectorRequirement(sr metav1.LabelSelectorRequirement, fldPath *field.Path) field.ErrorList {
+func ValidateLabelSelectorRequirement(sr metav1.LabelSelectorRequirement, opts LabelSelectorValidationOptions, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 	switch sr.Operator {
 	case metav1.LabelSelectorOpIn, metav1.LabelSelectorOpNotIn:
@@ -55,6 +61,13 @@ func ValidateLabelSelectorRequirement(sr metav1.LabelSelectorRequirement, fldPat
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("operator"), sr.Operator, "not a valid selector operator"))
 	}
 	allErrs = append(allErrs, ValidateLabelName(sr.Key, fldPath.Child("key"))...)
+	if !opts.AllowInvalidLabelValueInSelector {
+		for valueIndex, value := range sr.Values {
+			for _, msg := range validation.IsValidLabelValue(value) {
+				allErrs = append(allErrs, field.Invalid(fldPath.Child("values").Index(valueIndex), value, msg))
+			}
+		}
+	}
 	return allErrs
 }
 

pkg/apis/meta/v1/validation/validation_test.go

@@ -427,6 +427,99 @@ func TestValidateConditions(t *testing.T) {
 	}
 }
 
+func TestLabelSelectorMatchExpression(t *testing.T) {
+	// Success case
+	testCases := []struct {
+		name            string
+		labelSelector   *metav1.LabelSelector
+		wantErrorNumber int
+		validateErrs    func(t *testing.T, errs field.ErrorList)
+	}{
+		{
+			name: "Valid LabelSelector",
+			labelSelector: &metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{
+						Key:      "key",
+						Operator: metav1.LabelSelectorOpIn,
+						Values:   []string{"value"},
+					},
+				},
+			},
+			wantErrorNumber: 0,
+			validateErrs:    nil,
+		},
+		{
+			name: "MatchExpression's key name isn't valid",
+			labelSelector: &metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{
+						Key:      "-key",
+						Operator: metav1.LabelSelectorOpIn,
+						Values:   []string{"value"},
+					},
+				},
+			},
+			wantErrorNumber: 1,
+			validateErrs: func(t *testing.T, errs field.ErrorList) {
+				errMessage := "name part must consist of alphanumeric characters"
+				if !partStringInErrorMessage(errs, errMessage) {
+					t.Errorf("missing %q in\n%v", errMessage, errorsAsString(errs))
+				}
+			},
+		},
+		{
+			name: "MatchExpression's operator isn't valid",
+			labelSelector: &metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{
+						Key:      "key",
+						Operator: "abc",
+						Values:   []string{"value"},
+					},
+				},
+			},
+			wantErrorNumber: 1,
+			validateErrs: func(t *testing.T, errs field.ErrorList) {
+				errMessage := "not a valid selector operator"
+				if !partStringInErrorMessage(errs, errMessage) {
+					t.Errorf("missing %q in\n%v", errMessage, errorsAsString(errs))
+				}
+			},
+		},
+		{
+			name: "MatchExpression's value name isn't valid",
+			labelSelector: &metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{
+						Key:      "key",
+						Operator: metav1.LabelSelectorOpIn,
+						Values:   []string{"-value"},
+					},
+				},
+			},
+			wantErrorNumber: 1,
+			validateErrs: func(t *testing.T, errs field.ErrorList) {
+				errMessage := "a valid label must be an empty string or consist of"
+				if !partStringInErrorMessage(errs, errMessage) {
+					t.Errorf("missing %q in\n%v", errMessage, errorsAsString(errs))
+				}
+			},
+		},
+	}
+	for index, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			allErrs := ValidateLabelSelector(testCase.labelSelector, LabelSelectorValidationOptions{false}, field.NewPath("labelSelector"))
+			if len(allErrs) != testCase.wantErrorNumber {
+				t.Errorf("case[%d]: expected failure", index)
+			}
+			if len(allErrs) >= 1 && testCase.validateErrs != nil {
+				testCase.validateErrs(t, allErrs)
+			}
+		})
+	}
+}
+
 func hasError(errs field.ErrorList, needle string) bool {
 	for _, curr := range errs {
 		if curr.Error() == needle {
@@ -445,6 +538,15 @@ func hasPrefixError(errs field.ErrorList, prefix string) bool {
 	return false
 }
 
+func partStringInErrorMessage(errs field.ErrorList, prefix string) bool {
+	for _, curr := range errs {
+		if strings.Contains(curr.Error(), prefix) {
+			return true
+		}
+	}
+	return false
+}
+
 func errorsAsString(errs field.ErrorList) string {
 	messages := []string{}
 	for _, curr := range errs {