github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-27 13:37:30 +00:00
Code Issues Projects Releases Wiki Activity

DRA API: validate node selector labels

Browse Source 
Previously, ValidateNodeSelector did not check that labels are valid. Now it
does for resource.k8s.io, regardless whether an object already was created with
invalid labels in an earlier Kubernetes release. Theoretically this is a
breaking change and could cause problems during an upgrade, but that is highly
unlikely in practice.

In contrast to node affinity, DRA does not ignore parse errors
(= uses NewNodeSelector, not NewLazyErrorNodeSelector), so invalid labels would
have been found instead of being silently ignored.

Even if some object has invalid labels, this only affects an alpha -> beta
upgrade which isn't guaranteed to work seamlessly.
This commit is contained in:
AxeZhan 2024-10-23 17:28:27 +08:00 committed by Patrick Ohly
parent cf480a3a1a
commit 3075a9ae96
4 changed files with 113 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
pkg/apis/core/validation/validation.go
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"))
}

4
pkg/apis/resource/validation/validation.go
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.

47
pkg/apis/resource/validation/validation_resourceclaim_test.go
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 {

46
pkg/apis/resource/validation/validation_resourceslice_test.go
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 {