diff --git a/api/v1alpha1/proxmoxmachine_types.go b/api/v1alpha1/proxmoxmachine_types.go index 7b564641..39245590 100644 --- a/api/v1alpha1/proxmoxmachine_types.go +++ b/api/v1alpha1/proxmoxmachine_types.go @@ -281,7 +281,7 @@ type RoutingPolicySpec struct { // Table is the routing table ID. // +optional - Table uint32 `json:"table,omitempty"` + Table *uint32 `json:"table,omitempty"` // Priority is the position in the ip rule FIB table. // +kubebuilder:validation:Maximum=4294967295 diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 7ff935c2..e049639a 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -768,7 +768,9 @@ func (in *Routing) DeepCopyInto(out *Routing) { if in.RoutingPolicy != nil { in, out := &in.RoutingPolicy, &out.RoutingPolicy *out = make([]RoutingPolicySpec, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } } @@ -785,6 +787,11 @@ func (in *Routing) DeepCopy() *Routing { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RoutingPolicySpec) DeepCopyInto(out *RoutingPolicySpec) { *out = *in + if in.Table != nil { + in, out := &in.Table, &out.Table + *out = new(uint32) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoutingPolicySpec. diff --git a/internal/service/vmservice/bootstrap.go b/internal/service/vmservice/bootstrap.go index bc1b6767..73a55a92 100644 --- a/internal/service/vmservice/bootstrap.go +++ b/internal/service/vmservice/bootstrap.go @@ -168,7 +168,9 @@ func getRoutingPolicyData(rules []infrav1alpha1.RoutingPolicySpec) *[]cloudinit. ruleSpec.To = rule.To ruleSpec.From = rule.From ruleSpec.Priority = rule.Priority - ruleSpec.Table = rule.Table + if rule.Table != nil { + ruleSpec.Table = *rule.Table + } routingPolicyData = append(routingPolicyData, ruleSpec) } diff --git a/internal/service/vmservice/bootstrap_test.go b/internal/service/vmservice/bootstrap_test.go index 22a0462a..6c4c13bb 100644 --- a/internal/service/vmservice/bootstrap_test.go +++ b/internal/service/vmservice/bootstrap_test.go @@ -205,8 +205,8 @@ func TestGetCommonInterfaceConfig(t *testing.T) { {To: "172.24.16.0/24", Via: "192.168.178.1", Table: 100}, }, RoutingPolicy: []infrav1alpha1.RoutingPolicySpec{ - {To: "10.10.10.0/24", Table: 100}, - {From: "172.24.16.0/24", Table: 100}, + {To: "10.10.10.0/24", Table: ptr.To(uint32(100))}, + {From: "172.24.16.0/24", Table: ptr.To(uint32(100))}, }, }, }, diff --git a/internal/webhook/proxmoxmachine_webhook.go b/internal/webhook/proxmoxmachine_webhook.go index 5471822f..400916f7 100644 --- a/internal/webhook/proxmoxmachine_webhook.go +++ b/internal/webhook/proxmoxmachine_webhook.go @@ -120,8 +120,53 @@ func validateNetworks(machine *infrav1.ProxmoxMachine) error { field.NewPath("spec", "network", "additionalDevices", fmt.Sprint(i), "linkMtu"), machine.Spec.Network.AdditionalDevices[i], err.Error()), }) } + err = validateRoutingPolicy(&machine.Spec.Network.AdditionalDevices[i].InterfaceConfig.RoutingPolicy) + if err != nil { + return apierrors.NewInvalid( + gk, + name, + field.ErrorList{ + field.Invalid( + field.NewPath("spec", "network", "additionalDevices", fmt.Sprint(i), "routingPolicy"), machine.Spec.Network.AdditionalDevices[i], err.Error()), + }) + } + } + + for i := range machine.Spec.Network.VirtualNetworkDevices.VRFs { + err := validateVRFConfigRoutingPolicy(&machine.Spec.Network.VirtualNetworkDevices.VRFs[i]) + if err != nil { + return apierrors.NewInvalid( + gk, + name, + field.ErrorList{ + field.Invalid( + field.NewPath("spec", "network", "VirtualNetworkDevices", "VRFs", fmt.Sprint(i), "Table"), machine.Spec.Network.VirtualNetworkDevices.VRFs[i], err.Error()), + }) + } + } + + return nil +} + +func validateRoutingPolicy(policies *[]infrav1.RoutingPolicySpec) error { + for i, policy := range *policies { + if policy.Table == nil { + return fmt.Errorf("routing policy [%d] requires a table", i) + } } + return nil +} +func validateVRFConfigRoutingPolicy(vrf *infrav1.VRFDevice) error { + for _, policy := range vrf.Routing.RoutingPolicy { + // Netplan will not accept rules not matching the l3mdev table, although + // there is no technical reason for this limitation. + if policy.Table != nil { + if *policy.Table != vrf.Table { + return fmt.Errorf("VRF %s: device/rule routing table mismatch %d != %d", vrf.Name, vrf.Table, *policy.Table) + } + } + } return nil } diff --git a/internal/webhook/proxmoxmachine_webhook_test.go b/internal/webhook/proxmoxmachine_webhook_test.go index eb33c0a9..025a385f 100644 --- a/internal/webhook/proxmoxmachine_webhook_test.go +++ b/internal/webhook/proxmoxmachine_webhook_test.go @@ -65,6 +65,18 @@ var _ = Describe("Controller Test", func() { machine.Spec.Network.AdditionalDevices[0].LinkMTU = ptr.To(uint16(1000)) g.Expect(k8sClient.Create(testEnv.GetContext(), &machine)).To(MatchError(ContainSubstring("mtu must be at least 1280, but was 1000"))) }) + + It("should disallow conflicting l3mdev/routing policy table", func() { + machine := validProxmoxMachine("test-machine") + *machine.Spec.Network.VirtualNetworkDevices.VRFs[0].Routing.RoutingPolicy[0].Table = 667 + g.Expect(k8sClient.Create(testEnv.GetContext(), &machine)).To(MatchError(ContainSubstring("VRF vrf-green: device/rule routing table mismatch 665 != 667"))) + }) + + It("should disallow routing policy without table", func() { + machine := validProxmoxMachine("test-machine") + machine.Spec.Network.AdditionalDevices[0].InterfaceConfig.Routing.RoutingPolicy[0].Table = nil + g.Expect(k8sClient.Create(testEnv.GetContext(), &machine)).To(MatchError(ContainSubstring("routing policy [0] requires a table"))) + }) }) Context("update proxmox cluster", func() { @@ -131,9 +143,25 @@ func validProxmoxMachine(name string) infrav1.ProxmoxMachine { Kind: "InClusterIPPool", APIGroup: ptr.To("ipam.cluster.x-k8s.io"), }, + Routing: infrav1.Routing{ + RoutingPolicy: []infrav1.RoutingPolicySpec{{ + Table: ptr.To(uint32(665)), + }}, + }, }, }, }, + VirtualNetworkDevices: infrav1.VirtualNetworkDevices{ + VRFs: []infrav1.VRFDevice{{ + Table: 665, + Name: "vrf-green", + Routing: infrav1.Routing{ + RoutingPolicy: []infrav1.RoutingPolicySpec{{ + Table: ptr.To(uint32(665)), + }}, + }}, + }, + }, }, }, }