github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-21 10:51:29 +00:00
Code Issues Projects Releases Wiki Activity

api: extend validation of dynamic resource allocation fields in PodSpec

Browse Source 
The generated ResourceClaim name and the names of the ResourceClaimTemplate and
ResourceClaim referenced by a pod must be valid according to the resource API,
otherwise the pod cannot start.

Checking this was removed from the original implementation out of concerns
about validating fields in core against limitations imposed by a separate,
alpha API.  But as this was pointed out again in
https://github.com/kubernetes/kubernetes/pull/116254#discussion_r1134010324
it gets added back.

The same strings that worked before still work now. In particular, the
constraints for a spec.resourceClaim.name are still the same (DNS label).
This commit is contained in:
Patrick Ohly 2023-03-14 11:58:41 +01:00
parent ee18f60252
commit e97531b349
3 changed files with 101 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

95
pkg/apis/core/validation/validation_test.go
View File

@ -23680,34 +23680,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,
@ -23738,7 +23746,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)
}
})
@ -23883,10 +23891,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)
}
})
}
})
}
})
}

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