From f70142a6a90ed1423d376006509f69328ea7329c Mon Sep 17 00:00:00 2001 From: Aaron Prindle Date: Thu, 18 Sep 2025 20:47:08 +0000 Subject: [PATCH] feat(validation-gen): migrate ResourceClaimStatus.Allocation NoModify validation to declarative validation Co-Authored-by: Yongrui Lin --- .../resource/v1/zz_generated.validations.go | 4 + .../v1beta1/zz_generated.validations.go | 4 + .../v1beta2/zz_generated.validations.go | 4 + pkg/apis/resource/validation/validation.go | 2 +- .../validation_resourceclaim_test.go | 2 +- .../declarative_validation_test.go | 81 +++++++++++++++++++ .../k8s.io/api/resource/v1/generated.proto | 1 + staging/src/k8s.io/api/resource/v1/types.go | 1 + .../api/resource/v1beta1/generated.proto | 1 + .../src/k8s.io/api/resource/v1beta1/types.go | 1 + .../api/resource/v1beta2/generated.proto | 1 + .../src/k8s.io/api/resource/v1beta2/types.go | 1 + 12 files changed, 101 insertions(+), 2 deletions(-) diff --git a/pkg/apis/resource/v1/zz_generated.validations.go b/pkg/apis/resource/v1/zz_generated.validations.go index a34a9db8c02..b397dd0dc22 100644 --- a/pkg/apis/resource/v1/zz_generated.validations.go +++ b/pkg/apis/resource/v1/zz_generated.validations.go @@ -836,6 +836,10 @@ func Validate_ResourceClaimStatus(ctx context.Context, op operation.Operation, f if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj); len(e) != 0 { earlyReturn = true } + if e := validate.UpdatePointer(ctx, op, fldPath, obj, oldObj, validate.NoModify); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } if earlyReturn { return // do not proceed } diff --git a/pkg/apis/resource/v1beta1/zz_generated.validations.go b/pkg/apis/resource/v1beta1/zz_generated.validations.go index a0627d2507f..9f9dfec686f 100644 --- a/pkg/apis/resource/v1beta1/zz_generated.validations.go +++ b/pkg/apis/resource/v1beta1/zz_generated.validations.go @@ -838,6 +838,10 @@ func Validate_ResourceClaimStatus(ctx context.Context, op operation.Operation, f if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj); len(e) != 0 { earlyReturn = true } + if e := validate.UpdatePointer(ctx, op, fldPath, obj, oldObj, validate.NoModify); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } if earlyReturn { return // do not proceed } diff --git a/pkg/apis/resource/v1beta2/zz_generated.validations.go b/pkg/apis/resource/v1beta2/zz_generated.validations.go index 728687695db..aafa2bad120 100644 --- a/pkg/apis/resource/v1beta2/zz_generated.validations.go +++ b/pkg/apis/resource/v1beta2/zz_generated.validations.go @@ -858,6 +858,10 @@ func Validate_ResourceClaimStatus(ctx context.Context, op operation.Operation, f if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj); len(e) != 0 { earlyReturn = true } + if e := validate.UpdatePointer(ctx, op, fldPath, obj, oldObj, validate.NoModify); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } if earlyReturn { return // do not proceed } diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index bf9fe54c30d..c1a8c43ae03 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -437,7 +437,7 @@ func validateResourceClaimStatusUpdate(status, oldStatus *resource.ResourceClaim // in this particular case, must not be validated again because // validation for new results is tighter than it was before. if oldStatus.Allocation != nil && status.Allocation != nil { - allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(status.Allocation, oldStatus.Allocation, fldPath.Child("allocation"))...) + allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(status.Allocation, oldStatus.Allocation, fldPath.Child("allocation")).WithOrigin("update").MarkCoveredByDeclarative()...) } else if status.Allocation != nil { allErrs = append(allErrs, validateAllocationResult(status.Allocation, fldPath.Child("allocation"), requestNames, false)...) } diff --git a/pkg/apis/resource/validation/validation_resourceclaim_test.go b/pkg/apis/resource/validation/validation_resourceclaim_test.go index e2208a15cde..630983b04f1 100644 --- a/pkg/apis/resource/validation/validation_resourceclaim_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaim_test.go @@ -1165,7 +1165,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) { claim := validAllocatedClaim.DeepCopy() claim.Status.Allocation.Devices.Results[0].Driver += "-2" return claim.Status.Allocation - }(), "field is immutable")}, + }(), "field is immutable").MarkCoveredByDeclarative()}, oldClaim: validAllocatedClaim, update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { claim.Status.Allocation.Devices.Results[0].Driver += "-2" diff --git a/pkg/registry/resource/resourceclaim/declarative_validation_test.go b/pkg/registry/resource/resourceclaim/declarative_validation_test.go index f5fa1004650..ea2d95a054c 100644 --- a/pkg/registry/resource/resourceclaim/declarative_validation_test.go +++ b/pkg/registry/resource/resourceclaim/declarative_validation_test.go @@ -647,6 +647,53 @@ func TestValidateStatusUpdateForDeclarative(t *testing.T) { tweakStatusReservedFor(generateResourceClaimReferences(256)...), ), }, + "valid status.allocation unchanged": { + old: mkResourceClaimWithStatus(), + update: mkResourceClaimWithStatus(), + }, + "valid status.allocation set from nil": { + old: mkValidResourceClaim(), + update: mkResourceClaimWithStatus(), + }, + "valid status.allocation cleared (Unset is allowed)": { + old: mkResourceClaimWithStatus(), + update: mkValidResourceClaim(), + }, + "invalid status.allocation changed device (NoModify)": { + old: mkResourceClaimWithStatus(), + update: tweakStatusAllocationDevice(mkResourceClaimWithStatus(), "device-different"), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("status", "allocation"), nil, "field is immutable").WithOrigin("update"), + }, + }, + "invalid status.allocation changed driver (NoModify)": { + old: mkResourceClaimWithStatus(), + update: tweakStatusAllocationDriver(mkResourceClaimWithStatus(), "different.example.com"), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("status", "allocation"), nil, "field is immutable").WithOrigin("update"), + }, + }, + "invalid status.allocation changed pool (NoModify)": { + old: mkResourceClaimWithStatus(), + update: tweakStatusAllocationPool(mkResourceClaimWithStatus(), "different-pool"), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("status", "allocation"), nil, "field is immutable").WithOrigin("update"), + }, + }, + "invalid status.allocation added result (NoModify)": { + old: mkResourceClaimWithStatus(), + update: addStatusAllocationResult(mkResourceClaimWithStatus()), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("status", "allocation"), nil, "field is immutable").WithOrigin("update"), + }, + }, + "invalid status.allocation removed result (NoModify)": { + old: addStatusAllocationResult(mkResourceClaimWithStatus()), + update: mkResourceClaimWithStatus(), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("status", "allocation"), nil, "field is immutable").WithOrigin("update"), + }, + }, } for k, tc := range testCases { t.Run(k, func(t *testing.T) { @@ -849,3 +896,37 @@ func tweakFirstAvailableTolerations(tolerations []resource.DeviceToleration) fun } } } + +func tweakStatusAllocationDevice(obj resource.ResourceClaim, device string) resource.ResourceClaim { + if obj.Status.Allocation != nil && len(obj.Status.Allocation.Devices.Results) > 0 { + obj.Status.Allocation.Devices.Results[0].Device = device + } + return obj +} + +func tweakStatusAllocationDriver(obj resource.ResourceClaim, driver string) resource.ResourceClaim { + if obj.Status.Allocation != nil && len(obj.Status.Allocation.Devices.Results) > 0 { + obj.Status.Allocation.Devices.Results[0].Driver = driver + } + return obj +} + +func tweakStatusAllocationPool(obj resource.ResourceClaim, pool string) resource.ResourceClaim { + if obj.Status.Allocation != nil && len(obj.Status.Allocation.Devices.Results) > 0 { + obj.Status.Allocation.Devices.Results[0].Pool = pool + } + return obj +} + +func addStatusAllocationResult(obj resource.ResourceClaim) resource.ResourceClaim { + if obj.Status.Allocation != nil { + obj.Status.Allocation.Devices.Results = append(obj.Status.Allocation.Devices.Results, + resource.DeviceRequestAllocationResult{ + Request: "req-0", + Driver: "another.example.com", + Pool: "pool-1", + Device: "device-1", + }) + } + return obj +} diff --git a/staging/src/k8s.io/api/resource/v1/generated.proto b/staging/src/k8s.io/api/resource/v1/generated.proto index 556acaa6d52..3ab712aadc0 100644 --- a/staging/src/k8s.io/api/resource/v1/generated.proto +++ b/staging/src/k8s.io/api/resource/v1/generated.proto @@ -1362,6 +1362,7 @@ message ResourceClaimStatus { // // +optional // +k8s:optional + // +k8s:update=NoModify optional AllocationResult allocation = 1; // ReservedFor indicates which entities are currently allowed to use diff --git a/staging/src/k8s.io/api/resource/v1/types.go b/staging/src/k8s.io/api/resource/v1/types.go index 2304472d3da..0c96f1f05fc 100644 --- a/staging/src/k8s.io/api/resource/v1/types.go +++ b/staging/src/k8s.io/api/resource/v1/types.go @@ -1365,6 +1365,7 @@ type ResourceClaimStatus struct { // // +optional // +k8s:optional + // +k8s:update=NoModify Allocation *AllocationResult `json:"allocation,omitempty" protobuf:"bytes,1,opt,name=allocation"` // ReservedFor indicates which entities are currently allowed to use diff --git a/staging/src/k8s.io/api/resource/v1beta1/generated.proto b/staging/src/k8s.io/api/resource/v1beta1/generated.proto index 2e520f8a691..aa33e7e0e21 100644 --- a/staging/src/k8s.io/api/resource/v1beta1/generated.proto +++ b/staging/src/k8s.io/api/resource/v1beta1/generated.proto @@ -1376,6 +1376,7 @@ message ResourceClaimStatus { // // +optional // +k8s:optional + // +k8s:update=NoModify optional AllocationResult allocation = 1; // ReservedFor indicates which entities are currently allowed to use diff --git a/staging/src/k8s.io/api/resource/v1beta1/types.go b/staging/src/k8s.io/api/resource/v1beta1/types.go index 50e6ac0afdd..8dc41feb9fc 100644 --- a/staging/src/k8s.io/api/resource/v1beta1/types.go +++ b/staging/src/k8s.io/api/resource/v1beta1/types.go @@ -1372,6 +1372,7 @@ type ResourceClaimStatus struct { // // +optional // +k8s:optional + // +k8s:update=NoModify Allocation *AllocationResult `json:"allocation,omitempty" protobuf:"bytes,1,opt,name=allocation"` // ReservedFor indicates which entities are currently allowed to use diff --git a/staging/src/k8s.io/api/resource/v1beta2/generated.proto b/staging/src/k8s.io/api/resource/v1beta2/generated.proto index 6bea9212f18..6073ea4ba61 100644 --- a/staging/src/k8s.io/api/resource/v1beta2/generated.proto +++ b/staging/src/k8s.io/api/resource/v1beta2/generated.proto @@ -1362,6 +1362,7 @@ message ResourceClaimStatus { // // +optional // +k8s:optional + // +k8s:update=NoModify optional AllocationResult allocation = 1; // ReservedFor indicates which entities are currently allowed to use diff --git a/staging/src/k8s.io/api/resource/v1beta2/types.go b/staging/src/k8s.io/api/resource/v1beta2/types.go index 179127d47ed..41ce8b7800a 100644 --- a/staging/src/k8s.io/api/resource/v1beta2/types.go +++ b/staging/src/k8s.io/api/resource/v1beta2/types.go @@ -1365,6 +1365,7 @@ type ResourceClaimStatus struct { // // +optional // +k8s:optional + // +k8s:update=NoModify Allocation *AllocationResult `json:"allocation,omitempty" protobuf:"bytes,1,opt,name=allocation"` // ReservedFor indicates which entities are currently allowed to use