Skip to content

Commit 64c58ac

Browse files
refactor(source/wrappers): move wrappers logic away from execute file (#5888)
* refactor(source/wrappers): move wrappers away from Signed-off-by: ivan katliarchuk <[email protected]> * refactor(source/wrappers): move wrappers away from Signed-off-by: ivan katliarchuk <[email protected]> * refactor(source/wrappers): move wrappers away from Signed-off-by: ivan katliarchuk <[email protected]> * refactor(source/wrappers): move wrappers away from Signed-off-by: ivan katliarchuk <[email protected]> --------- Signed-off-by: ivan katliarchuk <[email protected]>
1 parent 66b698e commit 64c58ac

File tree

6 files changed

+276
-55
lines changed

6 files changed

+276
-55
lines changed

controller/execute.go

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -446,24 +446,14 @@ func buildSource(ctx context.Context, cfg *externaldns.Config) (source.Source, e
446446
if err != nil {
447447
return nil, err
448448
}
449-
// Combine multiple sources into a single, deduplicated source.
450-
combinedSource := wrappers.NewDedupSource(wrappers.NewMultiSource(sources, sourceCfg.DefaultTargets, sourceCfg.ForceDefaultTargets))
451-
cfg.AddSourceWrapper("dedup")
452-
if len(cfg.NAT64Networks) > 0 {
453-
combinedSource, err = wrappers.NewNAT64Source(combinedSource, cfg.NAT64Networks)
454-
if err != nil {
455-
return nil, fmt.Errorf("failed to create NAT64 source wrapper: %w", err)
456-
}
457-
cfg.AddSourceWrapper("nat64")
458-
}
459-
// Filter targets
460-
targetFilter := endpoint.NewTargetNetFilterWithExclusions(cfg.TargetNetFilter, cfg.ExcludeTargetNets)
461-
if targetFilter.IsEnabled() {
462-
combinedSource = wrappers.NewTargetFilterSource(combinedSource, targetFilter)
463-
cfg.AddSourceWrapper("target-filter")
464-
}
465-
combinedSource = wrappers.NewPostProcessor(combinedSource, wrappers.WithTTL(cfg.MinTTL))
466-
return combinedSource, nil
449+
opts := wrappers.NewConfig(
450+
wrappers.WithDefaultTargets(cfg.DefaultTargets),
451+
wrappers.WithForceDefaultTargets(cfg.ForceDefaultTargets),
452+
wrappers.WithNAT64Networks(cfg.NAT64Networks),
453+
wrappers.WithTargetNetFilter(cfg.TargetNetFilter),
454+
wrappers.WithExcludeTargetNets(cfg.ExcludeTargetNets),
455+
wrappers.WithMinTTL(cfg.MinTTL))
456+
return wrappers.WrapSources(sources, opts)
467457
}
468458

469459
// RegexDomainFilter overrides DomainFilter

controller/execute_test.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -449,9 +449,6 @@ func TestBuildSourceWithWrappers(t *testing.T) {
449449
Sources: []string{"fake"},
450450
TargetNetFilter: []string{"10.0.0.0/8"},
451451
},
452-
asserts: func(t *testing.T, cfg *externaldns.Config) {
453-
assert.True(t, cfg.IsSourceWrapperInstrumented("target-filter"))
454-
},
455452
},
456453
{
457454
name: "configuration with nat64 networks",
@@ -460,29 +457,20 @@ func TestBuildSourceWithWrappers(t *testing.T) {
460457
Sources: []string{"fake"},
461458
NAT64Networks: []string{"2001:db8::/96"},
462459
},
463-
asserts: func(t *testing.T, cfg *externaldns.Config) {
464-
assert.True(t, cfg.IsSourceWrapperInstrumented("nat64"))
465-
},
466460
},
467461
{
468462
name: "default configuration",
469463
cfg: &externaldns.Config{
470464
APIServerURL: svr.URL,
471465
Sources: []string{"fake"},
472466
},
473-
asserts: func(t *testing.T, cfg *externaldns.Config) {
474-
assert.True(t, cfg.IsSourceWrapperInstrumented("dedup"))
475-
assert.False(t, cfg.IsSourceWrapperInstrumented("nat64"))
476-
assert.False(t, cfg.IsSourceWrapperInstrumented("target-filter"))
477-
},
478467
},
479468
}
480469

