From da0b75f8f9ae40a7b785c178aa5edb659fb9e3c5 Mon Sep 17 00:00:00 2001 From: Kevin Klues Date: Tue, 7 Mar 2023 15:09:52 +0000 Subject: [PATCH] Update validation for recent changes to resource.k8s.io/v1alpha2 Signed-off-by: Kevin Klues --- pkg/apis/resource/validation/validation.go | 27 ++++- .../validation_resourceclaim_test.go | 103 +++++++++++++++++- 2 files changed, 122 insertions(+), 8 deletions(-) diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index a76d64e309e..f899a494e33 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -155,6 +155,11 @@ func ValidateClaimStatusUpdate(resourceClaim, oldClaim *resource.ResourceClaim) } } + // Updates to a populated resourceClaim.Status.Allocation are not allowed + if oldClaim.Status.Allocation != nil && resourceClaim.Status.Allocation != nil { + allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(resourceClaim.Status.Allocation, oldClaim.Status.Allocation, fldPath.Child("allocation"))...) + } + if !oldClaim.Status.DeallocationRequested && resourceClaim.Status.DeallocationRequested && len(resourceClaim.Status.ReservedFor) > 0 { @@ -185,8 +190,8 @@ func ValidateClaimStatusUpdate(resourceClaim, oldClaim *resource.ResourceClaim) func validateAllocationResult(allocation *resource.AllocationResult, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList if allocation != nil { - if len(allocation.ResourceHandle) > resource.ResourceHandleMaxSize { - allErrs = append(allErrs, field.TooLongMaxLength(fldPath.Child("resourceHandle"), len(allocation.ResourceHandle), resource.ResourceHandleMaxSize)) + if len(allocation.ResourceHandles) > 0 { + allErrs = append(allErrs, validateResourceHandles(allocation.ResourceHandles, resource.AllocationResultResourceHandlesMaxSize, fldPath.Child("resourceHandles"))...) } if allocation.AvailableOnNodes != nil { allErrs = append(allErrs, corevalidation.ValidateNodeSelector(allocation.AvailableOnNodes, fldPath.Child("availableOnNodes"))...) @@ -195,6 +200,24 @@ func validateAllocationResult(allocation *resource.AllocationResult, fldPath *fi return allErrs } +func validateResourceHandles(resourceHandles []resource.ResourceHandle, maxSize int, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + for i, resourceHandle := range resourceHandles { + idxPath := fldPath.Index(i) + allErrs = append(allErrs, validateResourceDriverName(resourceHandle.DriverName, idxPath.Child("driverName"))...) + if len(resourceHandle.Data) > resource.ResourceHandleDataMaxSize { + allErrs = append(allErrs, field.TooLongMaxLength(idxPath.Child("data"), len(resourceHandle.Data), resource.ResourceHandleDataMaxSize)) + } + } + if len(resourceHandles) > maxSize { + // Dumping the entire field into the error message is likely to be too long, + // in particular when it is already beyond the maximum size. Instead this + // just shows the number of entries. + allErrs = append(allErrs, field.TooLongMaxLength(fldPath, len(resourceHandles), maxSize)) + } + return allErrs +} + func validateResourceClaimUserReference(ref resource.ResourceClaimConsumerReference, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList if ref.Resource == "" { diff --git a/pkg/apis/resource/validation/validation_resourceclaim_test.go b/pkg/apis/resource/validation/validation_resourceclaim_test.go index 48b817a9f66..962336e3a5d 100644 --- a/pkg/apis/resource/validation/validation_resourceclaim_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaim_test.go @@ -315,6 +315,7 @@ func TestValidateClaimUpdate(t *testing.T) { } func TestValidateClaimStatusUpdate(t *testing.T) { + invalidName := "!@#$%^" validClaim := testClaim("foo", "ns", resource.ResourceClaimSpec{ ResourceClassName: "valid", AllocationMode: resource.AllocationModeImmediate, @@ -324,8 +325,18 @@ func TestValidateClaimStatusUpdate(t *testing.T) { validAllocatedClaim.Status = resource.ResourceClaimStatus{ DriverName: "valid", Allocation: &resource.AllocationResult{ - ResourceHandle: strings.Repeat(" ", resource.ResourceHandleMaxSize), - Shareable: true, + ResourceHandles: func() []resource.ResourceHandle { + var handles []resource.ResourceHandle + for i := 0; i < resource.AllocationResultResourceHandlesMaxSize; i++ { + handle := resource.ResourceHandle{ + DriverName: "valid", + Data: strings.Repeat(" ", resource.ResourceHandleDataMaxSize), + } + handles = append(handles, handle) + } + return handles + }(), + Shareable: true, }, } @@ -359,18 +370,60 @@ func TestValidateClaimStatusUpdate(t *testing.T) { update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { claim.Status.DriverName = "valid" claim.Status.Allocation = &resource.AllocationResult{ - ResourceHandle: strings.Repeat(" ", resource.ResourceHandleMaxSize), + ResourceHandles: []resource.ResourceHandle{ + { + DriverName: "valid", + Data: strings.Repeat(" ", resource.ResourceHandleDataMaxSize), + }, + }, } return claim }, }, - "invalid-allocation-handle": { - wantFailures: field.ErrorList{field.TooLongMaxLength(field.NewPath("status", "allocation", "resourceHandle"), resource.ResourceHandleMaxSize+1, resource.ResourceHandleMaxSize)}, + "invalid-allocation-resourceHandles": { + wantFailures: field.ErrorList{field.TooLongMaxLength(field.NewPath("status", "allocation", "resourceHandles"), resource.AllocationResultResourceHandlesMaxSize+1, resource.AllocationResultResourceHandlesMaxSize)}, oldClaim: validClaim, update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { claim.Status.DriverName = "valid" claim.Status.Allocation = &resource.AllocationResult{ - ResourceHandle: strings.Repeat(" ", resource.ResourceHandleMaxSize+1), + ResourceHandles: func() []resource.ResourceHandle { + var handles []resource.ResourceHandle + for i := 0; i < resource.AllocationResultResourceHandlesMaxSize+1; i++ { + handles = append(handles, resource.ResourceHandle{DriverName: "valid"}) + } + return handles + }(), + } + return claim + }, + }, + "invalid-allocation-resource-handle-drivername": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("status", "allocation", "resourceHandles[0]", "driverName"), invalidName, "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])?)*')")}, + oldClaim: validClaim, + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + claim.Status.DriverName = "valid" + claim.Status.Allocation = &resource.AllocationResult{ + ResourceHandles: []resource.ResourceHandle{ + { + DriverName: invalidName, + }, + }, + } + return claim + }, + }, + "invalid-allocation-resource-handle-data": { + wantFailures: field.ErrorList{field.TooLongMaxLength(field.NewPath("status", "allocation", "resourceHandles[0]", "data"), resource.ResourceHandleDataMaxSize+1, resource.ResourceHandleDataMaxSize)}, + oldClaim: validClaim, + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + claim.Status.DriverName = "valid" + claim.Status.Allocation = &resource.AllocationResult{ + ResourceHandles: []resource.ResourceHandle{ + { + DriverName: "valid", + Data: strings.Repeat(" ", resource.ResourceHandleDataMaxSize+1), + }, + }, } return claim }, @@ -564,6 +617,18 @@ func TestValidateClaimStatusUpdate(t *testing.T) { return claim }, }, + "remove-allocation": { + oldClaim: func() *resource.ResourceClaim { + claim := validAllocatedClaim.DeepCopy() + claim.Status.DeallocationRequested = true + return claim + }(), + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + claim.Status.DeallocationRequested = false + claim.Status.Allocation = nil + return claim + }, + }, "invalid-deallocation-requested-removal": { wantFailures: field.ErrorList{field.Forbidden(field.NewPath("status", "deallocationRequested"), "may not be cleared when `allocation` is set")}, oldClaim: func() *resource.ResourceClaim { @@ -576,6 +641,32 @@ func TestValidateClaimStatusUpdate(t *testing.T) { return claim }, }, + "invalid-allocation-modification": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("status.allocation"), func() *resource.AllocationResult { + claim := validAllocatedClaim.DeepCopy() + claim.Status.Allocation.ResourceHandles = []resource.ResourceHandle{ + { + DriverName: "valid", + Data: strings.Repeat(" ", resource.ResourceHandleDataMaxSize/2), + }, + } + return claim.Status.Allocation + }(), "field is immutable")}, + oldClaim: func() *resource.ResourceClaim { + claim := validAllocatedClaim.DeepCopy() + claim.Status.DeallocationRequested = false + return claim + }(), + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + claim.Status.Allocation.ResourceHandles = []resource.ResourceHandle{ + { + DriverName: "valid", + Data: strings.Repeat(" ", resource.ResourceHandleDataMaxSize/2), + }, + } + return claim + }, + }, "invalid-deallocation-requested-in-use": { wantFailures: field.ErrorList{field.Forbidden(field.NewPath("status", "deallocationRequested"), "deallocation cannot be requested while `reservedFor` is set")}, oldClaim: func() *resource.ResourceClaim {