Repository: metallb/metallb
Pull request #1056 (2021-11-16 16:57:48)

Fail when trying to load frr only features in native mode
Ideally, the checks of FRR only fields should happen in the config handler of the speaker, which is implementation aware.
This is not possible to achieve because of the defaulting mechanism in place.
Since it makes sense to have all the checks in the same place, and those checks must happen before the defaulting, we inject an implementation driven hook to check the raw config.

Current checks are meant to drop:
- ipv6 pools in case of native bgp mode
- ipv6 aggregation length
- BFD profiles



russellb: Can you take a look at the CI failures?
fedepaol: > Can you take a look at the CI failures?

There's definetely an issue with tests now, as we were creating always pools with v6 / v4 addresses regardless of the family we are testing, and native now will reject any bgp pool containing v6 addresses.
fedepaol: Should be fixed. There was another bfd flake I was not able to reproduce locally.
Need to push https://github.com/metallb/metallb/pull/1042 forward.
msherif1234: very nice!! Thanks for consolidating config validation this validation function will keep growing for sure

Patch:
diff --git a/controller/main.go b/controller/main.go
index 95759c7f..19a0826c 100644
--- a/controller/main.go
+++ b/controller/main.go
@@ -24,6 +24,7 @@ import (
 
 	"go.universe.tf/metallb/internal/allocator"
 	"go.universe.tf/metallb/internal/config"
+	metallbcfg "go.universe.tf/metallb/internal/config"
 	"go.universe.tf/metallb/internal/k8s"
 	"go.universe.tf/metallb/internal/logging"
 	"go.universe.tf/metallb/internal/version"
@@ -165,6 +166,7 @@ func main() {
 
 		ServiceChanged: c.SetBalancer,
 		ConfigChanged:  c.SetConfig,
+		ValidateConfig: metallbcfg.DontValidate, // the controller is not aware of the mode, we defer the validation to the speaker
 		Synced:         c.MarkSynced,
 	})
 	if err != nil {
diff --git a/internal/config/config.go b/internal/config/config.go
index 84beb77a..9efba3ec 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -242,12 +242,17 @@ func parseKeepaliveTime(ht time.Duration, ka string) (time.Duration, error) {
 }
 
 // Parse loads and validates a Config from bs.
-func Parse(bs []byte) (*Config, error) {
+func Parse(bs []byte, validate Validate) (*Config, error) {
 	var raw configFile
 	if err := yaml.UnmarshalStrict(bs, &raw); err != nil {
 		return nil, fmt.Errorf("could not parse config: %s", err)
 	}
 
+	err := validate(&raw)
+	if err != nil {
+		return nil, err
+	}
+
 	cfg := &Config{
 		Pools:       map[string]*Pool{},
 		BFDProfiles: map[string]*BFDProfile{},
diff --git a/internal/config/validate.go b/internal/config/validate.go
new file mode 100644
index 00000000..d229de30
--- /dev/null
+++ b/internal/config/validate.go
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier:Apache-2.0
+
+package config
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+)
+
+type Validate func(*configFile) error
+
+// DiscardFRROnly returns an error if the current configFile contains
+// any options that are available only in the FRR implementation.
+func DiscardFRROnly(c *configFile) error {
+	for _, p := range c.Peers {
+		if p.BFDProfile != "" {
+			return fmt.Errorf("peer %s has bfd-profile set on native bgp mode", p.Addr)
+		}
+		if p.KeepaliveTime != "" {
+			return fmt.Errorf("peer %s has keepalive-time set on native bgp mode", p.Addr)
+		}
+	}
+	if len(c.BFDProfiles) > 0 {
+		return errors.New("bfd profiles section set")
+	}
+	for _, p := range c.Pools {
+		for _, a := range p.BGPAdvertisements {
+			if a.AggregationLengthV6 != nil {
+				return fmt.Errorf("pool %s has aggregation-lenght-v6 set on native bgp mode", p.Name)
+			}
+		}
+		if p.Protocol == BGP {
+			for _, cidr := range p.Addresses {
+				nets, err := ParseCIDR(cidr)
+				if err != nil {
+					return fmt.Errorf("invalid CIDR %q in pool %q: %s", cidr, p.Name, err)
+				}
+				for _, n := range nets {
+					if n.IP.To4() == nil {
+						return fmt.Errorf("pool %q has ipv6 CIDR %s, native bgp mode does not support ipv6", p.Name, n)
+					}
+				}
+			}
+		}
+	}
+	return nil
+}
+
+// DontValidate is a Validate function that always returns
+// success.
+func DontValidate(c *configFile) error {
+	return nil
+}
diff --git a/internal/k8s/k8s.go b/internal/k8s/k8s.go
index 6b19fb1e..242912a4 100644
--- a/internal/k8s/k8s.go
+++ b/internal/k8s/k8s.go
@@ -54,6 +54,7 @@ type Client struct {
 
 	serviceChanged func(log.Logger, string, *v1.Service, EpsOrSlices) SyncState
 	configChanged  func(log.Logger, *config.Config) SyncState
+	validateConfig config.Validate
 	nodeChanged    func(log.Logger, *v1.Node) SyncState
 	synced         func(log.Logger)
 }
@@ -91,6 +92,7 @@ type Config struct {
 
 	ServiceChanged func(log.Logger, string, *v1.Service, EpsOrSlices) SyncState
 	ConfigChanged  func(log.Logger, *config.Config) SyncState
+	ValidateConfig config.Validate
 	NodeChanged    func(log.Logger, *v1.Node) SyncState
 	Synced         func(log.Logger)
 }
@@ -137,10 +139,11 @@ func New(cfg *Config) (*Client, error) {
 	queue := workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter())
 
 	c := &Client{
-		logger: cfg.Logger,
-		client: clientset,
-		events: recorder,
-		queue:  queue,
+		logger:         cfg.Logger,
+		client:         clientset,
+		events:         recorder,
+		queue:          queue,
+		validateConfig: cfg.ValidateConfig,
 	}
 
 	if cfg.ServiceChanged != nil {
@@ -535,7 +538,7 @@ func (c *Client) sync(key interface{}) SyncState {
 		// config is not going to parse any better until the k8s
 		// object changes to fix the issue.
 		cm := cmi.(*v1.ConfigMap)
-		cfg, err := config.Parse([]byte(cm.Data["config"]))
+		cfg, err := config.Parse([]byte(cm.Data["config"]), c.validateConfig)
 		if err != nil {
 			level.Error(l).Log("event", "configStale", "error", err, "msg", "config (re)load failed, config marked stale")
 			configStale.Set(1)
diff --git a/manifests/metallb-frr.yaml b/manifests/metallb-frr.yaml
index 5fa357a2..ab980fac 100644
--- a/manifests/metallb-frr.yaml
+++ b/manifests/metallb-frr.yaml
@@ -404,10 +404,13 @@ data:
   vtysh.conf: |+
     service integrated-vtysh-config
   frr.conf: |+
+    ! This file gets overriden the first time the speaker renders a config.
+    ! So anything configured here is only temporary.
     frr version 7.5.1
     frr defaults traditional
     hostname Router
     line vty
+    log file /etc/frr/frr.log informational
 
 ---
 apiVersion: apps/v1
@@ -484,6 +487,20 @@ spec:
               mountPath: /var/run/frr
             - name: frr-conf
               mountPath: /etc/frr
+          # The command is FRR's default entrypoint & waiting for the log file to appear and tailing it.
+          # If the log file isn't created in 60 seconds the tail fails and the container is restarted.
+          # This workaround is needed to have the frr logs as part of kubectl logs -c frr < speaker_pod_name >.
+          command:
+            - /bin/sh
+            - -c
+            - |
+              /sbin/tini -- /usr/lib/frr/docker-start &
+              attempts=0
+              until [[ -f /etc/frr/frr.log || $attempts -eq 60 ]]; do
+                sleep 1
+                attempts=$(( $attempts + 1 ))
+              done
+              tail -f /etc/frr/frr.log
         - name: reloader
           image: frrouting/frr:v7.5.1
           command: ["/etc/frr_reloader/frr-reloader.sh"]
diff --git a/speaker/bgp_controller.go b/speaker/bgp_controller.go
index 8b4fa23e..637dbd47 100644
--- a/speaker/bgp_controller.go
+++ b/speaker/bgp_controller.go
@@ -64,10 +64,6 @@ newPeers:
 			if ep == nil {
 				continue
 			}
-			if ep.cfg.KeepaliveTime != 0 && c.bgpType == bgpNative {
-				level.Error(l).Log("op", "setConfig", "peer", ep.cfg.Addr, "configuration ignored", "keep alive time configuration is not supported")
-				continue
-			}
 			if reflect.DeepEqual(p, ep.cfg) {
 				newPeers = append(newPeers, ep)
 				c.peers[i] = nil
diff --git a/speaker/main.go b/speaker/main.go
index 0610cd6c..ed2f4715 100644
--- a/speaker/main.go
+++ b/speaker/main.go
@@ -26,6 +26,7 @@ import (
 
 	"go.universe.tf/metallb/internal/bgp"
 	"go.universe.tf/metallb/internal/config"
+	metallbcfg "go.universe.tf/metallb/internal/config"
 	"go.universe.tf/metallb/internal/k8s"
 	"go.universe.tf/metallb/internal/layer2"
 	"go.universe.tf/metallb/internal/logging"
@@ -133,6 +134,11 @@ func main() {
 		os.Exit(1)
 	}
 
+	validateConfig := metallbcfg.DontValidate
+	if bgpType == "native" {
+		validateConfig = metallbcfg.DiscardFRROnly
+	}
+
 	client, err := k8s.New(&k8s.Config{
 		ProcessName:     "metallb-speaker",
 		ConfigMapName:   *config,
@@ -148,6 +154,7 @@ func main() {
 
 		ServiceChanged: ctrl.SetBalancer,
 		ConfigChanged:  ctrl.SetConfig,
+		ValidateConfig: validateConfig,
 		NodeChanged:    ctrl.SetNode,
 	})
 	if err != nil {
@@ -355,14 +362,6 @@ func (c *controller) SetConfig(l log.Logger, cfg *config.Config) k8s.SyncState {
 		return k8s.SyncStateErrorNoRetry
 	}
 
-	if c.bgpType == bgpNative {
-		err := validateFRROnlyConfiguration(cfg)
-		if err != nil {
-			level.Error(l).Log("op", "setConfig", "error", err)
-			return k8s.SyncStateError
-		}
-	}
-
 	for svc, ip := range c.svcIP {
 		if pool := poolFor(cfg.Pools, ip); pool == "" {
 			level.Error(l).Log("op", "setConfig", "service", svc, "ip", ip, "error", "service has no configuration under new config", "msg", "new configuration rejected")
diff --git a/speaker/validate.go b/speaker/validate.go
deleted file mode 100644
index 4fe52514..00000000
--- a/speaker/validate.go
+++ /dev/null
@@ -1,22 +0,0 @@
-// SPDX-License-Identifier:Apache-2.0
-
-package main
-
-import (
-	"fmt"
-
-	"github.com/pkg/errors"
-	"go.universe.tf/metallb/internal/config"
-)
-
-func validateFRROnlyConfiguration(cfg *config.Config) error {
-	for _, p := range cfg.Peers {
-		if p.BFDProfile != "" {
-			return fmt.Errorf("peer %s has bfd profile set", p.Addr)
-		}
-	}
-	if len(cfg.BFDProfiles) > 0 {
-		return errors.New("bfd profiles section set")
-	}
-	return nil
-}


Test patch:
diff --git a/e2etest/bgp_test.go b/e2etest/bgp_test.go
index dd704fd2..18b22c68 100644
--- a/e2etest/bgp_test.go
+++ b/e2etest/bgp_test.go
@@ -47,11 +47,13 @@ import (
 )
 
 const (
-	frrIBGP      = "frr-iBGP"
-	frrEBGP      = "frr-eBGP"
-	IBGPAsn      = 64512
-	EBGPAsn      = 64513
-	baseRouterID = "10.10.10.%d"
+	frrIBGP         = "frr-iBGP"
+	frrEBGP         = "frr-eBGP"
+	IBGPAsn         = 64512
+	EBGPAsn         = 64513
+	baseRouterID    = "10.10.10.%d"
+	v4PoolAddresses = "192.168.10.0/24"
+	v6PoolAddresses = "fc00:f853:0ccd:e799::/124"
 )
 
 var (
@@ -122,7 +124,7 @@ var _ = ginkgo.Describe("BGP", func() {
 		}
 	})
 
-	table.DescribeTable("A service of protocol load balancer should work with", func(ipFamily string, setProtocoltest string) {
+	table.DescribeTable("A service of protocol load balancer should work with", func(ipFamily, setProtocoltest, poolAddress string) {
 		var allNodes *corev1.NodeList
 		configData := configFile{
 			Pools: []addressPool{
@@ -130,8 +132,7 @@ var _ = ginkgo.Describe("BGP", func() {
 					Name:     "bgp-test",
 					Protocol: BGP,
 					Addresses: []string{
-						"192.168.10.0/24",
-						"fc00:f853:0ccd:e799::/124",
+						poolAddress,
 					},
 				},
 			},
@@ -190,12 +191,12 @@ var _ = ginkgo.Describe("BGP", func() {
 			}
 		}
 	},
-		table.Entry("IPV4 - ExternalTrafficPolicyCluster", "ipv4", "ExternalTrafficPolicyCluster"),
-		table.Entry("IPV4 - ExternalTrafficPolicyLocal", "ipv4", "ExternalTrafficPolicyLocal"),
-		table.Entry("IPV4 - FRR running in the speaker POD", "ipv4", "CheckSpeakerFRRPodRunning"),
-		table.Entry("IPV6 - ExternalTrafficPolicyCluster", "ipv6", "ExternalTrafficPolicyCluster"),
-		table.Entry("IPV6 - ExternalTrafficPolicyLocal", "ipv6", "ExternalTrafficPolicyLocal"),
-		table.Entry("IPV6 - FRR running in the speaker POD", "ipv6", "CheckSpeakerFRRPodRunning"))
+		table.Entry("IPV4 - ExternalTrafficPolicyCluster", "ipv4", "ExternalTrafficPolicyCluster", v4PoolAddresses),
+		table.Entry("IPV4 - ExternalTrafficPolicyLocal", "ipv4", "ExternalTrafficPolicyLocal", v4PoolAddresses),
+		table.Entry("IPV4 - FRR running in the speaker POD", "ipv4", "CheckSpeakerFRRPodRunning", v4PoolAddresses),
+		table.Entry("IPV6 - ExternalTrafficPolicyCluster", "ipv6", "ExternalTrafficPolicyCluster", v6PoolAddresses),
+		table.Entry("IPV6 - ExternalTrafficPolicyLocal", "ipv6", "ExternalTrafficPolicyLocal", v6PoolAddresses),
+		table.Entry("IPV6 - FRR running in the speaker POD", "ipv6", "CheckSpeakerFRRPodRunning", v6PoolAddresses))
 
 	ginkgo.Context("metrics", func() {
 		var controllerPod *corev1.Pod
@@ -220,7 +221,7 @@ var _ = ginkgo.Describe("BGP", func() {
 			}
 		})
 
-		table.DescribeTable("should be exposed by the controller", func(ipFamily string) {
+		table.DescribeTable("should be exposed by the controller", func(ipFamily, poolAddress string, addressTotal int) {
 			poolName := "bgp-test"
 
 			var peerAddrs []string
@@ -237,12 +238,9 @@ var _ = ginkgo.Describe("BGP", func() {
 			configData := configFile{
 				Pools: []addressPool{
 					{
-						Name:     poolName,
-						Protocol: BGP,
-						Addresses: []string{
-							"192.168.10.0/24",
-							"fc00:f853:0ccd:e799::/124",
-						},
+						Name:      poolName,
+						Protocol:  BGP,
+						Addresses: []string{poolAddress},
 					},
 				},
 				Peers: peersForContainers(frrContainers, ipFamily),
@@ -268,7 +266,7 @@ var _ = ginkgo.Describe("BGP", func() {
 				if err != nil {
 					return err
 				}
-				err = validateGaugeValue(272, "metallb_allocator_addresses_total", map[string]string{"pool": poolName}, controllerMetrics)
+				err = validateGaugeValue(addressTotal, "metallb_allocator_addresses_total", map[string]string{"pool": poolName}, controllerMetrics)
 				if err != nil {
 					return err
 				}
@@ -350,8 +348,8 @@ var _ = ginkgo.Describe("BGP", func() {
 				}, 2*time.Minute, 1*time.Second).Should(BeNil())
 			}
 		},
-			table.Entry("IPV4 - Checking service", "ipv4"),
-			table.Entry("IPV6 - Checking service", "ipv6"))
+			table.Entry("IPV4 - Checking service", "ipv4", v4PoolAddresses, 256),
+			table.Entry("IPV6 - Checking service", "ipv6", v6PoolAddresses, 16))
 	})
 
 	ginkgo.Context("validate different AddressPools for type=Loadbalancer", func() {
@@ -402,13 +400,21 @@ var _ = ginkgo.Describe("BGP", func() {
 				validateService(cs, svc, allNodes.Items, c, ipFamily)
 			}
 		},
+			table.Entry("IPV4 - test AddressPool defined by address range", []addressPool{
+				{
+					Name:     "bgp-test",
+					Protocol: BGP,
+					Addresses: []string{
+						"192.168.10.0-192.168.10.18",
+					},
+				}}, "ipv4",
+			),
 			table.Entry("IPV4 - test AddressPool defined by network prefix", []addressPool{
 				{
 					Name:     "bgp-test",
 					Protocol: BGP,
 					Addresses: []string{
 						"192.168.10.0/24",
-						"fc00:f853:0ccd:e799::/124",
 					},
 				}}, "ipv4",
 			),
@@ -417,24 +423,31 @@ var _ = ginkgo.Describe("BGP", func() {
 					Name:     "bgp-test",
 					Protocol: BGP,
 					Addresses: []string{
-						"192.168.10.0-192.168.10.18",
 						"fc00:f853:0ccd:e799::0-fc00:f853:0ccd:e799::18",
 					},
 				}}, "ipv6",
 			),
+			table.Entry("IPV6 - test AddressPool defined by network prefix", []addressPool{
+				{
+					Name:     "bgp-test",
+					Protocol: BGP,
+					Addresses: []string{
+						"fc00:f853:0ccd:e799::/124",
+					},
+				}}, "ipv6",
+			),
 		)
 	})
 
 	ginkgo.Context("BFD", func() {
-		table.DescribeTable("should work with the given bfd profile", func(bfd bfdProfile, ipFamily string) {
+		table.DescribeTable("should work with the given bfd profile", func(bfd bfdProfile, ipFamily, poolAddress string) {
 			configData := configFile{
 				Pools: []addressPool{
 					{
 						Name:     "bfd-test",
 						Protocol: BGP,
 						Addresses: []string{
-							"192.168.10.0/24",
-							"fc00:f853:0ccd:e799::/124",
+							poolAddress,
 						},
 					},
 				},
@@ -495,7 +508,7 @@ var _ = ginkgo.Describe("BGP", func() {
 			table.Entry("IPV4 - default",
 				bfdProfile{
 					Name: "bar",
-				}, "ipv4"),
+				}, "ipv4", v4PoolAddresses),
 			table.Entry("IPV4 - full params",
 				bfdProfile{
 					Name:             "full1",
@@ -505,7 +518,7 @@ var _ = ginkgo.Describe("BGP", func() {
 					EchoMode:         boolPtr(false),
 					PassiveMode:      boolPtr(false),
 					MinimumTTL:       uint32Ptr(254),
-				}, "ipv4"),
+				}, "ipv4", v4PoolAddresses),
 			table.Entry("IPV4 - echo mode enabled",
 				bfdProfile{
 					Name:             "echo",
@@ -515,11 +528,11 @@ var _ = ginkgo.Describe("BGP", func() {
 					EchoMode:         boolPtr(true),
 					PassiveMode:      boolPtr(false),
 					MinimumTTL:       uint32Ptr(254),
-				}, "ipv4"),
+				}, "ipv4", v4PoolAddresses),
 			table.Entry("IPV6 - default",
 				bfdProfile{
 					Name: "bar",
-				}, "ipv6"),
+				}, "ipv6", v6PoolAddresses),
 			table.Entry("IPV6 - full params",
 				bfdProfile{
 					Name:             "full1",
@@ -529,7 +542,7 @@ var _ = ginkgo.Describe("BGP", func() {
 					EchoMode:         boolPtr(false),
 					PassiveMode:      boolPtr(false),
 					MinimumTTL:       uint32Ptr(254),
-				}, "ipv6"),
+				}, "ipv6", v6PoolAddresses),
 			table.Entry("IPV6 - echo mode enabled",
 				bfdProfile{
 					Name:             "echo",
@@ -539,7 +552,7 @@ var _ = ginkgo.Describe("BGP", func() {
 					EchoMode:         boolPtr(true),
 					PassiveMode:      boolPtr(false),
 					MinimumTTL:       uint32Ptr(254),
-				}, "ipv6"),
+				}, "ipv6", v6PoolAddresses),
 		)
 	})
 })
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 8a54a014..757aca25 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -792,7 +792,7 @@ bfd-profiles:
 
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			got, err := Parse([]byte(test.raw))
+			got, err := Parse([]byte(test.raw), DontValidate)
 			if err != nil && test.want != nil {
 				t.Errorf("%q: parse failed: %s", test.desc, err)
 				return
diff --git a/internal/config/validate_test.go b/internal/config/validate_test.go
new file mode 100644
index 00000000..f1738fa7
--- /dev/null
+++ b/internal/config/validate_test.go
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier:Apache-2.0
+
+package config
+
+import (
+	"testing"
+)
+
+func TestValidate(t *testing.T) {
+	tests := []struct {
+		desc     string
+		config   *configFile
+		mustFail bool
+	}{
+		{
+			desc: "peer with bfd profile",
+			config: &configFile{
+				Peers: []peer{
+					{
+						Addr:       "1.2.3.4",
+						BFDProfile: "foo",
+					},
+				},
+			},
+			mustFail: true,
+		},
+		{
+			desc: "bfd profile set",
+			config: &configFile{
+				Peers: []peer{
+					{
+						Addr: "1.2.3.4",
+					},
+				},
+				BFDProfiles: []bfdProfile{
+					{
+						Name: "default",
+					},
+				},
+			},
+			mustFail: true,
+		},
+		{
+			desc: "v6 address",
+			config: &configFile{
+				Pools: []addressPool{
+					{
+						Protocol:  BGP,
+						Addresses: []string{"2001:db8::/64"},
+					},
+				},
+			},
+			mustFail: true,
+		},
+		{
+			desc: "keepalive time",
+			config: &configFile{
+				Peers: []peer{
+					{
+						KeepaliveTime: "1s",
+					},
+				},
+			},
+			mustFail: true,
+		},
+		{
+			desc: "aggregation v6",
+			config: &configFile{
+				Pools: []addressPool{
+					{
+						Name: "foo",
+						BGPAdvertisements: []bgpAdvertisement{
+							{
+								AggregationLengthV6: intPtr(47),
+							},
+						},
+					},
+				},
+			},
+			mustFail: true,
+		},
+		{
+			desc: "should pass",
+			config: &configFile{
+				Peers: []peer{
+					{
+						Addr: "1.2.3.4",
+					},
+				},
+			},
+			mustFail: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			err := DiscardFRROnly(test.config)
+			if test.mustFail && err == nil {
+				t.Fatalf("Expected error for %s", test.desc)
+			}
+			if !test.mustFail && err != nil {
+				t.Fatalf("Not expected error %s for %s", err, test.desc)
+			}
+		})
+	}
+}
+
+func intPtr(i int) *int {
+	return &i
+}
diff --git a/speaker/validate_test.go b/speaker/validate_test.go
deleted file mode 100644
index 4a407085..00000000
--- a/speaker/validate_test.go
+++ /dev/null
@@ -1,74 +0,0 @@
-// SPDX-License-Identifier:Apache-2.0
-
-package main
-
-import (
-	"net"
-	"testing"
-
-	"go.universe.tf/metallb/internal/config"
-	"k8s.io/apimachinery/pkg/labels"
-)
-
-func TestValidate(t *testing.T) {
-	tests := []struct {
-		desc     string
-		config   *config.Config
-		mustFail bool
-	}{
-		{
-			desc: "peer with bfd profile",
-			config: &config.Config{
-				Peers: []*config.Peer{
-					{
-						Addr:          net.ParseIP("1.2.3.4"),
-						NodeSelectors: []labels.Selector{labels.Everything()},
-						BFDProfile:    "foo",
-					},
-				},
-			},
-			mustFail: true,
-		},
-		{
-			desc: "bfd profile set",
-			config: &config.Config{
-				Peers: []*config.Peer{
-					{
-						Addr:          net.ParseIP("1.2.3.4"),
-						NodeSelectors: []labels.Selector{labels.Everything()},
-					},
-				},
-				BFDProfiles: map[string]*config.BFDProfile{
-					"default": {
-						Name: "default",
-					},
-				},
-			},
-			mustFail: true,
-		},
-		{
-			desc: "should pass",
-			config: &config.Config{
-				Peers: []*config.Peer{
-					{
-						Addr:          net.ParseIP("1.2.3.4"),
-						NodeSelectors: []labels.Selector{labels.Everything()},
-					},
-				},
-			},
-			mustFail: false,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.desc, func(t *testing.T) {
-			err := validateFRROnlyConfiguration(test.config)
-			if test.mustFail && err == nil {
-				t.Fatalf("Expected error for %s", test.desc)
-			}
-			if !test.mustFail && err != nil {
-				t.Fatalf("Not expected error %s for %s", err, test.desc)
-			}
-		})
-	}
-}