Skip to content

Commit

Permalink
Update validation webhooks
Browse files Browse the repository at this point in the history
  • Loading branch information
birksl committed Jun 7, 2024
1 parent eb17a44 commit 15d2696
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 61 deletions.
11 changes: 11 additions & 0 deletions internal/webhooks/vspheremachine.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,17 @@ func (webhook *VSphereMachineWebhook) ValidateCreate(_ context.Context, raw runt
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "guestSoftPowerOffTimeout"), spec.GuestSoftPowerOffTimeout, "should be greater than 0"))
}
}
for i, device := range spec.PciDevices {
if device.VGPUProfile == "" {
if device.DeviceID == nil || device.VendorID == nil {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "template", "spec", "pciDevices", fmt.Sprintf("%d", i)), device, "should have both deviceId and vendorId set"))
}
} else {
if device.DeviceID != nil || device.VendorID != nil {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "template", "spec", "pciDevices", fmt.Sprintf("%d", i)), device, "should have either deviceId + vendorId or vgpuProfile"))
}
}
}

return nil, AggregateObjErrors(obj.GroupVersionKind().GroupKind(), obj.Name, allErrs)
}
Expand Down
95 changes: 68 additions & 27 deletions internal/webhooks/vspheremachine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,52 +48,86 @@ func TestVSphereMachine_ValidateCreate(t *testing.T) {
}{
{
name: "preferredAPIServerCIDR set on creation ",
vsphereMachine: createVSphereMachine("foo.com", nil, "192.168.0.1/32", []string{}, infrav1.VirtualMachinePowerOpModeTrySoft, nil),
vsphereMachine: createVSphereMachine("foo.com", nil, "192.168.0.1/32", []string{}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, nil),
wantErr: true,
},
{
name: "IPs are not in CIDR format",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil),
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, nil),
wantErr: true,
},
{
name: "IPs are not valid IPs in CIDR format",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"<nil>/32", "192.168.0.644/33"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil),
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"<nil>/32", "192.168.0.644/33"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, nil),
wantErr: true,
},
{
name: "guestSoftPowerOffTimeout should not be set with powerOffMode set to hard",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeHard, &metav1.Duration{Duration: infrav1.GuestSoftPowerOffDefaultTimeout}),
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeHard, &metav1.Duration{Duration: infrav1.GuestSoftPowerOffDefaultTimeout}, nil),
wantErr: true,
},
{
name: "guestSoftPowerOffTimeout should not be set with powerOffMode set to soft",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeSoft, &metav1.Duration{Duration: infrav1.GuestSoftPowerOffDefaultTimeout}),
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeSoft, &metav1.Duration{Duration: infrav1.GuestSoftPowerOffDefaultTimeout}, nil),
wantErr: true,
},
{
name: "guestSoftPowerOffTimeout should not be negative",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeHard, &metav1.Duration{Duration: -1234}),
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeHard, &metav1.Duration{Duration: -1234}, nil),
wantErr: true,
},

{
name: "empty pciDevice",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, []infrav1.PCIDeviceSpec{{VGPUProfile: ""}}),
wantErr: true,
},
{
name: "incorrect pciDevice",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, []infrav1.PCIDeviceSpec{{VGPUProfile: "vgpu", DeviceID: new(int32)}}),
wantErr: true,
},
{
name: "incorrect pciDevice",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, []infrav1.PCIDeviceSpec{{VGPUProfile: "vgpu", DeviceID: new(int32), VendorID: new(int32)}}),
wantErr: true,
},
{
name: "incomplete pciDevice",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, []infrav1.PCIDeviceSpec{{DeviceID: new(int32)}}),
wantErr: true,
},
{
name: "incomplete pciDevice",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, []infrav1.PCIDeviceSpec{{VendorID: new(int32)}}),
wantErr: true,
},
{
name: "successful VSphereMachine creation with PCI device",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, []infrav1.PCIDeviceSpec{{DeviceID: new(int32), VendorID: new(int32)}}),
},
{
name: "successful VSphereMachine creation with vgpu",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, []infrav1.PCIDeviceSpec{{VGPUProfile: "vgpu"}}),
},
{
name: "successful VSphereMachine creation",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil),
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, nil),
wantErr: false,
},
{
name: "successful VSphereMachine creation with powerOffMode set to hard",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeHard, nil),
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeHard, nil, nil),
wantErr: false,
},
{
name: "successful VSphereMachine creation with powerOffMode set to soft",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil),
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
wantErr: false,
},
{
name: "successful VSphereMachine creation with powerOffMode set to trySoft and non-default timeout",
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, &metav1.Duration{Duration: 1234}),
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32", "192.168.0.3/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, &metav1.Duration{Duration: 1234}, nil),
wantErr: false,
},
}
Expand Down Expand Up @@ -121,50 +155,56 @@ func TestVSphereMachine_ValidateUpdate(t *testing.T) {
}{
{
name: "ProviderID can be updated",
oldVSphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil),
oldVSphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
wantErr: false,
},
{
name: "updating ips can be done",
oldVSphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32", "192.168.0.10/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil),
oldVSphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32", "192.168.0.10/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
wantErr: false,
},
{
name: "updating non-existing IP with invalid ips can not be done",
oldVSphereMachine: createVSphereMachine("foo.com", nil, "", nil, infrav1.VirtualMachinePowerOpModeSoft, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"<nil>/32", "192.168.0.10/33"}, infrav1.VirtualMachinePowerOpModeSoft, nil),
oldVSphereMachine: createVSphereMachine("foo.com", nil, "", nil, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"<nil>/32", "192.168.0.10/33"}, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
wantErr: true,
},
{
name: "updating existing IP with invalid ips can not be done",
oldVSphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"<nil>/32", "192.168.0.10/33"}, infrav1.VirtualMachinePowerOpModeSoft, nil),
oldVSphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"<nil>/32", "192.168.0.10/33"}, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
wantErr: true,
},
{
name: "updating server cannot be done",
oldVSphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil),
vsphereMachine: createVSphereMachine("bar.com", &someProviderID, "", []string{"192.168.0.1/32", "192.168.0.10/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil),
oldVSphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
vsphereMachine: createVSphereMachine("bar.com", &someProviderID, "", []string{"192.168.0.1/32", "192.168.0.10/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
wantErr: true,
},
{
name: "updating pci devices cannot be done",
oldVSphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil, []infrav1.PCIDeviceSpec{{VGPUProfile: "vgpu"}}),
vsphereMachine: createVSphereMachine("foo.com", nil, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil, []infrav1.PCIDeviceSpec{{VGPUProfile: "new-vgpu"}}),
wantErr: true,
},
{
name: "powerOffMode cannot be updated when new powerOffMode is not valid",
oldVSphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeHard, &metav1.Duration{Duration: infrav1.GuestSoftPowerOffDefaultTimeout}),
oldVSphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, nil, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeHard, &metav1.Duration{Duration: infrav1.GuestSoftPowerOffDefaultTimeout}, nil),
wantErr: true,
},
{
name: "powerOffMode can be updated to hard",
oldVSphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, &metav1.Duration{Duration: infrav1.GuestSoftPowerOffDefaultTimeout}),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeHard, nil),
oldVSphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, &metav1.Duration{Duration: infrav1.GuestSoftPowerOffDefaultTimeout}, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeHard, nil, nil),
wantErr: false,
},
{
name: "powerOffMode can be updated to soft",
oldVSphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, &metav1.Duration{Duration: infrav1.GuestSoftPowerOffDefaultTimeout}),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil),
oldVSphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeTrySoft, &metav1.Duration{Duration: infrav1.GuestSoftPowerOffDefaultTimeout}, nil),
vsphereMachine: createVSphereMachine("foo.com", &someProviderID, "", []string{"192.168.0.1/32"}, infrav1.VirtualMachinePowerOpModeSoft, nil, nil),
wantErr: false,
},
}
Expand All @@ -181,7 +221,7 @@ func TestVSphereMachine_ValidateUpdate(t *testing.T) {
}
}

func createVSphereMachine(server string, providerID *string, preferredAPIServerCIDR string, ips []string, powerOffMode infrav1.VirtualMachinePowerOpMode, guestSoftPowerOffTimeout *metav1.Duration) *infrav1.VSphereMachine {
func createVSphereMachine(server string, providerID *string, preferredAPIServerCIDR string, ips []string, powerOffMode infrav1.VirtualMachinePowerOpMode, guestSoftPowerOffTimeout *metav1.Duration, pciDevices []infrav1.PCIDeviceSpec) *infrav1.VSphereMachine {
VSphereMachine := &infrav1.VSphereMachine{
Spec: infrav1.VSphereMachineSpec{
VirtualMachineCloneSpec: infrav1.VirtualMachineCloneSpec{
Expand All @@ -190,6 +230,7 @@ func createVSphereMachine(server string, providerID *string, preferredAPIServerC
PreferredAPIServerCIDR: preferredAPIServerCIDR,
Devices: []infrav1.NetworkDeviceSpec{},
},
PciDevices: pciDevices,
},
ProviderID: providerID,
PowerOffMode: powerOffMode,
Expand Down
12 changes: 8 additions & 4 deletions internal/webhooks/vspheremachinetemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,14 @@ func (webhook *VSphereMachineTemplateWebhook) ValidateCreate(_ context.Context,
}
}
for i, device := range spec.PciDevices {
hasVGPU := device.VGPUProfile != ""
hasPCI := device.DeviceID != nil && device.VendorID != nil
if (hasPCI && hasVGPU) || (!hasPCI && !hasVGPU) {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "template", "spec", "pciDevices", fmt.Sprintf("%d, i)), device, "should have either deviceID + vendorID or vgpuProfile"))
if device.VGPUProfile == "" {
if device.DeviceID == nil || device.VendorID == nil {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "template", "spec", "pciDevices", fmt.Sprintf("%d", i)), device, "should have both deviceId and vendorId set"))
}
} else {
if device.DeviceID != nil || device.VendorID != nil {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "template", "spec", "pciDevices", fmt.Sprintf("%d", i)), device, "should have either deviceId + vendorId or vgpuProfile"))
}
}
}
return nil, AggregateObjErrors(obj.GroupVersionKind().GroupKind(), obj.Name, allErrs)
Expand Down
Loading

0 comments on commit 15d2696

Please sign in to comment.