diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index dfebbf43daf..694896ee750 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4477,7 +4477,7 @@ func validateWindows(spec *core.PodSpec, fldPath *field.Path) field.ErrorList { } // ValidateNodeSelectorRequirement tests that the specified NodeSelectorRequirement fields has valid data -func ValidateNodeSelectorRequirement(rq core.NodeSelectorRequirement, fldPath *field.Path) field.ErrorList { +func ValidateNodeSelectorRequirement(rq core.NodeSelectorRequirement, allowInvalidLabelValueInRequiredNodeAffinity bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} switch rq.Operator { case core.NodeSelectorOpIn, core.NodeSelectorOpNotIn: @@ -4496,9 +4496,15 @@ func ValidateNodeSelectorRequirement(rq core.NodeSelectorRequirement, fldPath *f default: allErrs = append(allErrs, field.Invalid(fldPath.Child("operator"), rq.Operator, "not a valid selector operator")) } - allErrs = append(allErrs, unversionedvalidation.ValidateLabelName(rq.Key, fldPath.Child("key"))...) - + if !allowInvalidLabelValueInRequiredNodeAffinity { + path := fldPath.Child("values") + for valueIndex, value := range rq.Values { + for _, msg := range validation.IsValidLabelValue(value) { + allErrs = append(allErrs, field.Invalid(path.Index(valueIndex), value, msg)) + } + } + } return allErrs } @@ -4534,11 +4540,11 @@ func ValidateNodeFieldSelectorRequirement(req core.NodeSelectorRequirement, fldP } // ValidateNodeSelectorTerm tests that the specified node selector term has valid data -func ValidateNodeSelectorTerm(term core.NodeSelectorTerm, fldPath *field.Path) field.ErrorList { +func ValidateNodeSelectorTerm(term core.NodeSelectorTerm, allowInvalidLabelValueInRequiredNodeAffinity bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} for j, req := range term.MatchExpressions { - allErrs = append(allErrs, ValidateNodeSelectorRequirement(req, fldPath.Child("matchExpressions").Index(j))...) + allErrs = append(allErrs, ValidateNodeSelectorRequirement(req, allowInvalidLabelValueInRequiredNodeAffinity, fldPath.Child("matchExpressions").Index(j))...) } for j, req := range term.MatchFields { @@ -4549,7 +4555,7 @@ func ValidateNodeSelectorTerm(term core.NodeSelectorTerm, fldPath *field.Path) f } // ValidateNodeSelector tests that the specified nodeSelector fields has valid data -func ValidateNodeSelector(nodeSelector *core.NodeSelector, fldPath *field.Path) field.ErrorList { +func ValidateNodeSelector(nodeSelector *core.NodeSelector, allowInvalidLabelValueInRequiredNodeAffinity bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} termFldPath := fldPath.Child("nodeSelectorTerms") @@ -4558,7 +4564,7 @@ func ValidateNodeSelector(nodeSelector *core.NodeSelector, fldPath *field.Path) } for i, term := range nodeSelector.NodeSelectorTerms { - allErrs = append(allErrs, ValidateNodeSelectorTerm(term, termFldPath.Index(i))...) + allErrs = append(allErrs, ValidateNodeSelectorTerm(term, allowInvalidLabelValueInRequiredNodeAffinity, termFldPath.Index(i))...) } return allErrs @@ -4659,7 +4665,9 @@ func ValidatePreferredSchedulingTerms(terms []core.PreferredSchedulingTerm, fldP allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("weight"), term.Weight, "must be in the range 1-100")) } - allErrs = append(allErrs, ValidateNodeSelectorTerm(term.Preference, fldPath.Index(i).Child("preference"))...) + // we always allow invalid label-value for preferred affinity + // as they can success when cluster has only one node + allErrs = append(allErrs, ValidateNodeSelectorTerm(term.Preference, true, fldPath.Index(i).Child("preference"))...) } return allErrs } @@ -4729,7 +4737,7 @@ func validateNodeAffinity(na *core.NodeAffinity, fldPath *field.Path) field.Erro // allErrs = append(allErrs, ValidateNodeSelector(na.RequiredDuringSchedulingRequiredDuringExecution, fldPath.Child("requiredDuringSchedulingRequiredDuringExecution"))...) // } if na.RequiredDuringSchedulingIgnoredDuringExecution != nil { - allErrs = append(allErrs, ValidateNodeSelector(na.RequiredDuringSchedulingIgnoredDuringExecution, fldPath.Child("requiredDuringSchedulingIgnoredDuringExecution"))...) + allErrs = append(allErrs, ValidateNodeSelector(na.RequiredDuringSchedulingIgnoredDuringExecution, true /* TODO: opts.AllowInvalidLabelValueInRequiredNodeAffinity */, fldPath.Child("requiredDuringSchedulingIgnoredDuringExecution"))...) } if len(na.PreferredDuringSchedulingIgnoredDuringExecution) > 0 { allErrs = append(allErrs, ValidatePreferredSchedulingTerms(na.PreferredDuringSchedulingIgnoredDuringExecution, fldPath.Child("preferredDuringSchedulingIgnoredDuringExecution"))...) @@ -7783,7 +7791,7 @@ func validateVolumeNodeAffinity(nodeAffinity *core.VolumeNodeAffinity, fldPath * } if nodeAffinity.Required != nil { - allErrs = append(allErrs, ValidateNodeSelector(nodeAffinity.Required, fldPath.Child("required"))...) + allErrs = append(allErrs, ValidateNodeSelector(nodeAffinity.Required, true /* TODO: opts.AllowInvalidLabelValueInRequiredNodeAffinity */, fldPath.Child("required"))...) } else { allErrs = append(allErrs, field.Required(fldPath.Child("required"), "must specify required node constraints")) } diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index 4f06c114d85..2f58721ddc1 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -332,7 +332,7 @@ func validateAllocationResult(allocation *resource.AllocationResult, fldPath *fi var allErrs field.ErrorList allErrs = append(allErrs, validateDeviceAllocationResult(allocation.Devices, fldPath.Child("devices"), requestNames, stored)...) if allocation.NodeSelector != nil { - allErrs = append(allErrs, corevalidation.ValidateNodeSelector(allocation.NodeSelector, fldPath.Child("nodeSelector"))...) + allErrs = append(allErrs, corevalidation.ValidateNodeSelector(allocation.NodeSelector, false, fldPath.Child("nodeSelector"))...) } return allErrs } @@ -490,7 +490,7 @@ func validateResourceSliceSpec(spec, oldSpec *resource.ResourceSliceSpec, fldPat } if spec.NodeSelector != nil { numNodeSelectionFields++ - allErrs = append(allErrs, corevalidation.ValidateNodeSelector(spec.NodeSelector, fldPath.Child("nodeSelector"))...) + allErrs = append(allErrs, corevalidation.ValidateNodeSelector(spec.NodeSelector, false, fldPath.Child("nodeSelector"))...) if len(spec.NodeSelector.NodeSelectorTerms) != 1 { // This additional constraint simplifies merging of different selectors // when devices are allocated from different slices. diff --git a/pkg/apis/resource/validation/validation_resourceclaim_test.go b/pkg/apis/resource/validation/validation_resourceclaim_test.go index ac202caf6b2..c48a5bd3079 100644 --- a/pkg/apis/resource/validation/validation_resourceclaim_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaim_test.go @@ -1282,6 +1282,53 @@ func TestValidateClaimStatusUpdate(t *testing.T) { }, deviceStatusFeatureGate: false, }, + "invalid-update-invalid-label-value": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("status", "allocation", "nodeSelector", "nodeSelectorTerms").Index(0).Child("matchExpressions").Index(0).Child("values").Index(0), "-1", "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')"), + }, + oldClaim: validClaim, + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + claim = claim.DeepCopy() + claim.Status.Allocation = validAllocatedClaim.Status.Allocation.DeepCopy() + claim.Status.Allocation.NodeSelector = &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{{ + MatchExpressions: []core.NodeSelectorRequirement{{ + Key: "foo", + Operator: core.NodeSelectorOpIn, + Values: []string{"-1"}, + }}, + }}, + } + return claim + }, + }, + "valid-update-with-invalid-label-value": { + oldClaim: func() *resource.ResourceClaim { + claim := validAllocatedClaim.DeepCopy() + claim.Status.Allocation = validAllocatedClaim.Status.Allocation.DeepCopy() + claim.Status.Allocation.NodeSelector = &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{{ + MatchExpressions: []core.NodeSelectorRequirement{{ + Key: "foo", + Operator: core.NodeSelectorOpIn, + Values: []string{"-1"}, + }}, + }}, + } + return claim + }(), + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + for i := 0; i < resource.ResourceClaimReservedForMaxSize; i++ { + claim.Status.ReservedFor = append(claim.Status.ReservedFor, + resource.ResourceClaimConsumerReference{ + Resource: "pods", + Name: fmt.Sprintf("foo-%d", i), + UID: types.UID(fmt.Sprintf("%d", i)), + }) + } + return claim + }, + }, } for name, scenario := range scenarios { diff --git a/pkg/apis/resource/validation/validation_resourceslice_test.go b/pkg/apis/resource/validation/validation_resourceslice_test.go index 7901a655acb..c4d57a2b09a 100644 --- a/pkg/apis/resource/validation/validation_resourceslice_test.go +++ b/pkg/apis/resource/validation/validation_resourceslice_test.go @@ -430,6 +430,23 @@ func TestValidateResourceSlice(t *testing.T) { return slice }(), }, + "invalid-node-selecor-label-value": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "nodeSelector", "nodeSelectorTerms").Index(0).Child("matchExpressions").Index(0).Child("values").Index(0), "-1", "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')")}, + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSlice(goodName, goodName, goodName, 3) + slice.Spec.NodeName = "" + slice.Spec.NodeSelector = &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{{ + MatchExpressions: []core.NodeSelectorRequirement{{ + Key: "foo", + Operator: core.NodeSelectorOpIn, + Values: []string{"-1"}, + }}, + }}, + } + return slice + }(), + }, } for name, scenario := range scenarios { @@ -485,6 +502,35 @@ func TestValidateResourceSliceUpdate(t *testing.T) { return slice }, }, + "invalid-update-to-invalid-nodeselector-label-value": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "nodeSelector", "nodeSelectorTerms").Index(0).Child("matchExpressions").Index(0).Child("values").Index(0), "-1", "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')")}, + oldResourceSlice: func() *resourceapi.ResourceSlice { + slice := validResourceSlice.DeepCopy() + slice.Spec.NodeName = "" + slice.Spec.NodeSelector = &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{{ + MatchExpressions: []core.NodeSelectorRequirement{{ + Key: "foo", + Operator: core.NodeSelectorOpIn, + Values: []string{"bar"}, + }}, + }}, + } + return slice + }(), + update: func(slice *resourceapi.ResourceSlice) *resourceapi.ResourceSlice { + slice.Spec.NodeSelector = &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{{ + MatchExpressions: []core.NodeSelectorRequirement{{ + Key: "foo", + Operator: core.NodeSelectorOpIn, + Values: []string{"-1"}, + }}, + }}, + } + return slice + }, + }, } for name, scenario := range scenarios {