github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-22 19:31:44 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #97538 from lingsamuel/requirement-return-field-error

Browse Source 
make labels.NewRequirement returns aggregated field.ErrorList
This commit is contained in:
Kubernetes Prow Robot 2021-01-13 14:17:17 -08:00 committed by GitHub
commit a28c802db7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 336 additions and 131 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/scheduler/apis/config/validation/validation_pluginargs.go
View File

@ -284,14 +284,14 @@ func ValidateNodeAffinityArgs(args *config.NodeAffinityArgs) error {
path := field.NewPath("addedAffinity")
var errs []error
if ns := affinity.RequiredDuringSchedulingIgnoredDuringExecution; ns != nil {
_, err := nodeaffinity.NewNodeSelector(ns, nodeaffinity.WithPath(path.Child("requiredDuringSchedulingIgnoredDuringExecution")))
_, err := nodeaffinity.NewNodeSelector(ns, field.WithPath(path.Child("requiredDuringSchedulingIgnoredDuringExecution")))
if err != nil {
errs = append(errs, err)
}
}
// TODO: Add validation for requiredDuringSchedulingRequiredDuringExecution when it gets added to the API.
if terms := affinity.PreferredDuringSchedulingIgnoredDuringExecution; len(terms) != 0 {
_, err := nodeaffinity.NewPreferredSchedulingTerms(terms, nodeaffinity.WithPath(path.Child("preferredDuringSchedulingIgnoredDuringExecution")))
_, err := nodeaffinity.NewPreferredSchedulingTerms(terms, field.WithPath(path.Child("preferredDuringSchedulingIgnoredDuringExecution")))
if err != nil {
errs = append(errs, err)
}

18
pkg/scheduler/apis/config/validation/validation_pluginargs_test.go
View File

@ -114,7 +114,7 @@ func TestValidateDefaultPreemptionArgs(t *testing.T) {
t.Run(name, func(t *testing.T) {
err := ValidateDefaultPreemptionArgs(tc.args)
if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
t.Fatalf("ValidateDefaultPreemptionArgs returned err (-want,+got):\n%s", diff)
t.Errorf("ValidateDefaultPreemptionArgs returned err (-want,+got):\n%s", diff)
}
})
}
@ -154,7 +154,7 @@ func TestValidateInterPodAffinityArgs(t *testing.T) {
t.Run(name, func(t *testing.T) {
err := ValidateInterPodAffinityArgs(tc.args)
if diff := cmp.Diff(tc.wantErr, err, ignoreBadValueDetail); diff != "" {
t.Fatalf("ValidateInterPodAffinityArgs returned err (-want,+got):\n%s", diff)
t.Errorf("ValidateInterPodAffinityArgs returned err (-want,+got):\n%s", diff)
}
})
}
@ -235,7 +235,7 @@ func TestValidateNodeLabelArgs(t *testing.T) {
t.Run(name, func(t *testing.T) {
err := ValidateNodeLabelArgs(tc.args)
if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
t.Fatalf("ValidateNodeLabelArgs returned err (-want,+got):\n%s", diff)
t.Errorf("ValidateNodeLabelArgs returned err (-want,+got):\n%s", diff)
}
})
}
@ -426,7 +426,7 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) {
t.Run(name, func(t *testing.T) {
err := ValidatePodTopologySpreadArgs(tc.args)
if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
t.Fatalf("ValidatePodTopologySpreadArgs returned err (-want,+got):\n%s", diff)
t.Errorf("ValidatePodTopologySpreadArgs returned err (-want,+got):\n%s", diff)
}
})
}
@ -668,7 +668,7 @@ func TestValidateRequestedToCapacityRatioArgs(t *testing.T) {
t.Run(name, func(t *testing.T) {
err := ValidateRequestedToCapacityRatioArgs(tc.args)
if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
t.Fatalf("ValidateRequestedToCapacityRatioArgs returned err (-want,+got):\n%s", diff)
t.Errorf("ValidateRequestedToCapacityRatioArgs returned err (-want,+got):\n%s", diff)
}
})
}
@ -755,7 +755,7 @@ func TestValidateNodeResourcesLeastAllocatedArgs(t *testing.T) {
t.Run(name, func(t *testing.T) {
err := ValidateNodeResourcesLeastAllocatedArgs(tc.args)
if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
t.Fatalf("ValidateNodeResourcesLeastAllocatedArgs returned err (-want,+got):\n%s", diff)
t.Errorf("ValidateNodeResourcesLeastAllocatedArgs returned err (-want,+got):\n%s", diff)
}
})
}
@ -842,7 +842,7 @@ func TestValidateNodeResourcesMostAllocatedArgs(t *testing.T) {
t.Run(name, func(t *testing.T) {
err := ValidateNodeResourcesMostAllocatedArgs(tc.args)
if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
t.Fatalf("ValidateNodeResourcesLeastAllocatedArgs returned err (-want,+got):\n%s", diff)
t.Errorf("ValidateNodeResourcesLeastAllocatedArgs returned err (-want,+got):\n%s", diff)
}
})
}
@ -927,7 +927,7 @@ func TestValidateNodeAffinityArgs(t *testing.T) {
wantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "addedAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0]",
Field: "addedAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].key",
},
&field.Error{
Type: field.ErrorTypeInvalid,
@ -940,7 +940,7 @@ func TestValidateNodeAffinityArgs(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
err := ValidateNodeAffinityArgs(&tc.args)
if diff := cmp.Diff(tc.wantErr, err, ignoreBadValueDetail); diff != "" {
t.Fatalf("ValidatedNodeAffinityArgs returned err (-want,+got):\n%s", diff)
t.Errorf("ValidatedNodeAffinityArgs returned err (-want,+got):\n%s", diff)
}
})
}

