diff --git a/PROJECT b/PROJECT index 08f77d0a..4137afa2 100644 --- a/PROJECT +++ b/PROJECT @@ -30,6 +30,10 @@ resources: kind: ProxmoxMachine path: github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1 version: v1alpha1 + webhooks: + defaulting: true + validation: true + webhookVersion: v1 - api: crdVersion: v1 namespaced: true diff --git a/api/v1alpha1/proxmoxmachine_types.go b/api/v1alpha1/proxmoxmachine_types.go index c843dd3a..851d7e39 100644 --- a/api/v1alpha1/proxmoxmachine_types.go +++ b/api/v1alpha1/proxmoxmachine_types.go @@ -216,6 +216,13 @@ type NetworkDevice struct { // +kubebuilder:validation:Enum=e1000;virtio;rtl8139;vmxnet3 // +kubebuilder:default=virtio Model *string `json:"model,omitempty"` + + // MTU is the network device Maximum Transmission Unit. + // Only works with virtio Model. + // +optional + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=65520 + MTU *uint16 `json:"mtu,omitempty"` } // AdditionalNetworkDevice the definition of a Proxmox network device. diff --git a/api/v1alpha1/proxmoxmachine_types_test.go b/api/v1alpha1/proxmoxmachine_types_test.go index 4a77b375..4390769d 100644 --- a/api/v1alpha1/proxmoxmachine_types_test.go +++ b/api/v1alpha1/proxmoxmachine_types_test.go @@ -200,5 +200,29 @@ var _ = Describe("ProxmoxMachine Test", func() { } Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("at least one pool reference must be set, either ipv4PoolRef or ipv6PoolRef"))) }) + + It("Should not allow machine with network device mtu less than 1", func() { + dm := defaultMachine() + dm.Spec.Network = &NetworkSpec{ + Default: &NetworkDevice{ + Bridge: "vmbr0", + MTU: ptr.To(uint16(0)), + }, + } + + Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be greater than or equal to 1"))) + }) + + It("Should not allow machine with network device mtu greater than 65520", func() { + dm := defaultMachine() + dm.Spec.Network = &NetworkSpec{ + Default: &NetworkDevice{ + Bridge: "vmbr0", + MTU: ptr.To(uint16(65521)), + }, + } + + Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("should be less than or equal to 65520"))) + }) }) }) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 792ce632..0471b26a 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -98,6 +98,11 @@ func (in *NetworkDevice) DeepCopyInto(out *NetworkDevice) { *out = new(string) **out = **in } + if in.MTU != nil { + in, out := &in.MTU, &out.MTU + *out = new(uint16) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkDevice. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml index 14dc44cd..0f23df91 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml @@ -205,6 +205,12 @@ spec: - rtl8139 - vmxnet3 type: string + mtu: + description: MTU is the network device Maximum Transmission + Unit. Only works with virtio Model. + maximum: 65520 + minimum: 1 + type: integer name: description: Name is the network device name. must be unique within the virtual machine and different from the primary @@ -245,6 +251,12 @@ spec: - rtl8139 - vmxnet3 type: string + mtu: + description: MTU is the network device Maximum Transmission + Unit. Only works with virtio Model. + maximum: 65520 + minimum: 1 + type: integer required: - bridge type: object diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml index 180655aa..de931bdd 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml @@ -223,6 +223,12 @@ spec: - rtl8139 - vmxnet3 type: string + mtu: + description: MTU is the network device Maximum Transmission + Unit. Only works with virtio Model. + maximum: 65520 + minimum: 1 + type: integer name: description: Name is the network device name. must be unique within the virtual machine and different @@ -265,6 +271,12 @@ spec: - rtl8139 - vmxnet3 type: string + mtu: + description: MTU is the network device Maximum Transmission + Unit. Only works with virtio Model. + maximum: 65520 + minimum: 1 + type: integer required: - bridge type: object diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 51a36fa1..c4fdf247 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -26,3 +26,24 @@ webhooks: resources: - proxmoxclusters sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-infrastructure-cluster-x-k8s-io-v1alpha1-proxmoxmachine + failurePolicy: Fail + matchPolicy: Equivalent + name: validation.proxmoxmachine.infrastructure.cluster.x-k8s.io + rules: + - apiGroups: + - infrastructure.cluster.x-k8s.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - proxmoxmachines + sideEffects: None diff --git a/internal/service/vmservice/utils.go b/internal/service/vmservice/utils.go index f0935f01..1bdd5650 100644 --- a/internal/service/vmservice/utils.go +++ b/internal/service/vmservice/utils.go @@ -19,8 +19,10 @@ package vmservice import ( "fmt" "regexp" + "strconv" "github.com/google/uuid" + infrav1alpha1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" "github.com/ionos-cloud/cluster-api-provider-proxmox/pkg/scope" ) @@ -55,14 +57,39 @@ func IPAddressWithPrefix(ip string, prefix int) string { return fmt.Sprintf("%s/%d", ip, prefix) } -// extractNetworkModelAndBridge returns the model & bridge out of net device input e.g. virtio=A6:23:64:4D:84:CB,bridge=vmbr1. -func extractNetworkModelAndBridge(input string) (string, string) { - re := regexp.MustCompile(`([^=,]+)=([^,]+),bridge=([^,]+)`) +// extractNetworkModel returns the model out of net device input e.g. virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500. +func extractNetworkModel(input string) string { + re := regexp.MustCompile(`([^,=]+)(?:=[^,]*)?,bridge=([^,]+)`) matches := re.FindStringSubmatch(input) - if len(matches) == 4 { - return matches[1], matches[3] + if len(matches) >= 2 { + return matches[1] + } + return "" +} + +// extractNetworkBridge returns the bridge out of net device input e.g. virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500. +func extractNetworkBridge(input string) string { + re := regexp.MustCompile(`bridge=(\w+)`) + match := re.FindStringSubmatch(input) + if len(match) > 1 { + return match[1] + } + return "unknown" +} + +// extractNetworkMTU returns the mtu out of net device input e.g. virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500. +func extractNetworkMTU(input string) uint16 { + re := regexp.MustCompile(`mtu=(\d+)`) + match := re.FindStringSubmatch(input) + if len(match) > 1 { + mtu, err := strconv.ParseUint(match[1], 10, 16) + if err != nil { + return 0 + } + return uint16(mtu) } - return "", "" + + return 0 } func shouldUpdateNetworkDevices(machineScope *scope.MachineScope) bool { @@ -78,10 +105,23 @@ func shouldUpdateNetworkDevices(machineScope *scope.MachineScope) bool { if net0 == "" { return true } - model, bridge := extractNetworkModelAndBridge(net0) - if model != *machineScope.ProxmoxMachine.Spec.Network.Default.Model || bridge != machineScope.ProxmoxMachine.Spec.Network.Default.Bridge { + + desiredDefault := *machineScope.ProxmoxMachine.Spec.Network.Default + + model := extractNetworkModel(net0) + bridge := extractNetworkBridge(net0) + + if model != *desiredDefault.Model || bridge != desiredDefault.Bridge { return true } + + if desiredDefault.MTU != nil { + mtu := extractNetworkMTU(net0) + + if mtu != *desiredDefault.MTU { + return true + } + } } devices := machineScope.ProxmoxMachine.Spec.Network.AdditionalDevices @@ -91,11 +131,22 @@ func shouldUpdateNetworkDevices(machineScope *scope.MachineScope) bool { if len(net) == 0 { return true } - model, bridge := extractNetworkModelAndBridge(net) + + model := extractNetworkModel(net) + bridge := extractNetworkBridge(net) + // current is different from the desired spec. if model != *v.Model || bridge != v.Bridge { return true } + + if v.MTU != nil { + mtu := extractNetworkMTU(net) + + if mtu != *v.MTU { + return true + } + } } return false @@ -103,8 +154,12 @@ func shouldUpdateNetworkDevices(machineScope *scope.MachineScope) bool { // formatNetworkDevice formats a network device config // example 'virtio,bridge=vmbr0'. -func formatNetworkDevice(model, bridge string) string { - return fmt.Sprintf("%s,bridge=%s", model, bridge) +func formatNetworkDevice(model, bridge string, mtu *uint16) string { + if mtu == nil { + return fmt.Sprintf("%s,bridge=%s", model, bridge) + } + + return fmt.Sprintf("%s,bridge=%s,mtu=%d", model, bridge, *mtu) } // extractMACAddress returns the macaddress out of net device input e.g. virtio=A6:23:64:4D:84:CB,bridge=vmbr1. diff --git a/internal/service/vmservice/utils_test.go b/internal/service/vmservice/utils_test.go index 119401d0..53d230b1 100644 --- a/internal/service/vmservice/utils_test.go +++ b/internal/service/vmservice/utils_test.go @@ -54,39 +54,103 @@ func TestExtractUUID(t *testing.T) { } } -func TestExtractNetworkModelAndBridge(t *testing.T) { +func TestExtractNetworkModel(t *testing.T) { type match struct { - test string - expectedModel string - expectedBridge string + test string + expected string } goodstrings := []match{ - {"virtio=A6:23:64:4D:84:CB,bridge=vmbr1", "virtio", "vmbr1"}, - {"foo,virtio=A6:23:64:4D:84:CB,bridge=vmbr1", "virtio", "vmbr1"}, - {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1", "virtio", "vmbr1"}, - {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,foo", "virtio", "vmbr1"}, - {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,foo=bar", "virtio", "vmbr1"}, + {"virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500", "virtio"}, + {"foo,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500", "virtio"}, + {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500", "virtio"}, + {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500,foo", "virtio"}, + {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500,foo=bar", "virtio"}, + {"foo=bar,e1000=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=9000,foo=bar", "e1000"}, + {"foo=bar,e1000=a6:23:64:4d:84:Cb,bridge=vmbr1,mtu=9000,foo=bar", "e1000"}, } badstrings := []string{ "bridge=vmbr1", - "virtio=,bridge=vmbr1", - "virtio=,bridge=", + "virtio=", "uuid=7dd9b137-6a3c-4661-a4fa-375075e1776b", + "a6:23:64:4d:84:Cb", + "=", "", } for _, m := range goodstrings { - model, bridge := extractNetworkModelAndBridge(m.test) - require.Equal(t, m.expectedModel, model) - require.Equal(t, m.expectedBridge, bridge) + model := extractNetworkModel(m.test) + require.Equal(t, m.expected, model) } for _, s := range badstrings { - model, bridge := extractNetworkModelAndBridge(s) + model := extractNetworkModel(s) require.Empty(t, model) - require.Empty(t, bridge) + } +} + +func TestExtractNetworkBridge(t *testing.T) { + type match struct { + test string + expected string + } + + goodstrings := []match{ + {"virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500", "vmbr1"}, + {"foo,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500", "vmbr1"}, + {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500", "vmbr1"}, + {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500,foo", "vmbr1"}, + {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500,foo=bar", "vmbr1"}, + } + + badstrings := []string{ + "virtio=", + "bridge=", + "uuid=7dd9b137-6a3c-4661-a4fa-375075e1776b", + "", + } + + for _, m := range goodstrings { + bridge := extractNetworkBridge(m.test) + require.Equal(t, m.expected, bridge) + } + + for _, s := range badstrings { + bridge := extractNetworkBridge(s) + require.Equal(t, "unknown", bridge) + } +} + +func TestExtractNetworkMTU(t *testing.T) { + type match struct { + test string + expected uint16 + } + + goodstrings := []match{ + {"virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500", 1500}, + {"foo,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500", 1500}, + {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500", 1500}, + {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=9000,foo", 9000}, + {"foo=bar,virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=9000,foo=bar", 9000}, + } + + badstrings := []string{ + "virtio=", + "bridge=", + "uuid=7dd9b137-6a3c-4661-a4fa-375075e1776b", + "", + } + + for _, m := range goodstrings { + mtu := extractNetworkMTU(m.test) + require.Equal(t, m.expected, mtu) + } + + for _, s := range badstrings { + mtu := extractNetworkMTU(s) + require.Equal(t, uint16(0), mtu) } } @@ -143,12 +207,12 @@ func TestShouldUpdateNetworkDevices_AdditionalDeviceNeedsUpdate(t *testing.T) { func TestShouldUpdateNetworkDevices_NoUpdate(t *testing.T) { machineScope, _, _ := setupReconcilerTest(t) machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ - Default: &infrav1alpha1.NetworkDevice{Bridge: "vmbr0", Model: ptr.To("virtio")}, + Default: &infrav1alpha1.NetworkDevice{Bridge: "vmbr0", Model: ptr.To("virtio"), MTU: ptr.To(uint16(1500))}, AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ - {Name: "net1", NetworkDevice: infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}}, + {Name: "net1", NetworkDevice: infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio"), MTU: ptr.To(uint16(1500))}}, }, } - machineScope.SetVirtualMachine(newVMWithNets("virtio=A6:23:64:4D:84:CD,bridge=vmbr0", "virtio=A6:23:64:4D:84:CD,bridge=vmbr1")) + machineScope.SetVirtualMachine(newVMWithNets("virtio=A6:23:64:4D:84:CD,bridge=vmbr0,mtu=1500", "virtio=A6:23:64:4D:84:CD,bridge=vmbr1,mtu=1500")) require.False(t, shouldUpdateNetworkDevices(machineScope)) } diff --git a/internal/service/vmservice/vm.go b/internal/service/vmservice/vm.go index 978c301d..5d9caa47 100644 --- a/internal/service/vmservice/vm.go +++ b/internal/service/vmservice/vm.go @@ -191,8 +191,12 @@ func reconcileVirtualMachineConfig(ctx context.Context, machineScope *scope.Mach if machineScope.ProxmoxMachine.Spec.Network != nil && shouldUpdateNetworkDevices(machineScope) { // adding the default network device. vmOptions = append(vmOptions, proxmox.VirtualMachineOption{ - Name: infrav1alpha1.DefaultNetworkDevice, - Value: formatNetworkDevice(*machineScope.ProxmoxMachine.Spec.Network.Default.Model, machineScope.ProxmoxMachine.Spec.Network.Default.Bridge), + Name: infrav1alpha1.DefaultNetworkDevice, + Value: formatNetworkDevice( + *machineScope.ProxmoxMachine.Spec.Network.Default.Model, + machineScope.ProxmoxMachine.Spec.Network.Default.Bridge, + machineScope.ProxmoxMachine.Spec.Network.Default.MTU, + ), }) // handing additional network devices. @@ -200,7 +204,7 @@ func reconcileVirtualMachineConfig(ctx context.Context, machineScope *scope.Mach for _, v := range devices { vmOptions = append(vmOptions, proxmox.VirtualMachineOption{ Name: v.Name, - Value: formatNetworkDevice(*v.Model, v.Bridge), + Value: formatNetworkDevice(*v.Model, v.Bridge, v.MTU), }) } } diff --git a/internal/service/vmservice/vm_test.go b/internal/service/vmservice/vm_test.go index 3965c295..7b88d0f6 100644 --- a/internal/service/vmservice/vm_test.go +++ b/internal/service/vmservice/vm_test.go @@ -160,11 +160,11 @@ func TestReconcileVirtualMachineConfig_ApplyConfig(t *testing.T) { machineScope.ProxmoxMachine.Spec.NumCores = 4 machineScope.ProxmoxMachine.Spec.MemoryMiB = 16 * 1024 machineScope.ProxmoxMachine.Spec.Network = &infrav1alpha1.NetworkSpec{ - Default: &infrav1alpha1.NetworkDevice{Bridge: "vmbr0", Model: ptr.To("virtio")}, + Default: &infrav1alpha1.NetworkDevice{Bridge: "vmbr0", Model: ptr.To("virtio"), MTU: ptr.To(uint16(1500))}, AdditionalDevices: []infrav1alpha1.AdditionalNetworkDevice{ { Name: "net1", - NetworkDevice: infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio")}, + NetworkDevice: infrav1alpha1.NetworkDevice{Bridge: "vmbr1", Model: ptr.To("virtio"), MTU: ptr.To(uint16(1500))}, }, }, } @@ -176,8 +176,8 @@ func TestReconcileVirtualMachineConfig_ApplyConfig(t *testing.T) { proxmox.VirtualMachineOption{Name: optionSockets, Value: machineScope.ProxmoxMachine.Spec.NumSockets}, proxmox.VirtualMachineOption{Name: optionCores, Value: machineScope.ProxmoxMachine.Spec.NumCores}, proxmox.VirtualMachineOption{Name: optionMemory, Value: machineScope.ProxmoxMachine.Spec.MemoryMiB}, - proxmox.VirtualMachineOption{Name: "net0", Value: formatNetworkDevice("virtio", "vmbr0")}, - proxmox.VirtualMachineOption{Name: "net1", Value: formatNetworkDevice("virtio", "vmbr1")}, + proxmox.VirtualMachineOption{Name: "net0", Value: formatNetworkDevice("virtio", "vmbr0", ptr.To(uint16(1500)))}, + proxmox.VirtualMachineOption{Name: "net1", Value: formatNetworkDevice("virtio", "vmbr1", ptr.To(uint16(1500)))}, } proxmoxClient.EXPECT().ConfigureVM(context.TODO(), vm, expectedOptions...).Return(task, nil).Once() diff --git a/internal/webhook/proxmoxmachine_webhook.go b/internal/webhook/proxmoxmachine_webhook.go new file mode 100644 index 00000000..b258b5d5 --- /dev/null +++ b/internal/webhook/proxmoxmachine_webhook.go @@ -0,0 +1,132 @@ +/* +Copyright 2023 IONOS Cloud. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package webhook contains webhooks for the custom resources. +package webhook + +import ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" +) + +// ProxmoxMachine is a type that implements +// the interfaces from the admission package. +type ProxmoxMachine struct{} + +// SetupWebhookWithManager sets up the webhook with the +// custom interfaces. +func (p *ProxmoxMachine) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&infrav1.ProxmoxMachine{}). + WithValidator(p). + Complete() +} + +//+kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha1-proxmoxmachine,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=proxmoxmachines,versions=v1alpha1,name=validation.proxmoxmachine.infrastructure.cluster.x-k8s.io,admissionReviewVersions=v1 + +// ValidateCreate implements the creation validation function. +func (p *ProxmoxMachine) ValidateCreate(_ context.Context, obj runtime.Object) (warnings admission.Warnings, err error) { + machine, ok := obj.(*infrav1.ProxmoxMachine) + if !ok { + return warnings, apierrors.NewBadRequest(fmt.Sprintf("expected a ProxmoxMachine but got %T", obj)) + } + + err = validateNetworks(machine) + if err != nil { + warnings = append(warnings, fmt.Sprintf("cannot create proxmox machine %s", machine.GetName())) + return warnings, err + } + + return warnings, nil +} + +// ValidateUpdate implements the update validation function. +func (p *ProxmoxMachine) ValidateUpdate(_ context.Context, _, newObj runtime.Object) (warnings admission.Warnings, err error) { + newMachine, ok := newObj.(*infrav1.ProxmoxMachine) + if !ok { + return warnings, apierrors.NewBadRequest(fmt.Sprintf("expected a ProxmoxMachine but got %T", newObj)) + } + + err = validateNetworks(newMachine) + if err != nil { + warnings = append(warnings, fmt.Sprintf("cannot update proxmox machine %s", newMachine.GetName())) + return warnings, err + } + + return warnings, nil +} + +// ValidateDelete implements the deletion validation function. +func (p *ProxmoxMachine) ValidateDelete(_ context.Context, _ runtime.Object) (warnings admission.Warnings, err error) { + return nil, nil +} + +func validateNetworks(machine *infrav1.ProxmoxMachine) error { + gk, name := machine.GroupVersionKind().GroupKind(), machine.GetName() + + if machine.Spec.Network.Default != nil { + err := validateNetworkDevice(machine.Spec.Network.Default) + if err != nil { + return apierrors.NewInvalid( + gk, + name, + field.ErrorList{ + field.Invalid( + field.NewPath("spec", "network", "default", "mtu"), machine.Spec.Network.Default, err.Error()), + }) + } + } + + for i := range machine.Spec.Network.AdditionalDevices { + err := validateNetworkDevice(&machine.Spec.Network.AdditionalDevices[i].NetworkDevice) + if err != nil { + return apierrors.NewInvalid( + gk, + name, + field.ErrorList{ + field.Invalid( + field.NewPath("spec", "network", "additionalDevices", fmt.Sprint(i), "mtu"), machine.Spec.Network.Default, err.Error()), + }) + } + } + + return nil +} + +func validateNetworkDevice(device *infrav1.NetworkDevice) error { + if device.MTU == nil { + return nil + } + + if *device.MTU == 1 { + return nil + } + + if *device.MTU > 999 { + return nil + } + + return fmt.Errorf("mtu must be at least 1000 or 1, but was %d", *device.MTU) +} diff --git a/internal/webhook/proxmoxmachine_webhook_test.go b/internal/webhook/proxmoxmachine_webhook_test.go new file mode 100644 index 00000000..388ed289 --- /dev/null +++ b/internal/webhook/proxmoxmachine_webhook_test.go @@ -0,0 +1,105 @@ +/* +Copyright 2023 IONOS Cloud. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package webhook + +import ( + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1" +) + +var _ = Describe("Controller Test", func() { + g := NewWithT(GinkgoT()) + + Context("create proxmox machine", func() { + It("should disallow invalid network mtu", func() { + machine := invalidProxmoxMachine("test-machine") + g.Expect(k8sClient.Create(testEnv.GetContext(), &machine)).To(MatchError(ContainSubstring("spec.network.default.mtu: Invalid value"))) + }) + + It("should create a valid proxmox machine", func() { + machine := validProxmoxMachine("test-machine") + g.Expect(k8sClient.Create(testEnv.GetContext(), &machine)).To(Succeed()) + }) + }) + + Context("update proxmox cluster", func() { + It("should disallow invalid network mtu", func() { + clusterName := "test-cluster" + machine := validProxmoxMachine(clusterName) + g.Expect(k8sClient.Create(testEnv.GetContext(), &machine)).To(Succeed()) + + g.Expect(k8sClient.Get(testEnv.GetContext(), client.ObjectKeyFromObject(&machine), &machine)).To(Succeed()) + machine.Spec.Network.Default.MTU = ptr.To(uint16(50)) + + g.Expect(k8sClient.Create(testEnv.GetContext(), &machine)).To(MatchError(ContainSubstring("spec.network.default.mtu: Invalid value"))) + + g.Eventually(func(g Gomega) { + g.Expect(client.IgnoreNotFound(k8sClient.Delete(testEnv.GetContext(), &machine))).To(Succeed()) + }).WithTimeout(time.Second * 10). + WithPolling(time.Second). + Should(Succeed()) + }) + }) +}) + +func validProxmoxMachine(name string) infrav1.ProxmoxMachine { + return infrav1.ProxmoxMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + Spec: infrav1.ProxmoxMachineSpec{ + VirtualMachineCloneSpec: infrav1.VirtualMachineCloneSpec{ + SourceNode: "pve", + }, + NumSockets: 1, + NumCores: 1, + MemoryMiB: 1024, + Disks: &infrav1.Storage{ + BootVolume: &infrav1.DiskSize{ + Disk: "scsi[0]", + SizeGB: 10, + }, + }, + Network: &infrav1.NetworkSpec{ + Default: &infrav1.NetworkDevice{ + Bridge: "vmbr1", + Model: ptr.To("virtio"), + MTU: ptr.To(uint16(1500)), + }, + }, + }, + } +} + +func invalidProxmoxMachine(name string) infrav1.ProxmoxMachine { + machine := validProxmoxMachine(name) + machine.Spec.Network.Default = &infrav1.NetworkDevice{ + Bridge: "vmbr1", + Model: ptr.To("virtio"), + MTU: ptr.To(uint16(50)), + } + return machine +} diff --git a/internal/webhook/webhook_suite_test.go b/internal/webhook/webhook_suite_test.go index ed28aa7d..69561dce 100644 --- a/internal/webhook/webhook_suite_test.go +++ b/internal/webhook/webhook_suite_test.go @@ -65,6 +65,9 @@ var _ = BeforeSuite(func() { err = (&ProxmoxCluster{}).SetupWebhookWithManager(testEnv.Manager) Expect(err).NotTo(HaveOccurred()) + err = (&ProxmoxMachine{}).SetupWebhookWithManager(testEnv.Manager) + Expect(err).NotTo(HaveOccurred()) + //+kubebuilder:scaffold:webhook go func() {