Skip to content

Commit 446f20a

Browse files
committed
DRA API: add maximum length of opaque parameters
This had been left out unintentionally earlier. Because theoretically there might now be existing objects with parameters that are larger than whatever limit gets enforced now, the limit only gets checked when parameters get created or modified. This is similar to the validation of CEL expressions and for consistency, the same 10 Ki limit as for those is chosen. Because the limit is not enforced for stored parameters, it can be increased in the future, with the caveat that users who need larger parameters then depend on the newer Kubernetes release with a higher limit. Lowering the limit is harder because creating deployments that worked in older Kubernetes will not work anymore with newer Kubernetes.
1 parent e273349 commit 446f20a

14 files changed

+209
-26
lines changed

api/openapi-spec/swagger.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/openapi-spec/v3/apis__resource.k8s.io__v1alpha3_openapi.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/openapi-spec/v3/apis__resource.k8s.io__v1beta1_openapi.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/apis/resource/types.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,10 +654,16 @@ type OpaqueDeviceConfiguration struct {
654654
// includes self-identification and a version ("kind" + "apiVersion" for
655655
// Kubernetes types), with conversion between different versions.
656656
//
657+
// The length of the raw data must be smaller or equal to 10 Ki.
658+
//
657659
// +required
658660
Parameters runtime.RawExtension
659661
}
660662

663+
// OpaqueParametersMaxLength is the maximum length of the raw data in an
664+
// [OpaqueDeviceConfiguration.Parameters] field.
665+
const OpaqueParametersMaxLength = 10 * 1024
666+
661667
// ResourceClaimStatus tracks whether the resource has been allocated and what
662668
// the result of that was.
663669
type ResourceClaimStatus struct {

pkg/apis/resource/validation/validation.go

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func validateDeviceClaim(deviceClaim *resource.DeviceClaim, fldPath *field.Path,
110110
}, fldPath.Child("constraints"))...)
111111
allErrs = append(allErrs, validateSlice(deviceClaim.Config, resource.DeviceConfigMaxSize,
112112
func(config resource.DeviceClaimConfiguration, fldPath *field.Path) field.ErrorList {
113-
return validateDeviceClaimConfiguration(config, fldPath, requestNames)
113+
return validateDeviceClaimConfiguration(config, fldPath, requestNames, stored)
114114
}, fldPath.Child("config"))...)
115115
return allErrs
116116
}
@@ -212,13 +212,13 @@ func validateDeviceConstraint(constraint resource.DeviceConstraint, fldPath *fie
212212
return allErrs
213213
}
214214

