From 80f3a8d105f5b8d8d30b149ae00d64cb05cc0343 Mon Sep 17 00:00:00 2001 From: Denozordec Date: Mon, 6 Apr 2026 01:01:40 +0700 Subject: [PATCH] feat: enhance peer neighbor handling by introducing NormalizePeerNeighborString function. Update CreatePeer and UpdatePeer methods in Postgres and Memory stores to validate and normalize neighbor input, improving data integrity and error handling. --- internal/pipeline/refresh.go | 9 ++----- internal/repository/postgres.go | 12 +++++++-- internal/store/memory_crud.go | 14 +++++++--- internal/store/peer_neighbor.go | 40 ++++++++++++++++++++++++++++ internal/store/peer_neighbor_test.go | 33 +++++++++++++++++++++++ 5 files changed, 96 insertions(+), 12 deletions(-) create mode 100644 internal/store/peer_neighbor.go create mode 100644 internal/store/peer_neighbor_test.go diff --git a/internal/pipeline/refresh.go b/internal/pipeline/refresh.go index faad5ff..05f0545 100644 --- a/internal/pipeline/refresh.go +++ b/internal/pipeline/refresh.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "net/http" - "net/netip" "os" "sort" "strconv" @@ -345,12 +344,8 @@ func renderPeersBirdFragment(st store.Backend, tenantID string, loc birdLocals) if p == nil || !p.Enabled { continue } - neighbor := strings.TrimSpace(p.Neighbor) - if neighbor == "" { - continue - } - addr, err := netip.ParseAddr(neighbor) - if err != nil { + addr, ok := store.ParsePeerNeighbor(p.Neighbor) + if !ok { continue } if !store.ValidASN(p.RemoteASN) { diff --git a/internal/repository/postgres.go b/internal/repository/postgres.go index 06fa8bd..510cd6b 100644 --- a/internal/repository/postgres.go +++ b/internal/repository/postgres.go @@ -315,6 +315,10 @@ func (p *Postgres) CreatePeer(tenantID string, in *store.BGPPeer) (*store.BGPPee if in == nil || in.RemoteASN == 0 { return nil, store.ErrInvalidInput } + neighbor, ok := store.NormalizePeerNeighborString(in.Neighbor) + if !ok { + return nil, store.ErrInvalidInput + } ctx := context.Background() id := uuid.NewString() meta := map[string]any{"name": in.Name, "session_state": in.SessionState} @@ -330,7 +334,7 @@ func (p *Postgres) CreatePeer(tenantID string, in *store.BGPPeer) (*store.BGPPee _, err := p.pool.Exec(ctx, ` INSERT INTO bgp_peer (id, tenant_id, bgp_speaker_id, neighbor, remote_asn, enabled, policies_json, meta_json) VALUES ($1,$2,$3,$4::inet, $5, $6, $7::jsonb, $8::jsonb)`, - id, tenantID, sp, in.Neighbor, in.RemoteASN, in.Enabled, pol, string(mb)) + id, tenantID, sp, neighbor, in.RemoteASN, in.Enabled, pol, string(mb)) if err != nil { return nil, err } @@ -343,7 +347,11 @@ func (p *Postgres) UpdatePeer(tenantID, id string, patch *store.PeerPatch) (*sto return nil, err } if patch.Neighbor != nil { - cur.Neighbor = strings.TrimSpace(*patch.Neighbor) + n, ok := store.NormalizePeerNeighborString(*patch.Neighbor) + if !ok { + return nil, store.ErrInvalidInput + } + cur.Neighbor = n } if patch.RemoteASN != nil { cur.RemoteASN = *patch.RemoteASN diff --git a/internal/store/memory_crud.go b/internal/store/memory_crud.go index 780904b..271c075 100644 --- a/internal/store/memory_crud.go +++ b/internal/store/memory_crud.go @@ -614,7 +614,11 @@ func (m *Memory) GetPeer(tenantID, id string) (*BGPPeer, error) { } func (m *Memory) CreatePeer(tenantID string, in *BGPPeer) (*BGPPeer, error) { - if in == nil || strings.TrimSpace(in.Neighbor) == "" || in.RemoteASN == 0 { + if in == nil || in.RemoteASN == 0 { + return nil, ErrInvalidInput + } + neighbor, ok := NormalizePeerNeighborString(in.Neighbor) + if !ok { return nil, ErrInvalidInput } m.mu.Lock() @@ -625,7 +629,7 @@ func (m *Memory) CreatePeer(tenantID string, in *BGPPeer) (*BGPPeer, error) { id := uuid.NewString() p := &BGPPeer{ ID: id, TenantID: tenantID, SpeakerID: in.SpeakerID, Name: in.Name, - Neighbor: strings.TrimSpace(in.Neighbor), RemoteASN: in.RemoteASN, Enabled: in.Enabled, + Neighbor: neighbor, RemoteASN: in.RemoteASN, Enabled: in.Enabled, SessionState: in.SessionState, PoliciesJSON: in.PoliciesJSON, } if !p.Enabled && p.SessionState == "" { @@ -646,7 +650,11 @@ func (m *Memory) UpdatePeer(tenantID, id string, patch *PeerPatch) (*BGPPeer, er return nil, ErrNotFound } if patch.Neighbor != nil { - p.Neighbor = strings.TrimSpace(*patch.Neighbor) + n, ok := NormalizePeerNeighborString(*patch.Neighbor) + if !ok { + return nil, ErrInvalidInput + } + p.Neighbor = n } if patch.RemoteASN != nil { p.RemoteASN = *patch.RemoteASN diff --git a/internal/store/peer_neighbor.go b/internal/store/peer_neighbor.go new file mode 100644 index 0000000..039b969 --- /dev/null +++ b/internal/store/peer_neighbor.go @@ -0,0 +1,40 @@ +package store + +import ( + "net/netip" + "strings" +) + +// ParsePeerNeighbor parses a BGP neighbor value for BIRD output: a plain IPv4/IPv6 +// address, or a host prefix (/32 or /128) which is a common input mistake. +func ParsePeerNeighbor(s string) (netip.Addr, bool) { + s = strings.TrimSpace(s) + if s == "" { + return netip.Addr{}, false + } + if addr, err := netip.ParseAddr(s); err == nil { + return addr, true + } + pfx, err := netip.ParsePrefix(s) + if err != nil { + return netip.Addr{}, false + } + addr := pfx.Addr() + if addr.Is4() && pfx.Bits() == 32 { + return addr, true + } + if addr.Is6() && pfx.Bits() == 128 { + return addr, true + } + return netip.Addr{}, false +} + +// NormalizePeerNeighborString returns the canonical host address string for storage +// and BIRD, or false if s is not a usable neighbor. +func NormalizePeerNeighborString(s string) (string, bool) { + addr, ok := ParsePeerNeighbor(s) + if !ok { + return "", false + } + return addr.String(), true +} diff --git a/internal/store/peer_neighbor_test.go b/internal/store/peer_neighbor_test.go new file mode 100644 index 0000000..d870810 --- /dev/null +++ b/internal/store/peer_neighbor_test.go @@ -0,0 +1,33 @@ +package store + +import "testing" + +func TestParsePeerNeighbor(t *testing.T) { + tests := []struct { + in string + want string + wantOK bool + }{ + {"192.168.0.2", "192.168.0.2", true}, + {"192.168.0.2/32", "192.168.0.2", true}, + {" 192.168.0.2/32 ", "192.168.0.2", true}, + {"2001:db8::1", "2001:db8::1", true}, + {"2001:db8::1/128", "2001:db8::1", true}, + {"192.168.0.0/24", "", false}, + {"not-an-ip", "", false}, + {"", "", false}, + } + for _, tt := range tests { + addr, ok := ParsePeerNeighbor(tt.in) + if ok != tt.wantOK { + t.Errorf("ParsePeerNeighbor(%q) ok=%v want %v", tt.in, ok, tt.wantOK) + continue + } + if !tt.wantOK { + continue + } + if got := addr.String(); got != tt.want { + t.Errorf("ParsePeerNeighbor(%q) = %q want %q", tt.in, got, tt.want) + } + } +}