Skip to content

Commit afc2041

Browse files
authored
Merge pull request kubernetes#128601 from pohly/dra-api-opaque-parameters-length-limit
DRA API: opaque parameters length limit
2 parents f3498df + 446f20a commit afc2041

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)