Skip to content

Commit 62f3f58

Browse files
committed
Fix port and localip defaults logic, add more test cases
1 parent 19d9387 commit 62f3f58

File tree

5 files changed

+247
-105
lines changed

5 files changed

+247
-105
lines changed

pkg/bgp/peer_config.go

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,17 @@ import (
1515
type PeerConfig struct {
1616
LocalIP *string `yaml:"localip"`
1717
Password *utils.Base64String `yaml:"password"`
18-
Port uint32 `yaml:"port"`
18+
Port *uint32 `yaml:"port"`
1919
RemoteASN *uint32 `yaml:"remoteasn"`
2020
RemoteIP *net.IP `yaml:"remoteip"`
2121
}
2222

2323
// Custom Stringer to prevent leaking passwords when printed
2424
func (p PeerConfig) String() string {
2525
var fields []string
26-
fields = append(fields, fmt.Sprintf("Port: %d", p.Port))
27-
26+
if p.Port != nil {
27+
fields = append(fields, fmt.Sprintf("Port: %d", *p.Port))
28+
}
2829
if p.LocalIP != nil {
2930
fields = append(fields, fmt.Sprintf("LocalIP: %s", *p.LocalIP))
3031
}
@@ -41,27 +42,32 @@ func (p *PeerConfig) UnmarshalYAML(raw []byte) error {
4142
tmp := struct {
4243
LocalIP *string `yaml:"localip"`
4344
Password *utils.Base64String `yaml:"password"`
44-
Port uint32 `yaml:"port"`
45+
Port *uint32 `yaml:"port"`
4546
RemoteASN *uint32 `yaml:"remoteasn"`
46-
RemoteIP string `yaml:"remoteip"`
47+
RemoteIP *string `yaml:"remoteip"`
4748
}{}
4849

4950
if err := yaml.Unmarshal(raw, &tmp); err != nil {
5051
return fmt.Errorf("failed to unmarshal peer config: %w", err)
5152
}
5253

54+
if tmp.RemoteIP == nil {
55+
return errors.New("remoteip cannot be empty")
56+
}
57+
if tmp.RemoteASN == nil {
58+
return errors.New("remoteasn cannot be empty")
59+
}
60+
5361
p.LocalIP = tmp.LocalIP
5462
p.Password = tmp.Password
5563
p.Port = tmp.Port
5664
p.RemoteASN = tmp.RemoteASN
5765

58-
if tmp.RemoteIP != "" {
59-
ip := net.ParseIP(tmp.RemoteIP)
60-
if ip == nil {
61-
return fmt.Errorf("%s is not a valid IP address", tmp.RemoteIP)
62-
}
63-
p.RemoteIP = &ip
66+
ip := net.ParseIP(*tmp.RemoteIP)
67+
if ip == nil {
68+
return fmt.Errorf("%s is not a valid IP address", *tmp.RemoteIP)
6469
}
70+
p.RemoteIP = &ip
6571
return nil
6672
}
6773

@@ -72,6 +78,8 @@ func (p PeerConfigs) LocalIPs() []string {
7278
for _, cfg := range p {
7379
if cfg.LocalIP != nil {
7480
localIPs = append(localIPs, *cfg.LocalIP)
81+
} else {
82+
localIPs = append(localIPs, "")
7583
}
7684
}
7785
return localIPs
@@ -91,7 +99,11 @@ func (p PeerConfigs) Passwords() []string {
9199
func (p PeerConfigs) Ports() []uint32 {
92100
ports := make([]uint32, 0)
93101
for _, cfg := range p {
94-
ports = append(ports, cfg.Port)
102+
if cfg.Port != nil {
103+
ports = append(ports, *cfg.Port)
104+
} else {
105+
ports = append(ports, options.DefaultBgpPort)
106+
}
95107
}
96108
return ports
97109
}
@@ -172,7 +184,7 @@ func NewPeerConfigs(
172184
peerCfgs[i].RemoteASN = &remoteASNs[i]
173185

174186
if len(ports) != 0 {
175-
peerCfgs[i].Port = ports[i]
187+
peerCfgs[i].Port = &ports[i]
176188
}
177189

178190
if len(b64EncodedPasswords) != 0 {
@@ -206,13 +218,11 @@ func validatePeerConfigs(
206218
}
207219
if len(remoteIPs) != len(ports) && len(ports) != 0 {
208220
return fmt.Errorf("invalid peer router config. The number of ports should either be zero, or "+
209-
"one per peer router. If blank items are used, it will default to standard BGP port, %s. "+
210-
"Example: \"port,,port\" OR [\"port\",\"\",\"port\"]", strconv.Itoa(options.DefaultBgpPort))
221+
"one per peer router. If blank items are used, it will default to standard BGP port, %s. ", strconv.Itoa(options.DefaultBgpPort))
211222
}
212223
if len(remoteIPs) != len(localIPs) && len(localIPs) != 0 {
213224
return fmt.Errorf("invalid peer router config. The number of localIPs should either be zero, or "+
214-
"one per peer router. If blank items are used, it will default to nodeIP, %s. "+
215-
"Example: \"10.1.1.1,,10.1.1.2\" OR [\"10.1.1.1\",\"\",\"10.1.1.2\"]", localAddress)
225+
"one per peer router. If blank items are used, it will default to nodeIP, %s. ", localAddress)
216226
}
217227

218228
for _, asn := range remoteASNs {

0 commit comments

Comments
 (0)