diff --git a/config.yaml b/config.yaml index 534baa4..a3a44c4 100644 --- a/config.yaml +++ b/config.yaml @@ -17,6 +17,21 @@ bgp: remote_as: 6789 # override the peer IP to use instead of auto discovering peer_ip: 10.10.10.1 + + # Alternatively, define multiple BGP peers for redundancy + #peers: + # - peer_ip: 10.10.10.1 + # peer_as: 6789 + # communities: + # - 100:100 + # - 200:200 + # - peer_ip: 10.10.10.2 + # peer_as: 6789 + # communities: + # - 100:101 + # - 200:201 + # multi_hop: true # optional + communities: - asn:nnnn - asn:nnnn diff --git a/config/config.go b/config/config.go index 8947c49..eaeee9d 100644 --- a/config/config.go +++ b/config/config.go @@ -18,11 +18,21 @@ type AgentConfig struct { ConsulToken string `yaml:"consul_token"` } +type PeerConfig struct { + PeerIP string `yaml:"peer_ip"` + PeerAS int `yaml:"peer_as"` + MultiHop *bool `yaml:"multi_hop,omitempty"` + Communities []string `yaml:"communities,omitempty"` +} + type BgpConfig struct { - LocalAS int `yaml:"local_as"` - PeerAS int `yaml:"peer_as"` - LocalIP string `yaml:"local_ip"` - PeerIP string `yaml:"peer_ip"` + LocalAS int `yaml:"local_as"` + LocalIP string `yaml:"local_ip"` + // Legacy single-peer config (deprecated but supported for backward compatibility) + PeerAS int `yaml:"peer_as,omitempty"` + PeerIP string `yaml:"peer_ip,omitempty"` + // New multi-peer config + Peers []PeerConfig `yaml:"peers,omitempty"` Communities []string Origin string } diff --git a/controller/bgp.go b/controller/bgp.go index 96da20d..ce5b519 100644 --- a/controller/bgp.go +++ b/controller/bgp.go @@ -20,82 +20,110 @@ type Route struct { } type Controller struct { - peerAS int - localIP, peerIP net.IP - communities []string - origin uint32 - multiHop bool - s *gobgp.BgpServer + localAS int + localIP net.IP + peers []c.PeerConfig + communities []string + origin uint32 + s *gobgp.BgpServer } func NewController(config c.BgpConfig) (*Controller, error) { - c := &Controller{} + ctrl := &Controller{} var gw net.IP var err error - if config.PeerIP == "" { - gw, err := gateway() - if err != nil { - return nil, fmt.Errorf("Unable to get gw ip: %v", err) + + // Normalize config: convert legacy single-peer to new multi-peer format + peers := config.Peers + if len(peers) == 0 { + // Backward compatibility: convert legacy config + if config.PeerIP != "" { + // Explicit peer IP configured + peers = []c.PeerConfig{{ + PeerIP: config.PeerIP, + PeerAS: config.PeerAS, + }} + } else { + // No peer IP configured - use default gateway + gw, err = gateway() + if err != nil { + return nil, fmt.Errorf("Unable to get gateway ip: %v", err) + } + peers = []c.PeerConfig{{ + PeerIP: gw.String(), + PeerAS: config.PeerAS, + }} } - c.peerIP = gw - } else { - c.peerIP = net.ParseIP(config.PeerIP) } + + // Determine local IP if config.LocalIP == "" { - gw, err = via(c.peerIP) + // Use first peer to determine local IP + firstPeerIP := net.ParseIP(peers[0].PeerIP) + if firstPeerIP == nil { + gw, err = gateway() + if err != nil { + return nil, fmt.Errorf("Unable to get gw ip: %v", err) + } + firstPeerIP = gw + } + gw, err = via(firstPeerIP) if err != nil { return nil, fmt.Errorf("Unable to get gw ip: %v", err) } - c.localIP, err = localAddress(gw) + ctrl.localIP, err = localAddress(gw) if err != nil { return nil, err } } else { - c.localIP = net.ParseIP(config.LocalIP) + ctrl.localIP = net.ParseIP(config.LocalIP) } - c.communities = config.Communities + + ctrl.localAS = config.LocalAS + ctrl.peers = peers + ctrl.communities = config.Communities + switch config.Origin { case "igp": - c.origin = 0 + ctrl.origin = 0 case "egp": - c.origin = 1 + ctrl.origin = 1 case "unknown": - c.origin = 2 + ctrl.origin = 2 } + s := gobgp.NewBgpServer() go s.Serve() if err := s.StartBgp(context.Background(), &api.StartBgpRequest{ Global: &api.Global{ As: uint32(config.LocalAS), - RouterId: c.localIP.String(), + RouterId: ctrl.localIP.String(), ListenPort: -1, // gobgp won't listen on tcp:179 }, }); err != nil { return nil, fmt.Errorf("Unable to start bgp: %v", err) } - c.s = s - c.peerAS = config.PeerAS - // set mh by default for all ebgp peers - if c.peerAS != config.LocalAS { - c.multiHop = true - } - return c, nil + ctrl.s = s + + return ctrl, nil } -func (c *Controller) AddPeer(peer string) error { +func (c *Controller) addPeer(peer *c.PeerConfig) error { n := &api.Peer{ Conf: &api.PeerConf{ - NeighborAddress: peer, - PeerAs: uint32(c.peerAS), + NeighborAddress: peer.PeerIP, + PeerAs: uint32(peer.PeerAS), }, } - if c.multiHop { + + // Enable multihop only if explicitly configured + if peer.MultiHop != nil && *peer.MultiHop { n.EbgpMultihop = &api.EbgpMultihop{Enabled: true, MultihopTtl: uint32(255)} } return c.s.AddPeer(context.Background(), &api.AddPeerRequest{Peer: n}) } -func (c *Controller) getApiPath(route *Route) *api.Path { +func (c *Controller) getApiPath(route *Route, peer *c.PeerConfig) *api.Path { afi := api.Family_AFI_IP if route.Net.IP.To4() == nil { afi = api.Family_AFI_IP6 @@ -111,8 +139,15 @@ func (c *Controller) getApiPath(route *Route) *api.Path { a2, _ := ptypes.MarshalAny(&api.NextHopAttribute{ NextHop: c.localIP.String(), }) + + // Merge communities: global + per-peer + per-route + var allCommunities []string + allCommunities = append(allCommunities, c.communities...) + allCommunities = append(allCommunities, peer.Communities...) + allCommunities = append(allCommunities, route.Communities...) + var communities []uint32 - for _, comm := range append(c.communities, route.Communities...) { + for _, comm := range allCommunities { communities = append(communities, convertCommunity(comm)) } a3, _ := ptypes.MarshalAny(&api.CommunitiesAttribute{ @@ -127,49 +162,105 @@ func (c *Controller) getApiPath(route *Route) *api.Path { } func (c *Controller) Announce(route *Route) error { - var found bool - err := c.s.ListPeer(context.Background(), &api.ListPeerRequest{}, func(p *api.Peer) { - if p.Conf.NeighborAddress == c.peerIP.String() { - found = true + var errs []error + + for i := range c.peers { + peer := &c.peers[i] + + // Check if peer exists + var found bool + err := c.s.ListPeer(context.Background(), &api.ListPeerRequest{}, func(p *api.Peer) { + if p.Conf.NeighborAddress == peer.PeerIP { + found = true + } + }) + if err != nil { + errs = append(errs, fmt.Errorf("peer %s: list error: %v", peer.PeerIP, err)) + continue } - }) - if err != nil { - return err - } - if !found { - if err := c.AddPeer(c.peerIP.String()); err != nil { - return err + + // Add peer if not found + if !found { + if err := c.addPeer(peer); err != nil { + errs = append(errs, fmt.Errorf("peer %s: add error: %v", peer.PeerIP, err)) + continue + } + } + + // Announce route to this peer + path := c.getApiPath(route, peer) + if _, err := c.s.AddPath(context.Background(), &api.AddPathRequest{Path: path}); err != nil { + errs = append(errs, fmt.Errorf("peer %s: announce error: %v", peer.PeerIP, err)) + continue } } - _, err = c.s.AddPath(context.Background(), &api.AddPathRequest{Path: c.getApiPath(route)}) - return err + + // Return aggregated errors if any peer failed + if len(errs) > 0 { + return fmt.Errorf("announcement errors: %v", errs) + } + return nil } func (c *Controller) Withdraw(route *Route) error { - return c.s.DeletePath(context.Background(), &api.DeletePathRequest{Path: c.getApiPath(route)}) + var errs []error + + for i := range c.peers { + peer := &c.peers[i] + path := c.getApiPath(route, peer) + if err := c.s.DeletePath(context.Background(), &api.DeletePathRequest{Path: path}); err != nil { + errs = append(errs, fmt.Errorf("peer %s: withdraw error: %v", peer.PeerIP, err)) + continue + } + } + + // Return aggregated errors if any peer failed + if len(errs) > 0 { + return fmt.Errorf("withdrawal errors: %v", errs) + } + return nil } -func (c *Controller) PeerInfo() (*api.Peer, error) { - var peer *api.Peer +func (c *Controller) PeerInfo() ([]*api.Peer, error) { + var peers []*api.Peer + peerMap := make(map[string]bool) + + // Build map of configured peer IPs + for _, peer := range c.peers { + peerMap[peer.PeerIP] = true + } + + // Collect info for all configured peers err := c.s.ListPeer(context.Background(), &api.ListPeerRequest{}, func(p *api.Peer) { - if p.Conf.NeighborAddress == c.peerIP.String() { - peer = p + if peerMap[p.Conf.NeighborAddress] { + peers = append(peers, p) } }) if err != nil { return nil, err } - return peer, nil + return peers, nil } func (c *Controller) Shutdown() error { - if err := c.s.ShutdownPeer(context.Background(), &api.ShutdownPeerRequest{ - Address: c.peerIP.String(), - }); err != nil { - return err + var errs []error + + // Shutdown all peer sessions + for _, peer := range c.peers { + if err := c.s.ShutdownPeer(context.Background(), &api.ShutdownPeerRequest{ + Address: peer.PeerIP, + }); err != nil { + errs = append(errs, fmt.Errorf("peer %s: shutdown error: %v", peer.PeerIP, err)) + } } + + // Stop BGP server if err := c.s.StopBgp(context.Background(), &api.StopBgpRequest{}); err != nil { - return err + errs = append(errs, fmt.Errorf("stop bgp error: %v", err)) + } + + if len(errs) > 0 { + return fmt.Errorf("shutdown errors: %v", errs) } return nil } diff --git a/controller/bgp_test.go b/controller/bgp_test.go index b9ec48b..17c7b54 100644 --- a/controller/bgp_test.go +++ b/controller/bgp_test.go @@ -102,3 +102,389 @@ func TestBgpNew(t *testing.T) { a.Equal("20.30.40.0/24", path) ctrl.Shutdown() } + +func TestLegacyConfigConversion(t *testing.T) { + a := assert.New(t) + + // Test legacy single-peer config + legacyConfig := config.BgpConfig{ + LocalAS: 11111, + PeerAS: 22222, + PeerIP: "10.10.10.1", + LocalIP: "192.168.1.100", + Communities: []string{"100:100"}, + Origin: "igp", + } + + ctrl, err := NewController(legacyConfig) + if err != nil { + a.FailNow(err.Error()) + } + defer ctrl.Shutdown() + + // Verify legacy config was converted to multi-peer format + a.Equal(1, len(ctrl.peers), "Should have exactly 1 peer") + a.Equal("10.10.10.1", ctrl.peers[0].PeerIP) + a.Equal(22222, ctrl.peers[0].PeerAS) +} + +func TestMultiPeerConfig(t *testing.T) { + a := assert.New(t) + + // Test new multi-peer config + multiPeerConfig := config.BgpConfig{ + LocalAS: 11111, + LocalIP: "192.168.1.100", + Peers: []config.PeerConfig{ + {PeerIP: "10.10.10.1", PeerAS: 22222}, + {PeerIP: "10.10.10.2", PeerAS: 22222}, + }, + Communities: []string{"100:100"}, + Origin: "igp", + } + + ctrl, err := NewController(multiPeerConfig) + if err != nil { + a.FailNow(err.Error()) + } + defer ctrl.Shutdown() + + // Verify both peers are configured + a.Equal(2, len(ctrl.peers), "Should have exactly 2 peers") + a.Equal("10.10.10.1", ctrl.peers[0].PeerIP) + a.Equal("10.10.10.2", ctrl.peers[1].PeerIP) +} + +func TestDefaultGatewayPeer(t *testing.T) { + a := assert.New(t) + + // Test config with no peer_ip - should use default gateway + defaultGatewayConfig := config.BgpConfig{ + LocalAS: 11111, + LocalIP: "192.168.1.100", + PeerAS: 22222, + Origin: "igp", + } + + ctrl, err := NewController(defaultGatewayConfig) + a.NoError(err, "Should not error when peer_ip is not specified") + if err == nil { + defer ctrl.Shutdown() + + // Verify a peer was configured using gateway + a.Equal(1, len(ctrl.peers), "Should have exactly 1 peer") + a.NotEmpty(ctrl.peers[0].PeerIP, "Peer IP should be set from gateway") + a.Equal(22222, ctrl.peers[0].PeerAS, "Peer AS should match config") + } +} + +func TestCommunityMerging(t *testing.T) { + a := assert.New(t) + + ctrl := &Controller{ + localAS: 11111, + localIP: net.ParseIP("192.168.1.100"), + communities: []string{"1000:1000", "2000:2000"}, // Global + origin: 0, + } + + peer := &config.PeerConfig{ + PeerIP: "10.10.10.1", + PeerAS: 22222, + Communities: []string{"100:100", "200:200"}, // Per-peer + } + + route := &Route{ + Net: &net.IPNet{ + IP: net.ParseIP("20.30.40.0"), + Mask: net.CIDRMask(24, 32), + }, + Communities: []string{"5000:5000"}, // Per-route + } + + path := ctrl.getApiPath(route, peer) + + // Extract communities from path + var commAttr *api.CommunitiesAttribute + for _, attr := range path.Pattrs { + var dynAny ptypes.DynamicAny + if err := ptypes.UnmarshalAny(attr, &dynAny); err == nil { + if c, ok := dynAny.Message.(*api.CommunitiesAttribute); ok { + commAttr = c + break + } + } + } + + a.NotNil(commAttr, "Should have communities attribute") + a.Equal(5, len(commAttr.Communities), "Should have 5 communities (2 global + 2 peer + 1 route)") + + // Verify community values (converted to uint32) + expectedCommunities := []uint32{ + convertCommunity("1000:1000"), // Global + convertCommunity("2000:2000"), // Global + convertCommunity("100:100"), // Per-peer + convertCommunity("200:200"), // Per-peer + convertCommunity("5000:5000"), // Per-route + } + a.Equal(expectedCommunities, commAttr.Communities) +} + +func TestMultiHopConfiguration(t *testing.T) { + a := assert.New(t) + + testCases := []struct { + name string + localAS int + peerAS int + multiHopPtr *bool + expectMH bool + }{ + { + name: "default - multihop not enabled", + localAS: 11111, + peerAS: 22222, + multiHopPtr: nil, + expectMH: false, + }, + { + name: "explicit disable", + localAS: 11111, + peerAS: 22222, + multiHopPtr: boolPtr(false), + expectMH: false, + }, + { + name: "explicit enable", + localAS: 11111, + peerAS: 22222, + multiHopPtr: boolPtr(true), + expectMH: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctrl := &Controller{ + localAS: tc.localAS, + localIP: net.ParseIP("192.168.1.100"), + s: gobgp.NewBgpServer(), + } + go ctrl.s.Serve() + + if err := ctrl.s.StartBgp(context.Background(), &api.StartBgpRequest{ + Global: &api.Global{ + As: uint32(tc.localAS), + RouterId: ctrl.localIP.String(), + ListenPort: -1, + }, + }); err != nil { + a.FailNow(err.Error()) + } + defer ctrl.s.StopBgp(context.Background(), &api.StopBgpRequest{}) + + peer := &config.PeerConfig{ + PeerIP: "10.10.10.1", + PeerAS: tc.peerAS, + MultiHop: tc.multiHopPtr, + } + + err := ctrl.addPeer(peer) + a.NoError(err) + + // Verify multihop setting by checking peer config + var foundPeer *api.Peer + ctrl.s.ListPeer(context.Background(), &api.ListPeerRequest{}, func(p *api.Peer) { + if p.Conf.NeighborAddress == peer.PeerIP { + foundPeer = p + } + }) + + a.NotNil(foundPeer, "Peer should be added") + if tc.expectMH { + a.NotNil(foundPeer.EbgpMultihop, "Should have multihop configured") + a.True(foundPeer.EbgpMultihop.Enabled, "Multihop should be enabled") + } else { + if foundPeer.EbgpMultihop != nil { + a.False(foundPeer.EbgpMultihop.Enabled, "Multihop should not be enabled") + } + } + }) + } +} + +// Helper function to create bool pointer +func boolPtr(b bool) *bool { + return &b +} + +func TestMultiPeerAnnouncement(t *testing.T) { + if os.Getenv("CI") != "" { + t.Skip("Skipping testing in CI environment") + } + + a := assert.New(t) + + // Create two BGP listeners + listener1, err := NewBgpListener(22222) + if err != nil { + panic(err) + } + defer listener1.Shutdown() + + listener2, err := NewBgpListener(33333) + if err != nil { + panic(err) + } + defer listener2.Shutdown() + + // Create controller with two peers + multiPeerConfig := config.BgpConfig{ + LocalAS: 11111, + LocalIP: "192.168.1.100", + Peers: []config.PeerConfig{ + {PeerIP: "127.0.0.1", PeerAS: 22222}, + {PeerIP: "127.0.0.1", PeerAS: 33333}, + }, + Communities: []string{"100:100"}, + Origin: "igp", + } + + ctrl, err := NewController(multiPeerConfig) + if err != nil { + a.FailNow(err.Error()) + } + defer ctrl.Shutdown() + + // Announce a route + _, ipnet, _ := net.ParseCIDR("20.30.40.0/24") + r := &Route{Net: ipnet} + if err := ctrl.Announce(r); err != nil { + a.FailNow(err.Error()) + } + + // Verify both listeners received the route + path1 := <-listener1.recvdPaths + a.Equal("20.30.40.0/24", path1) + + path2 := <-listener2.recvdPaths + a.Equal("20.30.40.0/24", path2) +} + +func TestBestEffortAnnouncement(t *testing.T) { + a := assert.New(t) + + // Create controller with two peers + mixedConfig := config.BgpConfig{ + LocalAS: 11111, + LocalIP: "192.168.1.100", + Peers: []config.PeerConfig{ + {PeerIP: "127.0.0.1", PeerAS: 22222}, + {PeerIP: "127.0.0.2", PeerAS: 33333}, + }, + Origin: "igp", + } + + ctrl, err := NewController(mixedConfig) + if err != nil { + a.FailNow(err.Error()) + } + defer ctrl.Shutdown() + + // Announce a route - both peers will be added successfully + // (they won't have actual BGP sessions established, but peers are added to GoBGP) + _, ipnet, _ := net.ParseCIDR("20.30.40.0/24") + r := &Route{Net: ipnet} + + // The announcement should succeed for both peers being added + err = ctrl.Announce(r) + a.NoError(err, "Announcement should succeed for both peers") + + // Verify both peers were added + peers, err := ctrl.PeerInfo() + a.NoError(err) + a.Equal(2, len(peers), "Should have both peers configured") +} + +func TestWithdrawMultiplePeers(t *testing.T) { + a := assert.New(t) + + ctrl := &Controller{ + localAS: 11111, + localIP: net.ParseIP("192.168.1.100"), + peers: []config.PeerConfig{ + {PeerIP: "10.10.10.1", PeerAS: 22222}, + {PeerIP: "10.10.10.2", PeerAS: 22222}, + }, + origin: 0, + s: gobgp.NewBgpServer(), + } + go ctrl.s.Serve() + + if err := ctrl.s.StartBgp(context.Background(), &api.StartBgpRequest{ + Global: &api.Global{ + As: uint32(11111), + RouterId: ctrl.localIP.String(), + ListenPort: -1, + }, + }); err != nil { + a.FailNow(err.Error()) + } + defer ctrl.s.StopBgp(context.Background(), &api.StopBgpRequest{}) + + _, ipnet, _ := net.ParseCIDR("20.30.40.0/24") + r := &Route{Net: ipnet} + + // Withdraw should iterate through all peers + // This will fail because peers aren't established, but it should try both + err := ctrl.Withdraw(r) + // We expect an error but it should have tried both peers + if err != nil { + a.Contains(err.Error(), "withdrawal errors") + } +} + +func TestPeerInfoMultiplePeers(t *testing.T) { + a := assert.New(t) + + ctrl := &Controller{ + localAS: 11111, + localIP: net.ParseIP("192.168.1.100"), + peers: []config.PeerConfig{ + {PeerIP: "10.10.10.1", PeerAS: 22222}, + {PeerIP: "10.10.10.2", PeerAS: 33333}, + }, + origin: 0, + s: gobgp.NewBgpServer(), + } + go ctrl.s.Serve() + + if err := ctrl.s.StartBgp(context.Background(), &api.StartBgpRequest{ + Global: &api.Global{ + As: uint32(11111), + RouterId: ctrl.localIP.String(), + ListenPort: -1, + }, + }); err != nil { + a.FailNow(err.Error()) + } + defer ctrl.s.StopBgp(context.Background(), &api.StopBgpRequest{}) + + // Add peers + for i := range ctrl.peers { + ctrl.addPeer(&ctrl.peers[i]) + } + + // Get peer info + peers, err := ctrl.PeerInfo() + a.NoError(err) + a.Equal(2, len(peers), "Should return info for both peers") + + // Verify peer addresses + peerAddrs := make(map[string]bool) + for _, p := range peers { + peerAddrs[p.Conf.NeighborAddress] = true + } + a.True(peerAddrs["10.10.10.1"], "Should have first peer") + a.True(peerAddrs["10.10.10.2"], "Should have second peer") +} diff --git a/controller/monitor.go b/controller/monitor.go index f5b9ddf..24dd50b 100644 --- a/controller/monitor.go +++ b/controller/monitor.go @@ -356,6 +356,6 @@ func (m *MonitorMgr) Cleanup(app string, exit chan bool) { } // GetInfo returns basic BGP info for established peers -func (m *MonitorMgr) GetInfo() (*api.Peer, error) { +func (m *MonitorMgr) GetInfo() ([]*api.Peer, error) { return m.ctrl.PeerInfo() } diff --git a/server/server.go b/server/server.go index bf50c36..40b943c 100644 --- a/server/server.go +++ b/server/server.go @@ -72,11 +72,11 @@ func (s *Server) unregisterHandler(w http.ResponseWriter, r *http.Request) { } func (s *Server) infoHandler(w http.ResponseWriter, r *http.Request) { - peer, err := s.mon.GetInfo() + peers, err := s.mon.GetInfo() if err != nil { http.Error(w, fmt.Sprintf("Internal error getting peers: %v", err), http.StatusInternalServerError) return } w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(peer) + json.NewEncoder(w).Encode(peers) }