From 118356175d234d65efe28368cc896cef4f23d238 Mon Sep 17 00:00:00 2001 From: Lionel Jouin Date: Thu, 7 Nov 2024 22:18:04 +0100 Subject: [PATCH] [KEP-4817] Add limits on conditions and IPs + fix documentation Signed-off-by: Lionel Jouin --- pkg/apis/resource/types.go | 15 ++++- pkg/apis/resource/validation/validation.go | 7 +- .../validation_resourceclaim_test.go | 64 ++++++++++++++++--- .../src/k8s.io/api/resource/v1alpha3/types.go | 11 +++- .../src/k8s.io/api/resource/v1beta1/types.go | 11 +++- 5 files changed, 92 insertions(+), 16 deletions(-) diff --git a/pkg/apis/resource/types.go b/pkg/apis/resource/types.go index b09691a10c5..65fb34f9a14 100644 --- a/pkg/apis/resource/types.go +++ b/pkg/apis/resource/types.go @@ -1020,12 +1020,17 @@ type AllocatedDeviceStatus struct { // If the device has been configured according to the class and claim // config references, the `Ready` condition should be True. // + // Must not contain more than 8 entries. + // // +optional - // +listType=atomic + // +listType=map + // +listMapKey=type Conditions []metav1.Condition // Data contains arbitrary driver-specific data. // + // The length of the raw data must be smaller or equal to 10 Ki. + // // +optional Data runtime.RawExtension @@ -1041,7 +1046,9 @@ type AllocatedDeviceStatus struct { type NetworkDeviceData struct { // InterfaceName specifies the name of the network interface associated with // the allocated device. This might be the name of a physical or virtual - // network interface. + // network interface being configured in the pod. + // + // Must not be longer than 256 characters. // // +optional InterfaceName string @@ -1052,12 +1059,16 @@ type NetworkDeviceData struct { // associated subnet mask. // e.g.: "192.0.2.5/24" for IPv4 and "2001:db8::5/64" for IPv6. // + // Must not contain more than 16 entries. + // // +optional // +listType=atomic IPs []string // HardwareAddress represents the hardware address (e.g. MAC Address) of the device's network interface. // + // Must not be longer than 128 characters. + // // +optional HardwareAddress string } diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index ca7611cc66c..4f06c114d85 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -748,6 +748,9 @@ func validateDeviceStatus(device resource.AllocatedDeviceStatus, fldPath *field. if !allocatedDevices.Has(deviceID) { allErrs = append(allErrs, field.Invalid(fldPath, deviceID, "must be an allocated device in the claim")) } + if len(device.Conditions) > maxConditions { + allErrs = append(allErrs, field.TooMany(fldPath.Child("conditions"), len(device.Conditions), maxConditions)) + } allErrs = append(allErrs, metav1validation.ValidateConditions(device.Conditions, fldPath.Child("conditions"))...) if len(device.Data.Raw) > 0 { // Data is an optional field. allErrs = append(allErrs, validateRawExtension(device.Data, fldPath.Child("data"), false)...) @@ -778,6 +781,8 @@ func validateRawExtension(rawExtension runtime.RawExtension, fldPath *field.Path return allErrs } +const maxConditions int = 8 +const maxIPs int = 16 const interfaceNameMaxLength int = 256 const hardwareAddressMaxLength int = 128 @@ -795,7 +800,7 @@ func validateNetworkDeviceData(networkDeviceData *resource.NetworkDeviceData, fl allErrs = append(allErrs, field.TooLong(fldPath.Child("hardwareAddress"), "" /* unused */, hardwareAddressMaxLength)) } - allErrs = append(allErrs, validateSet(networkDeviceData.IPs, -1, + allErrs = append(allErrs, validateSet(networkDeviceData.IPs, maxIPs, func(address string, fldPath *field.Path) field.ErrorList { return validation.IsValidCIDR(fldPath, address) }, diff --git a/pkg/apis/resource/validation/validation_resourceclaim_test.go b/pkg/apis/resource/validation/validation_resourceclaim_test.go index 50b32312677..ac202caf6b2 100644 --- a/pkg/apis/resource/validation/validation_resourceclaim_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaim_test.go @@ -994,13 +994,14 @@ func TestValidateClaimStatusUpdate(t *testing.T) { Pool: goodName, Device: goodName, Conditions: []metav1.Condition{ - { - Type: "test", - Status: metav1.ConditionTrue, - Reason: "test_reason", - LastTransitionTime: metav1.Now(), - ObservedGeneration: 0, - }, + {Type: "test-0", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-1", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-2", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-3", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-4", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-5", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-6", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-7", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, }, Data: runtime.RawExtension{ Raw: []byte(`{"kind": "foo", "apiVersion": "dra.example.com/v1"}`), @@ -1013,6 +1014,8 @@ func TestValidateClaimStatusUpdate(t *testing.T) { "2001:db8::/64", "10.9.8.1/24", "2001:db8::1/64", + "10.9.8.2/24", "10.9.8.3/24", "10.9.8.4/24", "10.9.8.5/24", "10.9.8.6/24", "10.9.8.7/24", + "10.9.8.8/24", "10.9.8.9/24", "10.9.8.10/24", "10.9.8.11/24", "10.9.8.12/24", "10.9.8.13/24", }, }, }, @@ -1096,9 +1099,11 @@ func TestValidateClaimStatusUpdate(t *testing.T) { }, deviceStatusFeatureGate: true, }, - "invalid-data-device-status-too-long": { + "invalid-data-device-status-limits": { wantFailures: field.ErrorList{ + field.TooMany(field.NewPath("status", "devices").Index(0).Child("conditions"), maxConditions+1, maxConditions), field.TooLong(field.NewPath("status", "devices").Index(0).Child("data"), "" /* unused */, resource.OpaqueParametersMaxLength), + field.TooMany(field.NewPath("status", "devices").Index(0).Child("networkData", "ips"), maxIPs+1, maxIPs), }, oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(), update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { @@ -1108,6 +1113,23 @@ func TestValidateClaimStatusUpdate(t *testing.T) { Pool: goodName, Device: goodName, Data: runtime.RawExtension{Raw: []byte(`{"str": "` + strings.Repeat("x", resource.OpaqueParametersMaxLength-9-2+1 /* too large by one */) + `"}`)}, + Conditions: []metav1.Condition{ + {Type: "test-0", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-1", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-2", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-3", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-4", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-5", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-6", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-7", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-8", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + }, + NetworkData: &resource.NetworkDeviceData{ + IPs: []string{ + "10.9.8.0/24", "10.9.8.1/24", "10.9.8.2/24", "10.9.8.3/24", "10.9.8.4/24", "10.9.8.5/24", "10.9.8.6/24", "10.9.8.7/24", "10.9.8.8/24", + "10.9.8.9/24", "10.9.8.10/24", "10.9.8.11/24", "10.9.8.12/24", "10.9.8.13/24", "10.9.8.14/24", "10.9.8.15/24", "10.9.8.16/24", + }, + }, }, } return claim @@ -1206,9 +1228,11 @@ func TestValidateClaimStatusUpdate(t *testing.T) { }, deviceStatusFeatureGate: false, }, - "invalid-data-device-status-too-long-feature-gate": { + "invalid-data-device-status-limits-feature-gate": { wantFailures: field.ErrorList{ + field.TooMany(field.NewPath("status", "devices").Index(0).Child("conditions"), maxConditions+1, maxConditions), field.TooLong(field.NewPath("status", "devices").Index(0).Child("data"), "" /* unused */, resource.OpaqueParametersMaxLength), + field.TooMany(field.NewPath("status", "devices").Index(0).Child("networkData", "ips"), maxIPs+1, maxIPs), }, oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(), update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { @@ -1218,6 +1242,23 @@ func TestValidateClaimStatusUpdate(t *testing.T) { Pool: goodName, Device: goodName, Data: runtime.RawExtension{Raw: []byte(`{"str": "` + strings.Repeat("x", resource.OpaqueParametersMaxLength-9-2+1 /* too large by one */) + `"}`)}, + Conditions: []metav1.Condition{ + {Type: "test-0", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-1", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-2", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-3", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-4", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-5", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-6", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-7", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + {Type: "test-8", Status: metav1.ConditionTrue, Reason: "test_reason", LastTransitionTime: metav1.Now(), ObservedGeneration: 0}, + }, + NetworkData: &resource.NetworkDeviceData{ + IPs: []string{ + "10.9.8.0/24", "10.9.8.1/24", "10.9.8.2/24", "10.9.8.3/24", "10.9.8.4/24", "10.9.8.5/24", "10.9.8.6/24", "10.9.8.7/24", "10.9.8.8/24", + "10.9.8.9/24", "10.9.8.10/24", "10.9.8.11/24", "10.9.8.12/24", "10.9.8.13/24", "10.9.8.14/24", "10.9.8.15/24", "10.9.8.16/24", + }, + }, }, } return claim @@ -1250,6 +1291,11 @@ func TestValidateClaimStatusUpdate(t *testing.T) { scenario.oldClaim.ResourceVersion = "1" errs := ValidateResourceClaimStatusUpdate(scenario.update(scenario.oldClaim.DeepCopy()), scenario.oldClaim) + + if name == "invalid-data-device-status-limits-feature-gate" { + fmt.Println(errs) + fmt.Println(scenario.wantFailures) + } assertFailures(t, scenario.wantFailures, errs) }) } diff --git a/staging/src/k8s.io/api/resource/v1alpha3/types.go b/staging/src/k8s.io/api/resource/v1alpha3/types.go index 65bdd55f2cc..e3d7fd8945b 100644 --- a/staging/src/k8s.io/api/resource/v1alpha3/types.go +++ b/staging/src/k8s.io/api/resource/v1alpha3/types.go @@ -1032,11 +1032,14 @@ type AllocatedDeviceStatus struct { // config references, the `Ready` condition should be True. // // +optional - // +listType=atomic + // +listType=map + // +listMapKey=type Conditions []metav1.Condition `json:"conditions" protobuf:"bytes,4,opt,name=conditions"` // Data contains arbitrary driver-specific data. // + // The length of the raw data must be smaller or equal to 10 Ki. + // // +optional Data runtime.RawExtension `json:"data,omitempty" protobuf:"bytes,5,opt,name=data"` @@ -1052,7 +1055,9 @@ type AllocatedDeviceStatus struct { type NetworkDeviceData struct { // InterfaceName specifies the name of the network interface associated with // the allocated device. This might be the name of a physical or virtual - // network interface. + // network interface being configured in the pod. + // + // Must not be longer than 256 characters. // // +optional InterfaceName string `json:"interfaceName,omitempty" protobuf:"bytes,1,opt,name=interfaceName"` @@ -1069,6 +1074,8 @@ type NetworkDeviceData struct { // HardwareAddress represents the hardware address (e.g. MAC Address) of the device's network interface. // + // Must not be longer than 128 characters. + // // +optional HardwareAddress string `json:"hardwareAddress,omitempty" protobuf:"bytes,3,opt,name=hardwareAddress"` } diff --git a/staging/src/k8s.io/api/resource/v1beta1/types.go b/staging/src/k8s.io/api/resource/v1beta1/types.go index 1f1b7dcb746..a7f1ee7b54f 100644 --- a/staging/src/k8s.io/api/resource/v1beta1/types.go +++ b/staging/src/k8s.io/api/resource/v1beta1/types.go @@ -1035,11 +1035,14 @@ type AllocatedDeviceStatus struct { // config references, the `Ready` condition should be True. // // +optional - // +listType=atomic + // +listType=map + // +listMapKey=type Conditions []metav1.Condition `json:"conditions" protobuf:"bytes,4,opt,name=conditions"` // Data contains arbitrary driver-specific data. // + // The length of the raw data must be smaller or equal to 10 Ki. + // // +optional Data runtime.RawExtension `json:"data,omitempty" protobuf:"bytes,5,opt,name=data"` @@ -1055,7 +1058,9 @@ type AllocatedDeviceStatus struct { type NetworkDeviceData struct { // InterfaceName specifies the name of the network interface associated with // the allocated device. This might be the name of a physical or virtual - // network interface. + // network interface being configured in the pod. + // + // Must not be longer than 256 characters. // // +optional InterfaceName string `json:"interfaceName,omitempty" protobuf:"bytes,1,opt,name=interfaceName"` @@ -1072,6 +1077,8 @@ type NetworkDeviceData struct { // HardwareAddress represents the hardware address (e.g. MAC Address) of the device's network interface. // + // Must not be longer than 128 characters. + // // +optional HardwareAddress string `json:"hardwareAddress,omitempty" protobuf:"bytes,3,opt,name=hardwareAddress"` }