apimachinery/pkg/labels: add SelectorFromSet

While rambling again about how unsafe labels.SelectorFromSet is as it
just returns an empty selector that matches everything when it
encounters a parsing error, I noticed that we do not even have a safe
alternate. This commit fixes that by adding a `ValidatedSelectorFromSet`
func that either returns a Selector or an error.

It also changes SelectorFromSet to use SelectorFromValidatedSet under
the hood, so invalid Sets are send to the server and rejected there,
rather than silently doing the wrong thing by using am empty Selector.

Author: alvaroaleman

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
-// preform any validation.
+// assumes that labels are already validated and thus doesn't
+// 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 {
-			requirements = append(requirements, *r)
-		} else {
-			//TODO: double check errors when input comes from serialization?
-			return internalSelector{}
+		if err != nil {
+			return nil, err
 		}
+		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)
+		}
+	}
+}