From fe82462a84d11ff871ff61b117c52c129620da86 Mon Sep 17 00:00:00 2001 From: Manjusaka Date: Thu, 13 Jan 2022 22:10:10 +0800 Subject: [PATCH] Add extra value validation for matchExpression field in LabelSelector Kubernetes-commit: 0843c4dfcab93dd242044e51d6a2a21dc73d233e --- pkg/apis/meta/v1/validation/validation.go | 19 +++- .../meta/v1/validation/validation_test.go | 102 ++++++++++++++++++ 2 files changed, 118 insertions(+), 3 deletions(-) diff --git a/pkg/apis/meta/v1/validation/validation.go b/pkg/apis/meta/v1/validation/validation.go index 4c09898b8..89df038bc 100644 --- a/pkg/apis/meta/v1/validation/validation.go +++ b/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 } diff --git a/pkg/apis/meta/v1/validation/validation_test.go b/pkg/apis/meta/v1/validation/validation_test.go index 93c3284f7..f81e520d8 100644 --- a/pkg/apis/meta/v1/validation/validation_test.go +++ b/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 {