From 763c01ef68f4ddd18cdcd78bfeb61170a90a2f96 Mon Sep 17 00:00:00 2001 From: yongruilin Date: Fri, 3 Oct 2025 20:56:06 +0000 Subject: [PATCH] feat(validation): Add normalization rules for ResourceClaim validation --- .../declarative_validation_test.go | 130 +++++++++++++++++- .../resource/resourceclaim/strategy.go | 14 +- 2 files changed, 139 insertions(+), 5 deletions(-) diff --git a/pkg/registry/resource/resourceclaim/declarative_validation_test.go b/pkg/registry/resource/resourceclaim/declarative_validation_test.go index cfc722a6227..1682c88bdf4 100644 --- a/pkg/registry/resource/resourceclaim/declarative_validation_test.go +++ b/pkg/registry/resource/resourceclaim/declarative_validation_test.go @@ -84,11 +84,58 @@ func testDeclarativeValidate(t *testing.T, apiVersion string) { field.TooMany(field.NewPath("spec", "devices", "config"), 33, 32).WithOrigin("maxItems"), }, }, + "invalid firstAvailable, too many": { + input: mkValidResourceClaim(tweakFirstAvailable(9)), + expectedErrs: field.ErrorList{ + field.TooMany(field.NewPath("spec", "devices", "requests").Index(0).Child("firstAvailable"), 9, 8).WithOrigin("maxItems"), + }, + }, + "invalid selectors, too many": { + input: mkValidResourceClaim(tweakExactlySelectors(33)), + expectedErrs: field.ErrorList{ + field.TooMany(field.NewPath("spec", "devices", "requests").Index(0).Child("exactly", "selectors"), 33, 32).WithOrigin("maxItems").MarkCoveredByDeclarative(), + }, + }, + "invalid subrequest selectors, too many": { + input: mkValidResourceClaim(tweakSubRequestSelectors(33)), + expectedErrs: field.ErrorList{ + field.TooMany(field.NewPath("spec", "devices", "requests").Index(0).Child("firstAvailable").Index(0).Child("selectors"), 33, 32).WithOrigin("maxItems"), + }, + }, + "invalid constraint requests, too many": { + input: mkValidResourceClaim(tweakConstraintRequests(33)), + expectedErrs: field.ErrorList{ + field.TooMany(field.NewPath("spec", "devices", "requests"), 33, 32).WithOrigin("maxItems"), + field.TooMany(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests"), 33, 32).WithOrigin("maxItems"), + }, + }, + "invalid config requests, too many": { + input: mkValidResourceClaim(tweakConfigRequests(33)), + expectedErrs: field.ErrorList{ + field.TooMany(field.NewPath("spec", "devices", "requests"), 33, 32).WithOrigin("maxItems"), + field.TooMany(field.NewPath("spec", "devices", "config").Index(0).Child("requests"), 33, 32).WithOrigin("maxItems"), + }, + }, + "valid firstAvailable, max allowed": { + input: mkValidResourceClaim(tweakFirstAvailable(8)), + }, + "valid selectors, max allowed": { + input: mkValidResourceClaim(tweakExactlySelectors(32)), + }, + "valid subrequest selectors, max allowed": { + input: mkValidResourceClaim(tweakSubRequestSelectors(32)), + }, + "valid constraint requests, max allowed": { + input: mkValidResourceClaim(tweakConstraintRequests(32)), + }, + "valid config requests, max allowed": { + input: mkValidResourceClaim(tweakConfigRequests(32)), + }, // TODO: Add more test cases } for k, tc := range testCases { t.Run(k, func(t *testing.T) { - apitesting.VerifyValidationEquivalence(t, ctx, &tc.input, Strategy.Validate, tc.expectedErrs) + apitesting.VerifyValidationEquivalence(t, ctx, &tc.input, Strategy.Validate, tc.expectedErrs, apitesting.WithNormalizationRules(resourceClaimNormalizationRules...)) }) } } @@ -118,6 +165,83 @@ func tweakDevicesRequests(items int) func(*resource.ResourceClaim) { } } +func tweakExactlySelectors(items int) func(*resource.ResourceClaim) { + return func(rc *resource.ResourceClaim) { + for i := 0; i < items; i++ { + rc.Spec.Devices.Requests[0].Exactly.Selectors = append(rc.Spec.Devices.Requests[0].Exactly.Selectors, + resource.DeviceSelector{ + CEL: &resource.CELDeviceSelector{ + Expression: fmt.Sprintf("device.driver == \"test.driver.io%d\"", i), + }, + }, + ) + } + } +} + +func tweakSubRequestSelectors(items int) func(*resource.ResourceClaim) { + return func(rc *resource.ResourceClaim) { + rc.Spec.Devices.Requests[0].Exactly = nil + rc.Spec.Devices.Requests[0].FirstAvailable = []resource.DeviceSubRequest{ + { + Name: "sub-0", + DeviceClassName: "class", + AllocationMode: resource.DeviceAllocationModeAll, + }, + } + for i := 0; i < items; i++ { + rc.Spec.Devices.Requests[0].FirstAvailable[0].Selectors = append(rc.Spec.Devices.Requests[0].FirstAvailable[0].Selectors, + resource.DeviceSelector{ + CEL: &resource.CELDeviceSelector{ + Expression: fmt.Sprintf("device.driver == \"test.driver.io%d\"", i), + }, + }, + ) + } + } +} + +func tweakConstraintRequests(count int) func(*resource.ResourceClaim) { + return func(rc *resource.ResourceClaim) { + tweakDevicesRequests(count)(rc) + if len(rc.Spec.Devices.Constraints) == 0 { + rc.Spec.Devices.Constraints = append(rc.Spec.Devices.Constraints, mkDeviceConstraint()) + } + rc.Spec.Devices.Constraints[0].Requests = []string{} + for i := 0; i < count; i++ { + rc.Spec.Devices.Constraints[0].Requests = append(rc.Spec.Devices.Constraints[0].Requests, fmt.Sprintf("req-%d", i)) + } + } +} + +func tweakConfigRequests(count int) func(*resource.ResourceClaim) { + return func(rc *resource.ResourceClaim) { + tweakDevicesRequests(count)(rc) + if len(rc.Spec.Devices.Config) == 0 { + rc.Spec.Devices.Config = append(rc.Spec.Devices.Config, mkDeviceClaimConfiguration()) + } + rc.Spec.Devices.Config[0].Requests = []string{} + for i := 0; i < count; i++ { + rc.Spec.Devices.Config[0].Requests = append(rc.Spec.Devices.Config[0].Requests, fmt.Sprintf("req-%d", i)) + } + } +} + +func tweakFirstAvailable(items int) func(*resource.ResourceClaim) { + return func(rc *resource.ResourceClaim) { + rc.Spec.Devices.Requests[0].Exactly = nil + for i := 0; i < items; i++ { + rc.Spec.Devices.Requests[0].FirstAvailable = append(rc.Spec.Devices.Requests[0].FirstAvailable, + resource.DeviceSubRequest{ + Name: fmt.Sprintf("sub-%d", i), + DeviceClassName: "class", + AllocationMode: resource.DeviceAllocationModeAll, + }, + ) + } + } +} + func mkDeviceClaimConfiguration() resource.DeviceClaimConfiguration { return resource.DeviceClaimConfiguration{ Requests: []string{"req-0"}, @@ -181,7 +305,7 @@ func testDeclarativeValidateUpdate(t *testing.T, apiVersion string) { t.Run(k, func(t *testing.T) { tc.old.ResourceVersion = "1" tc.update.ResourceVersion = "2" - apitesting.VerifyUpdateValidationEquivalence(t, ctx, &tc.update, &tc.old, Strategy.ValidateUpdate, tc.expectedErrs) + apitesting.VerifyUpdateValidationEquivalence(t, ctx, &tc.update, &tc.old, Strategy.ValidateUpdate, tc.expectedErrs, apitesting.WithNormalizationRules(resourceClaimNormalizationRules...)) }) } } @@ -258,7 +382,7 @@ func TestValidateStatusUpdateForDeclarative(t *testing.T) { t.Run(k, func(t *testing.T) { tc.old.ObjectMeta.ResourceVersion = "1" tc.update.ObjectMeta.ResourceVersion = "1" - apitesting.VerifyUpdateValidationEquivalence(t, ctx, &tc.update, &tc.old, strategy.ValidateUpdate, tc.expectedErrs, "status") + apitesting.VerifyUpdateValidationEquivalence(t, ctx, &tc.update, &tc.old, strategy.ValidateUpdate, tc.expectedErrs, apitesting.WithSubResources("status")) }) } } diff --git a/pkg/registry/resource/resourceclaim/strategy.go b/pkg/registry/resource/resourceclaim/strategy.go index 4da4321ff3a..7c71f2355cb 100644 --- a/pkg/registry/resource/resourceclaim/strategy.go +++ b/pkg/registry/resource/resourceclaim/strategy.go @@ -19,6 +19,7 @@ package resourceclaim import ( "context" "errors" + "regexp" "sigs.k8s.io/structured-merge-diff/v6/fieldpath" @@ -52,6 +53,15 @@ type resourceclaimStrategy struct { nsClient v1.NamespaceInterface } +var resourceClaimNormalizationRules = []field.NormalizationRule{ + { + // The "exactly" struct was added in v1beta2. In earlier API + // versions, its fields were directly part of the DeviceRequest. + Regexp: regexp.MustCompile(`spec\.devices\.requests\[(\d+)\]\.selectors`), + Replacement: "spec.devices.requests[$1].exactly.selectors", + }, +} + // NewStrategy is the default logic that applies when creating and updating ResourceClaim objects. func NewStrategy(nsClient v1.NamespaceInterface) *resourceclaimStrategy { return &resourceclaimStrategy{ @@ -100,7 +110,7 @@ func (s *resourceclaimStrategy) Validate(ctx context.Context, obj runtime.Object allErrs := resourceutils.AuthorizedForAdmin(ctx, claim.Spec.Devices.Requests, claim.Namespace, s.nsClient) allErrs = append(allErrs, validation.ValidateResourceClaim(claim)...) - return rest.ValidateDeclarativelyWithMigrationChecks(ctx, legacyscheme.Scheme, claim, nil, allErrs, operation.Create) + return rest.ValidateDeclarativelyWithMigrationChecks(ctx, legacyscheme.Scheme, claim, nil, allErrs, operation.Create, rest.WithNormalizationRules(resourceClaimNormalizationRules)) } func (*resourceclaimStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { @@ -128,7 +138,7 @@ func (s *resourceclaimStrategy) ValidateUpdate(ctx context.Context, obj, old run // AuthorizedForAdmin isn't needed here because the spec is immutable. errorList := validation.ValidateResourceClaim(newClaim) errorList = append(errorList, validation.ValidateResourceClaimUpdate(newClaim, oldClaim)...) - return rest.ValidateDeclarativelyWithMigrationChecks(ctx, legacyscheme.Scheme, newClaim, oldClaim, errorList, operation.Update) + return rest.ValidateDeclarativelyWithMigrationChecks(ctx, legacyscheme.Scheme, newClaim, oldClaim, errorList, operation.Update, rest.WithNormalizationRules(resourceClaimNormalizationRules)) } func (*resourceclaimStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {