Merge pull request #114266 from dobsonj/vclib-index-out-of-range

legacy-cloud-providers: prevent index out-of-range in getNextUnitNumber

staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/utils.go

@@ -94,7 +94,7 @@ func getNextUnitNumber(devices object.VirtualDeviceList, c types.BaseVirtualCont
 	for _, device := range devices {
 		d := device.GetVirtualDevice()
 		if d.ControllerKey == key {
-			if d.UnitNumber != nil {
+			if d.UnitNumber != nil && *d.UnitNumber < SCSIDeviceSlots {
 				takenUnitNumbers[*d.UnitNumber] = true
 			}
 		}

staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/utils_test.go

@@ -21,7 +21,9 @@ import (
 	"testing"
 
 	"github.com/vmware/govmomi"
+	"github.com/vmware/govmomi/object"
 	"github.com/vmware/govmomi/simulator"
+	"github.com/vmware/govmomi/vim25/types"
 )
 
 func TestUtils(t *testing.T) {
@@ -103,3 +105,59 @@ func TestIsvCenterNotSupported(t *testing.T) {
 		}
 	}
 }
+
+func TestGetNextUnitNumber(t *testing.T) {
+	type testData struct {
+		name        string
+		deviceList  object.VirtualDeviceList
+		expectValue int32
+		expectError bool
+	}
+	tests := []testData{
+		{
+			name:        "should return 3 when devices 0-2 taken",
+			deviceList:  generateVirtualDeviceList([]int32{0, 1, 2}),
+			expectValue: 3,
+		},
+		{
+			name:        "should return 0 when devices 1-3 taken",
+			deviceList:  generateVirtualDeviceList([]int32{1, 2, 3}),
+			expectValue: 0,
+		},
+		{
+			name:        "should return error when no slots available",
+			deviceList:  generateVirtualDeviceList([]int32{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
+			expectValue: -1,
+			expectError: true,
+		},
+		{
+			name:        "should ignore invalid UnitNumber in device list",
+			deviceList:  generateVirtualDeviceList([]int32{0, 1, 16}),
+			expectValue: 2,
+		},
+	}
+
+	controller := &types.VirtualController{}
+	for _, test := range tests {
+		val, err := getNextUnitNumber(test.deviceList, controller)
+		if err != nil && !test.expectError {
+			t.Fatalf("%s: unexpected error: %v", test.name, err)
+		}
+		if val != test.expectValue {
+			t.Fatalf("%s: expected value %v but got %v", test.name, test.expectValue, val)
+		}
+	}
+}
+
+func generateVirtualDeviceList(unitNumbers []int32) object.VirtualDeviceList {
+	deviceList := object.VirtualDeviceList{}
+	for _, val := range unitNumbers {
+		unitNum := val
+		dev := &types.VirtualDevice{
+			Key:        unitNum,
+			UnitNumber: &unitNum,
+		}
+		deviceList = append(deviceList, dev)
+	}
+	return deviceList
+}