481470
for _, tt := range tests {
482471
t.Run(tt.name, func(t *testing.T) {
483472
_, err := buildSource(t.Context(), tt.cfg)
484473
require.NoError(t, err)
485-
tt.asserts(t, tt.cfg)
486474
})
487475
}
488476
}

pkg/apis/externaldns/types.go

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,6 @@ type Config struct {
217217
ExcludeUnschedulable bool
218218
EmitEvents []string
219219
ForceDefaultTargets bool
220-
sourceWrappers map[string]bool // map of source wrappers, e.g. "targetfilter", "nat64"
221220
}
222221

223222
var defaultConfig = &Config{
@@ -383,7 +382,6 @@ var defaultConfig = &Config{
383382
WebhookServer: false,
384383
ZoneIDFilter: []string{},
385384
ForceDefaultTargets: false,
386-
sourceWrappers: map[string]bool{},
387385
}
388386

389387
var providerNames = []string{
@@ -808,22 +806,6 @@ func bindFlags(b FlagBinder, cfg *Config) {
808806
b.BoolVar("webhook-server", "When enabled, runs as a webhook server instead of a controller. (default: false).", defaultConfig.WebhookServer, &cfg.WebhookServer)
809807
}
810808

811-
func (cfg *Config) AddSourceWrapper(name string) {
812-
if cfg.sourceWrappers == nil {
813-
cfg.sourceWrappers = make(map[string]bool)
814-
}
815-
cfg.sourceWrappers[name] = true
816-
}
817-
818-
// IsSourceWrapperInstrumented returns whether a source wrapper is enabled or not.
819-
func (cfg *Config) IsSourceWrapperInstrumented(name string) bool {
820-
if cfg.sourceWrappers == nil {
821-
return false
822-
}
823-
_, ok := cfg.sourceWrappers[name]
824-
return ok
825-
}
826-
827809
func App(cfg *Config) *kingpin.Application {
828810
app := kingpin.New("external-dns", "ExternalDNS synchronizes exposed Kubernetes Services and Ingresses with DNS providers.\n\nNote that all flags may be replaced with env vars - `--flag` -> `EXTERNAL_DNS_FLAG=1` or `--flag value` -> `EXTERNAL_DNS_FLAG=value`")
829811
app.Version(Version)

pkg/apis/externaldns/types_test.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,13 +1079,6 @@ func TestNewCobraCommandValidationValid(t *testing.T) {
10791079
require.NoError(t, err)
10801080
}
10811081

1082-
func TestSourceWrapperHelpers(t *testing.T) {
1083-
cfg := NewConfig()
1084-
assert.False(t, cfg.IsSourceWrapperInstrumented("nat64"))
1085-
cfg.AddSourceWrapper("nat64")
1086-
assert.True(t, cfg.IsSourceWrapperInstrumented("nat64"))
1087-
}
1088-
10891082
// Accepted binder/backend differences:
10901083
// - Enum validation
10911084
// - Boolean negation form

source/wrappers/types.go

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package wrappers
18+
19+
import (
20+
"fmt"
21+
"time"
22+
23+
"sigs.k8s.io/external-dns/endpoint"
24+
"sigs.k8s.io/external-dns/source"
25+
)
26+
27+
type Config struct {
28+
defaultTargets []string
29+
forceDefaultTargets bool
30+
nat64Networks []string
31+
targetNetFilter []string
32+
excludeTargetNets []string
33+
minTTL time.Duration
34+
sourceWrappers map[string]bool // map of source wrappers, e.g. "targetfilter", "nat64"
35+
}
36+
37+
func NewConfig(opts ...Option) *Config {
38+
o := &Config{}
39+
for _, opt := range opts {
40+
opt(o)
41+
}
42+
return o
43+
}
44+
45+
type Option func(config *Config)
46+
47+
func WithDefaultTargets(input []string) Option {
48+
return func(o *Config) {
49+
o.defaultTargets = input
50+
}
51+
}
52+
53+
func WithForceDefaultTargets(input bool) Option {
54+
return func(o *Config) {
55+
o.forceDefaultTargets = input
56+
}
57+
}
58+
59+
func WithNAT64Networks(input []string) Option {
60+
return func(o *Config) {
61+
o.nat64Networks = input
62+
}
63+
}
64+
65+
func WithTargetNetFilter(input []string) Option {
66+
return func(o *Config) {
67+
o.targetNetFilter = input
68+
}
69+
}
70+
71+
func WithExcludeTargetNets(input []string) Option {
72+
return func(o *Config) {
73+
o.excludeTargetNets = input
74+
}
75+
}
76+
77+
func WithMinTTL(ttl time.Duration) Option {
78+
return func(o *Config) {
79+
o.minTTL = ttl
80+
}
81+
}
82+
83+
// addSourceWrapper registers a source wrapper by name in the Config.
84+
// It initializes the sourceWrappers map if it is nil.
85+
func (o *Config) addSourceWrapper(name string) {
86+
if o.sourceWrappers == nil {
87+
o.sourceWrappers = make(map[string]bool)
88+
}
89+
o.sourceWrappers[name] = true
90+
}
91+
92+
// isSourceWrapperInstrumented returns whether a source wrapper is enabled or not.
93+
func (o *Config) isSourceWrapperInstrumented(name string) bool {
94+
if o.sourceWrappers == nil {
95+
return false
96+
}
97+
_, ok := o.sourceWrappers[name]
98+
return ok
99+
}
100+
101+
// WrapSources combines multiple sources into a single source,
102+
// applies optional NAT64 and target network filtering wrappers, and sets a minimum TTL.
103+
// It registers each applied wrapper in the Config for instrumentation.
104+
func WrapSources(
105+
sources []source.Source,
106+
opts *Config,
107+
) (source.Source, error) {
108+
combinedSource := NewDedupSource(NewMultiSource(sources, opts.defaultTargets, opts.forceDefaultTargets))
109+
opts.addSourceWrapper("dedup")
110+
if len(opts.nat64Networks) > 0 {
111+
var err error
112+
combinedSource, err = NewNAT64Source(combinedSource, opts.nat64Networks)
113+
if err != nil {
114+
return nil, fmt.Errorf("failed to create NAT64 source wrapper: %w", err)
115+
}
116+
opts.addSourceWrapper("nat64")
117+
}
118+
targetFilter := endpoint.NewTargetNetFilterWithExclusions(opts.targetNetFilter, opts.excludeTargetNets)
119+
if targetFilter.IsEnabled() {
120+
combinedSource = NewTargetFilterSource(combinedSource, targetFilter)
121+
opts.addSourceWrapper("target-filter")
122+
}
123+
combinedSource = NewPostProcessor(combinedSource, WithTTL(opts.minTTL))
124+
opts.addSourceWrapper("post-processor")
125+
return combinedSource, nil
126+
}

source/wrappers/types_test.go

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package wrappers
18+
19+
import (
20+
"testing"
21+
"time"
22+
23+
"github.com/stretchr/testify/assert"
24+
"github.com/stretchr/testify/require"
25+
)
26+
27+
func TestBuildSourceWithWrappers(t *testing.T) {
28+
tests := []struct {
29+
name string
30+
cfg *Config
31+
asserts func(*testing.T, *Config)
32+
}{
33+
{
34+
name: "configuration with target filter wrapper",
35+
cfg: NewConfig(
36+
WithTargetNetFilter([]string{"10.0.0.0/8"}),
37+
),
38+
asserts: func(t *testing.T, cfg *Config) {
39+
assert.True(t, cfg.isSourceWrapperInstrumented("target-filter"))
40+
},
41+
},
42+
{
43+
name: "configuration with nat64 networks",
44+
cfg: NewConfig(
45+
WithNAT64Networks([]string{"2001:db8::/96"}),
46+
),
47+
asserts: func(t *testing.T, cfg *Config) {
48+
assert.True(t, cfg.isSourceWrapperInstrumented("nat64"))
49+
},
50+
},
51+
{
52+
name: "default configuration",
53+
cfg: NewConfig(),
54+
asserts: func(t *testing.T, cfg *Config) {
55+
assert.True(t, cfg.isSourceWrapperInstrumented("dedup"))
56+
assert.False(t, cfg.isSourceWrapperInstrumented("nat64"))
57+
assert.False(t, cfg.isSourceWrapperInstrumented("target-filter"))
58+
},
59+
},
60+
{
61+
name: "with TTL and NAT64",
62+
cfg: NewConfig(
63+
WithMinTTL(300),
64+
WithNAT64Networks([]string{"2001:db8::/96"}),
65+
),
66+
asserts: func(t *testing.T, cfg *Config) {
67+
assert.True(t, cfg.isSourceWrapperInstrumented("dedup"))
68+
assert.True(t, cfg.isSourceWrapperInstrumented("nat64"))
69+
assert.True(t, cfg.isSourceWrapperInstrumented("post-processor"))
70+
assert.False(t, cfg.isSourceWrapperInstrumented("target-filter"))
71+
},
72+
},
73+
}
74+
75+
for _, tt := range tests {
76+
t.Run(tt.name, func(t *testing.T) {
77+
_, err := WrapSources(nil, tt.cfg)
78+
require.NoError(t, err)
79+
tt.asserts(t, tt.cfg)
80+
})
81+
}
82+
}
83+
84+
func TestWrapSources_NAT64Error(t *testing.T) {
85+
cfg := NewConfig(WithNAT64Networks([]string{"badnet"}))
86+
src, err := WrapSources(nil, cfg)
87+
assert.Nil(t, src)
88+
assert.Error(t, err)
89+
assert.Contains(t, err.Error(), "failed to create NAT64 source wrapper")
90+
}
91+
92+
func TestWithDefaultTargets(t *testing.T) {
93+
cfg := &Config{}
94+
opt := WithDefaultTargets([]string{"1.2.3.4"})
95+
opt(cfg)
96+
assert.Equal(t, []string{"1.2.3.4"}, cfg.defaultTargets)
97+
}
98+
99+
func TestWithForceDefaultTargets(t *testing.T) {
100+
cfg := &Config{}
101+
opt := WithForceDefaultTargets(true)
102+
opt(cfg)
103+
assert.True(t, cfg.forceDefaultTargets)
104+
}
105+
106+
func TestWithNAT64Networks(t *testing.T) {
107+
cfg := &Config{}
108+
opt := WithNAT64Networks([]string{"2001:db8::/96"})
109+
opt(cfg)
110+
assert.Equal(t, []string{"2001:db8::/96"}, cfg.nat64Networks)
111+
}
112+
113+
func TestWithTargetNetFilter(t *testing.T) {
114+
cfg := &Config{}
115+
opt := WithTargetNetFilter([]string{"10.0.0.0/8"})
116+
opt(cfg)
117+
assert.Equal(t, []string{"10.0.0.0/8"}, cfg.targetNetFilter)
118+
}
119+
120+
func TestWithExcludeTargetNets(t *testing.T) {
121+
cfg := &Config{}
122+
opt := WithExcludeTargetNets([]string{"192.168.0.0/16"})
123+
opt(cfg)
124+
assert.Equal(t, []string{"192.168.0.0/16"}, cfg.excludeTargetNets)
125+
}
126+
127+
func TestWithMinTTL(t *testing.T) {
128+
cfg := &Config{}
129+
opt := WithMinTTL(300 * time.Second)
130+
opt(cfg)
131+
assert.Equal(t, 300*time.Second, cfg.minTTL)
132+
}
133+
134+
func TestAddSourceWrapperAndIsSourceWrapperInstrumented(t *testing.T) {
135+
cfg := &Config{}
136+
assert.False(t, cfg.isSourceWrapperInstrumented("dedup"))
137+
cfg.addSourceWrapper("dedup")
138+
assert.True(t, cfg.isSourceWrapperInstrumented("dedup"))
139+
cfg.addSourceWrapper("nat64")
140+
assert.True(t, cfg.isSourceWrapperInstrumented("nat64"))
141+
assert.False(t, cfg.isSourceWrapperInstrumented("target-filter"))
142+
}

0 commit comments

Comments
 (0)