Skip to content

Commit cde4c95

Browse files
PoorungaPoorunga
authored
Merge pull request #584 from fanny7d/fix/autorelay-reservation-issue-583
fix: autorelay reservation for private nodes
2 parents 794fbf1 + 421e17d commit cde4c95

4 files changed

Lines changed: 251 additions & 21 deletions

File tree

‎pkg/tunnel/module.go‎

Lines changed: 58 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,12 @@ func newEdgeTunnel(c *v1alpha1.EdgeTunnelConfig) (*EdgeTunnel, error) {
107107
return nil, fmt.Errorf("failed to new conn manager: %w", err)
108108
}
109109

110-
listenAddr, err := generateListenAddr(c)
110+
ips, err := GetIPsFromInterfaces(c.ListenInterfaces, c.ExtraFilteredInterfaces)
111+
if err != nil {
112+
return nil, fmt.Errorf("failed to get ips from listen interfaces: %w", err)
113+
}
114+
115+
listenAddr, err := generateListenAddr(c, ips)
111116
if err != nil {
112117
return nil, fmt.Errorf("failed to generate listenAddr: %w", err)
113118
}
@@ -122,18 +127,19 @@ func newEdgeTunnel(c *v1alpha1.EdgeTunnelConfig) (*EdgeTunnel, error) {
122127
}))
123128
}
124129

