Merge pull request #89747 from alvaroaleman/add-ass-validated-selector

apimachinery/pkg/labels: add SelectorFromSet

staging/src/k8s.io/apimachinery/pkg/labels/labels.go

@@ -57,14 +57,22 @@ func (ls Set) Get(label string) string {
 	return ls[label]
 }
 
-// AsSelector converts labels into a selectors.
+// AsSelector converts labels into a selectors. It does not
+// perform any validation, which means the server will reject
+// the request if the Set contains invalid values.
 func (ls Set) AsSelector() Selector {
 	return SelectorFromSet(ls)
 }
 
+// AsValidatedSelector converts labels into a selectors.
+// The Set is validated client-side, which allows to catch errors early.
+func (ls Set) AsValidatedSelector() (Selector, error) {
+	return ValidatedSelectorFromSet(ls)
+}
+
 // AsSelectorPreValidated converts labels into a selector, but
-// assumes that labels are already validated and thus don't
+// assumes that labels are already validated and thus doesn't
-// preform any validation.
+// perform any validation.
 // According to our measurements this is significantly faster
 // in codepaths that matter at high scale.
 func (ls Set) AsSelectorPreValidated() Selector {

staging/src/k8s.io/apimachinery/pkg/labels/selector.go

@@ -869,23 +869,30 @@ func validateLabelValue(k, v string) error {
 
 // SelectorFromSet returns a Selector which will match exactly the given Set. A
 // nil and empty Sets are considered equivalent to Everything().
+// It does not perform any validation, which means the server will reject
+// the request if the Set contains invalid values.
 func SelectorFromSet(ls Set) Selector {
+	return SelectorFromValidatedSet(ls)
+}
+
+// ValidatedSelectorFromSet returns a Selector which will match exactly the given Set. A
+// nil and empty Sets are considered equivalent to Everything().
+// The Set is validated client-side, which allows to catch errors early.
+func ValidatedSelectorFromSet(ls Set) (Selector, error) {
 	if ls == nil || len(ls) == 0 {
-		return internalSelector{}
+		return internalSelector{}, nil
 	}
 	requirements := make([]Requirement, 0, len(ls))
 	for label, value := range ls {
 		r, err := NewRequirement(label, selection.Equals, []string{value})
-		if err == nil {
+		if err != nil {
-			requirements = append(requirements, *r)
+			return nil, err
-		} else {
-			//TODO: double check errors when input comes from serialization?
-			return internalSelector{}
 		}
+		requirements = append(requirements, *r)
 	}
 	// sort to have deterministic string representation
 	sort.Sort(ByKey(requirements))
-	return internalSelector(requirements)
+	return internalSelector(requirements), nil
 }
 
 // SelectorFromValidatedSet returns a Selector which will match exactly the given Set.

staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package labels
 
 import (
+	"fmt"
 	"reflect"
 	"strings"
 	"testing"
@@ -708,3 +709,33 @@ func TestRequiresExactMatch(t *testing.T) {
 		})
 	}
 }
+
+func TestValidatedSelectorFromSet(t *testing.T) {
+	tests := []struct {
+		name             string
+		input            Set
+		expectedSelector internalSelector
+		expectedError    error
+	}{
+		{
+			name:             "Simple Set, no error",
+			input:            Set{"key": "val"},
+			expectedSelector: internalSelector([]Requirement{{key: "key", operator: selection.Equals, strValues: []string{"val"}}}),
+		},
+		{
+			name:          "Invalid Set, value too long",
+			input:         Set{"Key": "axahm2EJ8Phiephe2eixohbee9eGeiyees1thuozi1xoh0GiuH3diewi8iem7Nui"},
+			expectedError: fmt.Errorf(`invalid label value: "axahm2EJ8Phiephe2eixohbee9eGeiyees1thuozi1xoh0GiuH3diewi8iem7Nui": at key: "Key": must be no more than 63 characters`),
+		},
+	}
+
+	for _, tc := range tests {
+		selector, err := ValidatedSelectorFromSet(tc.input)
+		if !reflect.DeepEqual(err, tc.expectedError) {
+			t.Fatalf("expected error %v, got error %v", tc.expectedError, err)
+		}
+		if err == nil && !reflect.DeepEqual(selector, tc.expectedSelector) {
+			t.Errorf("expected selector %v, got selector %v", tc.expectedSelector, selector)
+		}
+	}
+}