Skip to content

Commit 8ab33b8

Browse files
LionelJouinLionelJouin
committed
[KEP-4817] Improve NetworkData Validation
* Add max length for InterfaceName and HardwareAddress * Prevent duplicated Addresses Signed-off-by: Lionel Jouin <[email protected]>
1 parent a062f91 commit 8ab33b8

File tree

2 files changed

+59
-10
lines changed

2 files changed

+59
-10
lines changed

‎pkg/apis/resource/validation/validation.go‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
"k8s.io/dynamic-resource-allocation/structured"
3838
corevalidation "k8s.io/kubernetes/pkg/apis/core/validation"
3939
"k8s.io/kubernetes/pkg/apis/resource"
40+
netutils "k8s.io/utils/net"
4041
)
4142

4243
var (
@@ -740,6 +741,9 @@ func truncateIfTooLong(str string, maxLen int) string {
740741

741742
func validateDeviceStatus(device resource.AllocatedDeviceStatus, fldPath *field.Path, allocatedDevices sets.Set[structured.DeviceID]) field.ErrorList {
742743
var allErrs field.ErrorList
744+
allErrs = append(allErrs, validateDriverName(device.Driver, fldPath.Child("driver"))...)
745+
allErrs = append(allErrs, validatePoolName(device.Pool, fldPath.Child("pool"))...)
746+
allErrs = append(allErrs, validateDeviceName(device.Device, fldPath.Child("device"))...)
743747
deviceID := structured.DeviceID{Driver: device.Driver, Pool: device.Pool, Device: device.Device}
744748
if !allocatedDevices.Has(deviceID) {
745749
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
759763
if len(rawExtension.Raw) == 0 {
760764
allErrs = append(allErrs, field.Required(fldPath, ""))
761765
} else if err := json.Unmarshal(rawExtension.Raw, &v); err != nil {
762-
allErrs = append(allErrs, field.Invalid(fldPath, "<value omitted>", fmt.Sprintf("error parsing data: %v", err.Error())))
766+
allErrs = append(allErrs, field.Invalid(fldPath, "<value omitted>", fmt.Sprintf("error parsing data as JSON: %v", err.Error())))
763767
} else if v == nil {
764768
allErrs = append(allErrs, field.Required(fldPath, ""))
765769
} else if _, isObject := v.(map[string]any); !isObject {
@@ -768,16 +772,37 @@ func validateRawExtension(rawExtension runtime.RawExtension, fldPath *field.Path
768772
return allErrs
769773
}
770774

775+
const interfaceNameMaxLength int = 256
776+
const hardwareAddressMaxLength int = 128
777+
771778
func validateNetworkDeviceData(networkDeviceData *resource.NetworkDeviceData, fldPath *field.Path) field.ErrorList {
772779
var allErrs field.ErrorList
773780
if networkDeviceData == nil {
774781
return allErrs
775782
}
776-
allErrs = append(allErrs, validateSlice(networkDeviceData.Addresses, -1,
783+
784+
if len(networkDeviceData.InterfaceName) > interfaceNameMaxLength {
785+
allErrs = append(allErrs, field.TooLong(fldPath.Child("interfaceName"), "" /* unused */, interfaceNameMaxLength))
786+
}
787+
788+
if len(networkDeviceData.HardwareAddress) > hardwareAddressMaxLength {
789+
allErrs = append(allErrs, field.TooLong(fldPath.Child("hardwareAddress"), "" /* unused */, hardwareAddressMaxLength))
790+
}
791+
792+
allErrs = append(allErrs, validateSet(networkDeviceData.Addresses, -1,
777793
func(address string, fldPath *field.Path) field.ErrorList {
778-
var allErrs field.ErrorList
779-
allErrs = append(allErrs, validation.IsValidCIDR(fldPath, address)...)
780-
return allErrs
781-
}, fldPath.Child("addresses"))...)
794+
return validation.IsValidCIDR(fldPath, address)
795+
},
796+
func(address string) (string, string) {
797+
// reformat CIDR to handle different ways IPs can be written
798+
// (e.g. 2001:db8::1/64 == 2001:0db8::1/64)
799+
ip, ipNet, err := netutils.ParseCIDRSloppy(address)
800+
if err != nil {
801+
return "", "" // will fail at IsValidCIDR
802+
}
803+
maskSize, _ := ipNet.Mask.Size()
804+
return fmt.Sprintf("%s/%d", ip.String(), maskSize), ""
805+
},
806+
fldPath.Child("addresses"))...)
782807
return allErrs
783808
}

‎pkg/apis/resource/validation/validation_resourceclaim_test.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,12 +1006,14 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
10061006
Raw: []byte(`{"kind": "foo", "apiVersion": "dra.example.com/v1"}`),
10071007
},
10081008
NetworkData: &resource.NetworkDeviceData{
1009-
InterfaceName: "net-1",
1009+
InterfaceName: strings.Repeat("x", 256),
1010+
HardwareAddress: strings.Repeat("x", 128),
10101011
Addresses: []string{
10111012
"10.9.8.0/24",
10121013
"2001:db8::/64",
1014+
"10.9.8.1/24",
1015+
"2001:db8::1/64",
10131016
},
1014-
HardwareAddress: "ea:9f:cb:40:b1:7b",
10151017
},
10161018
},
10171019
}
@@ -1021,6 +1023,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
10211023
},
10221024
"invalid-device-status-duplicate": {
10231025
wantFailures: field.ErrorList{
1026+
field.Duplicate(field.NewPath("status", "devices").Index(0).Child("networkData", "addresses").Index(1), "2001:db8::1/64"),
10241027
field.Duplicate(field.NewPath("status", "devices").Index(1).Child("deviceID"), structured.DeviceID{Driver: goodName, Pool: goodName, Device: goodName}),
10251028
},
10261029
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
@@ -1030,6 +1033,12 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
10301033
Driver: goodName,
10311034
Pool: goodName,
10321035
Device: goodName,
1036+
NetworkData: &resource.NetworkDeviceData{
1037+
Addresses: []string{
1038+
"2001:db8::1/64",
1039+
"2001:0db8::1/64",
1040+
},
1041+
},
10331042
},
10341043
{
10351044
Driver: goodName,
@@ -1043,6 +1052,8 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
10431052
},
10441053
"invalid-network-device-status": {
10451054
wantFailures: field.ErrorList{
1055+
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", interfaceNameMaxLength),
1056+
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", hardwareAddressMaxLength),
10461057
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)"),
10471058
},
10481059
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
@@ -1053,6 +1064,8 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
10531064
Pool: goodName,
10541065
Device: goodName,
10551066
NetworkData: &resource.NetworkDeviceData{
1067+
InterfaceName: strings.Repeat("x", interfaceNameMaxLength+1),
1068+
HardwareAddress: strings.Repeat("x", hardwareAddressMaxLength+1),
10561069
Addresses: []string{
10571070
"300.9.8.0/24",
10581071
},
@@ -1065,7 +1078,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
10651078
},
10661079
"invalid-data-device-status": {
10671080
wantFailures: field.ErrorList{
1068-
field.Invalid(field.NewPath("status", "devices").Index(0).Child("data"), "<value omitted>", "error parsing data: invalid character 'o' in literal false (expecting 'a')"),
1081+
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')"),
10691082
},
10701083
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
10711084
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
@@ -1102,6 +1115,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
11021115
},
11031116
"invalid-device-status-duplicate-disabled-feature-gate": {
11041117
wantFailures: field.ErrorList{
1118+
field.Duplicate(field.NewPath("status", "devices").Index(0).Child("networkData", "addresses").Index(1), "2001:db8::1/64"),
11051119
field.Duplicate(field.NewPath("status", "devices").Index(1).Child("deviceID"), structured.DeviceID{Driver: goodName, Pool: goodName, Device: goodName}),
11061120
},
11071121
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
@@ -1111,6 +1125,12 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
11111125
Driver: goodName,
11121126
Pool: goodName,
11131127
Device: goodName,
1128+
NetworkData: &resource.NetworkDeviceData{
1129+
Addresses: []string{
1130+
"2001:db8::1/64",
1131+
"2001:0db8::1/64",
1132+
},
1133+
},
11141134
},
11151135
{
11161136
Driver: goodName,
@@ -1124,6 +1144,8 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
11241144
},
11251145
"invalid-network-device-status-disabled-feature-gate": {
11261146
wantFailures: field.ErrorList{
1147+
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", interfaceNameMaxLength),
1148+
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", hardwareAddressMaxLength),
11271149
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)"),
11281150
},
11291151
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
@@ -1134,6 +1156,8 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
11341156
Pool: goodName,
11351157
Device: goodName,
11361158
NetworkData: &resource.NetworkDeviceData{
1159+
InterfaceName: strings.Repeat("x", interfaceNameMaxLength+1),
1160+
HardwareAddress: strings.Repeat("x", hardwareAddressMaxLength+1),
11371161
Addresses: []string{
11381162
"300.9.8.0/24",
11391163
},
@@ -1146,7 +1170,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
11461170
},
11471171
"invalid-data-device-status-disabled-feature-gate": {
11481172
wantFailures: field.ErrorList{
1149-
field.Invalid(field.NewPath("status", "devices").Index(0).Child("data"), "<value omitted>", "error parsing data: invalid character 'o' in literal false (expecting 'a')"),
1173+
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')"),
11501174
},
11511175
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
11521176
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {

0 commit comments

Comments
 (0)