From b78cdb43ecf7b38e9a4fbb6fa98a9526baa15565 Mon Sep 17 00:00:00 2001 From: "christopher.dziomba@telekom.de" Date: Thu, 29 Jun 2023 17:05:09 +0200 Subject: [PATCH] Review --- api/v1alpha1/vrfrouteconfiguration_webhook.go | 6 +++ main.go | 2 +- pkg/reconciler/layer2.go | 13 ++--- pkg/reconciler/layer3.go | 48 +++++++++++++------ 4 files changed, 48 insertions(+), 21 deletions(-) diff --git a/api/v1alpha1/vrfrouteconfiguration_webhook.go b/api/v1alpha1/vrfrouteconfiguration_webhook.go index bb68c614..372e748b 100644 --- a/api/v1alpha1/vrfrouteconfiguration_webhook.go +++ b/api/v1alpha1/vrfrouteconfiguration_webhook.go @@ -83,6 +83,12 @@ func (r *VRFRouteConfiguration) validateItems() error { if err != nil { return err } + for _, item := range r.Spec.Aggregate { + _, _, err := net.ParseCIDR(item) + if err != nil { + return err + } + } return nil } diff --git a/main.go b/main.go index 884ddb85..c698f78e 100644 --- a/main.go +++ b/main.go @@ -97,11 +97,11 @@ func main() { anycastTracker := &anycast.AnycastTracker{} - // Start VRFRouteConfigurationReconciler when we are not running in only BPF mode. if err = (&networkv1alpha1.VRFRouteConfiguration{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "VRFRouteConfiguration") os.Exit(1) } + // Start VRFRouteConfigurationReconciler when we are not running in only BPF mode. if !onlyBPFMode { reconciler, err := reconciler.NewReconciler(mgr.GetClient(), anycastTracker) if err != nil { diff --git a/pkg/reconciler/layer2.go b/pkg/reconciler/layer2.go index 6342ca40..dff5d64c 100644 --- a/pkg/reconciler/layer2.go +++ b/pkg/reconciler/layer2.go @@ -41,7 +41,7 @@ func (r *reconcile) fetchLayer2() ([]networkv1alpha1.Layer2NetworkConfiguration, } if !selector.Matches(labels.Set(node.ObjectMeta.Labels)) { r.Logger.Info("local node does not match nodeSelector of layer2", "layer2", item.ObjectMeta.Name, "node", nodeName) - return nil, err + continue } } @@ -52,7 +52,7 @@ func (r *reconcile) fetchLayer2() ([]networkv1alpha1.Layer2NetworkConfiguration, } func (r *reconcile) reconcileLayer2(l2vnis []networkv1alpha1.Layer2NetworkConfiguration) error { - layer2Info := []nl.Layer2Information{} + desired := []nl.Layer2Information{} for _, layer2 := range l2vnis { spec := layer2.Spec @@ -68,7 +68,7 @@ func (r *reconcile) reconcileLayer2(l2vnis []networkv1alpha1.Layer2NetworkConfig return err } - layer2Info = append(layer2Info, nl.Layer2Information{ + desired = append(desired, nl.Layer2Information{ VlanID: spec.ID, MTU: spec.MTU, VNI: spec.VNI, @@ -88,10 +88,10 @@ func (r *reconcile) reconcileLayer2(l2vnis []networkv1alpha1.Layer2NetworkConfig delete := []nl.Layer2Information{} for _, cfg := range existing { stillExists := false - for _, info := range layer2Info { + for _, info := range desired { if info.VlanID == cfg.VlanID { - // Maybe reconcile to match MTU, gateways? stillExists = true + break } } if !stillExists { @@ -101,13 +101,14 @@ func (r *reconcile) reconcileLayer2(l2vnis []networkv1alpha1.Layer2NetworkConfig create := []nl.Layer2Information{} anycastTrackerInterfaces := []int{} - for _, info := range layer2Info { + for _, info := range desired { alreadyExists := false var currentConfig nl.Layer2Information for _, cfg := range existing { if info.VlanID == cfg.VlanID { alreadyExists = true currentConfig = cfg + break } } if !alreadyExists { diff --git a/pkg/reconciler/layer3.go b/pkg/reconciler/layer3.go index 33029eb3..117d2da5 100644 --- a/pkg/reconciler/layer3.go +++ b/pkg/reconciler/layer3.go @@ -33,13 +33,13 @@ func (r *reconcile) reconcileLayer3(l3vnis []networkv1alpha1.VRFRouteConfigurati var vni int var rt string - if val, ok := r.config.VRFToVNI[spec.VRF]; ok { - vni = val - r.Logger.Info("Configuring VRF from old VRFToVNI", "vrf", spec.VRF, "vni", val) - } else if val, ok := r.config.VRFConfig[spec.VRF]; ok { + if val, ok := r.config.VRFConfig[spec.VRF]; ok { vni = val.VNI rt = val.RT r.Logger.Info("Configuring VRF from new VRFConfig", "vrf", spec.VRF, "vni", val.VNI, "rt", rt) + } else if val, ok := r.config.VRFToVNI[spec.VRF]; ok { + vni = val + r.Logger.Info("Configuring VRF from old VRFToVNI", "vrf", spec.VRF, "vni", val) } else if r.config.ShouldSkipVRFConfig(spec.VRF) { vni = config.SKIP_VRF_TEMPLATE_VNI } else { @@ -60,13 +60,24 @@ func (r *reconcile) reconcileLayer3(l3vnis []networkv1alpha1.VRFRouteConfigurati config := vrfConfigMap[spec.VRF] if len(spec.Export) > 0 { - config.Export = append(config.Export, handlePrefixItemList(spec.Export, spec.Seq)) + prefixList, err := handlePrefixItemList(spec.Export, spec.Seq) + if err != nil { + return err + } + config.Export = append(config.Export, prefixList) } if len(spec.Import) > 0 { - config.Import = append(config.Import, handlePrefixItemList(spec.Import, spec.Seq)) + prefixList, err := handlePrefixItemList(spec.Import, spec.Seq) + if err != nil { + return err + } + config.Import = append(config.Import, prefixList) } for _, aggregate := range spec.Aggregate { - _, network, _ := net.ParseCIDR(aggregate) + _, network, err := net.ParseCIDR(aggregate) + if err != nil { + return err + } if network.IP.To4() == nil { config.AggregateIPv6 = append(config.AggregateIPv6, aggregate) } else { @@ -138,6 +149,7 @@ func (r *reconcile) reconcileL3Netlink(vrfConfigs []frr.VRFConfiguration) ([]nl. for _, vrf := range vrfConfigs { if vrf.Name == cfg.Name && vrf.VNI == cfg.VNI { stillExists = true + break } } if !stillExists { @@ -156,6 +168,7 @@ func (r *reconcile) reconcileL3Netlink(vrfConfigs []frr.VRFConfiguration) ([]nl. for _, cfg := range existing { if vrf.Name == cfg.Name && vrf.VNI == cfg.VNI { alreadyExists = true + break } } if !alreadyExists { @@ -186,22 +199,29 @@ func (r *reconcile) reconcileL3Netlink(vrfConfigs []frr.VRFConfiguration) ([]nl. return create, nil } -func handlePrefixItemList(input []networkv1alpha1.VrfRouteConfigurationPrefixItem, seq int) frr.PrefixList { +func handlePrefixItemList(input []networkv1alpha1.VrfRouteConfigurationPrefixItem, seq int) (frr.PrefixList, error) { prefixList := frr.PrefixList{ Seq: seq + 1, } for i, item := range input { - prefixList.Items = append(prefixList.Items, copyPrefixItemToFRRItem(i, item)) + frrItem, err := copyPrefixItemToFRRItem(i, item) + if err != nil { + return frr.PrefixList{}, err + } + prefixList.Items = append(prefixList.Items, frrItem) } - return prefixList + return prefixList, nil } -func copyPrefixItemToFRRItem(i int, item networkv1alpha1.VrfRouteConfigurationPrefixItem) frr.PrefixedRouteItem { - _, network, _ := net.ParseCIDR(item.CIDR) +func copyPrefixItemToFRRItem(n int, item networkv1alpha1.VrfRouteConfigurationPrefixItem) (frr.PrefixedRouteItem, error) { + _, network, err := net.ParseCIDR(item.CIDR) + if err != nil { + return frr.PrefixedRouteItem{}, err + } seq := item.Seq if seq <= 0 { - seq = i + 1 + seq = n + 1 } return frr.PrefixedRouteItem{ CIDR: *network, @@ -210,5 +230,5 @@ func copyPrefixItemToFRRItem(i int, item networkv1alpha1.VrfRouteConfigurationPr Action: item.Action, GE: item.GE, LE: item.LE, - } + }, nil }