[KEP-4817] Improve NetworkData Validation

* Add max length for InterfaceName and HardwareAddress
* Prevent duplicated Addresses

Signed-off-by: Lionel Jouin <lionel.jouin@est.tech>

pkg/apis/resource/validation/validation.go

@@ -37,6 +37,7 @@ import (
 	"k8s.io/dynamic-resource-allocation/structured"
 	corevalidation "k8s.io/kubernetes/pkg/apis/core/validation"
 	"k8s.io/kubernetes/pkg/apis/resource"
+	netutils "k8s.io/utils/net"
 )
 
 var (
@@ -740,6 +741,9 @@ func truncateIfTooLong(str string, maxLen int) string {
 
 func validateDeviceStatus(device resource.AllocatedDeviceStatus, fldPath *field.Path, allocatedDevices sets.Set[structured.DeviceID]) field.ErrorList {
 	var allErrs field.ErrorList
+	allErrs = append(allErrs, validateDriverName(device.Driver, fldPath.Child("driver"))...)
+	allErrs = append(allErrs, validatePoolName(device.Pool, fldPath.Child("pool"))...)
+	allErrs = append(allErrs, validateDeviceName(device.Device, fldPath.Child("device"))...)
 	deviceID := structured.DeviceID{Driver: device.Driver, Pool: device.Pool, Device: device.Device}
 	if !allocatedDevices.Has(deviceID) {
 		allErrs = append(allErrs, field.Invalid(fldPath, deviceID, "must be an allocated device in the claim"))
@@ -759,7 +763,7 @@ func validateRawExtension(rawExtension runtime.RawExtension, fldPath *field.Path
 	if len(rawExtension.Raw) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath, ""))
 	} else if err := json.Unmarshal(rawExtension.Raw, &v); err != nil {
-		allErrs = append(allErrs, field.Invalid(fldPath, "<value omitted>", fmt.Sprintf("error parsing data: %v", err.Error())))
+		allErrs = append(allErrs, field.Invalid(fldPath, "<value omitted>", fmt.Sprintf("error parsing data as JSON: %v", err.Error())))
 	} else if v == nil {
 		allErrs = append(allErrs, field.Required(fldPath, ""))
 	} else if _, isObject := v.(map[string]any); !isObject {
@@ -768,16 +772,37 @@ func validateRawExtension(rawExtension runtime.RawExtension, fldPath *field.Path
 	return allErrs
 }
 
+const interfaceNameMaxLength int = 256
+const hardwareAddressMaxLength int = 128
+
 func validateNetworkDeviceData(networkDeviceData *resource.NetworkDeviceData, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
 	if networkDeviceData == nil {
 		return allErrs
 	}
-	allErrs = append(allErrs, validateSlice(networkDeviceData.Addresses, -1,
+
+	if len(networkDeviceData.InterfaceName) > interfaceNameMaxLength {
+		allErrs = append(allErrs, field.TooLong(fldPath.Child("interfaceName"), "" /* unused */, interfaceNameMaxLength))
+	}
+
+	if len(networkDeviceData.HardwareAddress) > hardwareAddressMaxLength {
+		allErrs = append(allErrs, field.TooLong(fldPath.Child("hardwareAddress"), "" /* unused */, hardwareAddressMaxLength))
+	}
+
+	allErrs = append(allErrs, validateSet(networkDeviceData.Addresses, -1,
 		func(address string, fldPath *field.Path) field.ErrorList {
-			var allErrs field.ErrorList
+			return validation.IsValidCIDR(fldPath, address)
-			allErrs = append(allErrs, validation.IsValidCIDR(fldPath, address)...)
+		},
-			return allErrs
+		func(address string) (string, string) {
-		}, fldPath.Child("addresses"))...)
+			// reformat CIDR to handle different ways IPs can be written
+			// (e.g. 2001:db8::1/64 == 2001:0db8::1/64)
+			ip, ipNet, err := netutils.ParseCIDRSloppy(address)
+			if err != nil {
+				return "", "" // will fail at IsValidCIDR
+			}
+			maskSize, _ := ipNet.Mask.Size()
+			return fmt.Sprintf("%s/%d", ip.String(), maskSize), ""
+		},
+		fldPath.Child("addresses"))...)
 	return allErrs
 }

pkg/apis/resource/validation/validation_resourceclaim_test.go

@@ -1006,12 +1006,14 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 							Raw: []byte(`{"kind": "foo", "apiVersion": "dra.example.com/v1"}`),
 						},
 						NetworkData: &resource.NetworkDeviceData{
-							InterfaceName: "net-1",
+							InterfaceName:   strings.Repeat("x", 256),
+							HardwareAddress: strings.Repeat("x", 128),
 							Addresses: []string{
 								"10.9.8.0/24",
 								"2001:db8::/64",
+								"10.9.8.1/24",
+								"2001:db8::1/64",
 							},
-							HardwareAddress: "ea:9f:cb:40:b1:7b",
 						},
 					},
 				}
