diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index 9191bf5a348..fcbf8c55c18 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -239,15 +239,7 @@ func ValidatePodScheduling(resourceClaim *resource.PodScheduling) field.ErrorLis } func validatePodSchedulingSpec(spec *resource.PodSchedulingSpec, fldPath *field.Path) field.ErrorList { - var allErrs field.ErrorList - // Checking PotentialNodes for duplicates is intentionally not done. It - // could be fairly expensive and the only component which normally has - // permissions to set this field, kube-scheduler, is a trusted - // component. Also, if it gets this wrong because of a bug, then the - // effect is limited (same semantic). - if len(spec.PotentialNodes) > resource.PodSchedulingNodeListMaxSize { - allErrs = append(allErrs, field.TooLongMaxLength(fldPath.Child("potentialNodes"), nil, resource.PodSchedulingNodeListMaxSize)) - } + allErrs := validateSliceIsASet(spec.PotentialNodes, resource.PodSchedulingNodeListMaxSize, validateNodeName, fldPath.Child("potentialNodes")) return allErrs } @@ -283,15 +275,8 @@ func validatePodSchedulingClaims(claimStatuses []resource.ResourceClaimSchedulin return allErrs } -func validatePodSchedulingClaim(claim resource.ResourceClaimSchedulingStatus, fldPath *field.Path) field.ErrorList { - var allErrs field.ErrorList - // Checking UnsuitableNodes for duplicates is intentionally not done. It - // could be fairly expensive and if a resource driver gets this wrong, - // then it is only going to have a negative effect for the pods relying - // on this driver. - if len(claim.UnsuitableNodes) > resource.PodSchedulingNodeListMaxSize { - allErrs = append(allErrs, field.TooLongMaxLength(fldPath.Child("unsuitableNodes"), nil, resource.PodSchedulingNodeListMaxSize)) - } +func validatePodSchedulingClaim(status resource.ResourceClaimSchedulingStatus, fldPath *field.Path) field.ErrorList { + allErrs := validateSliceIsASet(status.UnsuitableNodes, resource.PodSchedulingNodeListMaxSize, validateNodeName, fldPath.Child("unsuitableNodes")) return allErrs } @@ -315,3 +300,11 @@ func ValidateClaimTemplateUpdate(template, oldTemplate *resource.ResourceClaimTe allErrs = append(allErrs, ValidateClaimTemplate(template)...) return allErrs } + +func validateNodeName(name string, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + for _, msg := range corevalidation.ValidateNodeName(name, false) { + allErrs = append(allErrs, field.Invalid(fldPath, name, msg)) + } + return allErrs +} diff --git a/pkg/apis/resource/validation/validation_podscheduling_test.go b/pkg/apis/resource/validation/validation_podscheduling_test.go index 138f70004bf..30193eb5a3a 100644 --- a/pkg/apis/resource/validation/validation_podscheduling_test.go +++ b/pkg/apis/resource/validation/validation_podscheduling_test.go @@ -194,6 +194,7 @@ func TestValidatePodScheduling(t *testing.T) { func TestValidatePodSchedulingUpdate(t *testing.T) { validScheduling := testPodScheduling("foo", "ns", resource.PodSchedulingSpec{}) + badName := "!@#$%^" scenarios := map[string]struct { oldScheduling *resource.PodScheduling @@ -220,8 +221,8 @@ func TestValidatePodSchedulingUpdate(t *testing.T) { return scheduling }, }, - "invalid-potential-nodes": { - wantFailures: field.ErrorList{field.TooLongMaxLength(field.NewPath("spec", "potentialNodes"), nil, resource.PodSchedulingNodeListMaxSize)}, + "invalid-potential-nodes-too-long": { + wantFailures: field.ErrorList{field.TooLongMaxLength(field.NewPath("spec", "potentialNodes"), 129, resource.PodSchedulingNodeListMaxSize)}, oldScheduling: validScheduling, update: func(scheduling *resource.PodScheduling) *resource.PodScheduling { for i := 0; i < resource.PodSchedulingNodeListMaxSize+1; i++ { @@ -230,6 +231,14 @@ func TestValidatePodSchedulingUpdate(t *testing.T) { return scheduling }, }, + "invalid-potential-nodes-name": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "potentialNodes").Index(0), badName, "a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')")}, + oldScheduling: validScheduling, + update: func(scheduling *resource.PodScheduling) *resource.PodScheduling { + scheduling.Spec.PotentialNodes = append(scheduling.Spec.PotentialNodes, badName) + return scheduling + }, + }, } for name, scenario := range scenarios { @@ -243,6 +252,7 @@ func TestValidatePodSchedulingUpdate(t *testing.T) { func TestValidatePodSchedulingStatusUpdate(t *testing.T) { validScheduling := testPodScheduling("foo", "ns", resource.PodSchedulingSpec{}) + badName := "!@#$%^" scenarios := map[string]struct { oldScheduling *resource.PodScheduling @@ -283,7 +293,7 @@ func TestValidatePodSchedulingStatusUpdate(t *testing.T) { }, }, "invalid-too-long-claim-status": { - wantFailures: field.ErrorList{field.TooLongMaxLength(field.NewPath("status", "claims").Index(0).Child("unsuitableNodes"), nil, resource.PodSchedulingNodeListMaxSize)}, + wantFailures: field.ErrorList{field.TooLongMaxLength(field.NewPath("status", "claims").Index(0).Child("unsuitableNodes"), 129, resource.PodSchedulingNodeListMaxSize)}, oldScheduling: validScheduling, update: func(scheduling *resource.PodScheduling) *resource.PodScheduling { scheduling.Status.ResourceClaims = append(scheduling.Status.ResourceClaims, @@ -300,6 +310,22 @@ func TestValidatePodSchedulingStatusUpdate(t *testing.T) { return scheduling }, }, + "invalid-node-name": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("status", "claims").Index(0).Child("unsuitableNodes").Index(0), badName, "a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')")}, + oldScheduling: validScheduling, + update: func(scheduling *resource.PodScheduling) *resource.PodScheduling { + scheduling.Status.ResourceClaims = append(scheduling.Status.ResourceClaims, + resource.ResourceClaimSchedulingStatus{ + Name: "my-claim", + }, + ) + scheduling.Status.ResourceClaims[0].UnsuitableNodes = append( + scheduling.Status.ResourceClaims[0].UnsuitableNodes, + badName, + ) + return scheduling + }, + }, } for name, scenario := range scenarios {