Skip to content

Commit 55b24ad

Browse files
vflauxvflaux
authored
refactor(source/nat64): optional & early prefixes parsing (#5810)
1 parent 275715d commit 55b24ad

File tree

4 files changed

+104
-19
lines changed

4 files changed

+104
-19
lines changed

‎controller/execute.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,8 +448,13 @@ func buildSource(ctx context.Context, cfg *externaldns.Config) (source.Source, e
448448
// Combine multiple sources into a single, deduplicated source.
449449
combinedSource := wrappers.NewDedupSource(wrappers.NewMultiSource(sources, sourceCfg.DefaultTargets, sourceCfg.ForceDefaultTargets))
450450
cfg.AddSourceWrapper("dedup")
451-
combinedSource = wrappers.NewNAT64Source(combinedSource, cfg.NAT64Networks)
452-
cfg.AddSourceWrapper("nat64")
451+
if len(cfg.NAT64Networks) > 0 {
452+
combinedSource, err = wrappers.NewNAT64Source(combinedSource, cfg.NAT64Networks)
453+
if err != nil {
454+
return nil, fmt.Errorf("failed to create NAT64 source wrapper: %w", err)
455+
}
456+
cfg.AddSourceWrapper("nat64")
457+
}
453458
// Filter targets
454459
targetFilter := endpoint.NewTargetNetFilterWithExclusions(cfg.TargetNetFilter, cfg.ExcludeTargetNets)
455460
if targetFilter.IsEnabled() {

‎controller/execute_test.go‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,14 +454,25 @@ func TestBuildSourceWithWrappers(t *testing.T) {
454454
},
455455
},
456456
{
457-
name: "configuration without target filter wrapper",
457+
name: "configuration with nat64 networks",
458+
cfg: &externaldns.Config{
459+
APIServerURL: svr.URL,
460+
Sources: []string{"fake"},
461+
NAT64Networks: []string{"2001:db8::/96"},
462+
},
463+
asserts: func(t *testing.T, cfg *externaldns.Config) {
464+
assert.True(t, cfg.IsSourceWrapperInstrumented("nat64"))
465+
},
466+
},
467+
{
468+
name: "default configuration",
458469
cfg: &externaldns.Config{
459470
APIServerURL: svr.URL,
460471
Sources: []string{"fake"},
461472
},
462473
asserts: func(t *testing.T, cfg *externaldns.Config) {
463474
assert.True(t, cfg.IsSourceWrapperInstrumented("dedup"))
464-
assert.True(t, cfg.IsSourceWrapperInstrumented("nat64"))
475+
assert.False(t, cfg.IsSourceWrapperInstrumented("nat64"))
465476
assert.False(t, cfg.IsSourceWrapperInstrumented("target-filter"))
466477
},
467478
},

‎source/wrappers/nat64source.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,13 @@ import (
3030
// nat64Source is a Source that adds A endpoints for AAAA records including an NAT64 address.
3131
type nat64Source struct {
3232
source source.Source
33-
nat64Prefixes []string
33+
nat64Prefixes []netip.Prefix
3434
}
3535

3636
// NewNAT64Source creates a new nat64Source wrapping the provided Source.
37-
func NewNAT64Source(source source.Source, nat64Prefixes []string) source.Source {
38-
return &nat64Source{source: source, nat64Prefixes: nat64Prefixes}
39-
}
40-
41-
// Endpoints collects endpoints from its wrapped source and returns them without duplicates.
42-
func (s *nat64Source) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error) {
43-
log.Debug("nat64Source: collecting endpoints and processing NAT64 translation")
37+
func NewNAT64Source(source source.Source, nat64Prefixes []string) (source.Source, error) {
4438
parsedNAT64Prefixes := make([]netip.Prefix, 0)
45-
for _, prefix := range s.nat64Prefixes {
39+
for _, prefix := range nat64Prefixes {
4640
pPrefix, err := netip.ParsePrefix(prefix)
4741
if err != nil {
4842
return nil, err
@@ -53,6 +47,12 @@ func (s *nat64Source) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, erro
5347
}
5448
parsedNAT64Prefixes = append(parsedNAT64Prefixes, pPrefix)
5549
}
50+
return &nat64Source{source: source, nat64Prefixes: parsedNAT64Prefixes}, nil
51+
}
52+
53+
// Endpoints collects endpoints from its wrapped source and returns them without duplicates.
54+
func (s *nat64Source) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error) {
55+
log.Debug("nat64Source: collecting endpoints and processing NAT64 translation")
5656

5757
additionalEndpoints := []*endpoint.Endpoint{}
5858

@@ -76,7 +76,7 @@ func (s *nat64Source) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, erro
7676

7777
var sPrefix *netip.Prefix
7878

79-
for _, cPrefix := range parsedNAT64Prefixes {
79+
for _, cPrefix := range s.nat64Prefixes {
8080
if cPrefix.Contains(ip) {
8181
sPrefix = &cPrefix
8282
}

‎source/wrappers/nat64source_test.go‎

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@ package wrappers
1818

1919
import (
2020
"context"
21+
"net/netip"
2122
"testing"
2223

24+
"github.com/stretchr/testify/assert"
25+
"github.com/stretchr/testify/require"
2326
"sigs.k8s.io/external-dns/endpoint"
2427
"sigs.k8s.io/external-dns/internal/testutils"
2528
"sigs.k8s.io/external-dns/source"
@@ -74,12 +77,11 @@ func testNat64Source(t *testing.T) {
7477
mockSource.On("Endpoints").Return(tc.endpoints, nil)
7578

7679
// Create our object under test and get the endpoints.
77-
source := NewNAT64Source(mockSource, []string{"2001:DB8::/96"})
80+
source, err := NewNAT64Source(mockSource, []string{"2001:DB8::/96"})
81+
require.NoError(t, err)
7882

7983
endpoints, err := source.Endpoints(context.Background())
80-
if err != nil {
81-
t.Fatal(err)
82-
}
84+
require.NoError(t, err)
8385

8486
// Validate returned endpoints against desired endpoints.
8587
validateEndpoints(t, endpoints, tc.expected)
@@ -112,10 +114,77 @@ func TestNat64Source_AddEventHandler(t *testing.T) {
112114
t.Run(tt.title, func(t *testing.T) {
113115
mockSource := testutils.NewMockSource()
114116

115-
src := NewNAT64Source(mockSource, tt.input)
117+
src, err := NewNAT64Source(mockSource, tt.input)
118+
require.NoError(t, err)
119+
116120
src.AddEventHandler(t.Context(), func() {})
117121

118122
mockSource.AssertNumberOfCalls(t, "AddEventHandler", tt.times)
119123
})
120124
}
121125
}
126+
127+
func TestNewNAT64Source(t *testing.T) {
128+
type args struct {
129+
source source.Source
130+
nat64Prefixes []string
131+
}
132+
tests := []struct {
133+
name string
134+
args args
135+
want source.Source
136+
wantErr bool
137+
}{
138+
{
139+
name: "empty NAT64 prefixes should succeed",
140+
args: args{
141+
source: &testutils.MockSource{},
142+
nat64Prefixes: []string{},
143+
},
144+
want: &nat64Source{source: &testutils.MockSource{}, nat64Prefixes: []netip.Prefix{}},
145+
},
146+
{
147+
name: "multiple valid NAT64 prefixes should succeed",
148+
args: args{
149+
source: &testutils.MockSource{},
150+
nat64Prefixes: []string{"2001:db8::/96", "64:ff9b::/96"},
151+
},
152+
want: &nat64Source{source: &testutils.MockSource{}, nat64Prefixes: []netip.Prefix{netip.MustParsePrefix("2001:db8::/96"), netip.MustParsePrefix("64:ff9b::/96")}},
153+
},
154+
{
155+
name: "invalid NAT64 prefix should fail",
156+
args: args{
157+
source: &testutils.MockSource{},
158+
nat64Prefixes: []string{"invalid-prefix"},
159+
},
160+
wantErr: true,
161+
},
162+
{
163+
name: "NAT64 prefix with wrong mask length should fail",
164+
args: args{
165+
source: &testutils.MockSource{},
166+
nat64Prefixes: []string{"2001:db8::/64"},
167+
},
168+
wantErr: true,
169+
},
170+
{
171+
name: "IPv4 address as NAT64 prefix should fail",
172+
args: args{
173+
source: &testutils.MockSource{},
174+
nat64Prefixes: []string{"192.0.2.0/24"},
175+
},
176+
wantErr: true,
177+
},
178+
}
179+
for _, tt := range tests {
180+
t.Run(tt.name, func(t *testing.T) {
181+
src, err := NewNAT64Source(tt.args.source, tt.args.nat64Prefixes)
182+
if tt.wantErr {
183+
assert.Error(t, err)
184+
} else {
185+
assert.NoError(t, err)
186+
}
187+
assert.Equal(t, tt.want, src)
188+
})
189+
}
190+
}

0 commit comments

Comments
 (0)