125-
// If the relayMap does not contain any public IP, NATService will not be able to assist this non-relay node to
126-
// identify its own network(public, private or unknown), so it needs to configure libp2p.ForceReachabilityPrivate()
127-
if !isRelay && !relayMap.ContainsPublicIP() {
130+
// Nodes that only listen on non-public addresses cannot rely on AutoNAT to infer
131+
// public reachability in fronted-relay setups such as ACK+NLB, so force private reachability.
132+
if !isRelay && (!relayMap.ContainsPublicIP() || ShouldForceReachabilityPrivate(ips)) {
128133
klog.Infof("Configure libp2p.ForceReachabilityPrivate()")
129134
opts = append(opts, libp2p.ForceReachabilityPrivate())
130135
}
131136

132137
relayNums := len(relayMap)
133-
if c.MaxCandidates < relayNums {
134-
klog.Infof("MaxCandidates=%d is less than len(relayMap)=%d, set MaxCandidates to len(relayMap)",
135-
c.MaxCandidates, relayNums)
136-
c.MaxCandidates = relayNums
138+
minCandidates, maxCandidates, numRelays := normalizeAutoRelayConfig(c.MaxCandidates, relayNums)
139+
if maxCandidates != c.MaxCandidates {
140+
klog.Infof("MaxCandidates adjusted from %d to %d (relayNums=%d)",
141+
c.MaxCandidates, maxCandidates, relayNums)
142+
c.MaxCandidates = maxCandidates
137143
}
138144

139145
// configures libp2p to use the given private network protector
@@ -165,9 +171,7 @@ func newEdgeTunnel(c *v1alpha1.EdgeTunnelConfig) (*EdgeTunnel, error) {
165171
func(ctx context.Context, numPeers int) <-chan peer.AddrInfo {
166172
return peerSource
167173
},
168-
autorelay.WithMinCandidates(0),
169-
autorelay.WithMaxCandidates(c.MaxCandidates),
170-
autorelay.WithBackoff(30*time.Second),
174+
buildAutoRelayOpts(minCandidates, c.MaxCandidates, numRelays)...,
171175
),
172176
libp2p.EnableNATService(),
173177
libp2p.EnableHolePunching(),
@@ -270,12 +274,7 @@ func newEdgeTunnel(c *v1alpha1.EdgeTunnelConfig) (*EdgeTunnel, error) {
270274
return edgeTunnel, nil
271275
}
272276

273-
func generateListenAddr(c *v1alpha1.EdgeTunnelConfig) (libp2p.Option, error) {
274-
ips, err := GetIPsFromInterfaces(c.ListenInterfaces, c.ExtraFilteredInterfaces)
275-
if err != nil {
276-
return nil, fmt.Errorf("failed to get ips from listen interfaces: %w", err)
277-
}
278-
277+
func generateListenAddr(c *v1alpha1.EdgeTunnelConfig, ips []string) (libp2p.Option, error) {
279278
multiAddrStrings := make([]string, 0)
280279
if c.Mode == defaults.ServerClientMode {
281280
for _, ip := range ips {
@@ -290,3 +289,45 @@ func generateListenAddr(c *v1alpha1.EdgeTunnelConfig) (libp2p.Option, error) {
290289
listenAddr := libp2p.ListenAddrStrings(multiAddrStrings...)
291290
return listenAddr, nil
292291
}
292+
293+
// normalizeAutoRelayConfig computes normalized autorelay parameters.
294+
//
295+
// Fixes the bug where WithMinCandidates(0) prevents peerSource from ever being
296+
// triggered (issue #583):
297+
// - minCandidates is set to at least 1 so relay_finder calls peerSource and
298+
// enters the Reserve() path.
299+
// - maxCandidates is at least relayNums so the candidate pool is large enough.
300+
// - For single-relay setups, numRelays=1 avoids waiting for a second relay
301+
// (the default desiredRelays=2 would stall indefinitely with only one relay).
302+
func normalizeAutoRelayConfig(cfgMaxCandidates, relayNums int) (minCandidates, maxCandidates, numRelays int) {
303+
maxCandidates = cfgMaxCandidates
304+
if maxCandidates < relayNums {
305+
maxCandidates = relayNums
306+
}
307+
// minCandidates must be >= 1; otherwise the condition
308+
// `numCandidates < minCandidates` in relay_finder.findNodes() is always
309+
// false and peerSource is never called.
310+
minCandidates = 1
311+
if maxCandidates > 0 && minCandidates > maxCandidates {
312+
minCandidates = maxCandidates
313+
}
314+
// For single-relay setups, explicitly set desiredRelays=1 to avoid
315+
// waiting forever for a second relay (default desiredRelays=2).
316+
if relayNums == 1 {
317+
numRelays = 1
318+
}
319+
return
320+
}
321+
322+
// buildAutoRelayOpts constructs the autorelay option list from normalized parameters.
323+
func buildAutoRelayOpts(minCandidates, maxCandidates, numRelays int) []autorelay.Option {
324+
opts := []autorelay.Option{
325+
autorelay.WithMinCandidates(minCandidates),
326+
autorelay.WithMaxCandidates(maxCandidates),
327+
autorelay.WithBackoff(30 * time.Second),
328+
}
329+
if numRelays > 0 {
330+
opts = append(opts, autorelay.WithNumRelays(numRelays))
331+
}
332+
return opts
333+
}

‎pkg/tunnel/module_test.go‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package tunnel
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestNormalizeAutoRelayConfig(t *testing.T) {
8+
tests := []struct {
9+
name string
10+
cfgMaxCandidates int
11+
relayNums int
12+
wantMinCandidates int
13+
wantMaxCandidates int
14+
wantNumRelays int
15+
}{
16+
{
17+
// root cause scenario from issue #583: single relay with default MaxCandidates;
18+
// after the fix minCandidates must be >= 1 and numRelays must be 1
19+
name: "single relay – min candidates fixed to 1, numRelays set to 1",
20+
cfgMaxCandidates: 4,
21+
relayNums: 1,
22+
wantMinCandidates: 1,
23+
wantMaxCandidates: 4,
24+
wantNumRelays: 1,
25+
},
26+
{
27+
// multiple relays, maxCandidates is large enough
28+
name: "multi relay – maxCandidates sufficient, numRelays stays 0",
29+
cfgMaxCandidates: 4,
30+
relayNums: 3,
31+
wantMinCandidates: 1,
32+
wantMaxCandidates: 4,
33+
wantNumRelays: 0,
34+
},
35+
{
36+
// multiple relays, maxCandidates is too small and must be boosted to relayNums
37+
name: "multi relay – maxCandidates boosted to relayNums",
38+
cfgMaxCandidates: 2,
39+
relayNums: 5,
40+
wantMinCandidates: 1,
41+
wantMaxCandidates: 5,
42+
wantNumRelays: 0,
43+
},
44+
{
45+
// cfgMaxCandidates=0 with single relay:
46+
// maxCandidates should be boosted to relayNums=1, minCandidates=1
47+
name: "zero maxCandidates with single relay – both boosted to 1",
48+
cfgMaxCandidates: 0,
49+
relayNums: 1,
50+
wantMinCandidates: 1,
51+
wantMaxCandidates: 1,
52+
wantNumRelays: 1,
53+
},
54+
{
55+
// no relay nodes configured; minCandidates=1 must still hold
56+
name: "no relay nodes",
57+
cfgMaxCandidates: 4,
58+
relayNums: 0,
59+
wantMinCandidates: 1,
60+
wantMaxCandidates: 4,
61+
wantNumRelays: 0,
62+
},
63+
}
64+
65+
for _, tt := range tests {
66+
t.Run(tt.name, func(t *testing.T) {
67+
gotMin, gotMax, gotNum := normalizeAutoRelayConfig(tt.cfgMaxCandidates, tt.relayNums)
68+
if gotMin != tt.wantMinCandidates {
69+
t.Errorf("minCandidates: got %d, want %d", gotMin, tt.wantMinCandidates)
70+
}
71+
if gotMax != tt.wantMaxCandidates {
72+
t.Errorf("maxCandidates: got %d, want %d", gotMax, tt.wantMaxCandidates)
73+
}
74+
if gotNum != tt.wantNumRelays {
75+
t.Errorf("numRelays: got %d, want %d", gotNum, tt.wantNumRelays)
76+
}
77+
})
78+
}
79+
}
80+
81+
func TestBuildAutoRelayOpts(t *testing.T) {
82+
t.Run("without numRelays", func(t *testing.T) {
83+
opts := buildAutoRelayOpts(1, 4, 0)
84+
// should contain 3 options: WithMinCandidates, WithMaxCandidates, WithBackoff
85+
if len(opts) != 3 {
86+
t.Errorf("expected 3 options, got %d", len(opts))
87+
}
88+
})
89+
90+
t.Run("with numRelays", func(t *testing.T) {
91+
opts := buildAutoRelayOpts(1, 1, 1)
92+
// should contain 4 options: WithMinCandidates, WithMaxCandidates, WithBackoff, WithNumRelays
93+
if len(opts) != 4 {
94+
t.Errorf("expected 4 options, got %d", len(opts))
95+
}
96+
})
97+
}

‎pkg/tunnel/util.go‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/libp2p/go-libp2p/p2p/transport/tcp"
2121
ws "github.com/libp2p/go-libp2p/p2p/transport/websocket"
2222
ma "github.com/multiformats/go-multiaddr"
23+
manet "github.com/multiformats/go-multiaddr/net"
2324
"k8s.io/klog/v2"
2425

2526
"github.com/kubeedge/edgemesh/pkg/apis/config/v1alpha1"
@@ -182,16 +183,48 @@ func GenerateRelayMap(relayNodes []*v1alpha1.RelayNode, protocol string, listenP
182183
func FilterPrivateMaddr(maddrs []ma.Multiaddr) []ma.Multiaddr {
183184
result := make([]ma.Multiaddr, 0)
184185
for _, maddr := range maddrs {
185-
maddrElements := strings.Split(maddr.String(), "/")
186-
ip := maddrElements[ipIndex]
187-
ipAddress := net.ParseIP(ip)
188-
if !ipAddress.IsLoopback() && !ipAddress.IsPrivate() {
186+
if manet.IsPublicAddr(maddr) || isDNSMaddr(maddr) {
189187
result = append(result, maddr)
190188
}
191189
}
192190
return result
193191
}
194192

193+
func isDNSMaddr(maddr ma.Multiaddr) bool {
194+
first, _ := ma.SplitFirst(maddr)
195+
if first == nil {
196+
return false
197+
}
198+
199+
switch first.Protocol().Code {
200+
case ma.P_DNS, ma.P_DNS4, ma.P_DNS6, ma.P_DNSADDR:
201+
return true
202+
default:
203+
return false
204+
}
205+
}
206+
207+
func ShouldForceReachabilityPrivate(ips []string) bool {
208+
if len(ips) == 0 {
209+
return false
210+
}
211+
212+
for _, ip := range ips {
213+
if isPublicIP(ip) {
214+
return false
215+
}
216+
}
217+
return true
218+
}
219+
220+
func isPublicIP(ip string) bool {
221+
maddr, err := ma.NewMultiaddr(GenerateMultiAddrString(TCP, ip, 1))
222+
if err != nil {
223+
return false
224+
}
225+
return manet.IsPublicAddr(maddr)
226+
}
227+
195228
func FilterCircuitMaddr(maddrs []ma.Multiaddr) []ma.Multiaddr {
196229
result := make([]ma.Multiaddr, 0)
197230
for _, maddr := range maddrs {

‎pkg/tunnel/util_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,65 @@ func TestAddCircuitAddrsToPeer(t *testing.T) {
124124
assertEqual(relayPeer.String(), want, t)
125125
}
126126

127+
func TestFilterPrivateMaddrDropsNonRoutableRelayAddrs(t *testing.T) {
128+
maddrs, err := StringsToMaddrs([]string{
129+
"/ip4/47.95.1.47/tcp/20006",
130+
"/ip4/10.112.27.223/tcp/20006",
131+
"/ip4/169.254.20.10/tcp/20006",
132+
"/ip4/198.18.1.255/tcp/20006",
133+
"/dns4/nlb-9qpaugg3doihkhemjw.cn-beijing.nlb.aliyuncsslb.com/tcp/20006",
134+
})
135+
if !assertEqual(err, nil, t) {
136+
t.Fatalf("failed to create multiaddrs: %v", err)
137+
}
138+
139+
want, err := StringsToMaddrs([]string{
140+
"/ip4/47.95.1.47/tcp/20006",
141+
"/dns4/nlb-9qpaugg3doihkhemjw.cn-beijing.nlb.aliyuncsslb.com/tcp/20006",
142+
})
143+
if !assertEqual(err, nil, t) {
144+
t.Fatalf("failed to create expected multiaddrs: %v", err)
145+
}
146+
147+
got := FilterPrivateMaddr(maddrs)
148+
if !assertEqual(got, want, t) {
149+
t.Fatalf("expected only routable relay addrs %v, got %v", want, got)
150+
}
151+
}
152+
153+
func TestShouldForceReachabilityPrivate(t *testing.T) {
154+
tests := []struct {
155+
name string
156+
ips []string
157+
want bool
158+
}{
159+
{
160+
name: "only private and loopback addresses",
161+
ips: []string{"10.80.9.54", "127.0.0.1"},
162+
want: true,
163+
},
164+
{
165+
name: "public address present",
166+
ips: []string{"10.80.9.54", "47.95.1.47"},
167+
want: false,
168+
},
169+
{
170+
name: "empty addresses",
171+
ips: nil,
172+
want: false,
173+
},
174+
}
175+
176+
for _, tt := range tests {
177+
t.Run(tt.name, func(t *testing.T) {
178+
got := ShouldForceReachabilityPrivate(tt.ips)
179+
if !assertEqual(got, tt.want, t) {
180+
t.Fatalf("ShouldForceReachabilityPrivate(%v) = %v, want %v", tt.ips, got, tt.want)
181+
}
182+
})
183+
}
184+
}
185+
127186
func TestAppendMultiaddrs(t *testing.T) {
128187
// generate multiAddress
129188
var nodes = []*testNode{

0 commit comments

Comments
 (0)