@@ -1021,6 +1023,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 		},
 		"invalid-device-status-duplicate": {
 			wantFailures: field.ErrorList{
+				field.Duplicate(field.NewPath("status", "devices").Index(0).Child("networkData", "addresses").Index(1), "2001:db8::1/64"),
 				field.Duplicate(field.NewPath("status", "devices").Index(1).Child("deviceID"), structured.DeviceID{Driver: goodName, Pool: goodName, Device: goodName}),
 			},
 			oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
@@ -1030,6 +1033,12 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 						Driver: goodName,
 						Pool:   goodName,
 						Device: goodName,
+						NetworkData: &resource.NetworkDeviceData{
+							Addresses: []string{
+								"2001:db8::1/64",
+								"2001:0db8::1/64",
+							},
+						},
 					},
 					{
 						Driver: goodName,
@@ -1043,6 +1052,8 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 		},
 		"invalid-network-device-status": {
 			wantFailures: field.ErrorList{
+				field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", interfaceNameMaxLength),
+				field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", hardwareAddressMaxLength),
 				field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "addresses").Index(0), "300.9.8.0/24", "must be a valid CIDR value, (e.g. 10.9.8.0/24 or 2001:db8::/64)"),
 			},
 			oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
@@ -1053,6 +1064,8 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 						Pool:   goodName,
 						Device: goodName,
 						NetworkData: &resource.NetworkDeviceData{
+							InterfaceName:   strings.Repeat("x", interfaceNameMaxLength+1),
+							HardwareAddress: strings.Repeat("x", hardwareAddressMaxLength+1),
 							Addresses: []string{
 								"300.9.8.0/24",
 							},
@@ -1065,7 +1078,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 		},
 		"invalid-data-device-status": {
 			wantFailures: field.ErrorList{
-				field.Invalid(field.NewPath("status", "devices").Index(0).Child("data"), "<value omitted>", "error parsing data: invalid character 'o' in literal false (expecting 'a')"),
+				field.Invalid(field.NewPath("status", "devices").Index(0).Child("data"), "<value omitted>", "error parsing data as JSON: invalid character 'o' in literal false (expecting 'a')"),
 			},
 			oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
 			update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
@@ -1102,6 +1115,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 		},
 		"invalid-device-status-duplicate-disabled-feature-gate": {
 			wantFailures: field.ErrorList{
+				field.Duplicate(field.NewPath("status", "devices").Index(0).Child("networkData", "addresses").Index(1), "2001:db8::1/64"),
 				field.Duplicate(field.NewPath("status", "devices").Index(1).Child("deviceID"), structured.DeviceID{Driver: goodName, Pool: goodName, Device: goodName}),
 			},
 			oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
@@ -1111,6 +1125,12 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 						Driver: goodName,
 						Pool:   goodName,
 						Device: goodName,
+						NetworkData: &resource.NetworkDeviceData{
+							Addresses: []string{
+								"2001:db8::1/64",
+								"2001:0db8::1/64",
+							},
+						},
 					},
 					{
 						Driver: goodName,
@@ -1124,6 +1144,8 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 		},
 		"invalid-network-device-status-disabled-feature-gate": {
 			wantFailures: field.ErrorList{
+				field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", interfaceNameMaxLength),
+				field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", hardwareAddressMaxLength),
 				field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "addresses").Index(0), "300.9.8.0/24", "must be a valid CIDR value, (e.g. 10.9.8.0/24 or 2001:db8::/64)"),
 			},
 			oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
@@ -1134,6 +1156,8 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 						Pool:   goodName,
 						Device: goodName,
 						NetworkData: &resource.NetworkDeviceData{
+							InterfaceName:   strings.Repeat("x", interfaceNameMaxLength+1),
+							HardwareAddress: strings.Repeat("x", hardwareAddressMaxLength+1),
 							Addresses: []string{
 								"300.9.8.0/24",
 							},
@@ -1146,7 +1170,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 		},
 		"invalid-data-device-status-disabled-feature-gate": {
 			wantFailures: field.ErrorList{
-				field.Invalid(field.NewPath("status", "devices").Index(0).Child("data"), "<value omitted>", "error parsing data: invalid character 'o' in literal false (expecting 'a')"),
+				field.Invalid(field.NewPath("status", "devices").Index(0).Child("data"), "<value omitted>", "error parsing data as JSON: invalid character 'o' in literal false (expecting 'a')"),
 			},
 			oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
 			update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {