Merge pull request #116576 from pohly/dra-core-validation

api: extend validation of dynamic resource allocation fields in PodSpec
This commit is contained in:
Kubernetes Prow Robot 2023-03-14 16:34:48 -07:00 committed by GitHub
commit f315a4669a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 101 additions and 42 deletions

View File

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

View File

@ -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 = &notLabel
return *spec
}(),
"invalid claim reference name": func() core.PodSpec {
spec := goodClaimReference.DeepCopy()
notLabel := ".foo_bar"
spec.ResourceClaims[0].Source.ResourceClaimName = &notLabel
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)
}
})
}
})
}
})
}

View File

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