Merge pull request #128932 from pohly/dra-node-selector-validation

DRA API: validate node selector labels
This commit is contained in:
Kubernetes Prow Robot 2024-11-22 20:22:55 +00:00 committed by GitHub
commit e4c1f980b7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 113 additions and 12 deletions

View File

@ -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"))
}

View File

@ -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.

View File

@ -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 {

View File

@ -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 {