diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index 753bfd2e229..361b2856f79 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -119,10 +119,41 @@ func validateDeviceClaim(deviceClaim *resource.DeviceClaim, fldPath *field.Path, return allErrs } -func gatherRequestNames(deviceClaim *resource.DeviceClaim) sets.Set[string] { - requestNames := sets.New[string]() +type requestNames map[string]sets.Set[string] + +func (r requestNames) Has(s string) bool { + segments := strings.Split(s, "/") + // If there are more than one / in the string, we + // know there can't be any match. + if len(segments) > 2 { + return false + } + // If the first segment doesn't have a match, we + // don't need to check the other one. + subRequestNames, found := r[segments[0]] + if !found { + return false + } + if len(segments) == 1 { + return true + } + // If the first segment matched and we have another one, + // check for a match for that too. + return subRequestNames.Has(segments[1]) +} + +func gatherRequestNames(deviceClaim *resource.DeviceClaim) requestNames { + requestNames := make(requestNames) for _, request := range deviceClaim.Requests { - requestNames.Insert(request.Name) + if len(request.FirstAvailable) == 0 { + requestNames[request.Name] = nil + continue + } + subRequestNames := sets.New[string]() + for _, subRequest := range request.FirstAvailable { + subRequestNames.Insert(subRequest.Name) + } + requestNames[request.Name] = subRequestNames } return requestNames } @@ -138,29 +169,79 @@ func gatherAllocatedDevices(allocationResult *resource.DeviceAllocationResult) s func validateDeviceRequest(request resource.DeviceRequest, fldPath *field.Path, stored bool) field.ErrorList { allErrs := validateRequestName(request.Name, fldPath.Child("name")) - if request.DeviceClassName == "" { - allErrs = append(allErrs, field.Required(fldPath.Child("deviceClassName"), "")) - } else { - allErrs = append(allErrs, validateDeviceClassName(request.DeviceClassName, fldPath.Child("deviceClassName"))...) + if len(request.FirstAvailable) > 0 { + if request.DeviceClassName != "" { + allErrs = append(allErrs, field.Invalid(fldPath.Child("deviceClassName"), request.DeviceClassName, "must not be specified when firstAvailable is set")) + } + if request.Selectors != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("selectors"), request.Selectors, "must not be specified when firstAvailable is set")) + } + if request.AllocationMode != "" { + allErrs = append(allErrs, field.Invalid(fldPath.Child("allocationMode"), request.AllocationMode, "must not be specified when firstAvailable is set")) + } + if request.Count != 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("count"), request.Count, "must not be specified when firstAvailable is set")) + } + if request.AdminAccess != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("adminAccess"), request.AdminAccess, "must not be specified when firstAvailable is set")) + } + allErrs = append(allErrs, validateSet(request.FirstAvailable, resource.FirstAvailableDeviceRequestMaxSize, + func(subRequest resource.DeviceSubRequest, fldPath *field.Path) field.ErrorList { + return validateDeviceSubRequest(subRequest, fldPath, stored) + }, + func(subRequest resource.DeviceSubRequest) (string, string) { + return subRequest.Name, "name" + }, + fldPath.Child("firstAvailable"))...) + return allErrs } - allErrs = append(allErrs, validateSlice(request.Selectors, resource.DeviceSelectorsMaxSize, + allErrs = append(allErrs, validateDeviceClass(request.DeviceClassName, fldPath.Child("deviceClassName"))...) + allErrs = append(allErrs, validateSelectorSlice(request.Selectors, fldPath.Child("selectors"), stored)...) + allErrs = append(allErrs, validateDeviceAllocationMode(request.AllocationMode, request.Count, fldPath.Child("allocationMode"), fldPath.Child("count"))...) + return allErrs +} + +func validateDeviceSubRequest(subRequest resource.DeviceSubRequest, fldPath *field.Path, stored bool) field.ErrorList { + allErrs := validateRequestName(subRequest.Name, fldPath.Child("name")) + allErrs = append(allErrs, validateDeviceClass(subRequest.DeviceClassName, fldPath.Child("deviceClassName"))...) + allErrs = append(allErrs, validateSelectorSlice(subRequest.Selectors, fldPath.Child("selectors"), stored)...) + allErrs = append(allErrs, validateDeviceAllocationMode(subRequest.AllocationMode, subRequest.Count, fldPath.Child("allocationMode"), fldPath.Child("count"))...) + return allErrs +} + +func validateDeviceAllocationMode(deviceAllocationMode resource.DeviceAllocationMode, count int64, allocModeFldPath, countFldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + switch deviceAllocationMode { + case resource.DeviceAllocationModeAll: + if count != 0 { + allErrs = append(allErrs, field.Invalid(countFldPath, count, fmt.Sprintf("must not be specified when allocationMode is '%s'", deviceAllocationMode))) + } + case resource.DeviceAllocationModeExactCount: + if count <= 0 { + allErrs = append(allErrs, field.Invalid(countFldPath, count, "must be greater than zero")) + } + default: + allErrs = append(allErrs, field.NotSupported(allocModeFldPath, deviceAllocationMode, []resource.DeviceAllocationMode{resource.DeviceAllocationModeAll, resource.DeviceAllocationModeExactCount})) + } + return allErrs +} + +func validateDeviceClass(deviceClass string, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + if deviceClass == "" { + allErrs = append(allErrs, field.Required(fldPath, "")) + } else { + allErrs = append(allErrs, validateDeviceClassName(deviceClass, fldPath)...) + } + return allErrs +} + +func validateSelectorSlice(selectors []resource.DeviceSelector, fldPath *field.Path, stored bool) field.ErrorList { + return validateSlice(selectors, resource.DeviceSelectorsMaxSize, func(selector resource.DeviceSelector, fldPath *field.Path) field.ErrorList { return validateSelector(selector, fldPath, stored) }, - fldPath.Child("selectors"))...) - switch request.AllocationMode { - case resource.DeviceAllocationModeAll: - if request.Count != 0 { - allErrs = append(allErrs, field.Invalid(fldPath.Child("count"), request.Count, fmt.Sprintf("must not be specified when allocationMode is '%s'", request.AllocationMode))) - } - case resource.DeviceAllocationModeExactCount: - if request.Count <= 0 { - allErrs = append(allErrs, field.Invalid(fldPath.Child("count"), request.Count, "must be greater than zero")) - } - default: - allErrs = append(allErrs, field.NotSupported(fldPath.Child("allocationMode"), request.AllocationMode, []resource.DeviceAllocationMode{resource.DeviceAllocationModeAll, resource.DeviceAllocationModeExactCount})) - } - return allErrs + fldPath) } func validateSelector(selector resource.DeviceSelector, fldPath *field.Path, stored bool) field.ErrorList { @@ -210,7 +291,7 @@ func convertCELErrorToValidationError(fldPath *field.Path, expression string, er return field.InternalError(fldPath, fmt.Errorf("unsupported error type: %w", err)) } -func validateDeviceConstraint(constraint resource.DeviceConstraint, fldPath *field.Path, requestNames sets.Set[string]) field.ErrorList { +func validateDeviceConstraint(constraint resource.DeviceConstraint, fldPath *field.Path, requestNames requestNames) field.ErrorList { var allErrs field.ErrorList allErrs = append(allErrs, validateSet(constraint.Requests, resource.DeviceRequestsMaxSize, func(name string, fldPath *field.Path) field.ErrorList { @@ -225,7 +306,7 @@ func validateDeviceConstraint(constraint resource.DeviceConstraint, fldPath *fie return allErrs } -func validateDeviceClaimConfiguration(config resource.DeviceClaimConfiguration, fldPath *field.Path, requestNames sets.Set[string], stored bool) field.ErrorList { +func validateDeviceClaimConfiguration(config resource.DeviceClaimConfiguration, fldPath *field.Path, requestNames requestNames, stored bool) field.ErrorList { var allErrs field.ErrorList allErrs = append(allErrs, validateSet(config.Requests, resource.DeviceRequestsMaxSize, func(name string, fldPath *field.Path) field.ErrorList { @@ -235,10 +316,20 @@ func validateDeviceClaimConfiguration(config resource.DeviceClaimConfiguration, return allErrs } -func validateRequestNameRef(name string, fldPath *field.Path, requestNames sets.Set[string]) field.ErrorList { - allErrs := validateRequestName(name, fldPath) +func validateRequestNameRef(name string, fldPath *field.Path, requestNames requestNames) field.ErrorList { + var allErrs field.ErrorList + segments := strings.Split(name, "/") + if len(segments) > 2 { + allErrs = append(allErrs, field.Invalid(fldPath, name, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'")) + return allErrs + } + + for i := range segments { + allErrs = append(allErrs, validateRequestName(segments[i], fldPath)...) + } + if !requestNames.Has(name) { - allErrs = append(allErrs, field.Invalid(fldPath, name, "must be the name of a request in the claim")) + allErrs = append(allErrs, field.Invalid(fldPath, name, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'")) } return allErrs } @@ -260,7 +351,7 @@ func validateOpaqueConfiguration(config resource.OpaqueDeviceConfiguration, fldP return allErrs } -func validateResourceClaimStatusUpdate(status, oldStatus *resource.ResourceClaimStatus, claimDeleted bool, requestNames sets.Set[string], fldPath *field.Path) field.ErrorList { +func validateResourceClaimStatusUpdate(status, oldStatus *resource.ResourceClaimStatus, claimDeleted bool, requestNames requestNames, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList allErrs = append(allErrs, validateSet(status.ReservedFor, resource.ResourceClaimReservedForMaxSize, validateResourceClaimUserReference, @@ -328,7 +419,7 @@ func validateResourceClaimUserReference(ref resource.ResourceClaimConsumerRefere // validateAllocationResult enforces constraints for *new* results, which in at // least one case (admin access) are more strict than before. Therefore it // may not be called to re-validate results which were stored earlier. -func validateAllocationResult(allocation *resource.AllocationResult, fldPath *field.Path, requestNames sets.Set[string], stored bool) field.ErrorList { +func validateAllocationResult(allocation *resource.AllocationResult, fldPath *field.Path, requestNames requestNames, stored bool) field.ErrorList { var allErrs field.ErrorList allErrs = append(allErrs, validateDeviceAllocationResult(allocation.Devices, fldPath.Child("devices"), requestNames, stored)...) if allocation.NodeSelector != nil { @@ -337,7 +428,7 @@ func validateAllocationResult(allocation *resource.AllocationResult, fldPath *fi return allErrs } -func validateDeviceAllocationResult(allocation resource.DeviceAllocationResult, fldPath *field.Path, requestNames sets.Set[string], stored bool) field.ErrorList { +func validateDeviceAllocationResult(allocation resource.DeviceAllocationResult, fldPath *field.Path, requestNames requestNames, stored bool) field.ErrorList { var allErrs field.ErrorList allErrs = append(allErrs, validateSlice(allocation.Results, resource.AllocationResultsMaxSize, func(result resource.DeviceRequestAllocationResult, fldPath *field.Path) field.ErrorList { @@ -351,7 +442,7 @@ func validateDeviceAllocationResult(allocation resource.DeviceAllocationResult, return allErrs } -func validateDeviceRequestAllocationResult(result resource.DeviceRequestAllocationResult, fldPath *field.Path, requestNames sets.Set[string]) field.ErrorList { +func validateDeviceRequestAllocationResult(result resource.DeviceRequestAllocationResult, fldPath *field.Path, requestNames requestNames) field.ErrorList { var allErrs field.ErrorList allErrs = append(allErrs, validateRequestNameRef(result.Request, fldPath.Child("request"), requestNames)...) allErrs = append(allErrs, validateDriverName(result.Driver, fldPath.Child("driver"))...) @@ -360,7 +451,7 @@ func validateDeviceRequestAllocationResult(result resource.DeviceRequestAllocati return allErrs } -func validateDeviceAllocationConfiguration(config resource.DeviceAllocationConfiguration, fldPath *field.Path, requestNames sets.Set[string], stored bool) field.ErrorList { +func validateDeviceAllocationConfiguration(config resource.DeviceAllocationConfiguration, fldPath *field.Path, requestNames requestNames, stored bool) field.ErrorList { var allErrs field.ErrorList allErrs = append(allErrs, validateAllocationConfigSource(config.Source, fldPath.Child("source"))...) allErrs = append(allErrs, validateSet(config.Requests, resource.DeviceRequestsMaxSize, diff --git a/pkg/apis/resource/validation/validation_resourceclaim_test.go b/pkg/apis/resource/validation/validation_resourceclaim_test.go index 5f3fe3308a8..3308024ecd1 100644 --- a/pkg/apis/resource/validation/validation_resourceclaim_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaim_test.go @@ -46,13 +46,17 @@ func testClaim(name, namespace string, spec resource.ResourceClaimSpec) *resourc } const ( - goodName = "foo" - badName = "!@#$%^" - goodNS = "ns" + goodName = "foo" + goodName2 = "bar" + badName = "!@#$%^" + goodNS = "ns" + badSubrequestName = "&^%$" ) var ( - validClaimSpec = resource.ResourceClaimSpec{ + badRequestFormat = fmt.Sprintf("%s/%s/%s", goodName, goodName, goodName) + badFullSubrequestName = fmt.Sprintf("%s/%s", badName, badSubrequestName) + validClaimSpec = resource.ResourceClaimSpec{ Devices: resource.DeviceClaim{ Requests: []resource.DeviceRequest{{ Name: goodName, @@ -62,6 +66,34 @@ var ( }}, }, } + validClaimSpecWithFirstAvailable = resource.ResourceClaimSpec{ + Devices: resource.DeviceClaim{ + Requests: []resource.DeviceRequest{{ + Name: goodName, + FirstAvailable: []resource.DeviceSubRequest{ + { + Name: goodName, + DeviceClassName: goodName, + AllocationMode: resource.DeviceAllocationModeExactCount, + Count: 1, + }, + { + Name: goodName2, + DeviceClassName: goodName, + AllocationMode: resource.DeviceAllocationModeExactCount, + Count: 1, + }, + }, + }}, + }, + } + validSelector = []resource.DeviceSelector{ + { + CEL: &resource.CELDeviceSelector{ + Expression: `device.driver == "dra.example.com"`, + }, + }, + } validClaim = testClaim(goodName, goodNS, validClaimSpec) ) @@ -225,14 +257,14 @@ func TestValidateClaim(t *testing.T) { wantFailures: field.ErrorList{ field.TooMany(field.NewPath("spec", "devices", "requests"), resource.DeviceRequestsMaxSize+1, resource.DeviceRequestsMaxSize), field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests").Index(1), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"), - field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim"), + field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"), field.TypeInvalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), "missing-domain", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')"), field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), resource.FullyQualifiedName("missing-domain"), "must include a domain"), field.Required(field.NewPath("spec", "devices", "constraints").Index(1).Child("matchAttribute"), "name required"), field.Required(field.NewPath("spec", "devices", "constraints").Index(2).Child("matchAttribute"), ""), field.TooMany(field.NewPath("spec", "devices", "constraints"), resource.DeviceConstraintsMaxSize+1, resource.DeviceConstraintsMaxSize), field.Invalid(field.NewPath("spec", "devices", "config").Index(0).Child("requests").Index(1), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"), - field.Invalid(field.NewPath("spec", "devices", "config").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim"), + field.Invalid(field.NewPath("spec", "devices", "config").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"), field.TooMany(field.NewPath("spec", "devices", "config"), resource.DeviceConfigMaxSize+1, resource.DeviceConfigMaxSize), }, claim: func() *resource.ResourceClaim { @@ -312,13 +344,13 @@ func TestValidateClaim(t *testing.T) { "invalid-spec": { wantFailures: field.ErrorList{ field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests").Index(1), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"), - field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim"), + field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"), field.TypeInvalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), "missing-domain", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')"), field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), resource.FullyQualifiedName("missing-domain"), "must include a domain"), field.Required(field.NewPath("spec", "devices", "constraints").Index(1).Child("matchAttribute"), "name required"), field.Required(field.NewPath("spec", "devices", "constraints").Index(2).Child("matchAttribute"), ""), field.Invalid(field.NewPath("spec", "devices", "config").Index(0).Child("requests").Index(1), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"), - field.Invalid(field.NewPath("spec", "devices", "config").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim"), + field.Invalid(field.NewPath("spec", "devices", "config").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"), }, claim: func() *resource.ResourceClaim { claim := testClaim(goodName, goodNS, validClaimSpec) @@ -537,6 +569,176 @@ func TestValidateClaim(t *testing.T) { return claim }(), }, + "prioritized-list-valid": { + wantFailures: nil, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable) + return claim + }(), + }, + "prioritized-list-field-on-parent": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices", "requests").Index(0).Child("deviceClassName"), goodName, "must not be specified when firstAvailable is set"), + field.Invalid(field.NewPath("spec", "devices", "requests").Index(0).Child("selectors"), validSelector, "must not be specified when firstAvailable is set"), + field.Invalid(field.NewPath("spec", "devices", "requests").Index(0).Child("allocationMode"), resource.DeviceAllocationModeAll, "must not be specified when firstAvailable is set"), + field.Invalid(field.NewPath("spec", "devices", "requests").Index(0).Child("count"), int64(2), "must not be specified when firstAvailable is set"), + field.Invalid(field.NewPath("spec", "devices", "requests").Index(0).Child("adminAccess"), ptr.To(true), "must not be specified when firstAvailable is set"), + }, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable) + claim.Spec.Devices.Requests[0].DeviceClassName = goodName + claim.Spec.Devices.Requests[0].Selectors = validSelector + claim.Spec.Devices.Requests[0].AllocationMode = resource.DeviceAllocationModeAll + claim.Spec.Devices.Requests[0].Count = 2 + claim.Spec.Devices.Requests[0].AdminAccess = ptr.To(true) + return claim + }(), + }, + "prioritized-list-invalid-nested-request": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices", "requests").Index(0).Child("firstAvailable").Index(0).Child("name"), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"), + field.Required(field.NewPath("spec", "devices", "requests").Index(0).Child("firstAvailable").Index(0).Child("deviceClassName"), ""), + field.NotSupported(field.NewPath("spec", "devices", "requests").Index(0).Child("firstAvailable").Index(0).Child("allocationMode"), resource.DeviceAllocationMode(""), []resource.DeviceAllocationMode{resource.DeviceAllocationModeAll, resource.DeviceAllocationModeExactCount}), + }, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable) + claim.Spec.Devices.Requests[0].FirstAvailable[0] = resource.DeviceSubRequest{ + Name: badName, + } + return claim + }(), + }, + "prioritized-list-nested-requests-same-name": { + wantFailures: field.ErrorList{ + field.Duplicate(field.NewPath("spec", "devices", "requests").Index(0).Child("firstAvailable").Index(1).Child("name"), "foo"), + }, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable) + claim.Spec.Devices.Requests[0].FirstAvailable[1].Name = goodName + return claim + }(), + }, + "prioritized-list-too-many-subrequests": { + wantFailures: field.ErrorList{ + field.TooMany(field.NewPath("spec", "devices", "requests").Index(0).Child("firstAvailable"), 9, 8), + }, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpec) + claim.Spec.Devices.Requests[0].DeviceClassName = "" + claim.Spec.Devices.Requests[0].AllocationMode = "" + claim.Spec.Devices.Requests[0].Count = 0 + var subRequests []resource.DeviceSubRequest + for i := 0; i <= 8; i++ { + subRequests = append(subRequests, resource.DeviceSubRequest{ + Name: fmt.Sprintf("subreq-%d", i), + DeviceClassName: goodName, + AllocationMode: resource.DeviceAllocationModeExactCount, + Count: 1, + }) + } + claim.Spec.Devices.Requests[0].FirstAvailable = subRequests + return claim + }(), + }, + "prioritized-list-config-requests-with-subrequest-reference": { + wantFailures: nil, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable) + claim.Spec.Devices.Config = []resource.DeviceClaimConfiguration{ + { + Requests: []string{"foo/bar"}, + DeviceConfiguration: resource.DeviceConfiguration{ + Opaque: &resource.OpaqueDeviceConfiguration{ + Driver: "dra.example.com", + Parameters: runtime.RawExtension{ + Raw: []byte(`{"kind": "foo", "apiVersion": "dra.example.com/v1"}`), + }, + }, + }, + }, + } + return claim + }(), + }, + "prioritized-list-config-requests-with-parent-request-reference": { + wantFailures: nil, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable) + claim.Spec.Devices.Config = []resource.DeviceClaimConfiguration{ + { + Requests: []string{"foo"}, + DeviceConfiguration: resource.DeviceConfiguration{ + Opaque: &resource.OpaqueDeviceConfiguration{ + Driver: "dra.example.com", + Parameters: runtime.RawExtension{ + Raw: []byte(`{"kind": "foo", "apiVersion": "dra.example.com/v1"}`), + }, + }, + }, + }, + } + return claim + }(), + }, + "prioritized-list-config-requests-with-invalid-subrequest-reference": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "devices", "config").Index(0).Child("requests").Index(0), "foo/baz", "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'")}, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable) + claim.Spec.Devices.Config = []resource.DeviceClaimConfiguration{ + { + Requests: []string{"foo/baz"}, + DeviceConfiguration: resource.DeviceConfiguration{ + Opaque: &resource.OpaqueDeviceConfiguration{ + Driver: "dra.example.com", + Parameters: runtime.RawExtension{ + Raw: []byte(`{"kind": "foo", "apiVersion": "dra.example.com/v1"}`), + }, + }, + }, + }, + } + return claim + }(), + }, + "prioritized-list-constraints-requests-with-subrequest-reference": { + wantFailures: nil, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable) + claim.Spec.Devices.Constraints = []resource.DeviceConstraint{ + { + Requests: []string{"foo/bar"}, + MatchAttribute: ptr.To(resource.FullyQualifiedName("dra.example.com/driverVersion")), + }, + } + return claim + }(), + }, + "prioritized-list-constraints-requests-with-parent-request-reference": { + wantFailures: nil, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable) + claim.Spec.Devices.Constraints = []resource.DeviceConstraint{ + { + Requests: []string{"foo"}, + MatchAttribute: ptr.To(resource.FullyQualifiedName("dra.example.com/driverVersion")), + }, + } + return claim + }(), + }, + "prioritized-list-constraints-requests-with-invalid-subrequest-reference": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests").Index(0), "foo/baz", "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'")}, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable) + claim.Spec.Devices.Constraints = []resource.DeviceConstraint{ + { + Requests: []string{"foo/baz"}, + MatchAttribute: ptr.To(resource.FullyQualifiedName("dra.example.com/driverVersion")), + }, + } + return claim + }(), + }, } for name, scenario := range scenarios { @@ -617,11 +819,12 @@ func TestValidateClaimStatusUpdate(t *testing.T) { validAllocatedClaimOld.Status.Allocation.Devices.Results[0].AdminAccess = nil // Not required in 1.31. scenarios := map[string]struct { - adminAccess bool - deviceStatusFeatureGate bool - oldClaim *resource.ResourceClaim - update func(claim *resource.ResourceClaim) *resource.ResourceClaim - wantFailures field.ErrorList + adminAccess bool + deviceStatusFeatureGate bool + prioritizedListFeatureGate bool + oldClaim *resource.ResourceClaim + update func(claim *resource.ResourceClaim) *resource.ResourceClaim + wantFailures field.ErrorList }{ "valid-no-op-update": { oldClaim: validClaim, @@ -654,7 +857,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) { "invalid-add-allocation-bad-request": { wantFailures: field.ErrorList{ field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("request"), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"), - field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("request"), badName, "must be the name of a request in the claim"), + field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("request"), badName, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"), }, oldClaim: validClaim, update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { @@ -862,7 +1065,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) { "invalid-request-name": { wantFailures: field.ErrorList{ field.Invalid(field.NewPath("status", "allocation", "devices", "config").Index(0).Child("requests").Index(1), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"), - field.Invalid(field.NewPath("status", "allocation", "devices", "config").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim"), + field.Invalid(field.NewPath("status", "allocation", "devices", "config").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"), }, oldClaim: validClaim, update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { @@ -1334,12 +1537,115 @@ func TestValidateClaimStatusUpdate(t *testing.T) { return claim }, }, + "valid-add-allocation-with-sub-requests": { + oldClaim: testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable), + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + claim.Status.Allocation = &resource.AllocationResult{ + Devices: resource.DeviceAllocationResult{ + Results: []resource.DeviceRequestAllocationResult{{ + Request: fmt.Sprintf("%s/%s", goodName, goodName), + Driver: goodName, + Pool: goodName, + Device: goodName, + AdminAccess: ptr.To(false), + }}, + }, + } + return claim + }, + prioritizedListFeatureGate: true, + }, + "invalid-add-allocation-with-sub-requests-invalid-format": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("request"), badRequestFormat, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"), + }, + oldClaim: testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable), + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + claim.Status.Allocation = &resource.AllocationResult{ + Devices: resource.DeviceAllocationResult{ + Results: []resource.DeviceRequestAllocationResult{{ + Request: badRequestFormat, + Driver: goodName, + Pool: goodName, + Device: goodName, + AdminAccess: ptr.To(false), + }}, + }, + } + return claim + }, + prioritizedListFeatureGate: true, + }, + "invalid-add-allocation-with-sub-requests-no-corresponding-sub-request": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("request"), "foo/baz", "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"), + }, + oldClaim: testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable), + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + claim.Status.Allocation = &resource.AllocationResult{ + Devices: resource.DeviceAllocationResult{ + Results: []resource.DeviceRequestAllocationResult{{ + Request: "foo/baz", + Driver: goodName, + Pool: goodName, + Device: goodName, + AdminAccess: ptr.To(false), + }}, + }, + } + return claim + }, + prioritizedListFeatureGate: true, + }, + "invalid-add-allocation-with-sub-requests-invalid-request-names": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("request"), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"), + field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("request"), badSubrequestName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"), + field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("request"), badFullSubrequestName, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"), + }, + oldClaim: testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable), + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + claim.Status.Allocation = &resource.AllocationResult{ + Devices: resource.DeviceAllocationResult{ + Results: []resource.DeviceRequestAllocationResult{{ + Request: badFullSubrequestName, + Driver: goodName, + Pool: goodName, + Device: goodName, + AdminAccess: ptr.To(false), + }}, + }, + } + return claim + }, + prioritizedListFeatureGate: true, + }, + "add-allocation-old-claim-with-prioritized-list": { + wantFailures: nil, + oldClaim: testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable), + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + claim.Status.Allocation = &resource.AllocationResult{ + Devices: resource.DeviceAllocationResult{ + Results: []resource.DeviceRequestAllocationResult{{ + Request: "foo/bar", + Driver: goodName, + Pool: goodName, + Device: goodName, + AdminAccess: ptr.To(false), + }}, + }, + } + return claim + }, + prioritizedListFeatureGate: false, + }, } for name, scenario := range scenarios { t.Run(name, func(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAAdminAccess, scenario.adminAccess) featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAResourceClaimDeviceStatus, scenario.deviceStatusFeatureGate) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAPrioritizedList, scenario.prioritizedListFeatureGate) scenario.oldClaim.ResourceVersion = "1" errs := ValidateResourceClaimStatusUpdate(scenario.update(scenario.oldClaim.DeepCopy()), scenario.oldClaim) diff --git a/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go b/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go index a22cc776cb2..586c804c702 100644 --- a/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go @@ -185,6 +185,26 @@ func TestValidateClaimTemplate(t *testing.T) { return template }(), }, + "prioritized-list": { + wantFailures: nil, + template: testClaimTemplate(goodName, goodNS, validClaimSpecWithFirstAvailable), + }, + "proritized-list-class-name-on-parent": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "spec", "devices", "requests").Index(0).Child("deviceClassName"), goodName, "must not be specified when firstAvailable is set")}, + template: func() *resource.ResourceClaimTemplate { + template := testClaimTemplate(goodName, goodNS, validClaimSpecWithFirstAvailable) + template.Spec.Spec.Devices.Requests[0].DeviceClassName = goodName + return template + }(), + }, + "prioritized-list-bad-class-name-on-subrequest": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "spec", "devices", "requests").Index(0).Child("firstAvailable").Index(0).Child("deviceClassName"), badName, "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])?)*')")}, + template: func() *resource.ResourceClaimTemplate { + template := testClaimTemplate(goodName, goodNS, validClaimSpecWithFirstAvailable) + template.Spec.Spec.Devices.Requests[0].FirstAvailable[0].DeviceClassName = badName + return template + }(), + }, } for name, scenario := range scenarios { @@ -219,6 +239,18 @@ func TestValidateClaimTemplateUpdate(t *testing.T) { return template }, }, + "prioritized-listinvalid-update-class": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec"), func() resource.ResourceClaimTemplateSpec { + template := testClaimTemplate(goodName, goodNS, validClaimSpecWithFirstAvailable) + template.Spec.Spec.Devices.Requests[0].FirstAvailable[0].DeviceClassName += "2" + return template.Spec + }(), "field is immutable")}, + oldClaimTemplate: testClaimTemplate(goodName, goodNS, validClaimSpecWithFirstAvailable), + update: func(template *resource.ResourceClaimTemplate) *resource.ResourceClaimTemplate { + template.Spec.Spec.Devices.Requests[0].FirstAvailable[0].DeviceClassName += "2" + return template + }, + }, } for name, scenario := range scenarios { diff --git a/pkg/registry/resource/resourceclaim/strategy.go b/pkg/registry/resource/resourceclaim/strategy.go index 336f09b1515..cb770b2a6e1 100644 --- a/pkg/registry/resource/resourceclaim/strategy.go +++ b/pkg/registry/resource/resourceclaim/strategy.go @@ -183,10 +183,38 @@ func toSelectableFields(claim *resource.ResourceClaim) fields.Set { // dropDisabledFields removes fields which are covered by a feature gate. func dropDisabledFields(newClaim, oldClaim *resource.ResourceClaim) { + dropDisabledDRAPrioritizedListFields(newClaim, oldClaim) dropDisabledDRAAdminAccessFields(newClaim, oldClaim) dropDisabledDRAResourceClaimDeviceStatusFields(newClaim, oldClaim) } +func dropDisabledDRAPrioritizedListFields(newClaim, oldClaim *resource.ResourceClaim) { + if utilfeature.DefaultFeatureGate.Enabled(features.DRAPrioritizedList) { + return + } + if draPrioritizedListFeatureInUse(oldClaim) { + return + } + + for i := range newClaim.Spec.Devices.Requests { + newClaim.Spec.Devices.Requests[i].FirstAvailable = nil + } +} + +func draPrioritizedListFeatureInUse(claim *resource.ResourceClaim) bool { + if claim == nil { + return false + } + + for _, request := range claim.Spec.Devices.Requests { + if len(request.FirstAvailable) > 0 { + return true + } + } + + return false +} + func dropDisabledDRAAdminAccessFields(newClaim, oldClaim *resource.ResourceClaim) { if utilfeature.DefaultFeatureGate.Enabled(features.DRAAdminAccess) { // No need to drop anything. diff --git a/pkg/registry/resource/resourceclaim/strategy_test.go b/pkg/registry/resource/resourceclaim/strategy_test.go index c65092cb5a4..fb855f0bf96 100644 --- a/pkg/registry/resource/resourceclaim/strategy_test.go +++ b/pkg/registry/resource/resourceclaim/strategy_test.go @@ -133,6 +133,30 @@ var objWithAdminAccessStatus = &resource.ResourceClaim{ }, } +var objWithPrioritizedList = &resource.ResourceClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-claim", + Namespace: "default", + }, + Spec: resource.ResourceClaimSpec{ + Devices: resource.DeviceClaim{ + Requests: []resource.DeviceRequest{ + { + Name: "req-0", + FirstAvailable: []resource.DeviceSubRequest{ + { + Name: "subreq-0", + DeviceClassName: "class", + AllocationMode: resource.DeviceAllocationModeExactCount, + Count: 1, + }, + }, + }, + }, + }, + }, +} + const ( testRequest = "test-request" testDriver = "test-driver" @@ -155,6 +179,7 @@ func TestStrategyCreate(t *testing.T) { testcases := map[string]struct { obj *resource.ResourceClaim adminAccess bool + prioritizedList bool expectValidationError bool expectObj *resource.ResourceClaim }{ @@ -180,11 +205,22 @@ func TestStrategyCreate(t *testing.T) { adminAccess: true, expectObj: objWithAdminAccess, }, + "drop-fields-prioritized-list": { + obj: objWithPrioritizedList, + prioritizedList: false, + expectValidationError: true, + }, + "keep-fields-prioritized-list": { + obj: objWithPrioritizedList, + prioritizedList: true, + expectObj: objWithPrioritizedList, + }, } for name, tc := range testcases { t.Run(name, func(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAAdminAccess, tc.adminAccess) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAPrioritizedList, tc.prioritizedList) obj := tc.obj.DeepCopy() Strategy.PrepareForCreate(ctx, obj) @@ -212,6 +248,7 @@ func TestStrategyUpdate(t *testing.T) { oldObj *resource.ResourceClaim newObj *resource.ResourceClaim adminAccess bool + prioritizedList bool expectValidationError bool expectObj *resource.ResourceClaim }{ @@ -247,11 +284,36 @@ func TestStrategyUpdate(t *testing.T) { adminAccess: true, expectObj: objWithAdminAccess, }, + "drop-fields-prioritized-list": { + oldObj: obj, + newObj: objWithPrioritizedList, + prioritizedList: false, + expectValidationError: true, + }, + "keep-fields-prioritized-list": { + oldObj: obj, + newObj: objWithPrioritizedList, + prioritizedList: true, + expectValidationError: true, // Spec is immutable. + }, + "keep-existing-fields-prioritized-list": { + oldObj: objWithPrioritizedList, + newObj: objWithPrioritizedList, + prioritizedList: true, + expectObj: objWithPrioritizedList, + }, + "keep-existing-fields-prioritized-list-disabled-feature": { + oldObj: objWithPrioritizedList, + newObj: objWithPrioritizedList, + prioritizedList: false, + expectObj: objWithPrioritizedList, + }, } for name, tc := range testcases { t.Run(name, func(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAAdminAccess, tc.adminAccess) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAPrioritizedList, tc.prioritizedList) oldObj := tc.oldObj.DeepCopy() newObj := tc.newObj.DeepCopy() diff --git a/pkg/registry/resource/resourceclaimtemplate/strategy.go b/pkg/registry/resource/resourceclaimtemplate/strategy.go index 3677683e9f7..3fa78b2a7f0 100644 --- a/pkg/registry/resource/resourceclaimtemplate/strategy.go +++ b/pkg/registry/resource/resourceclaimtemplate/strategy.go @@ -100,9 +100,37 @@ func toSelectableFields(template *resource.ResourceClaimTemplate) fields.Set { } func dropDisabledFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) { + dropDisabledDRAPrioritizedListFields(newClaimTemplate, oldClaimTemplate) dropDisabledDRAAdminAccessFields(newClaimTemplate, oldClaimTemplate) } +func dropDisabledDRAPrioritizedListFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) { + if utilfeature.DefaultFeatureGate.Enabled(features.DRAPrioritizedList) { + return + } + if draPrioritizedListFeatureInUse(oldClaimTemplate) { + return + } + + for i := range newClaimTemplate.Spec.Spec.Devices.Requests { + newClaimTemplate.Spec.Spec.Devices.Requests[i].FirstAvailable = nil + } +} + +func draPrioritizedListFeatureInUse(claimTemplate *resource.ResourceClaimTemplate) bool { + if claimTemplate == nil { + return false + } + + for _, request := range claimTemplate.Spec.Spec.Devices.Requests { + if len(request.FirstAvailable) > 0 { + return true + } + } + + return false +} + func dropDisabledDRAAdminAccessFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) { if utilfeature.DefaultFeatureGate.Enabled(features.DRAAdminAccess) { // No need to drop anything. diff --git a/pkg/registry/resource/resourceclaimtemplate/strategy_test.go b/pkg/registry/resource/resourceclaimtemplate/strategy_test.go index b48f78d7bee..eb547cfe469 100644 --- a/pkg/registry/resource/resourceclaimtemplate/strategy_test.go +++ b/pkg/registry/resource/resourceclaimtemplate/strategy_test.go @@ -71,6 +71,32 @@ var objWithAdminAccess = &resource.ResourceClaimTemplate{ }, } +var objWithPrioritizedList = &resource.ResourceClaimTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-claim-template", + Namespace: "default", + }, + Spec: resource.ResourceClaimTemplateSpec{ + Spec: resource.ResourceClaimSpec{ + Devices: resource.DeviceClaim{ + Requests: []resource.DeviceRequest{ + { + Name: "req-0", + FirstAvailable: []resource.DeviceSubRequest{ + { + Name: "subreq-0", + DeviceClassName: "class", + AllocationMode: resource.DeviceAllocationModeExactCount, + Count: 1, + }, + }, + }, + }, + }, + }, + }, +} + func TestClaimTemplateStrategy(t *testing.T) { if !Strategy.NamespaceScoped() { t.Errorf("ResourceClaimTemplate must be namespace scoped") @@ -86,6 +112,7 @@ func TestClaimTemplateStrategyCreate(t *testing.T) { testcases := map[string]struct { obj *resource.ResourceClaimTemplate adminAccess bool + prioritizedList bool expectValidationError bool expectObj *resource.ResourceClaimTemplate }{ @@ -111,11 +138,22 @@ func TestClaimTemplateStrategyCreate(t *testing.T) { adminAccess: true, expectObj: objWithAdminAccess, }, + "drop-fields-prioritized-list": { + obj: objWithPrioritizedList, + prioritizedList: false, + expectValidationError: true, + }, + "keep-fields-prioritized-list": { + obj: objWithPrioritizedList, + prioritizedList: true, + expectObj: objWithPrioritizedList, + }, } for name, tc := range testcases { t.Run(name, func(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAAdminAccess, tc.adminAccess) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAPrioritizedList, tc.prioritizedList) obj := tc.obj.DeepCopy() Strategy.PrepareForCreate(ctx, obj)