4
pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go
View File

@ -807,7 +807,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) {
node: &node1,
wantStatus: framework.NewStatus(
framework.UnschedulableAndUnresolvable,
"invalid label value",
`Invalid value: "{{.bad-value.}}"`,
),
},
{
@ -844,7 +844,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) {
node: &node1,
wantStatus: framework.NewStatus(
framework.UnschedulableAndUnresolvable,
"invalid label value",
`Invalid value: "{{.bad-value.}}"`,
),
},
}

4
pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go
View File

@ -561,7 +561,7 @@ func TestPreferredAffinity(t *testing.T) {
{
name: "invalid Affinity fails PreScore",
pod: &v1.Pod{Spec: v1.PodSpec{NodeName: "", Affinity: invalidAffinityLabels}},
wantStatus: framework.NewStatus(framework.Error, "invalid label value"),
wantStatus: framework.NewStatus(framework.Error, `Invalid value: "{{.bad-value.}}"`),
nodes: []*v1.Node{
{ObjectMeta: metav1.ObjectMeta{Name: "machine1", Labels: labelRgChina}},
{ObjectMeta: metav1.ObjectMeta{Name: "machine2", Labels: labelRgChina}},
@ -570,7 +570,7 @@ func TestPreferredAffinity(t *testing.T) {
{
name: "invalid AntiAffinity fails PreScore",
pod: &v1.Pod{Spec: v1.PodSpec{NodeName: "", Affinity: invalidAntiAffinityLabels}},
wantStatus: framework.NewStatus(framework.Error, "invalid label value"),
wantStatus: framework.NewStatus(framework.Error, `Invalid value: "{{.bad-value.}}"`),
nodes: []*v1.Node{
{ObjectMeta: metav1.ObjectMeta{Name: "machine1", Labels: labelRgChina}},
{ObjectMeta: metav1.ObjectMeta{Name: "machine2", Labels: labelRgChina}},

4
staging/src/k8s.io/apimachinery/pkg/labels/BUILD
View File

@ -16,6 +16,9 @@ go_test(
deps = [
"//staging/src/k8s.io/apimachinery/pkg/selection:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//vendor/github.com/google/go-cmp/cmp:go_default_library",
"//vendor/github.com/google/go-cmp/cmp/cmpopts:go_default_library",
],
)
@ -33,6 +36,7 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/selection:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//vendor/k8s.io/klog/v2:go_default_library",
],
)

8
staging/src/k8s.io/apimachinery/pkg/labels/labels.go
View File

@ -20,6 +20,8 @@ import (
"fmt"
"sort"
"strings"
"k8s.io/apimachinery/pkg/util/validation/field"
)
// Labels allows you to present labels independently from their storage.
@ -143,7 +145,7 @@ func Equals(labels1, labels2 Set) bool {
// ConvertSelectorToLabelsMap converts selector string to labels map
// and validates keys and values
func ConvertSelectorToLabelsMap(selector string) (Set, error) {
func ConvertSelectorToLabelsMap(selector string, opts ...field.PathOption) (Set, error) {
labelsMap := Set{}
if len(selector) == 0 {
@ -157,11 +159,11 @@ func ConvertSelectorToLabelsMap(selector string) (Set, error) {
return labelsMap, fmt.Errorf("invalid selector: %s", l)
}
key := strings.TrimSpace(l[0])
if err := validateLabelKey(key); err != nil {
if err := validateLabelKey(key, field.ToPath(opts...)); err != nil {
return labelsMap, err
}
value := strings.TrimSpace(l[1])
if err := validateLabelValue(key, value); err != nil {
if err := validateLabelValue(key, value, field.ToPath(opts...)); err != nil {
return labelsMap, err
}
labelsMap[key] = value

66
staging/src/k8s.io/apimachinery/pkg/labels/selector.go
View File

@ -26,9 +26,19 @@ import (
"k8s.io/apimachinery/pkg/selection"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/klog/v2"
)
var (
validRequirementOperators = []string{
string(selection.In), string(selection.NotIn),
string(selection.Equals), string(selection.DoubleEquals), string(selection.NotEquals),
string(selection.Exists), string(selection.DoesNotExist),
string(selection.GreaterThan), string(selection.LessThan),
}
)
// Requirements is AND of all requirements.
type Requirements []Requirement
@ -139,42 +149,47 @@ type Requirement struct {
// of characters. See validateLabelKey for more details.
//
// The empty string is a valid value in the input values set.
func NewRequirement(key string, op selection.Operator, vals []string) (*Requirement, error) {
if err := validateLabelKey(key); err != nil {
return nil, err
// Returned error, if not nil, is guaranteed to be an aggregated field.ErrorList
func NewRequirement(key string, op selection.Operator, vals []string, opts ...field.PathOption) (*Requirement, error) {
var allErrs field.ErrorList
path := field.ToPath(opts...)
if err := validateLabelKey(key, path.Child("key")); err != nil {
allErrs = append(allErrs, err)
}
valuePath := path.Child("values")
switch op {
case selection.In, selection.NotIn:
if len(vals) == 0 {
return nil, fmt.Errorf("for 'in', 'notin' operators, values set can't be empty")
allErrs = append(allErrs, field.Invalid(valuePath, vals, "for 'in', 'notin' operators, values set can't be empty"))
}
case selection.Equals, selection.DoubleEquals, selection.NotEquals:
if len(vals) != 1 {
return nil, fmt.Errorf("exact-match compatibility requires one single value")
allErrs = append(allErrs, field.Invalid(valuePath, vals, "exact-match compatibility requires one single value"))
}
case selection.Exists, selection.DoesNotExist:
if len(vals) != 0 {
return nil, fmt.Errorf("values set must be empty for exists and does not exist")
allErrs = append(allErrs, field.Invalid(valuePath, vals, "values set must be empty for exists and does not exist"))
}
case selection.GreaterThan, selection.LessThan:
if len(vals) != 1 {
return nil, fmt.Errorf("for 'Gt', 'Lt' operators, exactly one value is required")
allErrs = append(allErrs, field.Invalid(valuePath, vals, "for 'Gt', 'Lt' operators, exactly one value is required"))
}
for i := range vals {
if _, err := strconv.ParseInt(vals[i], 10, 64); err != nil {
return nil, fmt.Errorf("for 'Gt', 'Lt' operators, the value must be an integer")
allErrs = append(allErrs, field.Invalid(valuePath.Index(i), vals[i], "for 'Gt', 'Lt' operators, the value must be an integer"))
}
}
default:
return nil, fmt.Errorf("operator '%v' is not recognized", op)
allErrs = append(allErrs, field.NotSupported(path.Child("operator"), op, validRequirementOperators))
}
for i := range vals {
if err := validateLabelValue(key, vals[i]); err != nil {
return nil, err
if err := validateLabelValue(key, vals[i], valuePath.Index(i)); err != nil {
allErrs = append(allErrs, err)
}
}
return &Requirement{key: key, operator: op, strValues: vals}, nil
return &Requirement{key: key, operator: op, strValues: vals}, allErrs.ToAggregate()
}
func (r *Requirement) hasValue(value string) bool {
@ -560,6 +575,7 @@ type Parser struct {
l *Lexer
scannedItems []ScannedItem
position int
path *field.Path
}
// ParserContext represents context during parsing:
@ -653,7 +669,7 @@ func (p *Parser) parseRequirement() (*Requirement, error) {
return nil, err
}
if operator == selection.Exists || operator == selection.DoesNotExist { // operator found lookahead set checked
return NewRequirement(key, operator, []string{})
return NewRequirement(key, operator, []string{}, field.WithPath(p.path))
}
operator, err = p.parseOperator()
if err != nil {
@ -669,7 +685,7 @@ func (p *Parser) parseRequirement() (*Requirement, error) {
if err != nil {
return nil, err
}
return NewRequirement(key, operator, values.List())
return NewRequirement(key, operator, values.List(), field.WithPath(p.path))
}
@ -687,7 +703,7 @@ func (p *Parser) parseKeyAndInferOperator() (string, selection.Operator, error)
err := fmt.Errorf("found '%s', expected: identifier", literal)
return "", "", err
}
if err := validateLabelKey(literal); err != nil {
if err := validateLabelKey(literal, p.path); err != nil {
return "", "", err
}
if t, _ := p.lookahead(Values); t == EndOfStringToken || t == CommaToken {
@ -833,8 +849,8 @@ func (p *Parser) parseExactValue() (sets.String, error) {
// the KEY exists and can be any VALUE.
// (5) A requirement with just !KEY requires that the KEY not exist.
//
func Parse(selector string) (Selector, error) {
parsedSelector, err := parse(selector)
func Parse(selector string, opts ...field.PathOption) (Selector, error) {
parsedSelector, err := parse(selector, field.ToPath(opts...))
if err == nil {
return parsedSelector, nil
}
@ -845,8 +861,8 @@ func Parse(selector string) (Selector, error) {
// The callers of this method can then decide how to return the internalSelector struct to their
// callers. This function has two callers now, one returns a Selector interface and the other
// returns a list of requirements.
func parse(selector string) (internalSelector, error) {
p := &Parser{l: &Lexer{s: selector, pos: 0}}
func parse(selector string, path *field.Path) (internalSelector, error) {
p := &Parser{l: &Lexer{s: selector, pos: 0}, path: path}
items, err := p.parse()
if err != nil {
return nil, err
@ -855,16 +871,16 @@ func parse(selector string) (internalSelector, error) {
return internalSelector(items), err
}
func validateLabelKey(k string) error {
func validateLabelKey(k string, path *field.Path) *field.Error {
if errs := validation.IsQualifiedName(k); len(errs) != 0 {
return fmt.Errorf("invalid label key %q: %s", k, strings.Join(errs, "; "))
return field.Invalid(path, k, strings.Join(errs, "; "))
}
return nil
}
func validateLabelValue(k, v string) error {
func validateLabelValue(k, v string, path *field.Path) *field.Error {
if errs := validation.IsValidLabelValue(v); len(errs) != 0 {
return fmt.Errorf("invalid label value: %q: at key: %q: %s", v, k, strings.Join(errs, "; "))
return field.Invalid(path.Key(k), v, strings.Join(errs, "; "))
}
return nil
}
@ -918,6 +934,6 @@ func SelectorFromValidatedSet(ls Set) Selector {
// processing on selector requirements.
// See the documentation for Parse() function for more details.
// TODO: Consider exporting the internalSelector type instead.
func ParseToRequirements(selector string) ([]Requirement, error) {
return parse(selector)
func ParseToRequirements(selector string, opts ...field.PathOption) ([]Requirement, error) {
return parse(selector, field.ToPath(opts...))
}

224
staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go
View File

@ -17,13 +17,19 @@ limitations under the License.
package labels
import (
"fmt"
"reflect"
"strings"
"testing"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"k8s.io/apimachinery/pkg/selection"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field"
)
var (
ignoreDetail = cmpopts.IgnoreFields(field.Error{}, "Detail")
)
func TestSelectorParse(t *testing.T) {
@ -303,27 +309,179 @@ func TestRequirementConstructor(t *testing.T) {
Key string
Op selection.Operator
Vals sets.String
Success bool
WantErr field.ErrorList
}{
{"x", selection.In, nil, false},
{"x", selection.NotIn, sets.NewString(), false},
{"x", selection.In, sets.NewString("foo"), true},
{"x", selection.NotIn, sets.NewString("foo"), true},
{"x", selection.Exists, nil, true},
{"x", selection.DoesNotExist, nil, true},
{"1foo", selection.In, sets.NewString("bar"), true},
{"1234", selection.In, sets.NewString("bar"), true},
{"y", selection.GreaterThan, sets.NewString("1"), true},
{"z", selection.LessThan, sets.NewString("6"), true},
{"foo", selection.GreaterThan, sets.NewString("bar"), false},
{"barz", selection.LessThan, sets.NewString("blah"), false},
{strings.Repeat("a", 254), selection.Exists, nil, false}, //breaks DNS rule that len(key) <= 253
{
Key: "x1",
Op: selection.In,
WantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "values",
BadValue: []string{},
},
},
},
{
Key: "x2",
Op: selection.NotIn,
Vals: sets.NewString(),
WantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "values",
BadValue: []string{},
},
},
},
{
Key: "x3",
Op: selection.In,
Vals: sets.NewString("foo"),
},
{
Key: "x4",
Op: selection.NotIn,
Vals: sets.NewString("foo"),
},
{
Key: "x5",
Op: selection.Equals,
Vals: sets.NewString("foo", "bar"),
WantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "values",
BadValue: []string{"bar", "foo"},
},
},
},
{
Key: "x6",
Op: selection.Exists,
},
{
Key: "x7",
Op: selection.DoesNotExist,
},
{
Key: "x8",
Op: selection.Exists,
Vals: sets.NewString("foo"),
WantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "values",
BadValue: []string{"foo"},
},
},
},
{
Key: "x9",
Op: selection.In,
Vals: sets.NewString("bar"),
},
{
Key: "x10",
Op: selection.In,
Vals: sets.NewString("bar"),
},
{
Key: "x11",
Op: selection.GreaterThan,
Vals: sets.NewString("1"),
},
{
Key: "x12",
Op: selection.LessThan,
Vals: sets.NewString("6"),
},
{
Key: "x13",
Op: selection.GreaterThan,
WantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "values",
BadValue: []string{},
},
},
},
{
Key: "x14",
Op: selection.GreaterThan,
Vals: sets.NewString("bar"),
WantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "values[0]",
BadValue: "bar",
},
},
},
{
Key: "x15",
Op: selection.LessThan,
Vals: sets.NewString("bar"),
WantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "values[0]",
BadValue: "bar",
},
},
},
{
Key: strings.Repeat("a", 254), //breaks DNS rule that len(key) <= 253
Op: selection.Exists,
WantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "key",
BadValue: strings.Repeat("a", 254),
},
},
},
{
Key: "x16",
Op: selection.Equals,
Vals: sets.NewString(strings.Repeat("a", 254)),
WantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "values[0][x16]",
BadValue: strings.Repeat("a", 254),
},
},
},
{
Key: "x17",
Op: selection.Equals,
Vals: sets.NewString("a b"),
WantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "values[0][x17]",
BadValue: "a b",
},
},
},
{
Key: "x18",
Op: "unsupportedOp",
WantErr: field.ErrorList{
&field.Error{
Type: field.ErrorTypeNotSupported,
Field: "operator",
BadValue: selection.Operator("unsupportedOp"),
},
},
},
}
for _, rc := range requirementConstructorTests {
if _, err := NewRequirement(rc.Key, rc.Op, rc.Vals.List()); err == nil && !rc.Success {
t.Errorf("expected error with key:%#v op:%v vals:%v, got no error", rc.Key, rc.Op, rc.Vals)
} else if err != nil && rc.Success {
t.Errorf("expected no error with key:%#v op:%v vals:%v, got:%v", rc.Key, rc.Op, rc.Vals, err)
_, err := NewRequirement(rc.Key, rc.Op, rc.Vals.List())
if diff := cmp.Diff(rc.WantErr.ToAggregate(), err, ignoreDetail); diff != "" {
t.Errorf("NewRequirement test %v returned unexpected error (-want,+got):\n%s", rc.Key, diff)
}
}
}
@ -714,27 +872,41 @@ func TestValidatedSelectorFromSet(t *testing.T) {
name string
input Set
expectedSelector internalSelector
expectedError error
expectedError field.ErrorList
}{
{
name: "Simple Set, no error",
input: Set{"key": "val"},
expectedSelector: internalSelector([]Requirement{{key: "key", operator: selection.Equals, strValues: []string{"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`),
expectedError: field.ErrorList{
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "values[0][Key]",
BadValue: "axahm2EJ8Phiephe2eixohbee9eGeiyees1thuozi1xoh0GiuH3diewi8iem7Nui",
},
},
},
}
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 diff := cmp.Diff(tc.expectedError.ToAggregate(), err, ignoreDetail); diff != "" {
t.Errorf("ValidatedSelectorFromSet %#v returned unexpected error (-want,+got):\n%s", tc.name, diff)
}
if err == nil {
if diff := cmp.Diff(tc.expectedSelector, selector, cmp.AllowUnexported(Requirement{})); diff != "" {
t.Errorf("ValidatedSelectorFromSet %#v returned unexpected selector (-want,+got):\n%s", tc.name, diff)
}
if err == nil && !reflect.DeepEqual(selector, tc.expectedSelector) {
t.Errorf("expected selector %v, got selector %v", tc.expectedSelector, selector)
}
}
}

23
staging/src/k8s.io/apimachinery/pkg/util/validation/field/path.go
View File

@ -22,6 +22,29 @@ import (
"strconv"
)
type pathOptions struct {
path *Path
}
// PathOption modifies a pathOptions
type PathOption func(o *pathOptions)
// WithPath generates a PathOption
func WithPath(p *Path) PathOption {
return func(o *pathOptions) {
o.path = p
}
}
// ToPath produces *Path from a set of PathOption
func ToPath(opts ...PathOption) *Path {
c := &pathOptions{}
for _, opt := range opts {
opt(c)
}
return c.path
}
// Path represents the path from some root to a particular field.
type Path struct {
name string // the name of this field or "" if this is an index

1
staging/src/k8s.io/component-helpers/scheduling/corev1/nodeaffinity/BUILD
View File

@ -11,6 +11,7 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/selection:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
],
)

77
staging/src/k8s.io/component-helpers/scheduling/corev1/nodeaffinity/nodeaffinity.go
View File

@ -21,12 +21,10 @@ import (
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/selection"
"k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/validation/field"
)
// ParseOption is an option for parsing.
type ParseOption func(*parseOptions)
// NodeSelector is a runtime representation of v1.NodeSelector.
type NodeSelector struct {
lazy LazyErrorNodeSelector
@ -38,37 +36,27 @@ type LazyErrorNodeSelector struct {
terms []nodeSelectorTerm
}
// WithPath sets a field.Path to be used as root for parse errors.
func WithPath(p *field.Path) ParseOption {
return func(o *parseOptions) {
o.path = p
}
}
// NewNodeSelector returns a NodeSelector or aggregate parsing errors found.
func NewNodeSelector(ns *v1.NodeSelector, opts ...ParseOption) (*NodeSelector, error) {
func NewNodeSelector(ns *v1.NodeSelector, opts ...field.PathOption) (*NodeSelector, error) {
lazy := NewLazyErrorNodeSelector(ns, opts...)
var errs field.ErrorList
var errs []error
for _, term := range lazy.terms {
if len(term.parseErrs) > 0 {
errs = append(errs, term.parseErrs...)
}
}
if len(errs) != 0 {
return nil, errs.ToAggregate()
return nil, errors.Flatten(errors.NewAggregate(errs))
}
return &NodeSelector{lazy: *lazy}, nil
}
// NewLazyErrorNodeSelector creates a NodeSelector that only reports parse
// errors when no terms match.
func NewLazyErrorNodeSelector(ns *v1.NodeSelector, opts ...ParseOption) *LazyErrorNodeSelector {
o := parseOptions{}
for _, opt := range opts {
opt(&o)
}
func NewLazyErrorNodeSelector(ns *v1.NodeSelector, opts ...field.PathOption) *LazyErrorNodeSelector {
p := field.ToPath(opts...)
parsedTerms := make([]nodeSelectorTerm, 0, len(ns.NodeSelectorTerms))
path := o.path.Child("nodeSelectorTerms")
path := p.Child("nodeSelectorTerms")
for i, term := range ns.NodeSelectorTerms {
// nil or empty term selects no objects
if isEmptyNodeSelectorTerm(&term) {
@ -100,7 +88,7 @@ func (ns *LazyErrorNodeSelector) Match(node *v1.Node) (bool, error) {
nodeLabels := labels.Set(node.Labels)
nodeFields := extractNodeFields(node)
var errs field.ErrorList
var errs []error
for _, term := range ns.terms {
match, tErrs := term.match(nodeLabels, nodeFields)
if len(tErrs) > 0 {
@ -111,7 +99,7 @@ func (ns *LazyErrorNodeSelector) Match(node *v1.Node) (bool, error) {
return true, nil
}
}
return false, errs.ToAggregate()
return false, errors.Flatten(errors.NewAggregate(errs))
}
// PreferredSchedulingTerms is a runtime representation of []v1.PreferredSchedulingTerms.
@ -121,15 +109,12 @@ type PreferredSchedulingTerms struct {
// NewPreferredSchedulingTerms returns a PreferredSchedulingTerms or all the parsing errors found.
// If a v1.PreferredSchedulingTerm has a 0 weight, its parsing is skipped.
func NewPreferredSchedulingTerms(terms []v1.PreferredSchedulingTerm, opts ...ParseOption) (*PreferredSchedulingTerms, error) {
o := parseOptions{}
for _, opt := range opts {
opt(&o)
}
var errs field.ErrorList
func NewPreferredSchedulingTerms(terms []v1.PreferredSchedulingTerm, opts ...field.PathOption) (*PreferredSchedulingTerms, error) {
p := field.ToPath(opts...)
var errs []error
parsedTerms := make([]preferredSchedulingTerm, 0, len(terms))
for i, term := range terms {
path := o.path.Index(i)
path := p.Index(i)
if term.Weight == 0 || isEmptyNodeSelectorTerm(&term.Preference) {
continue
}
@ -144,7 +129,7 @@ func NewPreferredSchedulingTerms(terms []v1.PreferredSchedulingTerm, opts ...Par
}
}
if len(errs) != 0 {
return nil, errs.ToAggregate()
return nil, errors.Flatten(errors.NewAggregate(errs))
}
return &PreferredSchedulingTerms{terms: parsedTerms}, nil
}
@ -179,26 +164,30 @@ func extractNodeFields(n *v1.Node) fields.Set {
type nodeSelectorTerm struct {
matchLabels labels.Selector
matchFields fields.Selector
parseErrs field.ErrorList
parseErrs []error
}
func newNodeSelectorTerm(term *v1.NodeSelectorTerm, path *field.Path) nodeSelectorTerm {
var parsedTerm nodeSelectorTerm
var errs []error
if len(term.MatchExpressions) != 0 {
p := path.Child("matchExpressions")
parsedTerm.matchLabels, parsedTerm.parseErrs = nodeSelectorRequirementsAsSelector(term.MatchExpressions, p)
if parsedTerm.parseErrs != nil {
return parsedTerm
parsedTerm.matchLabels, errs = nodeSelectorRequirementsAsSelector(term.MatchExpressions, p)
if errs != nil {
parsedTerm.parseErrs = append(parsedTerm.parseErrs, errs...)
}
}
if len(term.MatchFields) != 0 {
p := path.Child("matchFields")
parsedTerm.matchFields, parsedTerm.parseErrs = nodeSelectorRequirementsAsFieldSelector(term.MatchFields, p)
parsedTerm.matchFields, errs = nodeSelectorRequirementsAsFieldSelector(term.MatchFields, p)
if errs != nil {
parsedTerm.parseErrs = append(parsedTerm.parseErrs, errs...)
}
}
return parsedTerm
}
func (t *nodeSelectorTerm) match(nodeLabels labels.Set, nodeFields fields.Set) (bool, field.ErrorList) {
func (t *nodeSelectorTerm) match(nodeLabels labels.Set, nodeFields fields.Set) (bool, []error) {
if t.parseErrs != nil {
return false, t.parseErrs
}
@ -213,11 +202,11 @@ func (t *nodeSelectorTerm) match(nodeLabels labels.Set, nodeFields fields.Set) (
// nodeSelectorRequirementsAsSelector converts the []NodeSelectorRequirement api type into a struct that implements
// labels.Selector.
func nodeSelectorRequirementsAsSelector(nsm []v1.NodeSelectorRequirement, path *field.Path) (labels.Selector, field.ErrorList) {
func nodeSelectorRequirementsAsSelector(nsm []v1.NodeSelectorRequirement, path *field.Path) (labels.Selector, []error) {
if len(nsm) == 0 {
return labels.Nothing(), nil
}
var errs field.ErrorList
var errs []error
selector := labels.NewSelector()
for i, expr := range nsm {
p := path.Index(i)
@ -239,11 +228,9 @@ func nodeSelectorRequirementsAsSelector(nsm []v1.NodeSelectorRequirement, path *
errs = append(errs, field.NotSupported(p.Child("operator"), expr.Operator, nil))
continue
}
r, err := labels.NewRequirement(expr.Key, op, expr.Values)
r, err := labels.NewRequirement(expr.Key, op, expr.Values, field.WithPath(p))
if err != nil {
// TODO(#96167): Use error as returned by labels.NewRequirement once it
// is a field.Error.
errs = append(errs, field.Invalid(p, expr, err.Error()))
errs = append(errs, err)
} else {
selector = selector.Add(*r)
}
@ -261,11 +248,11 @@ var validFieldSelectorOperators = []string{
// nodeSelectorRequirementsAsFieldSelector converts the []NodeSelectorRequirement core type into a struct that implements
// fields.Selector.
func nodeSelectorRequirementsAsFieldSelector(nsr []v1.NodeSelectorRequirement, path *field.Path) (fields.Selector, field.ErrorList) {
func nodeSelectorRequirementsAsFieldSelector(nsr []v1.NodeSelectorRequirement, path *field.Path) (fields.Selector, []error) {
if len(nsr) == 0 {
return fields.Nothing(), nil
}
var errs field.ErrorList
var errs []error
var selectors []fields.Selector
for i, expr := range nsr {
@ -300,7 +287,3 @@ type preferredSchedulingTerm struct {
nodeSelectorTerm
weight int
}
type parseOptions struct {
path *field.Path
}

22
staging/src/k8s.io/component-helpers/scheduling/corev1/nodeaffinity/nodeaffinity_test.go
View File

@ -82,8 +82,8 @@ func TestNodeSelectorMatch(t *testing.T) {
},
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "nodeSelectorTerms[2].matchExpressions[0]",
Detail: `invalid label key "invalid key": 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]')`,
Field: "nodeSelectorTerms[2].matchExpressions[0].key",
Detail: `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]')`,
},
}.ToAggregate(),
},
@ -150,7 +150,7 @@ func TestNodeSelectorMatch(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
nodeSelector, err := NewNodeSelector(&tt.nodeSelector)
if diff := cmp.Diff(tt.wantErr, err, ignoreBadValue); diff != "" {
t.Fatalf("NewNodeSelector returned unexpected error (-want,+got):\n%s", diff)
t.Errorf("NewNodeSelector returned unexpected error (-want,+got):\n%s", diff)
}
if tt.wantErr != nil {
return
@ -218,8 +218,8 @@ func TestPreferredSchedulingTermsScore(t *testing.T) {
},
&field.Error{
Type: field.ErrorTypeInvalid,
Field: "[2].matchExpressions[0]",
Detail: `invalid label key "invalid key": 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]')`,
Field: "[2].matchExpressions[0].key",
Detail: `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]')`,
},
}.ToAggregate(),
},
@ -281,7 +281,7 @@ func TestPreferredSchedulingTermsScore(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
prefSchedTerms, err := NewPreferredSchedulingTerms(tt.prefSchedTerms)
if diff := cmp.Diff(tt.wantErr, err, ignoreBadValue); diff != "" {
t.Fatalf("NewPreferredSchedulingTerms returned unexpected error (-want,+got):\n%s", diff)
t.Errorf("NewPreferredSchedulingTerms returned unexpected error (-want,+got):\n%s", diff)
}
if tt.wantErr != nil {
return
@ -310,7 +310,7 @@ func TestNodeSelectorRequirementsAsSelector(t *testing.T) {
tc := []struct {
in []v1.NodeSelectorRequirement
out labels.Selector
wantErrs field.ErrorList
wantErr []error
}{
{in: nil, out: labels.Nothing()},
{in: []v1.NodeSelectorRequirement{}, out: labels.Nothing()},
@ -324,7 +324,11 @@ func TestNodeSelectorRequirementsAsSelector(t *testing.T) {
Operator: v1.NodeSelectorOpExists,
Values: []string{"bar", "baz"},
}},
wantErrs: field.ErrorList{field.Invalid(field.NewPath("root").Index(0), nil, "values set must be empty for exists and does not exist")},
wantErr: []error{
field.ErrorList{
field.Invalid(field.NewPath("root").Index(0).Child("values"), nil, "values set must be empty for exists and does not exist"),
}.ToAggregate(),
},
},
{
in: []v1.NodeSelectorRequirement{{
@ -346,7 +350,7 @@ func TestNodeSelectorRequirementsAsSelector(t *testing.T) {
for i, tc := range tc {
out, err := nodeSelectorRequirementsAsSelector(tc.in, field.NewPath("root"))
if diff := cmp.Diff(tc.wantErrs, err, ignoreBadValue); diff != "" {
if diff := cmp.Diff(tc.wantErr, err, ignoreBadValue); diff != "" {
t.Errorf("nodeSelectorRequirementsAsSelector returned unexpected error (-want,+got):\n%s", diff)
}
if !reflect.DeepEqual(out, tc.out) {