diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 4d910e9c5b9..a7e2377cdbf 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -299,6 +299,14 @@ var ValidateClassName = apimachineryvalidation.NameIsDNSSubdomain // class name is valid. var ValidatePriorityClassName = apimachineryvalidation.NameIsDNSSubdomain +// ValidateResourceClaimName can be used to check whether the given +// name for a ResourceClaim is valid. +var ValidateResourceClaimName = apimachineryvalidation.NameIsDNSSubdomain + +// ValidateResourceClaimTemplateName can be used to check whether the given +// name for a ResourceClaimTemplate is valid. +var ValidateResourceClaimTemplateName = apimachineryvalidation.NameIsDNSSubdomain + // ValidateRuntimeClassName can be used to check whether the given RuntimeClass name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. @@ -2747,11 +2755,11 @@ func ValidateVolumeDevices(devices []core.VolumeDevice, volmounts map[string]str return allErrs } -func validatePodResourceClaims(claims []core.PodResourceClaim, fldPath *field.Path) field.ErrorList { +func validatePodResourceClaims(podMeta *metav1.ObjectMeta, claims []core.PodResourceClaim, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList podClaimNames := sets.NewString() for i, claim := range claims { - allErrs = append(allErrs, validatePodResourceClaim(claim, &podClaimNames, fldPath.Index(i))...) + allErrs = append(allErrs, validatePodResourceClaim(podMeta, claim, &podClaimNames, fldPath.Index(i))...) } return allErrs } @@ -2769,14 +2777,22 @@ func gatherPodResourceClaimNames(claims []core.PodResourceClaim) sets.String { return podClaimNames } -func validatePodResourceClaim(claim core.PodResourceClaim, podClaimNames *sets.String, fldPath *field.Path) field.ErrorList { +func validatePodResourceClaim(podMeta *metav1.ObjectMeta, claim core.PodResourceClaim, podClaimNames *sets.String, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList if claim.Name == "" { allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) } else if podClaimNames.Has(claim.Name) { allErrs = append(allErrs, field.Duplicate(fldPath.Child("name"), claim.Name)) } else { - allErrs = append(allErrs, ValidateDNS1123Label(claim.Name, fldPath.Child("name"))...) + nameErrs := ValidateDNS1123Label(claim.Name, fldPath.Child("name")) + if len(nameErrs) > 0 { + allErrs = append(allErrs, nameErrs...) + } else if podMeta != nil && claim.Source.ResourceClaimTemplateName != nil { + claimName := podMeta.Name + "-" + claim.Name + for _, detail := range ValidateResourceClaimName(claimName, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), claimName, "final ResourceClaim name: "+detail)) + } + } podClaimNames.Insert(claim.Name) } allErrs = append(allErrs, validatePodResourceClaimSource(claim.Source, fldPath.Child("source"))...) @@ -2792,6 +2808,16 @@ func validatePodResourceClaimSource(claimSource core.ClaimSource, fldPath *field if claimSource.ResourceClaimName == nil && claimSource.ResourceClaimTemplateName == nil { allErrs = append(allErrs, field.Invalid(fldPath, claimSource, "must specify one of: `resourceClaimName`, `resourceClaimTemplateName`")) } + if claimSource.ResourceClaimName != nil { + for _, detail := range ValidateResourceClaimName(*claimSource.ResourceClaimName, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("resourceClaimName"), *claimSource.ResourceClaimName, detail)) + } + } + if claimSource.ResourceClaimTemplateName != nil { + for _, detail := range ValidateResourceClaimTemplateName(*claimSource.ResourceClaimTemplateName, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("resourceClaimTemplateName"), *claimSource.ResourceClaimTemplateName, detail)) + } + } return allErrs } @@ -3759,7 +3785,7 @@ func ValidatePodSpec(spec *core.PodSpec, podMeta *metav1.ObjectMeta, fldPath *fi vols, vErrs := ValidateVolumes(spec.Volumes, podMeta, fldPath.Child("volumes"), opts) allErrs = append(allErrs, vErrs...) podClaimNames := gatherPodResourceClaimNames(spec.ResourceClaims) - allErrs = append(allErrs, validatePodResourceClaims(spec.ResourceClaims, fldPath.Child("resourceClaims"))...) + allErrs = append(allErrs, validatePodResourceClaims(podMeta, spec.ResourceClaims, fldPath.Child("resourceClaims"))...) allErrs = append(allErrs, validateContainers(spec.Containers, vols, podClaimNames, fldPath.Child("containers"), opts)...) allErrs = append(allErrs, validateInitContainers(spec.InitContainers, spec.Containers, vols, podClaimNames, fldPath.Child("initContainers"), opts)...) allErrs = append(allErrs, validateEphemeralContainers(spec.EphemeralContainers, spec.Containers, spec.InitContainers, vols, podClaimNames, fldPath.Child("ephemeralContainers"), opts)...) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 793b067a2ad..29346e5c61f 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -23693,34 +23693,42 @@ func TestValidateDynamicResourceAllocation(t *testing.T) { goodClaimSource := core.ClaimSource{ ResourceClaimName: &externalClaimName, } + shortPodName := &metav1.ObjectMeta{ + Name: "some-pod", + } + brokenPodName := &metav1.ObjectMeta{ + Name: ".dot.com", + } + goodClaimTemplate := core.PodSpec{ + Containers: []core.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", Resources: core.ResourceRequirements{Claims: []core.ResourceClaim{{Name: "my-claim-template"}}}}}, + RestartPolicy: core.RestartPolicyAlways, + DNSPolicy: core.DNSClusterFirst, + ResourceClaims: []core.PodResourceClaim{ + { + Name: "my-claim-template", + Source: core.ClaimSource{ + ResourceClaimTemplateName: &externalClaimTemplateName, + }, + }, + }, + } + goodClaimReference := core.PodSpec{ + Containers: []core.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", Resources: core.ResourceRequirements{Claims: []core.ResourceClaim{{Name: "my-claim-reference"}}}}}, + RestartPolicy: core.RestartPolicyAlways, + DNSPolicy: core.DNSClusterFirst, + ResourceClaims: []core.PodResourceClaim{ + { + Name: "my-claim-reference", + Source: core.ClaimSource{ + ResourceClaimName: &externalClaimName, + }, + }, + }, + } successCases := map[string]core.PodSpec{ - "resource claim reference": { - Containers: []core.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", Resources: core.ResourceRequirements{Claims: []core.ResourceClaim{{Name: "my-claim"}}}}}, - RestartPolicy: core.RestartPolicyAlways, - DNSPolicy: core.DNSClusterFirst, - ResourceClaims: []core.PodResourceClaim{ - { - Name: "my-claim", - Source: core.ClaimSource{ - ResourceClaimName: &externalClaimName, - }, - }, - }, - }, - "resource claim template": { - Containers: []core.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", Resources: core.ResourceRequirements{Claims: []core.ResourceClaim{{Name: "my-claim"}}}}}, - RestartPolicy: core.RestartPolicyAlways, - DNSPolicy: core.DNSClusterFirst, - ResourceClaims: []core.PodResourceClaim{ - { - Name: "my-claim", - Source: core.ClaimSource{ - ResourceClaimTemplateName: &externalClaimTemplateName, - }, - }, - }, - }, + "resource claim reference": goodClaimTemplate, + "resource claim template": goodClaimTemplate, "multiple claims": { Containers: []core.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", Resources: core.ResourceRequirements{Claims: []core.ResourceClaim{{Name: "my-claim"}, {Name: "another-claim"}}}}}, RestartPolicy: core.RestartPolicyAlways, @@ -23751,7 +23759,7 @@ func TestValidateDynamicResourceAllocation(t *testing.T) { } for k, v := range successCases { t.Run(k, func(t *testing.T) { - if errs := ValidatePodSpec(&v, nil, field.NewPath("field"), PodValidationOptions{}); len(errs) != 0 { + if errs := ValidatePodSpec(&v, shortPodName, field.NewPath("field"), PodValidationOptions{}); len(errs) != 0 { t.Errorf("expected success: %v", errs) } }) @@ -23896,10 +23904,43 @@ func TestValidateDynamicResourceAllocation(t *testing.T) { }, }, }, + "invalid claim template name": func() core.PodSpec { + spec := goodClaimTemplate.DeepCopy() + notLabel := ".foo_bar" + spec.ResourceClaims[0].Source.ResourceClaimTemplateName = ¬Label + return *spec + }(), + "invalid claim reference name": func() core.PodSpec { + spec := goodClaimReference.DeepCopy() + notLabel := ".foo_bar" + spec.ResourceClaims[0].Source.ResourceClaimName = ¬Label + return *spec + }(), } for k, v := range failureCases { if errs := ValidatePodSpec(&v, nil, field.NewPath("field"), PodValidationOptions{}); len(errs) == 0 { t.Errorf("expected failure for %q", k) } } + + t.Run("generated-claim-name", func(t *testing.T) { + for _, spec := range []*core.PodSpec{&goodClaimTemplate, &goodClaimReference} { + claimName := spec.ResourceClaims[0].Name + t.Run(claimName, func(t *testing.T) { + for _, podMeta := range []*metav1.ObjectMeta{shortPodName, brokenPodName} { + t.Run(podMeta.Name, func(t *testing.T) { + errs := ValidatePodSpec(spec, podMeta, field.NewPath("field"), PodValidationOptions{}) + // Only one out of the four combinations fails. + expectError := spec == &goodClaimTemplate && podMeta == brokenPodName + if expectError && len(errs) == 0 { + t.Error("did not get the expected failure") + } + if !expectError && len(errs) > 0 { + t.Errorf("unexpected failures: %+v", errs) + } + }) + } + }) + } + }) } diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index a76d64e309e..b6ae2640058 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -25,21 +25,13 @@ import ( "k8s.io/kubernetes/pkg/apis/resource" ) -// validateResourceClaimName can be used to check whether the given -// name for a ResourceClaim is valid. -var validateResourceClaimName = apimachineryvalidation.NameIsDNSSubdomain - -// validateResourceClaimTemplateName can be used to check whether the given -// name for a ResourceClaimTemplate is valid. -var validateResourceClaimTemplateName = apimachineryvalidation.NameIsDNSSubdomain - // validateResourceDriverName reuses the validation of a CSI driver because // the allowed values are exactly the same. var validateResourceDriverName = corevalidation.ValidateCSIDriverName // ValidateClaim validates a ResourceClaim. func ValidateClaim(resourceClaim *resource.ResourceClaim) field.ErrorList { - allErrs := corevalidation.ValidateObjectMeta(&resourceClaim.ObjectMeta, true, validateResourceClaimName, field.NewPath("metadata")) + allErrs := corevalidation.ValidateObjectMeta(&resourceClaim.ObjectMeta, true, corevalidation.ValidateResourceClaimName, field.NewPath("metadata")) allErrs = append(allErrs, validateResourceClaimSpec(&resourceClaim.Spec, field.NewPath("spec"))...) return allErrs } @@ -304,7 +296,7 @@ func validatePodSchedulingClaim(status resource.ResourceClaimSchedulingStatus, f // ValidateClaimTemplace validates a ResourceClaimTemplate. func ValidateClaimTemplate(template *resource.ResourceClaimTemplate) field.ErrorList { - allErrs := corevalidation.ValidateObjectMeta(&template.ObjectMeta, true, validateResourceClaimTemplateName, field.NewPath("metadata")) + allErrs := corevalidation.ValidateObjectMeta(&template.ObjectMeta, true, corevalidation.ValidateResourceClaimTemplateName, field.NewPath("metadata")) allErrs = append(allErrs, validateResourceClaimTemplateSpec(&template.Spec, field.NewPath("spec"))...) return allErrs }