215-
func validateDeviceClaimConfiguration(config resource.DeviceClaimConfiguration, fldPath *field.Path, requestNames sets.Set[string]) field.ErrorList {
215+
func validateDeviceClaimConfiguration(config resource.DeviceClaimConfiguration, fldPath *field.Path, requestNames sets.Set[string], stored bool) field.ErrorList {
216216
var allErrs field.ErrorList
217217
allErrs = append(allErrs, validateSet(config.Requests, resource.DeviceRequestsMaxSize,
218218
func(name string, fldPath *field.Path) field.ErrorList {
219219
return validateRequestNameRef(name, fldPath, requestNames)
220220
}, stringKey, fldPath.Child("requests"))...)
221-
allErrs = append(allErrs, validateDeviceConfiguration(config.DeviceConfiguration, fldPath)...)
221+
allErrs = append(allErrs, validateDeviceConfiguration(config.DeviceConfiguration, fldPath, stored)...)
222222
return allErrs
223223
}
224224

@@ -230,23 +230,29 @@ func validateRequestNameRef(name string, fldPath *field.Path, requestNames sets.
230230
return allErrs
231231
}
232232

233-
func validateDeviceConfiguration(config resource.DeviceConfiguration, fldPath *field.Path) field.ErrorList {
233+
func validateDeviceConfiguration(config resource.DeviceConfiguration, fldPath *field.Path, stored bool) field.ErrorList {
234234
var allErrs field.ErrorList
235235
if config.Opaque == nil {
236236
allErrs = append(allErrs, field.Required(fldPath.Child("opaque"), ""))
237237
} else {
238-
allErrs = append(allErrs, validateOpaqueConfiguration(*config.Opaque, fldPath.Child("opaque"))...)
238+
allErrs = append(allErrs, validateOpaqueConfiguration(*config.Opaque, fldPath.Child("opaque"), stored)...)
239239
}
240240
return allErrs
241241
}
242242

243-
func validateOpaqueConfiguration(config resource.OpaqueDeviceConfiguration, fldPath *field.Path) field.ErrorList {
243+
func validateOpaqueConfiguration(config resource.OpaqueDeviceConfiguration, fldPath *field.Path, stored bool) field.ErrorList {
244244
var allErrs field.ErrorList
245245
allErrs = append(allErrs, validateDriverName(config.Driver, fldPath.Child("driver"))...)
246246
// Validation of RawExtension as in https://github.com/kubernetes/kubernetes/pull/125549/
247247
var v any
248248
if len(config.Parameters.Raw) == 0 {
249249
allErrs = append(allErrs, field.Required(fldPath.Child("parameters"), ""))
250+
} else if !stored && len(config.Parameters.Raw) > resource.OpaqueParametersMaxLength {
251+
// Don't even bother with parsing when too large.
252+
// Only applies on create. Existing parameters are grand-fathered in
253+
// because the limit was introduced in 1.32. This also means that it
254+
// can be changed in the future.
255+
allErrs = append(allErrs, field.TooLong(fldPath.Child("parameters"), "" /* unused */, resource.OpaqueParametersMaxLength))
250256
} else if err := json.Unmarshal(config.Parameters.Raw, &v); err != nil {
251257
allErrs = append(allErrs, field.Invalid(fldPath.Child("parameters"), "<value omitted>", fmt.Sprintf("error parsing data as JSON: %v", err.Error())))
252258
} else if v == nil {
@@ -290,7 +296,7 @@ func validateResourceClaimStatusUpdate(status, oldStatus *resource.ResourceClaim
290296
if oldStatus.Allocation != nil && status.Allocation != nil {
291297
allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(status.Allocation, oldStatus.Allocation, fldPath.Child("allocation"))...)
292298
} else if status.Allocation != nil {
293-
allErrs = append(allErrs, validateAllocationResult(status.Allocation, fldPath.Child("allocation"), requestNames)...)
299+
allErrs = append(allErrs, validateAllocationResult(status.Allocation, fldPath.Child("allocation"), requestNames, false)...)
294300
}
295301

296302
return allErrs
@@ -313,24 +319,24 @@ func validateResourceClaimUserReference(ref resource.ResourceClaimConsumerRefere
313319
// validateAllocationResult enforces constraints for *new* results, which in at
314320
// least one case (admin access) are more strict than before. Therefore it
315321
// may not be called to re-validate results which were stored earlier.
316-
func validateAllocationResult(allocation *resource.AllocationResult, fldPath *field.Path, requestNames sets.Set[string]) field.ErrorList {
322+
func validateAllocationResult(allocation *resource.AllocationResult, fldPath *field.Path, requestNames sets.Set[string], stored bool) field.ErrorList {
317323
var allErrs field.ErrorList
318-
allErrs = append(allErrs, validateDeviceAllocationResult(allocation.Devices, fldPath.Child("devices"), requestNames)...)
324+
allErrs = append(allErrs, validateDeviceAllocationResult(allocation.Devices, fldPath.Child("devices"), requestNames, stored)...)
319325
if allocation.NodeSelector != nil {
320326
allErrs = append(allErrs, corevalidation.ValidateNodeSelector(allocation.NodeSelector, fldPath.Child("nodeSelector"))...)
321327
}
322328
return allErrs
323329
}
324330

325-
func validateDeviceAllocationResult(allocation resource.DeviceAllocationResult, fldPath *field.Path, requestNames sets.Set[string]) field.ErrorList {
331+
func validateDeviceAllocationResult(allocation resource.DeviceAllocationResult, fldPath *field.Path, requestNames sets.Set[string], stored bool) field.ErrorList {
326332
var allErrs field.ErrorList
327333
allErrs = append(allErrs, validateSlice(allocation.Results, resource.AllocationResultsMaxSize,
328334
func(result resource.DeviceRequestAllocationResult, fldPath *field.Path) field.ErrorList {
329335
return validateDeviceRequestAllocationResult(result, fldPath, requestNames)
330336
}, fldPath.Child("results"))...)
331337
allErrs = append(allErrs, validateSlice(allocation.Config, 2*resource.DeviceConfigMaxSize, /* class + claim */
332338
func(config resource.DeviceAllocationConfiguration, fldPath *field.Path) field.ErrorList {
333-
return validateDeviceAllocationConfiguration(config, fldPath, requestNames)
339+
return validateDeviceAllocationConfiguration(config, fldPath, requestNames, stored)
334340
}, fldPath.Child("config"))...)
335341

336342
return allErrs
@@ -345,14 +351,14 @@ func validateDeviceRequestAllocationResult(result resource.DeviceRequestAllocati
345351
return allErrs
346352
}
347353

348-
func validateDeviceAllocationConfiguration(config resource.DeviceAllocationConfiguration, fldPath *field.Path, requestNames sets.Set[string]) field.ErrorList {
354+
func validateDeviceAllocationConfiguration(config resource.DeviceAllocationConfiguration, fldPath *field.Path, requestNames sets.Set[string], stored bool) field.ErrorList {
349355
var allErrs field.ErrorList
350356
allErrs = append(allErrs, validateAllocationConfigSource(config.Source, fldPath.Child("source"))...)
351357
allErrs = append(allErrs, validateSet(config.Requests, resource.DeviceRequestsMaxSize,
352358
func(name string, fldPath *field.Path) field.ErrorList {
353359
return validateRequestNameRef(name, fldPath, requestNames)
354360
}, stringKey, fldPath.Child("requests"))...)
355-
allErrs = append(allErrs, validateDeviceConfiguration(config.DeviceConfiguration, fldPath)...)
361+
allErrs = append(allErrs, validateDeviceConfiguration(config.DeviceConfiguration, fldPath, stored)...)
356362
return allErrs
357363
}
358364

@@ -396,12 +402,20 @@ func validateDeviceClassSpec(spec, oldSpec *resource.DeviceClassSpec, fldPath *f
396402
return validateSelector(selector, fldPath, stored)
397403
},
398404
fldPath.Child("selectors"))...)
399-
allErrs = append(allErrs, validateSlice(spec.Config, resource.DeviceConfigMaxSize, validateDeviceClassConfiguration, fldPath.Child("config"))...)
405+
// Same logic as above for configs.
406+
if oldSpec != nil {
407+
stored = apiequality.Semantic.DeepEqual(spec.Config, oldSpec.Config)
408+
}
409+
allErrs = append(allErrs, validateSlice(spec.Config, resource.DeviceConfigMaxSize,
410+
func(config resource.DeviceClassConfiguration, fldPath *field.Path) field.ErrorList {
411+
return validateDeviceClassConfiguration(config, fldPath, stored)
412+
},
413+
fldPath.Child("config"))...)
400414
return allErrs
401415
}
402416

403-
func validateDeviceClassConfiguration(config resource.DeviceClassConfiguration, fldPath *field.Path) field.ErrorList {
404-
return validateDeviceConfiguration(config.DeviceConfiguration, fldPath)
417+
func validateDeviceClassConfiguration(config resource.DeviceClassConfiguration, fldPath *field.Path, stored bool) field.ErrorList {
418+
return validateDeviceConfiguration(config.DeviceConfiguration, fldPath, stored)
405419
}
406420

407421
// ValidateResourceClaimTemplate validates a ResourceClaimTemplate.

pkg/apis/resource/validation/validation_deviceclass_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package validation
1818

1919
import (
20+
"strings"
2021
"testing"
2122

2223
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -227,6 +228,7 @@ func TestValidateClass(t *testing.T) {
227228
field.Invalid(field.NewPath("spec", "config").Index(3).Child("opaque", "parameters"), "<value omitted>", "parameters must be a valid JSON object"),
228229
field.Required(field.NewPath("spec", "config").Index(4).Child("opaque", "parameters"), ""),
229230
field.Required(field.NewPath("spec", "config").Index(5).Child("opaque"), ""),
231+
field.TooLong(field.NewPath("spec", "config").Index(7).Child("opaque", "parameters"), "" /* unused */, resource.OpaqueParametersMaxLength),
230232
},
231233
class: func() *resource.DeviceClass {
232234
class := testClass(goodName)
@@ -272,6 +274,22 @@ func TestValidateClass(t *testing.T) {
272274
{
273275
DeviceConfiguration: resource.DeviceConfiguration{ /* Bad, empty. */ },
274276
},
277+
{
278+
DeviceConfiguration: resource.DeviceConfiguration{
279+
Opaque: &resource.OpaqueDeviceConfiguration{
280+
Driver: goodName,
281+
Parameters: runtime.RawExtension{Raw: []byte(`{"str": "` + strings.Repeat("x", resource.OpaqueParametersMaxLength-9-2) + `"}`)},
282+
},
283+
},
284+
},
285+
{
286+
DeviceConfiguration: resource.DeviceConfiguration{
287+
Opaque: &resource.OpaqueDeviceConfiguration{
288+
Driver: goodName,
289+
Parameters: runtime.RawExtension{Raw: []byte(`{"str": "` + strings.Repeat("x", resource.OpaqueParametersMaxLength-9-2+1 /* too large by one */) + `"}`)},
290+
},
291+
},
292+
},
275293
}
276294
for i := len(class.Spec.Config); i < resource.DeviceConfigMaxSize; i++ {
277295
class.Spec.Config = append(class.Spec.Config, validConfig)
@@ -321,6 +339,55 @@ func TestValidateClassUpdate(t *testing.T) {
321339
oldClass: validClass,
322340
update: func(class *resource.DeviceClass) *resource.DeviceClass { return class },
323341
},
342+
"valid-config-large": {
343+
oldClass: validClass,
344+
update: func(class *resource.DeviceClass) *resource.DeviceClass {
345+
class.Spec.Config = []resource.DeviceClassConfiguration{{
346+
DeviceConfiguration: resource.DeviceConfiguration{
347+
Opaque: &resource.OpaqueDeviceConfiguration{
348+
Driver: goodName,
349+
Parameters: runtime.RawExtension{Raw: []byte(`{"str": "` + strings.Repeat("x", resource.OpaqueParametersMaxLength-9-2) + `"}`)},
350+
},
351+
},
352+
}}
353+
return class
354+
},
355+
},
356+
"invalid-config-too-large": {
357+
wantFailures: field.ErrorList{
358+
field.TooLong(field.NewPath("spec", "config").Index(0).Child("opaque", "parameters"), "" /* unused */, resource.OpaqueParametersMaxLength),
359+
},
360+
oldClass: validClass,
361+
update: func(class *resource.DeviceClass) *resource.DeviceClass {
362+
class.Spec.Config = []resource.DeviceClassConfiguration{{
363+
DeviceConfiguration: resource.DeviceConfiguration{
364+
Opaque: &resource.OpaqueDeviceConfiguration{
365+
Driver: goodName,
366+
Parameters: runtime.RawExtension{Raw: []byte(`{"str": "` + strings.Repeat("x", resource.OpaqueParametersMaxLength-9-2+1 /* too large by one */) + `"}`)},
367+
},
368+
},
369+
}}
370+
return class
371+
},
372+
},
373+
"too-large-config-valid-if-stored": {
374+
oldClass: func() *resource.DeviceClass {
375+
class := validClass.DeepCopy()
376+
class.Spec.Config = []resource.DeviceClassConfiguration{{
377+
DeviceConfiguration: resource.DeviceConfiguration{
378+
Opaque: &resource.OpaqueDeviceConfiguration{
379+
Driver: goodName,
380+
Parameters: runtime.RawExtension{Raw: []byte(`{"str": "` + strings.Repeat("x", resource.OpaqueParametersMaxLength-9-2+1 /* too large by one */) + `"}`)},
381+
},
382+
},
383+
}}
384+
return class
385+
}(),
386+
update: func(class *resource.DeviceClass) *resource.DeviceClass {
387+
// No changes -> remains valid.
388+
return class
389+
},
390+
},
324391
}
325392

326393
for name, scenario := range scenarios {

0 commit comments

Comments
 (0)