From d217706973893e720d78cf84beee5fccd376be3e Mon Sep 17 00:00:00 2001 From: Ivan Ka <5395690+ivankatliarchuk@users.noreply.github.com> Date: Mon, 23 Mar 2026 14:10:19 +0000 Subject: [PATCH] refactor(fqdn): encapsulate FQDN template logic into TemplateEngine (#6292) * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(source): extract FQDN template logic into fqdn.TemplateEngine Signed-off-by: ivan katliarchuk * refactor(fqdn): encapsulate FQDN template logic into TemplateEngine Signed-off-by: ivan katliarchuk * refactor(fqdn): encapsulate FQDN template logic into TemplateEngine Signed-off-by: ivan katliarchuk * efactor(fqdn): encapsulate FQDN template logic into TemplateEngine Signed-off-by: ivan katliarchuk * refactor(fqdn): encapsulate FQDN template logic into TemplateEngine Signed-off-by: ivan katliarchuk * refactor(fqdn): encapsulate FQDN template logic into TemplateEngine Signed-off-by: ivan katliarchuk * refactor(fqdn): encapsulate FQDN template logic into TemplateEngine Signed-off-by: ivan katliarchuk * refactor(fqdn): encapsulate FQDN template logic into TemplateEngine Signed-off-by: ivan katliarchuk * refactor(fqdn): encapsulate FQDN template logic into TemplateEngine Signed-off-by: ivan katliarchuk * refactor(fqdn): encapsulate FQDN template logic into TemplateEngine Signed-off-by: ivan katliarchuk * refactor(fqdn): encapsulate FQDN template logic into TemplateEngine Signed-off-by: ivan katliarchuk --------- Signed-off-by: ivan katliarchuk --- controller/execute.go | 5 +- controller/execute_test.go | 3 +- source/contour_httpproxy.go | 20 +- source/contour_httpproxy_test.go | 81 +-- source/fake.go | 11 +- source/fake_test.go | 4 +- source/fqdn/fqdn.go | 171 ------- source/gateway.go | 17 +- source/gateway_grpcroute_test.go | 4 +- source/gateway_httproute_test.go | 6 +- source/gateway_tcproute_test.go | 4 +- source/gateway_tlsroute_test.go | 4 +- source/gateway_udproute_test.go | 4 +- source/ingress.go | 20 +- source/ingress_fqdn_test.go | 58 +-- source/ingress_test.go | 15 +- source/istio_gateway.go | 20 +- source/istio_gateway_fqdn_test.go | 25 +- source/istio_gateway_test.go | 82 +-- source/istio_virtualservice.go | 20 +- source/istio_virtualservice_fqdn_test.go | 25 +- source/istio_virtualservice_test.go | 94 +--- source/node.go | 38 +- source/node_fqdn_test.go | 60 +-- source/node_test.go | 66 +-- source/openshift_route.go | 20 +- source/openshift_route_fqdn_test.go | 12 +- source/openshift_route_test.go | 73 +-- source/pod.go | 26 +- source/pod_fqdn_test.go | 51 +- source/pod_indexer_test.go | 3 +- source/pod_test.go | 4 +- source/service.go | 27 +- source/service_fqdn_test.go | 4 +- source/service_test.go | 88 +--- source/skipper_routegroup.go | 78 ++- source/skipper_routegroup_test.go | 39 +- source/store.go | 21 +- source/store_test.go | 72 +++ source/template/OWNERS | 4 + source/template/engine.go | 187 +++++++ .../fqdn_test.go => template/engine_test.go} | 466 ++++-------------- source/template/functions.go | 85 ++++ source/template/functions_test.go | 242 +++++++++ source/template/testutil/testutil.go | 33 ++ source/unstructured.go | 55 +-- source/unstructured_fqdn_test.go | 16 +- source/unstructured_test.go | 9 +- source/wrappers/build_test.go | 21 +- tests/integration/toolkit/toolkit.go | 7 +- 50 files changed, 1018 insertions(+), 1482 deletions(-) delete mode 100644 source/fqdn/fqdn.go create mode 100644 source/template/OWNERS create mode 100644 source/template/engine.go rename source/{fqdn/fqdn_test.go => template/engine_test.go} (54%) create mode 100644 source/template/functions.go create mode 100644 source/template/functions_test.go create mode 100644 source/template/testutil/testutil.go diff --git a/controller/execute.go b/controller/execute.go index 482212267..06175743a 100644 --- a/controller/execute.go +++ b/controller/execute.go @@ -81,7 +81,10 @@ func Execute() { go serveMetrics(cfg.MetricsAddress) go handleSigterm(cancel) - sCfg := source.NewSourceConfig(cfg) + sCfg, err := source.NewSourceConfig(cfg) + if err != nil { + log.Fatal(err) // nolint: gocritic // exitAfterDefer + } endpointsSource, err := wrappers.Build(ctx, sCfg) if err != nil { log.Fatal(err) // nolint: gocritic // exitAfterDefer diff --git a/controller/execute_test.go b/controller/execute_test.go index ea539dc7d..d6e201315 100644 --- a/controller/execute_test.go +++ b/controller/execute_test.go @@ -290,7 +290,8 @@ func TestControllerRunCancelContextStopsLoop(t *testing.T) { Registry: "txt", TXTOwnerID: "test-owner", } - sCfg := source.NewSourceConfig(cfg) + sCfg, err := source.NewSourceConfig(cfg) + require.NoError(t, err) ctx, cancel := context.WithCancel(t.Context()) defer cancel() src, err := wrappers.Build(ctx, sCfg) diff --git a/source/contour_httpproxy.go b/source/contour_httpproxy.go index 57440a824..065af472f 100644 --- a/source/contour_httpproxy.go +++ b/source/contour_httpproxy.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "text/template" projectcontour "github.com/projectcontour/contour/apis/projectcontour/v1" log "github.com/sirupsen/logrus" @@ -34,8 +33,8 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" "sigs.k8s.io/external-dns/source/informers" + "sigs.k8s.io/external-dns/source/template" ) // HTTPProxySource is an implementation of Source for ProjectContour HTTPProxy objects. @@ -54,8 +53,7 @@ type httpProxySource struct { dynamicKubeClient dynamic.Interface namespace string annotationFilter string - fqdnTemplate *template.Template - combineFQDNAnnotation bool + templateEngine template.Engine ignoreHostnameAnnotation bool httpProxyInformer kubeinformers.GenericInformer unstructuredConverter *UnstructuredConverter @@ -67,11 +65,6 @@ func NewContourHTTPProxySource( dynamicKubeClient dynamic.Interface, cfg *Config, ) (Source, error) { - tmpl, err := fqdn.ParseTemplate(cfg.FQDNTemplate) - if err != nil { - return nil, err - } - // Use shared informer to listen for add/update/delete of HTTPProxys in the specified namespace. // Set resync period to 0, to prevent processing when nothing has changed. informerFactory := dynamicinformer.NewFilteredDynamicSharedInformerFactory(dynamicKubeClient, 0, cfg.Namespace, nil) @@ -96,8 +89,7 @@ func NewContourHTTPProxySource( dynamicKubeClient: dynamicKubeClient, namespace: cfg.Namespace, annotationFilter: cfg.AnnotationFilter, - fqdnTemplate: tmpl, - combineFQDNAnnotation: cfg.CombineFQDNAndAnnotation, + templateEngine: cfg.TemplateEngine, ignoreHostnameAnnotation: cfg.IgnoreHostnameAnnotation, httpProxyInformer: httpProxyInformer, unstructuredConverter: uc, @@ -142,10 +134,8 @@ func (sc *httpProxySource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, e hpEndpoints := sc.endpointsFromHTTPProxy(hp) // apply template if fqdn is missing on HTTPProxy - hpEndpoints, err = fqdn.CombineWithTemplatedEndpoints( + hpEndpoints, err = sc.templateEngine.CombineWithEndpoints( hpEndpoints, - sc.fqdnTemplate, - sc.combineFQDNAnnotation, func() ([]*endpoint.Endpoint, error) { return sc.endpointsFromTemplate(hp) }, ) if err != nil { @@ -164,7 +154,7 @@ func (sc *httpProxySource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, e } func (sc *httpProxySource) endpointsFromTemplate(httpProxy *projectcontour.HTTPProxy) ([]*endpoint.Endpoint, error) { - hostnames, err := fqdn.ExecTemplate(sc.fqdnTemplate, httpProxy) + hostnames, err := sc.templateEngine.ExecFQDN(httpProxy) if err != nil { return nil, err } diff --git a/source/contour_httpproxy_test.go b/source/contour_httpproxy_test.go index 491d4c89a..85a5f70ea 100644 --- a/source/contour_httpproxy_test.go +++ b/source/contour_httpproxy_test.go @@ -36,6 +36,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) // This is a compile-time validation that httpProxySource is a Source. @@ -95,8 +96,8 @@ func (suite *HTTPProxySuite) SetupTest() { context.TODO(), fakeDynamicClient, &Config{ - Namespace: "default", - FQDNTemplate: "{{.Name}}", + Namespace: "default", + TemplateEngine: templatetest.MustEngine(suite.T(), "{{.Name}}", "", "", false), }, ) suite.NoError(err, "should initialize httpproxy source") @@ -140,71 +141,6 @@ func TestHTTPProxy(t *testing.T) { t.Run("Endpoints", testHTTPProxyEndpoints) } -func TestNewContourHTTPProxySource(t *testing.T) { - t.Parallel() - - for _, ti := range []struct { - title string - annotationFilter string - fqdnTemplate string - combineFQDNAndAnnotation bool - expectError bool - }{ - { - title: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", - }, - { - title: "valid empty template", - expectError: false, - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", - combineFQDNAndAnnotation: true, - }, - { - title: "non-empty annotation filter label", - expectError: false, - annotationFilter: "contour.heptio.com/ingress.class=contour", - }, - } { - - t.Run(ti.title, func(t *testing.T) { - t.Parallel() - - fakeDynamicClient, _ := newDynamicKubernetesClient() - - _, err := NewContourHTTPProxySource( - t.Context(), - fakeDynamicClient, - &Config{ - AnnotationFilter: ti.annotationFilter, - FQDNTemplate: ti.fqdnTemplate, - CombineFQDNAndAnnotation: ti.combineFQDNAndAnnotation, - }, - ) - if ti.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} - func testEndpointsFromHTTPProxy(t *testing.T) { t.Parallel() @@ -311,7 +247,7 @@ func testEndpointsFromHTTPProxy(t *testing.T) { t.Run(ti.title, func(t *testing.T) { t.Parallel() - source, err := newTestHTTPProxySource() + source, err := newTestHTTPProxySource(t) require.NoError(t, err) endpoints := source.endpointsFromHTTPProxy(ti.httpProxy.HTTPProxy()) @@ -1081,8 +1017,7 @@ func testHTTPProxyEndpoints(t *testing.T) { &Config{ Namespace: ti.targetNamespace, AnnotationFilter: ti.annotationFilter, - FQDNTemplate: ti.fqdnTemplate, - CombineFQDNAndAnnotation: ti.combineFQDNAndAnnotation, + TemplateEngine: templatetest.MustEngine(t, ti.fqdnTemplate, "", "", ti.combineFQDNAndAnnotation), IgnoreHostnameAnnotation: ti.ignoreHostnameAnnotation, }, ) @@ -1101,14 +1036,14 @@ func testHTTPProxyEndpoints(t *testing.T) { } // httpproxy specific helper functions -func newTestHTTPProxySource() (*httpProxySource, error) { +func newTestHTTPProxySource(t *testing.T) (*httpProxySource, error) { fakeDynamicClient, _ := newDynamicKubernetesClient() src, err := NewContourHTTPProxySource( - context.TODO(), + t.Context(), fakeDynamicClient, &Config{ - FQDNTemplate: "{{.Name}}", + TemplateEngine: templatetest.MustEngine(t, "{{.Name}}", "", "", false), }, ) if err != nil { diff --git a/source/fake.go b/source/fake.go index f331cc00b..5a51ba4b2 100644 --- a/source/fake.go +++ b/source/fake.go @@ -55,13 +55,12 @@ const ( ) // NewFakeSource creates a new fakeSource with the given config. -func NewFakeSource(fqdnTemplate string) (Source, error) { - if fqdnTemplate == "" { - fqdnTemplate = defaultFQDNTemplate - } - +// TODO: support cfg.TemplateEngine by rendering the FQDN template against a synthetic +// Kubernetes object (e.g. metav1.PartialObjectMetadata) so that --fqdn-template +// is honored when --source=fake is used for dry-runs. +func NewFakeSource(_ *Config) (Source, error) { return &fakeSource{ - dnsName: fqdnTemplate, + dnsName: defaultFQDNTemplate, }, nil } diff --git a/source/fake_test.go b/source/fake_test.go index 7ad35a6ed..f4e409b95 100644 --- a/source/fake_test.go +++ b/source/fake_test.go @@ -31,7 +31,7 @@ import ( var _ Source = &fakeSource{} func generateTestEndpoints() []*endpoint.Endpoint { - sc, _ := NewFakeSource("") + sc, _ := NewFakeSource(&Config{}) endpoints, _ := sc.Endpoints(context.Background()) @@ -75,7 +75,7 @@ func TestFakeEndpointsResolveToIPAddresses(t *testing.T) { } func TestFakeSource_GenerateEndpoint_RefObject(t *testing.T) { - sc, _ := NewFakeSource("example.com") + sc, _ := NewFakeSource(&Config{}) fs := sc.(*fakeSource) ep := fs.generateEndpoint() diff --git a/source/fqdn/fqdn.go b/source/fqdn/fqdn.go deleted file mode 100644 index 39e185dd6..000000000 --- a/source/fqdn/fqdn.go +++ /dev/null @@ -1,171 +0,0 @@ -/* -Copyright 2025 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package fqdn - -import ( - "bytes" - "encoding/json" - "fmt" - "maps" - "reflect" - "slices" - "strings" - "text/template" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/kubernetes/scheme" - - "sigs.k8s.io/external-dns/endpoint" -) - -func ParseTemplate(input string) (*template.Template, error) { - if input == "" { - return nil, nil - } - funcs := template.FuncMap{ - "contains": strings.Contains, - "trimPrefix": strings.TrimPrefix, - "trimSuffix": strings.TrimSuffix, - "trim": strings.TrimSpace, - "toLower": strings.ToLower, - "replace": replace, - "isIPv6": isIPv6String, - "isIPv4": isIPv4String, - "hasKey": hasKey, - "fromJson": fromJson, - } - return template.New("endpoint").Funcs(funcs).Parse(input) -} - -type kubeObject interface { - runtime.Object - metav1.Object -} - -// ExecTemplate executes a template against a Kubernetes object and returns hostnames. -// It infers Kind if TypeMeta is missing. Returns error if obj is nil or execution fails. -func ExecTemplate(tmpl *template.Template, obj kubeObject) ([]string, error) { - if obj == nil { - return nil, fmt.Errorf("object is nil") - } - // Kubernetes API doesn't populate TypeMeta (Kind/APIVersion) when retrieving - // objects via informers. because the client already knows what type it requested. This reduces payload size. - // Set it so templates can use .Kind and .APIVersion - // TODO: all sources to transform Informer().SetTransform() - gvk := obj.GetObjectKind().GroupVersionKind() - if gvk.Kind == "" { - gvks, _, err := scheme.Scheme.ObjectKinds(obj) - if err == nil && len(gvks) > 0 { - gvk = gvks[0] - } else { - // Fallback to reflection for types not in scheme - gvk = schema.GroupVersionKind{Kind: reflect.TypeOf(obj).Elem().Name()} - } - obj.GetObjectKind().SetGroupVersionKind(gvk) - } - - var buf bytes.Buffer - if err := tmpl.Execute(&buf, obj); err != nil { - kind := obj.GetObjectKind().GroupVersionKind().Kind - return nil, fmt.Errorf("failed to apply template on %s %s/%s: %w", kind, obj.GetNamespace(), obj.GetName(), err) - } - hosts := strings.Split(buf.String(), ",") - hostnames := make(map[string]struct{}, len(hosts)) - for _, name := range hosts { - name = strings.TrimSpace(name) - name = strings.TrimSuffix(name, ".") - if name != "" { - hostnames[name] = struct{}{} - } - } - return slices.Sorted(maps.Keys(hostnames)), nil -} - -// replace all instances of oldValue with newValue in target string. -// adheres to syntax from https://masterminds.github.io/sprig/strings.html. -func replace(oldValue, newValue, target string) string { - return strings.ReplaceAll(target, oldValue, newValue) -} - -// isIPv6String reports whether the target string is an IPv6 address, -// including IPv4-mapped IPv6 addresses. -func isIPv6String(target string) bool { - return endpoint.SuitableType(target) == endpoint.RecordTypeAAAA -} - -// isIPv4String reports whether the target string is an IPv4 address. -func isIPv4String(target string) bool { - return endpoint.SuitableType(target) == endpoint.RecordTypeA -} - -// hasKey checks if a key exists in a map. This is needed because Go templates' -// `index` function returns the zero value ("") for missing keys, which is -// indistinguishable from keys with empty values. Kubernetes uses empty-value -// labels for markers (e.g., `service.kubernetes.io/headless: ""`), so we need -// explicit key existence checking. -func hasKey(m map[string]string, key string) bool { - _, ok := m[key] - return ok -} - -// fromJson decodes a JSON string into a Go value (map, slice, etc.). -// This enables templates to work with structured data stored as JSON strings -// in complex labels or annotations or Configmap data fields, e.g. ranging over a list of entries: -// -// {{ range $entry := (index .Data "entries" | fromJson) }}{{ index $entry "dns" }},{{ end }} -// -// Returns nil if the input is not valid JSON. -func fromJson(v string) any { - var output any - _ = json.Unmarshal([]byte(v), &output) - return output -} - -// CombineWithTemplatedEndpoints merges annotation-based endpoints with template-based endpoints -// according to the FQDN template configuration. -// -// Logic: -// - If fqdnTemplate is nil, returns original endpoints unchanged -// - If combineFQDNAnnotation is true, appends templated endpoints to existing -// - If combineFQDNAnnotation is false and endpoints is empty, uses templated endpoints -// - If combineFQDNAnnotation is false and endpoints exist, returns original unchanged -func CombineWithTemplatedEndpoints( - endpoints []*endpoint.Endpoint, - fqdnTemplate *template.Template, - combineFQDNAnnotation bool, - templateFunc func() ([]*endpoint.Endpoint, error), -) ([]*endpoint.Endpoint, error) { - if fqdnTemplate == nil { - return endpoints, nil - } - - if !combineFQDNAnnotation && len(endpoints) > 0 { - return endpoints, nil - } - - templatedEndpoints, err := templateFunc() - if err != nil { - return nil, fmt.Errorf("failed to get endpoints from template: %w", err) - } - - if combineFQDNAnnotation { - return append(endpoints, templatedEndpoints...), nil - } - return templatedEndpoints, nil -} diff --git a/source/gateway.go b/source/gateway.go index f68a0fd97..8066a677b 100644 --- a/source/gateway.go +++ b/source/gateway.go @@ -21,7 +21,6 @@ import ( "fmt" "sort" "strings" - "text/template" log "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" @@ -39,8 +38,8 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" "sigs.k8s.io/external-dns/source/informers" + "sigs.k8s.io/external-dns/source/template" ) const ( @@ -146,8 +145,7 @@ type gatewayRouteSource struct { nsInformer coreinformers.NamespaceInformer - fqdnTemplate *template.Template - combineFQDNAnnotation bool + templateEngine template.Engine ignoreHostnameAnnotation bool } @@ -169,10 +167,6 @@ func newGatewayRouteSource( if err != nil { return nil, err } - tmpl, err := fqdn.ParseTemplate(config.FQDNTemplate) - if err != nil { - return nil, err - } client, err := clients.GatewayClient() if err != nil { @@ -229,8 +223,7 @@ func newGatewayRouteSource( nsInformer: nsInformer, - fqdnTemplate: tmpl, - combineFQDNAnnotation: config.CombineFQDNAndAnnotation, + templateEngine: config.TemplateEngine, ignoreHostnameAnnotation: config.IgnoreHostnameAnnotation, } return src, nil @@ -452,8 +445,8 @@ func (c *gatewayRouteResolver) hosts(rt gatewayRoute) ([]string, error) { hostnames = append(hostnames, string(name)) } // TODO: The combine-fqdn-annotation flag is similarly vague. - if c.src.fqdnTemplate != nil && (len(hostnames) == 0 || c.src.combineFQDNAnnotation) { - hosts, err := fqdn.ExecTemplate(c.src.fqdnTemplate, rt.Object()) + if c.src.templateEngine.IsConfigured() && (len(hostnames) == 0 || c.src.templateEngine.Combining()) { + hosts, err := c.src.templateEngine.ExecFQDN(rt.Object()) if err != nil { return nil, err } diff --git a/source/gateway_grpcroute_test.go b/source/gateway_grpcroute_test.go index 8656f0ea0..52fcee883 100644 --- a/source/gateway_grpcroute_test.go +++ b/source/gateway_grpcroute_test.go @@ -33,6 +33,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func TestGatewayGRPCRouteSourceEndpoints(t *testing.T) { @@ -95,8 +96,7 @@ func TestGatewayGRPCRouteSourceEndpoints(t *testing.T) { require.NoError(t, err, "failed to create GRPCRoute") src, err := NewGatewayGRPCRouteSource(ctx, clients, &Config{ - FQDNTemplate: "{{.Name}}-template.foobar.internal", - CombineFQDNAndAnnotation: true, + TemplateEngine: templatetest.MustEngine(t, "{{.Name}}-template.foobar.internal", "", "", true), }) require.NoError(t, err, "failed to create Gateway GRPCRoute Source") diff --git a/source/gateway_httproute_test.go b/source/gateway_httproute_test.go index b7b87682c..b0cbaaf97 100644 --- a/source/gateway_httproute_test.go +++ b/source/gateway_httproute_test.go @@ -36,6 +36,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" logtest "sigs.k8s.io/external-dns/internal/testutils/log" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func mustGetLabelSelector(s string) labels.Selector { @@ -951,7 +952,7 @@ func TestGatewayHTTPRouteSourceEndpoints(t *testing.T) { { title: "FQDNTemplate", config: Config{ - FQDNTemplate: "{{.Name}}.zero.internal, {{.Name}}.one.internal. , {{.Name}}.two.internal ", + TemplateEngine: templatetest.MustEngine(t, "{{.Name}}.zero.internal, {{.Name}}.one.internal. , {{.Name}}.two.internal ", "", "", false), }, namespaces: namespaces("default"), gateways: []*v1beta1.Gateway{{ @@ -997,8 +998,7 @@ func TestGatewayHTTPRouteSourceEndpoints(t *testing.T) { { title: "CombineFQDN", config: Config{ - FQDNTemplate: "combine-{{.Name}}.internal", - CombineFQDNAndAnnotation: true, + TemplateEngine: templatetest.MustEngine(t, "combine-{{.Name}}.internal", "", "", true), }, namespaces: namespaces("default"), gateways: []*v1beta1.Gateway{{ diff --git a/source/gateway_tcproute_test.go b/source/gateway_tcproute_test.go index b4dc20731..7a0e765f5 100644 --- a/source/gateway_tcproute_test.go +++ b/source/gateway_tcproute_test.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func TestGatewayTCPRouteSourceEndpoints(t *testing.T) { @@ -95,8 +96,7 @@ func TestGatewayTCPRouteSourceEndpoints(t *testing.T) { require.NoError(t, err, "failed to create TCPRoute") src, err := NewGatewayTCPRouteSource(ctx, clients, &Config{ - FQDNTemplate: "{{.Name}}-template.foobar.internal", - CombineFQDNAndAnnotation: true, + TemplateEngine: templatetest.MustEngine(t, "{{.Name}}-template.foobar.internal", "", "", true), }) require.NoError(t, err, "failed to create Gateway TCPRoute Source") diff --git a/source/gateway_tlsroute_test.go b/source/gateway_tlsroute_test.go index 3e7eca54c..a865cf0a1 100644 --- a/source/gateway_tlsroute_test.go +++ b/source/gateway_tlsroute_test.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func TestGatewayTLSRouteSourceEndpoints(t *testing.T) { @@ -96,8 +97,7 @@ func TestGatewayTLSRouteSourceEndpoints(t *testing.T) { require.NoError(t, err, "failed to create TLSRoute") src, err := NewGatewayTLSRouteSource(ctx, clients, &Config{ - FQDNTemplate: "{{.Name}}-template.foobar.internal", - CombineFQDNAndAnnotation: true, + TemplateEngine: templatetest.MustEngine(t, "{{.Name}}-template.foobar.internal", "", "", true), }) require.NoError(t, err, "failed to create Gateway TLSRoute Source") diff --git a/source/gateway_udproute_test.go b/source/gateway_udproute_test.go index a391268d8..de71bd1e9 100644 --- a/source/gateway_udproute_test.go +++ b/source/gateway_udproute_test.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func TestGatewayUDPRouteSourceEndpoints(t *testing.T) { @@ -95,8 +96,7 @@ func TestGatewayUDPRouteSourceEndpoints(t *testing.T) { require.NoError(t, err, "failed to create UDPRoute") src, err := NewGatewayUDPRouteSource(ctx, clients, &Config{ - FQDNTemplate: "{{.Name}}-template.foobar.internal", - CombineFQDNAndAnnotation: true, + TemplateEngine: templatetest.MustEngine(t, "{{.Name}}-template.foobar.internal", "", "", true), }) require.NoError(t, err, "failed to create Gateway UDPRoute Source") diff --git a/source/ingress.go b/source/ingress.go index 223e2ba08..99fdeaee0 100644 --- a/source/ingress.go +++ b/source/ingress.go @@ -21,7 +21,6 @@ import ( "errors" "fmt" "strings" - "text/template" log "github.com/sirupsen/logrus" networkv1 "k8s.io/api/networking/v1" @@ -37,7 +36,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" + "sigs.k8s.io/external-dns/source/template" ) const ( @@ -67,8 +66,7 @@ type ingressSource struct { namespace string annotationFilter string ingressClassNames []string - fqdnTemplate *template.Template - combineFQDNAnnotation bool + templateEngine template.Engine ignoreHostnameAnnotation bool ingressInformer netinformers.IngressInformer ignoreIngressTLSSpec bool @@ -81,11 +79,6 @@ func NewIngressSource( ctx context.Context, kubeClient kubernetes.Interface, cfg *Config) (Source, error) { - tmpl, err := fqdn.ParseTemplate(cfg.FQDNTemplate) - if err != nil { - return nil, err - } - // ensure that ingress class is only set in either the ingressClassNames or // annotationFilter but not both if cfg.IngressClassNames != nil && cfg.AnnotationFilter != "" { @@ -126,8 +119,7 @@ func NewIngressSource( namespace: cfg.Namespace, annotationFilter: cfg.AnnotationFilter, ingressClassNames: cfg.IngressClassNames, - fqdnTemplate: tmpl, - combineFQDNAnnotation: cfg.CombineFQDNAndAnnotation, + templateEngine: cfg.TemplateEngine, ignoreHostnameAnnotation: cfg.IgnoreHostnameAnnotation, ingressInformer: ingressInformer, ignoreIngressTLSSpec: cfg.IgnoreIngressTLSSpec, @@ -164,10 +156,8 @@ func (sc *ingressSource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, err ingEndpoints := endpointsFromIngress(ing, sc.ignoreHostnameAnnotation, sc.ignoreIngressTLSSpec, sc.ignoreIngressRulesSpec) // apply template if host is missing on ingress - ingEndpoints, err = fqdn.CombineWithTemplatedEndpoints( + ingEndpoints, err = sc.templateEngine.CombineWithEndpoints( ingEndpoints, - sc.fqdnTemplate, - sc.combineFQDNAnnotation, func() ([]*endpoint.Endpoint, error) { return sc.endpointsFromTemplate(ing) }, ) if err != nil { @@ -188,7 +178,7 @@ func (sc *ingressSource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, err } func (sc *ingressSource) endpointsFromTemplate(ing *networkv1.Ingress) ([]*endpoint.Endpoint, error) { - hostnames, err := fqdn.ExecTemplate(sc.fqdnTemplate, ing) + hostnames, err := sc.templateEngine.ExecFQDN(ing) if err != nil { return nil, err } diff --git a/source/ingress_fqdn_test.go b/source/ingress_fqdn_test.go index a0b3c9e60..6a07c235e 100644 --- a/source/ingress_fqdn_test.go +++ b/source/ingress_fqdn_test.go @@ -16,7 +16,6 @@ package source import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "sigs.k8s.io/external-dns/internal/testutils" @@ -27,59 +26,9 @@ import ( "k8s.io/client-go/kubernetes/fake" "sigs.k8s.io/external-dns/endpoint" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) -func TestIngressSourceNewNodeSourceWithFqdn(t *testing.T) { - for _, tt := range []struct { - title string - annotationFilter string - fqdnTemplate string - expectError bool - }{ - { - title: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", - }, - { - title: "valid empty template", - expectError: false, - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", - }, - { - title: "complex template", - expectError: false, - fqdnTemplate: "{{range .Status.Addresses}}{{if and (eq .Type \"ExternalIP\") (isIPv4 .Address)}}{{.Address | replace \".\" \"-\"}}{{break}}{{end}}{{end}}.ext-dns.test.com", - }, - { - title: "valid template with multiple hosts", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", - }, - } { - t.Run(tt.title, func(t *testing.T) { - _, err := NewIngressSource( - t.Context(), - fake.NewClientset(), - &Config{ - FQDNTemplate: tt.fqdnTemplate, - LabelFilter: labels.NewSelector(), - }, - ) - - if tt.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} - func TestIngressSourceFqdnTemplatingExamples(t *testing.T) { for _, tt := range []struct { @@ -322,9 +271,8 @@ func TestIngressSourceFqdnTemplatingExamples(t *testing.T) { t.Context(), kubeClient, &Config{ - FQDNTemplate: tt.fqdnTemplate, - CombineFQDNAndAnnotation: true, - LabelFilter: labels.Everything(), + TemplateEngine: templatetest.MustEngine(t, tt.fqdnTemplate, "", "", true), + LabelFilter: labels.Everything(), }, ) diff --git a/source/ingress_test.go b/source/ingress_test.go index 636373a6e..bbec43c9d 100644 --- a/source/ingress_test.go +++ b/source/ingress_test.go @@ -35,6 +35,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) // Validates that ingressSource is a Source @@ -63,8 +64,8 @@ func (suite *IngressSuite) SetupTest() { context.TODO(), fakeClient, &Config{ - FQDNTemplate: "{{.Name}}", - LabelFilter: labels.Everything(), + TemplateEngine: templatetest.MustEngine(suite.T(), "{{.Name}}", "", "", false), + LabelFilter: labels.Everything(), }, ) suite.NoError(err, "should initialize ingress source") @@ -122,10 +123,9 @@ func TestNewIngressSource(t *testing.T) { t.Context(), fake.NewClientset(), &Config{ - AnnotationFilter: ti.annotationFilter, - FQDNTemplate: ti.fqdnTemplate, - CombineFQDNAndAnnotation: ti.combineFQDNAndAnnotation, - IngressClassNames: ti.ingressClassNames, + AnnotationFilter: ti.annotationFilter, + TemplateEngine: templatetest.MustEngine(t, ti.fqdnTemplate, "", "", ti.combineFQDNAndAnnotation), + IngressClassNames: ti.ingressClassNames, }, ) if ti.expectError { @@ -1418,8 +1418,7 @@ func testIngressEndpoints(t *testing.T) { &Config{ Namespace: ti.targetNamespace, AnnotationFilter: ti.annotationFilter, - FQDNTemplate: ti.fqdnTemplate, - CombineFQDNAndAnnotation: ti.combineFQDNAndAnnotation, + TemplateEngine: templatetest.MustEngine(t, ti.fqdnTemplate, "", "", ti.combineFQDNAndAnnotation), IgnoreHostnameAnnotation: ti.ignoreHostnameAnnotation, IgnoreIngressTLSSpec: ti.ignoreIngressTLSSpec, IgnoreIngressRulesSpec: ti.ignoreIngressRulesSpec, diff --git a/source/istio_gateway.go b/source/istio_gateway.go index c111fd36a..d68888338 100644 --- a/source/istio_gateway.go +++ b/source/istio_gateway.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "strings" - "text/template" log "github.com/sirupsen/logrus" networkingv1 "istio.io/client-go/pkg/apis/networking/v1" @@ -39,8 +38,8 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" "sigs.k8s.io/external-dns/source/informers" + "sigs.k8s.io/external-dns/source/template" ) // IstioGatewayIngressSource is the annotation used to determine if the gateway is implemented by an Ingress object @@ -65,8 +64,7 @@ type gatewaySource struct { istioClient istioclient.Interface namespace string annotationFilter string - fqdnTemplate *template.Template - combineFQDNAnnotation bool + templateEngine template.Engine ignoreHostnameAnnotation bool serviceInformer coreinformers.ServiceInformer gatewayInformer networkingv1informer.GatewayInformer @@ -80,11 +78,6 @@ func NewIstioGatewaySource( istioClient istioclient.Interface, cfg *Config, ) (Source, error) { - tmpl, err := fqdn.ParseTemplate(cfg.FQDNTemplate) - if err != nil { - return nil, err - } - // Use shared informers to listen for add/update/delete of services/pods/nodes in the specified namespace. // Set resync period to 0, to prevent processing when nothing has changed informerFactory := kubeinformers.NewSharedInformerFactoryWithOptions(kubeClient, 0, kubeinformers.WithNamespace(cfg.Namespace)) @@ -128,8 +121,7 @@ func NewIstioGatewaySource( istioClient: istioClient, namespace: cfg.Namespace, annotationFilter: cfg.AnnotationFilter, - fqdnTemplate: tmpl, - combineFQDNAnnotation: cfg.CombineFQDNAndAnnotation, + templateEngine: cfg.TemplateEngine, ignoreHostnameAnnotation: cfg.IgnoreHostnameAnnotation, serviceInformer: serviceInformer, gatewayInformer: gatewayInformer, @@ -172,12 +164,10 @@ func (sc *gatewaySource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, e } // apply template if host is missing on gateway - gwEndpoints, err = fqdn.CombineWithTemplatedEndpoints( + gwEndpoints, err = sc.templateEngine.CombineWithEndpoints( gwEndpoints, - sc.fqdnTemplate, - sc.combineFQDNAnnotation, func() ([]*endpoint.Endpoint, error) { - hostnames, err := fqdn.ExecTemplate(sc.fqdnTemplate, gateway) + hostnames, err := sc.templateEngine.ExecFQDN(gateway) if err != nil { return nil, err } diff --git a/source/istio_gateway_fqdn_test.go b/source/istio_gateway_fqdn_test.go index cfc300970..4b4f31f30 100644 --- a/source/istio_gateway_fqdn_test.go +++ b/source/istio_gateway_fqdn_test.go @@ -18,7 +18,6 @@ import ( "sigs.k8s.io/external-dns/internal/testutils" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" istionetworking "istio.io/api/networking/v1beta1" networkingv1 "istio.io/client-go/pkg/apis/networking/v1" @@ -31,6 +30,7 @@ import ( "sigs.k8s.io/external-dns/source/annotations" "sigs.k8s.io/external-dns/endpoint" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func TestIstioGatewaySourceNewSourceWithFqdn(t *testing.T) { @@ -38,25 +38,16 @@ func TestIstioGatewaySourceNewSourceWithFqdn(t *testing.T) { title string annotationFilter string fqdnTemplate string - expectError bool }{ { - title: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", - }, - { - title: "valid empty template", - expectError: false, + title: "valid empty template", }, { title: "valid template", - expectError: false, fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", }, { title: "valid template with multiple hosts", - expectError: false, fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", }, } { @@ -68,17 +59,12 @@ func TestIstioGatewaySourceNewSourceWithFqdn(t *testing.T) { &Config{ Namespace: "", AnnotationFilter: tt.annotationFilter, - FQDNTemplate: tt.fqdnTemplate, - CombineFQDNAndAnnotation: false, + TemplateEngine: templatetest.MustEngine(t, tt.fqdnTemplate, "", "", false), IgnoreHostnameAnnotation: false, }, ) - if tt.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } + require.NoError(t, err) }) } } @@ -580,8 +566,7 @@ func TestIstioGatewaySourceFqdnTemplatingExamples(t *testing.T) { &Config{ Namespace: "", AnnotationFilter: "", - FQDNTemplate: tt.fqdnTemplate, - CombineFQDNAndAnnotation: !tt.combineFqdn, + TemplateEngine: templatetest.MustEngine(t, tt.fqdnTemplate, "", "", !tt.combineFqdn), IgnoreHostnameAnnotation: false, }, ) diff --git a/source/istio_gateway_test.go b/source/istio_gateway_test.go index 979cbb8ce..cb5cc74d2 100644 --- a/source/istio_gateway_test.go +++ b/source/istio_gateway_test.go @@ -39,6 +39,7 @@ import ( "sigs.k8s.io/external-dns/source/annotations" "sigs.k8s.io/external-dns/endpoint" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) // This is a compile-time validation that gatewaySource is a Source. @@ -101,7 +102,7 @@ func (suite *GatewaySuite) SetupTest() { fakeKubernetesClient, fakeIstioClient, &Config{ - FQDNTemplate: "{{.Name}}", + TemplateEngine: templatetest.MustEngine(suite.T(), "{{.Name}}", "", "", false), }, ) suite.NoError(err, "should initialize gateway source") @@ -123,70 +124,6 @@ func TestGateway(t *testing.T) { t.Run("Endpoints", testGatewayEndpoints) } -func TestNewIstioGatewaySource(t *testing.T) { - t.Parallel() - - for _, ti := range []struct { - title string - annotationFilter string - fqdnTemplate string - combineFQDNAndAnnotation bool - expectError bool - }{ - { - title: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", - }, - { - title: "valid empty template", - expectError: false, - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", - combineFQDNAndAnnotation: true, - }, - { - title: "non-empty annotation filter label", - expectError: false, - annotationFilter: "kubernetes.io/gateway.class=nginx", - }, - } { - - t.Run(ti.title, func(t *testing.T) { - t.Parallel() - - _, err := NewIstioGatewaySource( - t.Context(), - fake.NewClientset(), - istiofake.NewSimpleClientset(), - &Config{ - FQDNTemplate: ti.fqdnTemplate, - CombineFQDNAndAnnotation: ti.combineFQDNAndAnnotation, - AnnotationFilter: ti.annotationFilter, - }, - ) - if ti.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} - func testEndpointsFromGatewayConfig(t *testing.T) { t.Parallel() @@ -527,7 +464,7 @@ func testEndpointsFromGatewayConfig(t *testing.T) { t.Parallel() gatewayCfg := ti.config.Config() - source, err := newTestGatewaySource(ti.lbServices, ti.ingresses) + source, err := newTestGatewaySource(t, ti.lbServices, ti.ingresses) require.NoError(t, err) hostnames := source.hostNamesFromGateway(gatewayCfg) endpoints, err := source.endpointsFromGateway(hostnames, gatewayCfg) @@ -1535,8 +1472,7 @@ func testGatewayEndpoints(t *testing.T) { fakeIstioClient, &Config{ Namespace: targetNamespace, - FQDNTemplate: ti.fqdnTemplate, - CombineFQDNAndAnnotation: ti.combineFQDNAndAnnotation, + TemplateEngine: templatetest.MustEngine(t, ti.fqdnTemplate, "", "", ti.combineFQDNAndAnnotation), IgnoreHostnameAnnotation: ti.ignoreHostnameAnnotation, AnnotationFilter: ti.annotationFilter, }, @@ -1941,31 +1877,31 @@ func TestSingleGatewayMultipleServicesPointingToSameLoadBalancer(t *testing.T) { } // gateway specific helper functions -func newTestGatewaySource(loadBalancerList []fakeIngressGatewayService, ingressList []fakeIngress) (*gatewaySource, error) { +func newTestGatewaySource(t *testing.T, loadBalancerList []fakeIngressGatewayService, ingressList []fakeIngress) (*gatewaySource, error) { fakeKubernetesClient := fake.NewClientset() fakeIstioClient := istiofake.NewSimpleClientset() for _, lb := range loadBalancerList { service := lb.Service() - _, err := fakeKubernetesClient.CoreV1().Services(service.Namespace).Create(context.Background(), service, metav1.CreateOptions{}) + _, err := fakeKubernetesClient.CoreV1().Services(service.Namespace).Create(t.Context(), service, metav1.CreateOptions{}) if err != nil { return nil, err } } for _, ing := range ingressList { ingress := ing.Ingress() - _, err := fakeKubernetesClient.NetworkingV1().Ingresses(ingress.Namespace).Create(context.Background(), ingress, metav1.CreateOptions{}) + _, err := fakeKubernetesClient.NetworkingV1().Ingresses(ingress.Namespace).Create(t.Context(), ingress, metav1.CreateOptions{}) if err != nil { return nil, err } } src, err := NewIstioGatewaySource( - context.TODO(), + t.Context(), fakeKubernetesClient, fakeIstioClient, &Config{ - FQDNTemplate: "{{.FQDN}}", + TemplateEngine: templatetest.MustEngine(t, "{{.FQDN}}", "", "", false), }, ) if err != nil { diff --git a/source/istio_virtualservice.go b/source/istio_virtualservice.go index 0ab38d6d0..cf01f6903 100644 --- a/source/istio_virtualservice.go +++ b/source/istio_virtualservice.go @@ -22,7 +22,6 @@ import ( "fmt" "slices" "strings" - "text/template" log "github.com/sirupsen/logrus" networkingv1 "istio.io/client-go/pkg/apis/networking/v1" @@ -42,8 +41,8 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" "sigs.k8s.io/external-dns/source/informers" + "sigs.k8s.io/external-dns/source/template" ) // IstioMeshGateway is the built in gateway for all sidecars @@ -66,8 +65,7 @@ type virtualServiceSource struct { istioClient istioclient.Interface namespace string annotationFilter string - fqdnTemplate *template.Template - combineFQDNAnnotation bool + templateEngine template.Engine ignoreHostnameAnnotation bool serviceInformer coreinformers.ServiceInformer vServiceInformer networkingv1informer.VirtualServiceInformer @@ -82,11 +80,6 @@ func NewIstioVirtualServiceSource( istioClient istioclient.Interface, cfg *Config, ) (Source, error) { - tmpl, err := fqdn.ParseTemplate(cfg.FQDNTemplate) - if err != nil { - return nil, err - } - // Use shared informers to listen for add/update/delete of services/pods/nodes in the specified namespace. // Set resync period to 0, to prevent processing when nothing has changed informerFactory := kubeinformers.NewSharedInformerFactoryWithOptions(kubeClient, 0, kubeinformers.WithNamespace(cfg.Namespace)) @@ -136,8 +129,7 @@ func NewIstioVirtualServiceSource( istioClient: istioClient, namespace: cfg.Namespace, annotationFilter: cfg.AnnotationFilter, - fqdnTemplate: tmpl, - combineFQDNAnnotation: cfg.CombineFQDNAndAnnotation, + templateEngine: cfg.TemplateEngine, ignoreHostnameAnnotation: cfg.IgnoreHostnameAnnotation, serviceInformer: serviceInformer, vServiceInformer: virtualServiceInformer, @@ -175,10 +167,8 @@ func (sc *virtualServiceSource) Endpoints(ctx context.Context) ([]*endpoint.Endp } // apply template if host is missing on VirtualService - gwEndpoints, err = fqdn.CombineWithTemplatedEndpoints( + gwEndpoints, err = sc.templateEngine.CombineWithEndpoints( gwEndpoints, - sc.fqdnTemplate, - sc.combineFQDNAnnotation, func() ([]*endpoint.Endpoint, error) { return sc.endpointsFromTemplate(ctx, vService) }, ) if err != nil { @@ -232,7 +222,7 @@ func (sc *virtualServiceSource) getGateway(_ context.Context, gatewayStr string, } func (sc *virtualServiceSource) endpointsFromTemplate(ctx context.Context, virtualService *networkingv1.VirtualService) ([]*endpoint.Endpoint, error) { - hostnames, err := fqdn.ExecTemplate(sc.fqdnTemplate, virtualService) + hostnames, err := sc.templateEngine.ExecFQDN(virtualService) if err != nil { return nil, err } diff --git a/source/istio_virtualservice_fqdn_test.go b/source/istio_virtualservice_fqdn_test.go index d4c8a7dd2..7f48eebbb 100644 --- a/source/istio_virtualservice_fqdn_test.go +++ b/source/istio_virtualservice_fqdn_test.go @@ -18,7 +18,6 @@ import ( "sigs.k8s.io/external-dns/internal/testutils" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" istionetworking "istio.io/api/networking/v1beta1" networkingv1 "istio.io/client-go/pkg/apis/networking/v1" @@ -30,6 +29,7 @@ import ( "sigs.k8s.io/external-dns/source/annotations" "sigs.k8s.io/external-dns/endpoint" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func TestIstioVirtualServiceSourceNewSourceWithFqdn(t *testing.T) { @@ -37,25 +37,16 @@ func TestIstioVirtualServiceSourceNewSourceWithFqdn(t *testing.T) { title string annotationFilter string fqdnTemplate string - expectError bool }{ { - title: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", - }, - { - title: "valid empty template", - expectError: false, + title: "valid empty template", }, { title: "valid template", - expectError: false, fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", }, { title: "valid template with multiple hosts", - expectError: false, fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", }, } { @@ -67,17 +58,12 @@ func TestIstioVirtualServiceSourceNewSourceWithFqdn(t *testing.T) { &Config{ Namespace: "", AnnotationFilter: "", - FQDNTemplate: tt.fqdnTemplate, - CombineFQDNAndAnnotation: false, + TemplateEngine: templatetest.MustEngine(t, tt.fqdnTemplate, "", "", false), IgnoreHostnameAnnotation: false, }, ) - if tt.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } + require.NoError(t, err) }) } } @@ -747,8 +733,7 @@ func TestIstioVirtualServiceSourceFqdnTemplatingExamples(t *testing.T) { &Config{ Namespace: "", AnnotationFilter: "", - FQDNTemplate: tt.fqdnTemplate, - CombineFQDNAndAnnotation: !tt.combineFqdn, + TemplateEngine: templatetest.MustEngine(t, tt.fqdnTemplate, "", "", !tt.combineFqdn), IgnoreHostnameAnnotation: false, }, ) diff --git a/source/istio_virtualservice_test.go b/source/istio_virtualservice_test.go index 77ae2f9bb..afe6ed9c3 100644 --- a/source/istio_virtualservice_test.go +++ b/source/istio_virtualservice_test.go @@ -39,6 +39,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) // This is a compile-time validation that istioVirtualServiceSource is a Source. @@ -120,7 +121,7 @@ func (suite *VirtualServiceSuite) SetupTest() { fakeKubernetesClient, fakeIstioClient, &Config{ - FQDNTemplate: "{{.Name}}", + TemplateEngine: templatetest.MustEngine(suite.T(), "{{.Name}}", "", "", false), }, ) suite.NoError(err, "should initialize virtualservice source") @@ -145,70 +146,6 @@ func TestVirtualService(t *testing.T) { t.Run("gatewaySelectorMatchesService", testGatewaySelectorMatchesService) } -func TestNewIstioVirtualServiceSource(t *testing.T) { - t.Parallel() - - for _, ti := range []struct { - title string - annotationFilter string - fqdnTemplate string - combineFQDNAndAnnotation bool - expectError bool - }{ - { - title: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", - }, - { - title: "valid empty template", - expectError: false, - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", - combineFQDNAndAnnotation: true, - }, - { - title: "non-empty annotation filter label", - expectError: false, - annotationFilter: "kubernetes.io/gateway.class=nginx", - }, - } { - - t.Run(ti.title, func(t *testing.T) { - t.Parallel() - - _, err := NewIstioVirtualServiceSource( - t.Context(), - fake.NewClientset(), - istiofake.NewSimpleClientset(), - &Config{ - FQDNTemplate: ti.fqdnTemplate, - CombineFQDNAndAnnotation: ti.combineFQDNAndAnnotation, - AnnotationFilter: ti.annotationFilter, - }, - ) - if ti.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} - func testVirtualServiceBindsToGateway(t *testing.T) { for _, ti := range []struct { title string @@ -742,7 +679,7 @@ func testEndpointsFromVirtualServiceConfig(t *testing.T) { ti.vsconfig.namespace = "test" } - if source, err := newTestVirtualServiceSource(ti.lbServices, ti.ingresses, []fakeGatewayConfig{ti.gwconfig}); err != nil { + if source, err := newTestVirtualServiceSource(t, ti.lbServices, ti.ingresses, []fakeGatewayConfig{ti.gwconfig}); err != nil { require.NoError(t, err) } else if endpoints, err := source.endpointsFromVirtualService(t.Context(), ti.vsconfig.Config()); err != nil { require.NoError(t, err) @@ -2023,8 +1960,7 @@ func testVirtualServiceEndpoints(t *testing.T) { &Config{ Namespace: ti.targetNamespace, AnnotationFilter: ti.annotationFilter, - FQDNTemplate: ti.fqdnTemplate, - CombineFQDNAndAnnotation: ti.combineFQDNAndAnnotation, + TemplateEngine: templatetest.MustEngine(t, ti.fqdnTemplate, "", "", ti.combineFQDNAndAnnotation), IgnoreHostnameAnnotation: ti.ignoreHostnameAnnotation, }, ) @@ -2074,13 +2010,13 @@ func testGatewaySelectorMatchesService(t *testing.T) { } } -func newTestVirtualServiceSource(loadBalancerList []fakeIngressGatewayService, ingressList []fakeIngress, gwList []fakeGatewayConfig) (*virtualServiceSource, error) { +func newTestVirtualServiceSource(t *testing.T, loadBalancerList []fakeIngressGatewayService, ingressList []fakeIngress, gwList []fakeGatewayConfig) (*virtualServiceSource, error) { fakeKubernetesClient := fake.NewClientset() fakeIstioClient := istiofake.NewSimpleClientset() for _, lb := range loadBalancerList { service := lb.Service() - _, err := fakeKubernetesClient.CoreV1().Services(service.Namespace).Create(context.Background(), service, metav1.CreateOptions{}) + _, err := fakeKubernetesClient.CoreV1().Services(service.Namespace).Create(t.Context(), service, metav1.CreateOptions{}) if err != nil { return nil, err } @@ -2088,7 +2024,7 @@ func newTestVirtualServiceSource(loadBalancerList []fakeIngressGatewayService, i for _, ing := range ingressList { ingress := ing.Ingress() - _, err := fakeKubernetesClient.NetworkingV1().Ingresses(ingress.Namespace).Create(context.Background(), ingress, metav1.CreateOptions{}) + _, err := fakeKubernetesClient.NetworkingV1().Ingresses(ingress.Namespace).Create(t.Context(), ingress, metav1.CreateOptions{}) if err != nil { return nil, err } @@ -2098,18 +2034,18 @@ func newTestVirtualServiceSource(loadBalancerList []fakeIngressGatewayService, i gwObj := gw.Config() // use create instead of add // https://github.com/kubernetes/client-go/blob/92512ee2b8cf6696e9909245624175b7f0c971d9/testing/fixture.go#LL336C3-L336C52 - _, err := fakeIstioClient.NetworkingV1().Gateways(gw.namespace).Create(context.Background(), gwObj, metav1.CreateOptions{}) + _, err := fakeIstioClient.NetworkingV1().Gateways(gw.namespace).Create(t.Context(), gwObj, metav1.CreateOptions{}) if err != nil { return nil, err } } src, err := NewIstioVirtualServiceSource( - context.TODO(), + t.Context(), fakeKubernetesClient, fakeIstioClient, &Config{ - FQDNTemplate: "{{ .Name }}", + TemplateEngine: templatetest.MustEngine(t, "{{ .Name }}", "", "", false), }, ) if err != nil { @@ -2169,21 +2105,21 @@ func TestVirtualServiceSourceGetGateway(t *testing.T) { expectedErrStr string }{ {name: "EmptyGateway", fields: fields{ - virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(nil, nil, nil); return vs }(), + virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(t, nil, nil, nil); return vs }(), }, args: args{ ctx: t.Context(), gatewayStr: "", virtualService: nil, }, want: nil, expectedErrStr: ""}, {name: "MeshGateway", fields: fields{ - virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(nil, nil, nil); return vs }(), + virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(t, nil, nil, nil); return vs }(), }, args: args{ ctx: t.Context(), gatewayStr: IstioMeshGateway, virtualService: nil, }, want: nil, expectedErrStr: ""}, {name: "MissingGateway", fields: fields{ - virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(nil, nil, nil); return vs }(), + virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(t, nil, nil, nil); return vs }(), }, args: args{ ctx: t.Context(), gatewayStr: "doesnt/exist", @@ -2195,7 +2131,7 @@ func TestVirtualServiceSourceGetGateway(t *testing.T) { }, }, want: nil, expectedErrStr: ""}, {name: "InvalidGatewayStr", fields: fields{ - virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(nil, nil, nil); return vs }(), + virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(t, nil, nil, nil); return vs }(), }, args: args{ ctx: t.Context(), gatewayStr: "1/2/3/", @@ -2203,7 +2139,7 @@ func TestVirtualServiceSourceGetGateway(t *testing.T) { }, want: nil, expectedErrStr: "invalid ingress name (name or namespace/name) found \"1/2/3/\""}, {name: "ExistingGateway", fields: fields{ virtualServiceSource: func() *virtualServiceSource { - vs, _ := newTestVirtualServiceSource(nil, nil, []fakeGatewayConfig{{ + vs, _ := newTestVirtualServiceSource(t, nil, nil, []fakeGatewayConfig{{ namespace: "bar", name: "foo", }}) diff --git a/source/node.go b/source/node.go index 9a2317ae3..71aec2850 100644 --- a/source/node.go +++ b/source/node.go @@ -19,7 +19,6 @@ package source import ( "context" "fmt" - "text/template" log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" @@ -33,8 +32,8 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/pkg/events" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" "sigs.k8s.io/external-dns/source/informers" + "sigs.k8s.io/external-dns/source/template" ) // nodeSource is an implementation of Source for Kubernetes Node objects. @@ -49,10 +48,9 @@ import ( // +externaldns:source:provider-specific=false // +externaldns:source:events=true type nodeSource struct { - client kubernetes.Interface - annotationFilter string - fqdnTemplate *template.Template - combineFQDNAnnotation bool + client kubernetes.Interface + annotationFilter string + templateEngine template.Engine nodeInformer coreinformers.NodeInformer labelSelector labels.Selector @@ -65,11 +63,6 @@ func NewNodeSource( ctx context.Context, kubeClient kubernetes.Interface, cfg *Config) (Source, error) { - tmpl, err := fqdn.ParseTemplate(cfg.FQDNTemplate) - if err != nil { - return nil, err - } - // Use shared informers to listen for add/update/delete of nodes. // Set resync period to 0, to prevent processing when nothing has changed informerFactory := kubeinformers.NewSharedInformerFactoryWithOptions(kubeClient, 0) @@ -92,14 +85,13 @@ func NewNodeSource( } return &nodeSource{ - client: kubeClient, - annotationFilter: cfg.AnnotationFilter, - fqdnTemplate: tmpl, - combineFQDNAnnotation: cfg.CombineFQDNAndAnnotation, - nodeInformer: nodeInformer, - labelSelector: cfg.LabelFilter, - excludeUnschedulable: cfg.ExcludeUnschedulable, - exposeInternalIPv6: cfg.ExposeInternalIPv6, + client: kubeClient, + annotationFilter: cfg.AnnotationFilter, + templateEngine: cfg.TemplateEngine, + nodeInformer: nodeInformer, + labelSelector: cfg.LabelFilter, + excludeUnschedulable: cfg.ExcludeUnschedulable, + exposeInternalIPv6: cfg.ExposeInternalIPv6, }, nil } @@ -132,17 +124,15 @@ func (ns *nodeSource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, error) // Only generate node name endpoints when there's no template or when combining var nodeEndpoints []*endpoint.Endpoint - if ns.fqdnTemplate == nil || ns.combineFQDNAnnotation { + if !ns.templateEngine.IsConfigured() || ns.templateEngine.Combining() { nodeEndpoints, err = ns.endpointsForDNSNames(node, []string{node.Name}) if err != nil { return nil, err } } - nodeEndpoints, err = fqdn.CombineWithTemplatedEndpoints( + nodeEndpoints, err = ns.templateEngine.CombineWithEndpoints( nodeEndpoints, - ns.fqdnTemplate, - ns.combineFQDNAnnotation, func() ([]*endpoint.Endpoint, error) { return ns.endpointsFromNodeTemplate(node) }, ) if err != nil { @@ -168,7 +158,7 @@ func (ns *nodeSource) AddEventHandler(_ context.Context, handler func()) { // endpointsFromNodeTemplate creates endpoints using DNS names from the FQDN template. func (ns *nodeSource) endpointsFromNodeTemplate(node *v1.Node) ([]*endpoint.Endpoint, error) { - names, err := fqdn.ExecTemplate(ns.fqdnTemplate, node) + names, err := ns.templateEngine.ExecFQDN(node) if err != nil { return nil, err } diff --git a/source/node_fqdn_test.go b/source/node_fqdn_test.go index a2ef18de1..5caa81090 100644 --- a/source/node_fqdn_test.go +++ b/source/node_fqdn_test.go @@ -21,7 +21,6 @@ import ( "sigs.k8s.io/external-dns/internal/testutils" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -29,57 +28,9 @@ import ( "k8s.io/client-go/kubernetes/fake" "sigs.k8s.io/external-dns/endpoint" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) -func TestNodeSourceNewNodeSourceWithFqdn(t *testing.T) { - for _, tt := range []struct { - title string - annotationFilter string - fqdnTemplate string - expectError bool - }{ - { - title: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", - }, - { - title: "valid empty template", - expectError: false, - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", - }, - { - title: "complex template", - expectError: false, - fqdnTemplate: "{{range .Status.Addresses}}{{if and (eq .Type \"ExternalIP\") (isIPv4 .Address)}}{{.Address | replace \".\" \"-\"}}{{break}}{{end}}{{end}}.ext-dns.test.com", - }, - } { - t.Run(tt.title, func(t *testing.T) { - _, err := NewNodeSource( - t.Context(), - fake.NewClientset(), - &Config{ - AnnotationFilter: tt.annotationFilter, - FQDNTemplate: tt.fqdnTemplate, - CombineFQDNAndAnnotation: false, - ExcludeUnschedulable: true, - ExposeInternalIPv6: true, - LabelFilter: labels.Everything(), - }, - ) - if tt.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} - func TestNodeSourceFqdnTemplatingExamples(t *testing.T) { for _, tt := range []struct { title string @@ -370,11 +321,10 @@ func TestNodeSourceFqdnTemplatingExamples(t *testing.T) { t.Context(), kubeClient, &Config{ - FQDNTemplate: tt.fqdnTemplate, - ExcludeUnschedulable: true, - ExposeInternalIPv6: true, - CombineFQDNAndAnnotation: tt.combineFQDN, - LabelFilter: labels.Everything(), + TemplateEngine: templatetest.MustEngine(t, tt.fqdnTemplate, "", "", tt.combineFQDN), + ExcludeUnschedulable: true, + ExposeInternalIPv6: true, + LabelFilter: labels.Everything(), }, ) require.NoError(t, err) diff --git a/source/node_test.go b/source/node_test.go index 86ca6bcde..7c9b6a76d 100644 --- a/source/node_test.go +++ b/source/node_test.go @@ -43,76 +43,16 @@ import ( "k8s.io/client-go/kubernetes/fake" "sigs.k8s.io/external-dns/endpoint" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func TestNodeSource(t *testing.T) { t.Parallel() - t.Run("NewNodeSource", testNodeSourceNewNodeSource) t.Run("Endpoints", testNodeSourceEndpoints) t.Run("EndpointsIPv6", testNodeEndpointsWithIPv6) } -// testNodeSourceNewNodeSource tests that NewNodeService doesn't return an error. -func testNodeSourceNewNodeSource(t *testing.T) { - t.Parallel() - - for _, ti := range []struct { - title string - annotationFilter string - fqdnTemplate string - expectError bool - }{ - { - title: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", - }, - { - title: "valid empty template", - expectError: false, - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", - }, - { - title: "complex template", - expectError: false, - fqdnTemplate: "{{range .Status.Addresses}}{{if and (eq .Type \"ExternalIP\") (isIPv4 .Address)}}{{.Address | replace \".\" \"-\"}}{{break}}{{end}}{{end}}.ext-dns.test.com", - }, - { - title: "non-empty annotation filter label", - expectError: false, - annotationFilter: "kubernetes.io/ingress.class=nginx", - }, - } { - - t.Run(ti.title, func(t *testing.T) { - t.Parallel() - - _, err := NewNodeSource( - t.Context(), - fake.NewClientset(), - &Config{ - AnnotationFilter: ti.annotationFilter, - FQDNTemplate: ti.fqdnTemplate, - LabelFilter: labels.Everything(), - ExcludeUnschedulable: true, - ExposeInternalIPv6: true, - }, - ) - - if ti.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} - // testNodeSourceEndpoints tests that various node generate the correct endpoints. func testNodeSourceEndpoints(t *testing.T) { for _, tc := range []struct { @@ -456,7 +396,7 @@ func testNodeSourceEndpoints(t *testing.T) { kubeClient, &Config{ AnnotationFilter: tc.annotationFilter, - FQDNTemplate: tc.fqdnTemplate, + TemplateEngine: templatetest.MustEngine(t, tc.fqdnTemplate, "", "", false), LabelFilter: labelSelector, ExposeInternalIPv6: tc.exposeInternalIPv6, ExcludeUnschedulable: tc.excludeUnschedulable, @@ -570,7 +510,7 @@ func testNodeEndpointsWithIPv6(t *testing.T) { kubeClient, &Config{ AnnotationFilter: tc.annotationFilter, - FQDNTemplate: tc.fqdnTemplate, + TemplateEngine: templatetest.MustEngine(t, tc.fqdnTemplate, "", "", false), LabelFilter: labelSelector, ExposeInternalIPv6: tc.exposeInternalIPv6, ExcludeUnschedulable: tc.excludeUnschedulable, diff --git a/source/openshift_route.go b/source/openshift_route.go index 807888a21..69693cf52 100644 --- a/source/openshift_route.go +++ b/source/openshift_route.go @@ -19,7 +19,6 @@ package source import ( "context" "fmt" - "text/template" "time" routev1 "github.com/openshift/api/route/v1" @@ -34,8 +33,8 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" "sigs.k8s.io/external-dns/source/informers" + "sigs.k8s.io/external-dns/source/template" ) // ocpRouteSource is an implementation of Source for OpenShift Route objects. @@ -56,8 +55,7 @@ type ocpRouteSource struct { client versioned.Interface namespace string annotationFilter string - fqdnTemplate *template.Template - combineFQDNAnnotation bool + templateEngine template.Engine ignoreHostnameAnnotation bool routeInformer routeInformer.RouteInformer labelSelector labels.Selector @@ -70,11 +68,6 @@ func NewOcpRouteSource( ocpClient versioned.Interface, cfg *Config, ) (Source, error) { - tmpl, err := fqdn.ParseTemplate(cfg.FQDNTemplate) - if err != nil { - return nil, err - } - // Use a shared informer to listen for add/update/delete of Routes in the specified namespace. // Set resync period to 0, to prevent processing when nothing has changed. informerFactory := extInformers.NewSharedInformerFactoryWithOptions(ocpClient, 0*time.Second, extInformers.WithNamespace(cfg.Namespace)) @@ -94,8 +87,7 @@ func NewOcpRouteSource( client: ocpClient, namespace: cfg.Namespace, annotationFilter: cfg.AnnotationFilter, - fqdnTemplate: tmpl, - combineFQDNAnnotation: cfg.CombineFQDNAndAnnotation, + templateEngine: cfg.TemplateEngine, ignoreHostnameAnnotation: cfg.IgnoreHostnameAnnotation, routeInformer: informer, labelSelector: cfg.LabelFilter, @@ -135,10 +127,8 @@ func (ors *ocpRouteSource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, e orEndpoints := ors.endpointsFromOcpRoute(ocpRoute, ors.ignoreHostnameAnnotation) // apply template if host is missing on OpenShift Route - orEndpoints, err = fqdn.CombineWithTemplatedEndpoints( + orEndpoints, err = ors.templateEngine.CombineWithEndpoints( orEndpoints, - ors.fqdnTemplate, - ors.combineFQDNAnnotation, func() ([]*endpoint.Endpoint, error) { return ors.endpointsFromTemplate(ocpRoute) }, ) if err != nil { @@ -157,7 +147,7 @@ func (ors *ocpRouteSource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, e } func (ors *ocpRouteSource) endpointsFromTemplate(ocpRoute *routev1.Route) ([]*endpoint.Endpoint, error) { - hostnames, err := fqdn.ExecTemplate(ors.fqdnTemplate, ocpRoute) + hostnames, err := ors.templateEngine.ExecFQDN(ocpRoute) if err != nil { return nil, err } diff --git a/source/openshift_route_fqdn_test.go b/source/openshift_route_fqdn_test.go index 6d302d824..8167bb5d7 100644 --- a/source/openshift_route_fqdn_test.go +++ b/source/openshift_route_fqdn_test.go @@ -28,6 +28,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func TestOpenShiftFqdnTemplatingExamples(t *testing.T) { @@ -340,12 +341,11 @@ func TestOpenShiftFqdnTemplatingExamples(t *testing.T) { t.Context(), kubeClient, &Config{ - Namespace: "", - AnnotationFilter: "", - FQDNTemplate: tt.fqdnTemplate, - CombineFQDNAndAnnotation: !tt.combineFqdn, - LabelFilter: labels.Everything(), - OCPRouterName: "", + Namespace: "", + AnnotationFilter: "", + TemplateEngine: templatetest.MustEngine(t, tt.fqdnTemplate, "", "", !tt.combineFqdn), + LabelFilter: labels.Everything(), + OCPRouterName: "", }, ) require.NoError(t, err) diff --git a/source/openshift_route_test.go b/source/openshift_route_test.go index 26b79519a..bd156bfa1 100644 --- a/source/openshift_route_test.go +++ b/source/openshift_route_test.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) type OCPRouteSuite struct { @@ -50,8 +51,8 @@ func (suite *OCPRouteSuite) SetupTest() { context.TODO(), fakeClient, &Config{ - FQDNTemplate: "{{.Name}}", - LabelFilter: labels.Everything(), + TemplateEngine: templatetest.MustEngine(suite.T(), "{{.Name}}", "", "", false), + LabelFilter: labels.Everything(), }, ) @@ -91,7 +92,6 @@ func TestOcpRouteSource(t *testing.T) { suite.Run(t, new(OCPRouteSuite)) t.Run("Interface", testOcpRouteSourceImplementsSource) - t.Run("NewOcpRouteSource", testOcpRouteSourceNewOcpRouteSource) t.Run("Endpoints", testOcpRouteSourceEndpoints) } @@ -100,67 +100,6 @@ func testOcpRouteSourceImplementsSource(t *testing.T) { assert.Implements(t, (*Source)(nil), new(ocpRouteSource)) } -// testOcpRouteSourceNewOcpRouteSource tests that NewOcpRouteSource doesn't return an error. -func testOcpRouteSourceNewOcpRouteSource(t *testing.T) { - t.Parallel() - - for _, ti := range []struct { - title string - annotationFilter string - fqdnTemplate string - expectError bool - labelFilter string - }{ - { - title: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", - }, - { - title: "valid empty template", - expectError: false, - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", - }, - { - title: "non-empty annotation filter label", - expectError: false, - annotationFilter: "kubernetes.io/ingress.class=nginx", - }, - { - title: "valid label selector", - expectError: false, - labelFilter: "app=web-external", - }, - } { - - labelSelector, err := labels.Parse(ti.labelFilter) - require.NoError(t, err) - t.Run(ti.title, func(t *testing.T) { - t.Parallel() - - _, err := NewOcpRouteSource( - t.Context(), - fake.NewClientset(), - &Config{ - AnnotationFilter: ti.annotationFilter, - FQDNTemplate: ti.fqdnTemplate, - LabelFilter: labelSelector, - }, - ) - - if ti.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} - // testOcpRouteSourceEndpoints tests that various OCP routes generate the correct endpoints. func testOcpRouteSourceEndpoints(t *testing.T) { for _, tc := range []struct { @@ -564,9 +503,9 @@ func testOcpRouteSourceEndpoints(t *testing.T) { t.Context(), fakeClient, &Config{ - FQDNTemplate: "{{.Name}}", - LabelFilter: labelSelector, - OCPRouterName: tc.ocpRouterName, + TemplateEngine: templatetest.MustEngine(t, "{{.Name}}", "", "", false), + LabelFilter: labelSelector, + OCPRouterName: tc.ocpRouterName, }, ) require.NoError(t, err) diff --git a/source/pod.go b/source/pod.go index 095b2966e..5ebfde27c 100644 --- a/source/pod.go +++ b/source/pod.go @@ -23,7 +23,6 @@ package source import ( "context" "fmt" - "text/template" log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" @@ -34,8 +33,8 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/pkg/events" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" "sigs.k8s.io/external-dns/source/informers" + "sigs.k8s.io/external-dns/source/template" "sigs.k8s.io/external-dns/source/types" ) @@ -51,10 +50,9 @@ import ( // +externaldns:source:provider-specific=false // +externaldns:source:events=true type podSource struct { - client kubernetes.Interface - namespace string - fqdnTemplate *template.Template - combineFQDNAnnotation bool + client kubernetes.Interface + namespace string + templateEngine template.Engine podInformer coreinformers.PodInformer nodeInformer coreinformers.NodeInformer @@ -102,11 +100,6 @@ func NewPodSource( return nil, err } - tmpl, err := fqdn.ParseTemplate(cfg.FQDNTemplate) - if err != nil { - return nil, err - } - return &podSource{ client: kubeClient, podInformer: podInformer, @@ -115,8 +108,7 @@ func NewPodSource( compatibility: cfg.Compatibility, ignoreNonHostNetworkPods: cfg.IgnoreNonHostNetworkPods, podSourceDomain: cfg.PodSourceDomain, - fqdnTemplate: tmpl, - combineFQDNAnnotation: cfg.CombineFQDNAndAnnotation, + templateEngine: cfg.TemplateEngine, }, nil } @@ -136,10 +128,8 @@ func (ps *podSource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, error) podEndpoints := ps.endpointsFromPodAnnotations(pod) - podEndpoints, err = fqdn.CombineWithTemplatedEndpoints( + podEndpoints, err = ps.templateEngine.CombineWithEndpoints( podEndpoints, - ps.fqdnTemplate, - ps.combineFQDNAnnotation, func() ([]*endpoint.Endpoint, error) { return ps.endpointsFromPodTemplate(pod) }, ) if err != nil { @@ -260,9 +250,9 @@ func (ps *podSource) addPodNodeEndpointsToEndpointMap(endpointMap map[endpoint.E } func (ps *podSource) hostsFromTemplate(pod *v1.Pod) (map[endpoint.EndpointKey][]string, error) { - hosts, err := fqdn.ExecTemplate(ps.fqdnTemplate, pod) + hosts, err := ps.templateEngine.ExecFQDN(pod) if err != nil { - return nil, fmt.Errorf("skipping generating endpoints from template for pod %s: %w", pod.Name, err) + return nil, err } result := make(map[endpoint.EndpointKey][]string) diff --git a/source/pod_fqdn_test.go b/source/pod_fqdn_test.go index 117aa5420..85b84c33e 100644 --- a/source/pod_fqdn_test.go +++ b/source/pod_fqdn_test.go @@ -21,54 +21,15 @@ import ( "sigs.k8s.io/external-dns/internal/testutils" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" "sigs.k8s.io/external-dns/endpoint" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) -func TestNewPodSourceWithFqdn(t *testing.T) { - for _, tt := range []struct { - title string - annotationFilter string - fqdnTemplate string - expectError bool - }{ - { - title: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", - }, - { - title: "valid empty template", - expectError: false, - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", - }, - } { - t.Run(tt.title, func(t *testing.T) { - _, err := NewPodSource( - t.Context(), - fake.NewClientset(), - &Config{ - FQDNTemplate: tt.fqdnTemplate, - }) - - if tt.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} - func TestPodSourceFqdnTemplatingExamples(t *testing.T) { for _, tt := range []struct { title string @@ -443,9 +404,8 @@ func TestPodSourceFqdnTemplatingExamples(t *testing.T) { t.Context(), kubeClient, &Config{ - FQDNTemplate: tt.fqdnTemplate, - CombineFQDNAndAnnotation: tt.combineFQDN, - PodSourceDomain: tt.sourceDomain, + TemplateEngine: templatetest.MustEngine(t, tt.fqdnTemplate, "", "", tt.combineFQDN), + PodSourceDomain: tt.sourceDomain, }) require.NoError(t, err) @@ -504,9 +464,8 @@ func TestPodSourceFqdnTemplatingExamples_Failed(t *testing.T) { t.Context(), kubeClient, &Config{ - FQDNTemplate: tt.fqdnTemplate, - CombineFQDNAndAnnotation: tt.combineFQDN, - PodSourceDomain: tt.sourceDomain, + TemplateEngine: templatetest.MustEngine(t, tt.fqdnTemplate, "", "", tt.combineFQDN), + PodSourceDomain: tt.sourceDomain, }) require.NoError(t, err) diff --git a/source/pod_indexer_test.go b/source/pod_indexer_test.go index 4322802e1..d7af68737 100644 --- a/source/pod_indexer_test.go +++ b/source/pod_indexer_test.go @@ -28,6 +28,7 @@ import ( "k8s.io/client-go/kubernetes/fake" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) type podSpec struct { @@ -220,7 +221,7 @@ func TestPodsWithAnnotationsAndLabels(t *testing.T) { t.Context(), client, &Config{ Namespace: tt.namespace, - FQDNTemplate: "{{ .Name }}.tld.org", + TemplateEngine: templatetest.MustEngine(t, "{{ .Name }}.tld.org", "", "", false), AnnotationFilter: tt.annotationFilter, LabelFilter: selector, }) diff --git a/source/pod_test.go b/source/pod_test.go index 14883c2e2..f58bed900 100644 --- a/source/pod_test.go +++ b/source/pod_test.go @@ -38,6 +38,7 @@ import ( "sigs.k8s.io/external-dns/internal/testutils" logtest "sigs.k8s.io/external-dns/internal/testutils/log" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" "k8s.io/client-go/kubernetes/fake" ) @@ -991,7 +992,6 @@ func TestPodSourceLogs(t *testing.T) { } src, err := NewPodSource(ctx, kubernetes, &Config{ - FQDNTemplate: "", IgnoreNonHostNetworkPods: tc.ignoreNonHostNetworkPods, }) require.NoError(t, err) @@ -1229,7 +1229,7 @@ func TestPodTransformerInPodSource(t *testing.T) { // Should not error when creating the source src, err := NewPodSource(t.Context(), fakeClient, &Config{ - FQDNTemplate: "template", + TemplateEngine: templatetest.MustEngine(t, "template", "", "", false), }) require.NoError(t, err) ps, ok := src.(*podSource) diff --git a/source/service.go b/source/service.go index bc729b930..08caa22a3 100644 --- a/source/service.go +++ b/source/service.go @@ -24,7 +24,6 @@ import ( "slices" "sort" "strings" - "text/template" log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" @@ -40,8 +39,8 @@ import ( "sigs.k8s.io/external-dns/pkg/events" "sigs.k8s.io/external-dns/provider" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" "sigs.k8s.io/external-dns/source/informers" + "sigs.k8s.io/external-dns/source/template" "sigs.k8s.io/external-dns/source/types" ) @@ -69,12 +68,11 @@ var ( // +externaldns:source:provider-specific=true // +externaldns:source:events=true type serviceSource struct { - client kubernetes.Interface - namespace string - annotationFilter string - labelSelector labels.Selector - fqdnTemplate *template.Template - combineFQDNAnnotation bool + client kubernetes.Interface + namespace string + annotationFilter string + labelSelector labels.Selector + templateEngine template.Engine ignoreHostnameAnnotation bool publishInternal bool @@ -100,10 +98,6 @@ func NewServiceSource( kubeClient kubernetes.Interface, config *Config, ) (Source, error) { - tmpl, err := fqdn.ParseTemplate(config.FQDNTemplate) - if err != nil { - return nil, err - } namespace := config.Namespace // Use shared informers to listen for add/update/delete of services/pods/nodes in the specified namespace. @@ -174,8 +168,7 @@ func NewServiceSource( namespace: namespace, annotationFilter: config.AnnotationFilter, compatibility: config.Compatibility, - fqdnTemplate: tmpl, - combineFQDNAnnotation: config.CombineFQDNAndAnnotation, + templateEngine: config.TemplateEngine, ignoreHostnameAnnotation: config.IgnoreHostnameAnnotation, publishInternal: config.PublishInternal, publishHostIP: config.PublishHostIP, @@ -226,10 +219,8 @@ func (sc *serviceSource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, err } // apply template if none of the above is found - svcEndpoints, err = fqdn.CombineWithTemplatedEndpoints( + svcEndpoints, err = sc.templateEngine.CombineWithEndpoints( svcEndpoints, - sc.fqdnTemplate, - sc.combineFQDNAnnotation, func() ([]*endpoint.Endpoint, error) { return sc.endpointsFromTemplate(svc) }, ) if err != nil { @@ -488,7 +479,7 @@ func buildHeadlessEndpoints(svc *v1.Service, targetsByHeadlessDomainAndType map[ } func (sc *serviceSource) endpointsFromTemplate(svc *v1.Service) ([]*endpoint.Endpoint, error) { - hostnames, err := fqdn.ExecTemplate(sc.fqdnTemplate, svc) + hostnames, err := sc.templateEngine.ExecFQDN(svc) if err != nil { return nil, err } diff --git a/source/service_fqdn_test.go b/source/service_fqdn_test.go index 448553c55..66f85dbb0 100644 --- a/source/service_fqdn_test.go +++ b/source/service_fqdn_test.go @@ -28,6 +28,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/internal/testutils" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func TestServiceSourceFqdnTemplatingExamples(t *testing.T) { @@ -875,8 +876,7 @@ func TestServiceSourceFqdnTemplatingExamples(t *testing.T) { } cfg := &Config{ - FQDNTemplate: tt.fqdnTemplate, - CombineFQDNAndAnnotation: tt.combineFQDN, + TemplateEngine: templatetest.MustEngine(t, tt.fqdnTemplate, "", "", tt.combineFQDN), PublishHostIP: tt.publishHostIp, ServiceTypeFilter: tt.serviceTypesFilter, PublishInternal: true, diff --git a/source/service_test.go b/source/service_test.go index e62a3ba51..ee508a80f 100644 --- a/source/service_test.go +++ b/source/service_test.go @@ -47,6 +47,7 @@ import ( "sigs.k8s.io/external-dns/source/annotations" "sigs.k8s.io/external-dns/source/informers" "sigs.k8s.io/external-dns/source/informers/fakes" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) type ServiceSuite struct { @@ -83,8 +84,8 @@ func (suite *ServiceSuite) SetupTest() { context.TODO(), fakeClient, &Config{ - FQDNTemplate: "{{.Name}}", - LabelFilter: labels.Everything(), + TemplateEngine: templatetest.MustEngine(suite.T(), "{{.Name}}", "", "", false), + LabelFilter: labels.Everything(), }, ) suite.NoError(err, "should initialize service source") @@ -102,7 +103,6 @@ func TestServiceSource(t *testing.T) { suite.Run(t, new(ServiceSuite)) t.Run("Interface", testServiceSourceImplementsSource) - t.Run("NewServiceSource", testServiceSourceNewServiceSource) t.Run("Endpoints", testServiceSourceEndpoints) t.Run("MultipleServices", testMultipleServicesEndpoints) } @@ -112,66 +112,6 @@ func testServiceSourceImplementsSource(t *testing.T) { assert.Implements(t, (*Source)(nil), new(serviceSource)) } -// testServiceSourceNewServiceSource tests that NewServiceSource doesn't return an error. -func testServiceSourceNewServiceSource(t *testing.T) { - t.Parallel() - - for _, tc := range []struct { - title string - annotationFilter string - fqdnTemplate string - serviceTypesFilter []string - expectError bool - }{ - { - title: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", - }, - { - title: "valid empty template", - expectError: false, - }, - { - title: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", - }, - { - title: "non-empty annotation filter label", - expectError: false, - annotationFilter: "kubernetes.io/ingress.class=nginx", - }, - { - title: "non-empty service types filter", - expectError: false, - serviceTypesFilter: []string{string(v1.ServiceTypeClusterIP)}, - }, - } { - - t.Run(tc.title, func(t *testing.T) { - t.Parallel() - - _, err := NewServiceSource( - t.Context(), - fake.NewClientset(), - &Config{ - FQDNTemplate: tc.fqdnTemplate, - AnnotationFilter: tc.annotationFilter, - ServiceTypeFilter: tc.serviceTypesFilter, - LabelFilter: labels.Everything(), - }, - ) - - if tc.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} - // testServiceSourceEndpoints tests that various services generate the correct endpoints. func testServiceSourceEndpoints(t *testing.T) { exampleDotComIP4, err := net.DefaultResolver.LookupNetIP(t.Context(), "ip4", "example.com") @@ -1152,10 +1092,9 @@ func testServiceSourceEndpoints(t *testing.T) { // Create our object under test and get the endpoints. client, err := NewServiceSource(t.Context(), kubernetes, &Config{ - FQDNTemplate: tc.fqdnTemplate, + TemplateEngine: templatetest.MustEngine(t, tc.fqdnTemplate, "", "", tc.combineFQDNAndAnnotation), AnnotationFilter: tc.annotationFilter, ServiceTypeFilter: tc.serviceTypesFilter, - CombineFQDNAndAnnotation: tc.combineFQDNAndAnnotation, Compatibility: tc.compatibility, Namespace: tc.targetNamespace, ResolveLoadBalancerHostname: tc.resolveLoadBalancerHostname, @@ -1363,10 +1302,9 @@ func testMultipleServicesEndpoints(t *testing.T) { // Create our object under test and get the endpoints. client, err := NewServiceSource(t.Context(), kubernetes, &Config{ - FQDNTemplate: tc.fqdnTemplate, + TemplateEngine: templatetest.MustEngine(t, tc.fqdnTemplate, "", "", tc.combineFQDNAndAnnotation), AnnotationFilter: tc.annotationFilter, ServiceTypeFilter: tc.serviceTypesFilter, - CombineFQDNAndAnnotation: tc.combineFQDNAndAnnotation, Compatibility: tc.compatibility, Namespace: tc.targetNamespace, IgnoreHostnameAnnotation: tc.ignoreHostnameAnnotation, @@ -1660,7 +1598,7 @@ func TestClusterIpServices(t *testing.T) { // Create our object under test and get the endpoints. client, _ := NewServiceSource(t.Context(), kubernetes, &Config{ - FQDNTemplate: tc.fqdnTemplate, + TemplateEngine: templatetest.MustEngine(t, tc.fqdnTemplate, "", "", false), AnnotationFilter: tc.annotationFilter, Compatibility: tc.compatibility, Namespace: tc.targetNamespace, @@ -2480,7 +2418,7 @@ func TestServiceSourceNodePortServices(t *testing.T) { // Create our object under test and get the endpoints. client, _ := NewServiceSource(t.Context(), kubernetes, &Config{ - FQDNTemplate: tc.fqdnTemplate, + TemplateEngine: templatetest.MustEngine(t, tc.fqdnTemplate, "", "", false), AnnotationFilter: tc.annotationFilter, Compatibility: tc.compatibility, Namespace: tc.targetNamespace, @@ -3382,7 +3320,7 @@ func TestHeadlessServices(t *testing.T) { // Create our object under test and get the endpoints. client, _ := NewServiceSource(t.Context(), kubernetes, &Config{ - FQDNTemplate: tc.fqdnTemplate, + TemplateEngine: templatetest.MustEngine(t, tc.fqdnTemplate, "", "", false), ServiceTypeFilter: tc.serviceTypesFilter, Compatibility: tc.compatibility, Namespace: tc.targetNamespace, @@ -4316,7 +4254,7 @@ func TestHeadlessServicesHostIP(t *testing.T) { Namespace: tc.targetNamespace, LabelFilter: labels.Everything(), Compatibility: tc.compatibility, - FQDNTemplate: tc.fqdnTemplate, + TemplateEngine: templatetest.MustEngine(t, tc.fqdnTemplate, "", "", false), IgnoreHostnameAnnotation: tc.ignoreHostnameAnnotation, ExcludeUnschedulable: true, PublishHostIP: true, @@ -4517,7 +4455,7 @@ func TestExternalServices(t *testing.T) { // Create our object under test and get the endpoints. client, _ := NewServiceSource(t.Context(), kubernetes, &Config{ - FQDNTemplate: tc.fqdnTemplate, + TemplateEngine: templatetest.MustEngine(t, tc.fqdnTemplate, "", "", false), Compatibility: tc.compatibility, ServiceTypeFilter: tc.serviceTypeFilter, Namespace: tc.targetNamespace, @@ -4855,7 +4793,7 @@ func TestEndpointSlicesIndexer(t *testing.T) { // Should not error when creating the source src, err := NewServiceSource(ctx, fakeClient, &Config{ - FQDNTemplate: "{{.Name}}", + TemplateEngine: templatetest.MustEngine(t, "{{.Name}}", "", "", false), Namespace: "default", ExcludeUnschedulable: true, LabelFilter: labels.Everything(), @@ -4951,8 +4889,8 @@ func TestPodTransformerInServiceSource(t *testing.T) { // Should not error when creating the source src, err := NewServiceSource(ctx, fakeClient, &Config{ - FQDNTemplate: "{{.Name}}", - LabelFilter: labels.Everything(), + TemplateEngine: templatetest.MustEngine(t, "{{.Name}}", "", "", false), + LabelFilter: labels.Everything(), }, ) require.NoError(t, err) diff --git a/source/skipper_routegroup.go b/source/skipper_routegroup.go index 83ce8e3c0..4290ba546 100644 --- a/source/skipper_routegroup.go +++ b/source/skipper_routegroup.go @@ -17,7 +17,6 @@ limitations under the License. package source import ( - "bytes" "context" "crypto/tls" "crypto/x509" @@ -29,17 +28,17 @@ import ( "os" "strings" "sync" - "text/template" "time" log "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/external-dns/source/types" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" + "sigs.k8s.io/external-dns/source/template" ) const ( @@ -64,8 +63,7 @@ type routeGroupSource struct { namespace string apiEndpoint string annotationFilter string - fqdnTemplate *template.Template - combineFQDNAnnotation bool + templateEngine template.Engine ignoreHostnameAnnotation bool } @@ -204,11 +202,6 @@ func (cli *routeGroupClient) do(req *http.Request) (*http.Response, error) { // NewRouteGroupSource creates a new routeGroupSource with the given config. func NewRouteGroupSource(cfg *Config, token, tokenPath, apiServerURL string) (Source, error) { - tmpl, err := fqdn.ParseTemplate(cfg.FQDNTemplate) - if err != nil { - return nil, err - } - routeGroupVersion := cfg.SkipperRouteGroupVersion if routeGroupVersion == "" { routeGroupVersion = DefaultRoutegroupVersion @@ -238,8 +231,7 @@ func NewRouteGroupSource(cfg *Config, token, tokenPath, apiServerURL string) (So namespace: cfg.Namespace, apiEndpoint: apiEndpoint, annotationFilter: cfg.AnnotationFilter, - fqdnTemplate: tmpl, - combineFQDNAnnotation: cfg.CombineFQDNAndAnnotation, + templateEngine: cfg.TemplateEngine, ignoreHostnameAnnotation: cfg.IgnoreHostnameAnnotation, }, nil } @@ -270,10 +262,8 @@ func (sc *routeGroupSource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, eps := sc.endpointsFromRouteGroup(rg) - eps, err = fqdn.CombineWithTemplatedEndpoints( + eps, err = sc.templateEngine.CombineWithEndpoints( eps, - sc.fqdnTemplate, - sc.combineFQDNAnnotation, func() ([]*endpoint.Endpoint, error) { return sc.endpointsFromTemplate(rg) }, ) if err != nil { @@ -284,7 +274,7 @@ func (sc *routeGroupSource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, continue } - log.Debugf("Endpoints generated from ingress: %s/%s: %v", rg.Metadata.Namespace, rg.Metadata.Name, eps) + log.Debugf("Endpoints generated from ingress: %s/%s: %v", rg.Namespace, rg.Name, eps) endpoints = append(endpoints, eps...) } @@ -292,47 +282,38 @@ func (sc *routeGroupSource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, } func (sc *routeGroupSource) endpointsFromTemplate(rg *routeGroup) ([]*endpoint.Endpoint, error) { - // Process the whole template string - var buf bytes.Buffer - err := sc.fqdnTemplate.Execute(&buf, rg) + hostnames, err := sc.templateEngine.ExecFQDN(rg) if err != nil { - return nil, fmt.Errorf("failed to apply template on routegroup %s/%s: %w", rg.Metadata.Namespace, rg.Metadata.Name, err) + return nil, err } - hostnames := buf.String() - - resource := fmt.Sprintf("routegroup/%s/%s", rg.Metadata.Namespace, rg.Metadata.Name) + resource := fmt.Sprintf("routegroup/%s/%s", rg.Namespace, rg.Name) // error handled in endpointsFromRouteGroup(), otherwise duplicate log - ttl := annotations.TTLFromAnnotations(rg.Metadata.Annotations, resource) + ttl := annotations.TTLFromAnnotations(rg.Annotations, resource) - targets := annotations.TargetsFromTargetAnnotation(rg.Metadata.Annotations) + targets := annotations.TargetsFromTargetAnnotation(rg.Annotations) if len(targets) == 0 { targets = targetsFromRouteGroupStatus(rg.Status) } - providerSpecific, setIdentifier := annotations.ProviderSpecificAnnotations(rg.Metadata.Annotations) + providerSpecific, setIdentifier := annotations.ProviderSpecificAnnotations(rg.Annotations) var endpoints []*endpoint.Endpoint - // splits the FQDN template and removes the trailing periods - hostnameList := strings.SplitSeq(strings.ReplaceAll(hostnames, " ", ""), ",") - for hostname := range hostnameList { - hostname = strings.TrimSuffix(hostname, ".") + for _, hostname := range hostnames { endpoints = append(endpoints, endpoint.EndpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...) } return endpoints, nil } - -// annotation logic ported from source/ingress.go without Spec.TLS part, because it's not supported in RouteGroup func (sc *routeGroupSource) endpointsFromRouteGroup(rg *routeGroup) []*endpoint.Endpoint { endpoints := []*endpoint.Endpoint{} - resource := fmt.Sprintf("routegroup/%s/%s", rg.Metadata.Namespace, rg.Metadata.Name) + resource := fmt.Sprintf("routegroup/%s/%s", rg.Namespace, rg.Name) - ttl := annotations.TTLFromAnnotations(rg.Metadata.Annotations, resource) + ttl := annotations.TTLFromAnnotations(rg.Annotations, resource) - targets := annotations.TargetsFromTargetAnnotation(rg.Metadata.Annotations) + targets := annotations.TargetsFromTargetAnnotation(rg.Annotations) if len(targets) == 0 { for _, lb := range rg.Status.LoadBalancer.RouteGroup { if lb.IP != "" { @@ -344,7 +325,7 @@ func (sc *routeGroupSource) endpointsFromRouteGroup(rg *routeGroup) []*endpoint. } } - providerSpecific, setIdentifier := annotations.ProviderSpecificAnnotations(rg.Metadata.Annotations) + providerSpecific, setIdentifier := annotations.ProviderSpecificAnnotations(rg.Annotations) for _, src := range rg.Spec.Hosts { if src == "" { @@ -355,7 +336,7 @@ func (sc *routeGroupSource) endpointsFromRouteGroup(rg *routeGroup) []*endpoint. // Skip endpoints if we do not want entries from annotations if !sc.ignoreHostnameAnnotation { - hostnameList := annotations.HostnamesFromAnnotations(rg.Metadata.Annotations) + hostnameList := annotations.HostnamesFromAnnotations(rg.Annotations) for _, hostname := range hostnameList { endpoints = append(endpoints, endpoint.EndpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...) } @@ -391,13 +372,22 @@ type routeGroupListMetadata struct { } type routeGroup struct { - Metadata metav1.ObjectMeta `json:"metadata"` - Spec routeGroupSpec `json:"spec"` - Status routeGroupStatus `json:"status"` + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata"` + Spec routeGroupSpec `json:"spec"` + Status routeGroupStatus `json:"status"` } -func (rg *routeGroup) GetObjectMeta() metav1.Object { - return rg.Metadata.GetObjectMeta() +// Metadata returns the ObjectMeta for backward-compatible template access. +// +// Deprecated: use top-level fields directly (e.g. {{.Name}} instead of {{.Metadata.Name}}). +func (rg *routeGroup) Metadata() *metav1.ObjectMeta { + return &rg.ObjectMeta +} + +func (rg *routeGroup) DeepCopyObject() runtime.Object { + out := *rg + return &out } type routeGroupSpec struct { @@ -416,7 +406,3 @@ type routeGroupLoadBalancer struct { IP string `json:"ip,omitempty"` Hostname string `json:"hostname,omitempty"` } - -func (rg *routeGroup) GetAnnotations() map[string]string { - return rg.Metadata.Annotations -} diff --git a/source/skipper_routegroup_test.go b/source/skipper_routegroup_test.go index c5d5ed429..7b8251266 100644 --- a/source/skipper_routegroup_test.go +++ b/source/skipper_routegroup_test.go @@ -26,12 +26,12 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func createTestRouteGroup(ns, name string, annotations map[string]string, hosts []string, destinations []routeGroupLoadBalancer) *routeGroup { return &routeGroup{ - Metadata: metav1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Namespace: ns, Name: name, Annotations: annotations, @@ -328,11 +328,12 @@ func (f *fakeRouteGroupClient) getRouteGroupList(string) (*routeGroupList, error func TestRouteGroupsEndpoints(t *testing.T) { for _, tt := range []struct { - name string - source *routeGroupSource - fqdnTemplate string - want []*endpoint.Endpoint - wantErr bool + name string + source *routeGroupSource + templates string + combineFQDN bool + want []*endpoint.Endpoint + wantErr bool }{ { name: "Empty routegroup should return empty endpoints", @@ -374,10 +375,10 @@ func TestRouteGroupsEndpoints(t *testing.T) { }, }, { - name: "Single routegroup with combineFQDNAnnotation with fqdn template should return endpoints from fqdnTemplate and routegroup", - fqdnTemplate: "{{.Metadata.Name}}.{{.Metadata.Namespace}}.example", + name: "Single routegroup with combineFQDNAnnotation with fqdn template should return endpoints from fqdnTemplate and routegroup", + templates: "{{.Metadata.Name}}.{{.Metadata.Namespace}}.example", + combineFQDN: true, source: &routeGroupSource{ - combineFQDNAnnotation: true, cli: &fakeRouteGroupClient{ rg: &routeGroupList{ Items: []*routeGroup{ @@ -410,8 +411,8 @@ func TestRouteGroupsEndpoints(t *testing.T) { }, }, { - name: "Single routegroup without, with fqdn template should return endpoints from fqdnTemplate", - fqdnTemplate: "{{.Metadata.Name}}.{{.Metadata.Namespace}}.example", + name: "Single routegroup without, with fqdn template should return endpoints from fqdnTemplate", + templates: "{{.Metadata.Name}}.{{.Metadata.Namespace}}.example", source: &routeGroupSource{ cli: &fakeRouteGroupClient{ rg: &routeGroupList{ @@ -440,8 +441,8 @@ func TestRouteGroupsEndpoints(t *testing.T) { }, }, { - name: "Single routegroup without combineFQDNAnnotation with fqdn template should return endpoints not from fqdnTemplate", - fqdnTemplate: "{{.Metadata.Name}}.{{.Metadata.Namespace}}.example", + name: "Single routegroup without combineFQDNAnnotation with fqdn template should return endpoints not from fqdnTemplate", + templates: "{{.Metadata.Name}}.{{.Metadata.Namespace}}.example", source: &routeGroupSource{ cli: &fakeRouteGroupClient{ rg: &routeGroupList{ @@ -818,12 +819,12 @@ func TestRouteGroupsEndpoints(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - if tt.fqdnTemplate != "" { - tmpl, err := fqdn.ParseTemplate(tt.fqdnTemplate) - if err != nil { - t.Fatalf("Failed to parse template: %v", err) + if tt.templates != "" { + if tt.combineFQDN { + tt.source.templateEngine = templatetest.MustEngine(t, tt.templates, "", "", true) + } else { + tt.source.templateEngine = templatetest.MustEngine(t, tt.templates, "", "", false) } - tt.source.fqdnTemplate = tmpl } got, err := tt.source.Endpoints(t.Context()) diff --git a/source/store.go b/source/store.go index 185fcb2c8..8afd3e17d 100644 --- a/source/store.go +++ b/source/store.go @@ -36,6 +36,7 @@ import ( "sigs.k8s.io/external-dns/pkg/apis/externaldns" kubeclient "sigs.k8s.io/external-dns/pkg/client" + "sigs.k8s.io/external-dns/source/template" "sigs.k8s.io/external-dns/source/types" ) @@ -62,10 +63,7 @@ type Config struct { AnnotationFilter string LabelFilter labels.Selector IngressClassNames []string - FQDNTemplate string - TargetTemplate string - FQDNTargetTemplate string - CombineFQDNAndAnnotation bool + TemplateEngine template.Engine IgnoreHostnameAnnotation bool IgnoreNonHostNetworkPods bool IgnoreIngressTLSSpec bool @@ -124,15 +122,18 @@ func WithClientGenerator(gen ClientGenerator) OverrideConfigOption { } } -func NewSourceConfig(cfg *externaldns.Config, opts ...OverrideConfigOption) *Config { +func NewSourceConfig(cfg *externaldns.Config, opts ...OverrideConfigOption) (*Config, error) { // error is explicitly ignored because the filter is already validated in validation.ValidateConfig labelSelector, _ := labels.Parse(cfg.LabelFilter) + tmpls, err := template.NewEngine(cfg.FQDNTemplate, cfg.TargetTemplate, cfg.FQDNTargetTemplate, cfg.CombineFQDNAndAnnotation) + if err != nil { + return nil, err + } c := &Config{ Namespace: cfg.Namespace, AnnotationFilter: cfg.AnnotationFilter, LabelFilter: labelSelector, IngressClassNames: cfg.IngressClassNames, - CombineFQDNAndAnnotation: cfg.CombineFQDNAndAnnotation, IgnoreHostnameAnnotation: cfg.IgnoreHostnameAnnotation, IgnoreNonHostNetworkPods: cfg.IgnoreNonHostNetworkPods, IgnoreIngressTLSSpec: cfg.IgnoreIngressTLSSpec, @@ -170,16 +171,14 @@ func NewSourceConfig(cfg *externaldns.Config, opts ...OverrideConfigOption) *Con NAT64Networks: cfg.NAT64Networks, MinTTL: cfg.MinTTL, UnstructuredResources: cfg.UnstructuredResources, - FQDNTemplate: cfg.FQDNTemplate, - TargetTemplate: cfg.TargetTemplate, - FQDNTargetTemplate: cfg.FQDNTargetTemplate, + TemplateEngine: tmpls, PreferAlias: cfg.PreferAlias, sources: cfg.Sources, } for _, opt := range opts { opt(c) } - return c + return c, nil } // ClientGenerator returns the ClientGenerator for this Config. @@ -446,7 +445,7 @@ func BuildWithConfig(ctx context.Context, source string, p ClientGenerator, cfg case types.OpenShiftRoute: return buildOpenShiftRouteSource(ctx, p, cfg) case types.Fake: - return NewFakeSource(cfg.FQDNTemplate) + return NewFakeSource(cfg) case types.Connector: return NewConnectorSource(cfg.ConnectorServer) case types.CRD: diff --git a/source/store_test.go b/source/store_test.go index 3910b60d2..8a9fc47ec 100644 --- a/source/store_test.go +++ b/source/store_test.go @@ -33,6 +33,7 @@ import ( fakeKube "k8s.io/client-go/kubernetes/fake" "sigs.k8s.io/external-dns/internal/testutils" + externaldns "sigs.k8s.io/external-dns/pkg/apis/externaldns" "sigs.k8s.io/external-dns/source/types" ) @@ -320,3 +321,74 @@ func TestSingletonClientGenerator_RESTConfig_SharedAcrossClients(t *testing.T) { require.NoError(t, err2, "Second call does not return error due to sync.Once bug") require.NoError(t, err3, "Third call does not return error due to sync.Once bug") } + +func TestNewSourceConfig(t *testing.T) { + tests := []struct { + name string + cfg *externaldns.Config + wantConfigured bool + wantCombining bool + wantErr bool + }{ + { + name: "no templates configured", + cfg: &externaldns.Config{}, + }, + { + name: "fqdn template only", + cfg: &externaldns.Config{ + FQDNTemplate: "{{.Name}}.example.com", + }, + wantConfigured: true, + }, + { + name: "fqdn template with combine", + cfg: &externaldns.Config{ + FQDNTemplate: "{{.Name}}.example.com", + CombineFQDNAndAnnotation: true, + }, + wantConfigured: true, + wantCombining: true, + }, + { + name: "all three templates configured", + cfg: &externaldns.Config{ + FQDNTemplate: "{{.Name}}.example.com", + TargetTemplate: "{{.Name}}.targets.example.com", + FQDNTargetTemplate: "{{.Name}}.example.com:{{.Name}}.targets.example.com", + CombineFQDNAndAnnotation: true, + }, + wantConfigured: true, + wantCombining: true, + }, + { + name: "invalid fqdn template", + cfg: &externaldns.Config{FQDNTemplate: "{{.Name"}, + wantErr: true, + }, + { + name: "invalid target template", + cfg: &externaldns.Config{TargetTemplate: "{{.Status.LoadBalancer.Ingress"}, + wantErr: true, + }, + { + name: "invalid fqdn-target template", + cfg: &externaldns.Config{FQDNTargetTemplate: "{{.Name}}.example.com:{{.Status"}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := NewSourceConfig(tt.cfg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + tmpl := got.TemplateEngine + assert.Equal(t, tt.wantConfigured, tmpl.IsConfigured(), "IsConfigured") + assert.Equal(t, tt.wantCombining, tmpl.Combining(), "Combining") + }) + } +} diff --git a/source/template/OWNERS b/source/template/OWNERS new file mode 100644 index 000000000..d1709d27c --- /dev/null +++ b/source/template/OWNERS @@ -0,0 +1,4 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +labels: +- template-engine diff --git a/source/template/engine.go b/source/template/engine.go new file mode 100644 index 000000000..57b6af613 --- /dev/null +++ b/source/template/engine.go @@ -0,0 +1,187 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package template + +import ( + "bytes" + "fmt" + "maps" + "reflect" + "slices" + "strings" + "text/template" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes/scheme" + + "sigs.k8s.io/external-dns/endpoint" +) + +// Engine holds the parsed Go templates used to derive DNS names and targets +// from Kubernetes objects. It is shared across source implementations. +// The zero value is valid and represents a no-op engine. +type Engine struct { + // fqdn is the template that generates fully-qualified domain names from a Kubernetes object. + // Parsed from --fqdn-template. + fqdn *template.Template + // target is the optional template that overrides the DNS target values. + // Parsed from --target-template. + target *template.Template + // fqdnTarget is the optional template that generates "hostname:target" pairs in a single pass, + // superseding fqdn and target when set. + // Parsed from --fqdn-target-template. + fqdnTarget *template.Template + // combine controls whether template-derived endpoints are merged with annotation-derived endpoints. + // Set by --combine-fqdn-annotation. + combine bool +} + +// NewEngine parses the provided Go template strings into a Engine. +// An empty string leaves the corresponding template unset; IsConfigured reflects +// whether the FQDN template was provided. Returns an error on the first parse failure. +func NewEngine(fqdnStr, targetStr, fqdnTargetStr string, combineFQDN bool) (Engine, error) { + fqdnTmpl, err := parseTemplate(fqdnStr) + if err != nil { + return Engine{}, fmt.Errorf("parse --fqdn-template: %w", err) + } + targetTmpl, err := parseTemplate(targetStr) + if err != nil { + return Engine{}, fmt.Errorf("parse --target-template: %w", err) + } + fqdnTargetTmpl, err := parseTemplate(fqdnTargetStr) + if err != nil { + return Engine{}, fmt.Errorf("parse --fqdn-target-template: %w", err) + } + return Engine{ + fqdn: fqdnTmpl, + target: targetTmpl, + fqdnTarget: fqdnTargetTmpl, + combine: combineFQDN, + }, nil +} + +// IsConfigured reports whether the FQDN template is set and ready to use. +func (e Engine) IsConfigured() bool { + return e.fqdn != nil +} + +// Combining reports whether the engine is configured to combine template-based +// endpoints with annotation-based endpoints. +func (e Engine) Combining() bool { + return e.combine +} + +// ExecFQDN executes the FQDN template against a Kubernetes object and returns hostnames. +func (e Engine) ExecFQDN(obj kubeObject) ([]string, error) { + return execTemplate(e.fqdn, obj) +} + +// ExecTarget executes the Target template against a Kubernetes object and returns targets. +func (e Engine) ExecTarget(obj kubeObject) ([]string, error) { + return execTemplate(e.target, obj) +} + +// ExecFQDNTarget executes the FQDNTarget template against a Kubernetes object and returns hostname:target pairs. +func (e Engine) ExecFQDNTarget(obj kubeObject) ([]string, error) { + return execTemplate(e.fqdnTarget, obj) +} + +// CombineWithEndpoints merges annotation-based endpoints with template-based endpoints. +func (e Engine) CombineWithEndpoints( + endpoints []*endpoint.Endpoint, + templateFunc func() ([]*endpoint.Endpoint, error), +) ([]*endpoint.Endpoint, error) { + if e.fqdn == nil && e.target == nil && e.fqdnTarget == nil { + return endpoints, nil + } + + if !e.combine && len(endpoints) > 0 { + return endpoints, nil + } + + templatedEndpoints, err := templateFunc() + if err != nil { + return nil, fmt.Errorf("failed to get endpoints from template: %w", err) + } + + if e.combine { + return append(endpoints, templatedEndpoints...), nil + } + return templatedEndpoints, nil +} + +func parseTemplate(input string) (*template.Template, error) { + if strings.TrimSpace(input) == "" { + return nil, nil //nolint:nilnil // nil template signals "not configured"; callers check IsConfigured() + } + // Clone is cheaper than re-registering all functions on a new template each call. + t, err := baseTemplate.Clone() + if err != nil { + return nil, err + } + if _, err = t.Parse(input); err != nil { + return nil, fmt.Errorf("%q: %w", input, err) + } + return t, nil +} + +type kubeObject interface { + runtime.Object + metav1.Object +} + +func execTemplate(tmpl *template.Template, obj kubeObject) ([]string, error) { + if tmpl == nil { + return []string{}, nil + } + if obj == nil { + return nil, fmt.Errorf("object is nil") + } + // Kubernetes API doesn't populate TypeMeta (Kind/APIVersion) when retrieving + // objects via informers, because the client already knows what type it requested. + // Set it so templates can use .Kind and .APIVersion. + // TODO: all sources to transform Informer().SetTransform() + gvk := obj.GetObjectKind().GroupVersionKind() + if gvk.Kind == "" { + gvks, _, err := scheme.Scheme.ObjectKinds(obj) + if err == nil && len(gvks) > 0 { + gvk = gvks[0] + } else { + // Fallback to reflection for types not in scheme + gvk = schema.GroupVersionKind{Kind: reflect.TypeOf(obj).Elem().Name()} + } + obj.GetObjectKind().SetGroupVersionKind(gvk) + } + + var buf bytes.Buffer + if err := tmpl.Execute(&buf, obj); err != nil { + kind := obj.GetObjectKind().GroupVersionKind().Kind + return nil, fmt.Errorf("failed to apply template on %s %s/%s: %w", kind, obj.GetNamespace(), obj.GetName(), err) + } + hosts := strings.Split(buf.String(), ",") + hostnames := make(map[string]struct{}, len(hosts)) + for _, name := range hosts { + name = strings.TrimSpace(name) + name = strings.TrimSuffix(name, ".") + if name != "" { + hostnames[name] = struct{}{} + } + } + return slices.Sorted(maps.Keys(hostnames)), nil +} diff --git a/source/fqdn/fqdn_test.go b/source/template/engine_test.go similarity index 54% rename from source/fqdn/fqdn_test.go rename to source/template/engine_test.go index 6a380e582..ea805cab8 100644 --- a/source/fqdn/fqdn_test.go +++ b/source/template/engine_test.go @@ -14,12 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -package fqdn +package template import ( "errors" "testing" - "text/template" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -30,74 +29,73 @@ import ( "sigs.k8s.io/external-dns/endpoint" ) -func TestParseTemplate(t *testing.T) { +func TestNewEngine(t *testing.T) { for _, tt := range []struct { - name string - annotationFilter string - fqdnTemplate string - combineFQDNAndAnnotation bool - expectError bool + name string + fqdn string + target string + fqdnTarget string + errContains string }{ { - name: "invalid template", - expectError: true, - fqdnTemplate: "{{.Name", + name: "invalid fqdn template", + fqdn: "{{.Name", + errContains: `parse --fqdn-template: "{{.Name"`, }, { - name: "valid empty template", - expectError: false, + name: "empty fqdn template", }, { - name: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", + name: "valid fqdn template", + fqdn: "{{.Name}}-{{.Namespace}}.ext-dns.test.com", }, { - name: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", + name: "valid fqdn template with multiple hosts", + fqdn: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", }, { - name: "valid template", - expectError: false, - fqdnTemplate: "{{.Name}}-{{.Namespace}}.ext-dns.test.com, {{.Name}}-{{.Namespace}}.ext-dna.test.com", - combineFQDNAndAnnotation: true, + name: "replace template function", + fqdn: "{{\"hello.world\" | replace \".\" \"-\"}}.ext-dns.test.com", }, { - name: "non-empty annotation filter label", - expectError: false, - annotationFilter: "kubernetes.io/ingress.class=nginx", + name: "isIPv4 template function with valid IPv4", + fqdn: "{{if isIPv4 \"192.168.1.1\"}}valid{{else}}invalid{{end}}.ext-dns.test.com", }, { - name: "replace template function", - expectError: false, - fqdnTemplate: "{{\"hello.world\" | replace \".\" \"-\"}}.ext-dns.test.com", + name: "isIPv4 template function with invalid IPv4", + fqdn: "{{if isIPv4 \"not.an.ip.addr\"}}valid{{else}}invalid{{end}}.ext-dns.test.com", }, { - name: "isIPv4 template function with valid IPv4", - expectError: false, - fqdnTemplate: "{{if isIPv4 \"192.168.1.1\"}}valid{{else}}invalid{{end}}.ext-dns.test.com", + name: "isIPv6 template function with valid IPv6", + fqdn: "{{if isIPv6 \"2001:db8::1\"}}valid{{else}}invalid{{end}}.ext-dns.test.com", }, { - name: "isIPv4 template function with invalid IPv4", - expectError: false, - fqdnTemplate: "{{if isIPv4 \"not.an.ip.addr\"}}valid{{else}}invalid{{end}}.ext-dns.test.com", + name: "isIPv6 template function with invalid IPv6", + fqdn: "{{if isIPv6 \"not:ipv6:addr\"}}valid{{else}}invalid{{end}}.ext-dns.test.com", }, { - name: "isIPv6 template function with valid IPv6", - expectError: false, - fqdnTemplate: "{{if isIPv6 \"2001:db8::1\"}}valid{{else}}invalid{{end}}.ext-dns.test.com", + name: "invalid target template", + target: "{{.Status.LoadBalancer.Ingress", + errContains: `parse --target-template: "{{.Status.LoadBalancer.Ingress"`, }, { - name: "isIPv6 template function with invalid IPv6", - expectError: false, - fqdnTemplate: "{{if isIPv6 \"not:ipv6:addr\"}}valid{{else}}invalid{{end}}.ext-dns.test.com", + name: "valid target template", + target: "{{.Name}}.targets.example.com", + }, + { + name: "invalid fqdn-target template", + fqdnTarget: "{{.Name}}.example.com:{{.Status", + errContains: `parse --fqdn-target-template: "{{.Name}}.example.com:{{.Status"`, + }, + { + name: "valid fqdn-target template", + fqdnTarget: "{{.Name}}.example.com:{{.Name}}.targets.example.com", }, } { t.Run(tt.name, func(t *testing.T) { - _, err := ParseTemplate(tt.fqdnTemplate) - if tt.expectError { - assert.Error(t, err) + _, err := NewEngine(tt.fqdn, tt.target, tt.fqdnTarget, false) + if tt.errContains != "" { + assert.ErrorContains(t, err, tt.errContains) } else { assert.NoError(t, err) } @@ -105,7 +103,17 @@ func TestParseTemplate(t *testing.T) { } } -func TestExecTemplate(t *testing.T) { +func TestTemplateEngineIsConfigured(t *testing.T) { + empty, err := NewEngine("", "", "", false) + require.NoError(t, err) + assert.False(t, empty.IsConfigured()) + + configured, err := NewEngine("{{ .Name }}.example.com", "", "", false) + require.NoError(t, err) + assert.True(t, configured.IsConfigured()) +} + +func TestExecFQDN(t *testing.T) { tests := []struct { name string tmpl string @@ -334,26 +342,26 @@ func TestExecTemplate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tmpl, err := ParseTemplate(tt.tmpl) + engine, err := NewEngine(tt.tmpl, "", "", false) require.NoError(t, err) - got, err := ExecTemplate(tmpl, tt.obj) + got, err := engine.ExecFQDN(tt.obj) require.NoError(t, err) assert.Equal(t, tt.want, got) }) } } -func TestExecTemplateEmptyObject(t *testing.T) { - tmpl, err := ParseTemplate("{{ toLower .Labels.department }}.example.org") +func TestExecFQDNNilObject(t *testing.T) { + engine, err := NewEngine("{{ toLower .Labels.department }}.example.org", "", "", false) require.NoError(t, err) - _, err = ExecTemplate(tmpl, nil) + _, err = engine.ExecFQDN(nil) assert.Error(t, err) } -func TestExecTemplatePopulatesEmptyKind(t *testing.T) { +func TestExecFQDNPopulatesEmptyKind(t *testing.T) { // Test that Kind is populated when initially empty (simulates informer behavior) - tmpl, err := ParseTemplate("{{ .Kind }}.{{ .Name }}.example.com") + engine, err := NewEngine("{{ .Kind }}.{{ .Name }}.example.com", "", "", false) require.NoError(t, err) // Create object with empty TypeMeta (Kind == "") @@ -367,7 +375,7 @@ func TestExecTemplatePopulatesEmptyKind(t *testing.T) { // Kind should be empty initially assert.Empty(t, obj.GetObjectKind().GroupVersionKind().Kind) - got, err := ExecTemplate(tmpl, obj) + got, err := engine.ExecFQDN(obj) require.NoError(t, err) // Kind should now be populated via reflection @@ -375,9 +383,9 @@ func TestExecTemplatePopulatesEmptyKind(t *testing.T) { assert.Equal(t, []string{"testObject.test.example.com"}, got) } -func TestExecTemplatePreservesExistingKind(t *testing.T) { +func TestExecFQDNPreservesExistingKind(t *testing.T) { // Test that existing Kind is not overwritten - tmpl, err := ParseTemplate("{{ .Kind }}.{{ .Name }}.example.com") + engine, err := NewEngine("{{ .Kind }}.{{ .Name }}.example.com", "", "", false) require.NoError(t, err) obj := &testObject{ @@ -391,7 +399,7 @@ func TestExecTemplatePreservesExistingKind(t *testing.T) { }, } - got, err := ExecTemplate(tmpl, obj) + got, err := engine.ExecFQDN(obj) require.NoError(t, err) // Kind should remain unchanged @@ -399,275 +407,8 @@ func TestExecTemplatePreservesExistingKind(t *testing.T) { assert.Equal(t, []string{"CustomKind.test.example.com"}, got) } -func TestFqdnTemplate(t *testing.T) { - tests := []struct { - name string - fqdnTemplate string - expectedError bool - }{ - { - name: "empty template", - fqdnTemplate: "", - expectedError: false, - }, - { - name: "valid template", - fqdnTemplate: "{{ .Name }}.example.com", - expectedError: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tmpl, err := ParseTemplate(tt.fqdnTemplate) - if tt.expectedError { - require.Error(t, err) - assert.Nil(t, tmpl) - } else { - require.NoError(t, err) - if tt.fqdnTemplate == "" { - assert.Nil(t, tmpl) - } else { - assert.NotNil(t, tmpl) - } - } - }) - } -} - -func TestReplace(t *testing.T) { - for _, tt := range []struct { - name string - oldValue string - newValue string - target string - expected string - }{ - { - name: "simple replacement", - oldValue: "old", - newValue: "new", - target: "old-value", - expected: "new-value", - }, - { - name: "multiple replacements", - oldValue: ".", - newValue: "-", - target: "hello.world.com", - expected: "hello-world-com", - }, - { - name: "no replacement needed", - oldValue: "x", - newValue: "y", - target: "hello-world", - expected: "hello-world", - }, - { - name: "empty strings", - oldValue: "", - newValue: "", - target: "test", - expected: "test", - }, - } { - t.Run(tt.name, func(t *testing.T) { - result := replace(tt.oldValue, tt.newValue, tt.target) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestIsIPv6String(t *testing.T) { - for _, tt := range []struct { - name string - input string - expected bool - }{ - { - name: "valid IPv6", - input: "2001:db8::1", - expected: true, - }, - { - name: "valid IPv6 with multiple segments", - input: "2001:0db8:85a3:0000:0000:8a2e:0370:7334", - expected: true, - }, - { - name: "valid IPv4-mapped IPv6", - input: "::ffff:192.168.1.1", - expected: true, - }, - { - name: "invalid IPv6", - input: "not:ipv6:addr", - expected: false, - }, - { - name: "IPv4 address", - input: "192.168.1.1", - expected: false, - }, - { - name: "empty string", - input: "", - expected: false, - }, - } { - t.Run(tt.name, func(t *testing.T) { - result := isIPv6String(tt.input) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestIsIPv4String(t *testing.T) { - for _, tt := range []struct { - name string - input string - expected bool - }{ - { - name: "valid IPv4", - input: "192.168.1.1", - expected: true, - }, - { - name: "invalid IPv4", - input: "256.256.256.256", - expected: false, - }, - { - name: "IPv6 address", - input: "2001:db8::1", - expected: false, - }, - { - name: "invalid format", - input: "not.an.ip", - expected: false, - }, - { - name: "empty string", - input: "", - expected: false, - }, - } { - t.Run(tt.name, func(t *testing.T) { - result := isIPv4String(tt.input) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestHasKey(t *testing.T) { - for _, tt := range []struct { - name string - m map[string]string - key string - expected bool - }{ - { - name: "key exists with non-empty value", - m: map[string]string{"foo": "bar"}, - key: "foo", - expected: true, - }, - { - name: "key exists with empty value", - m: map[string]string{"service.kubernetes.io/headless": ""}, - key: "service.kubernetes.io/headless", - expected: true, - }, - { - name: "key does not exist", - m: map[string]string{"foo": "bar"}, - key: "baz", - expected: false, - }, - { - name: "nil map", - m: nil, - key: "foo", - expected: false, - }, - { - name: "empty map", - m: map[string]string{}, - key: "foo", - expected: false, - }, - } { - t.Run(tt.name, func(t *testing.T) { - result := hasKey(tt.m, tt.key) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestFromJson(t *testing.T) { - for _, tt := range []struct { - name string - input string - expected any - }{ - { - name: "map of strings", - input: `{"dns":"entry1.internal.tld","target":"10.10.10.10"}`, - expected: map[string]any{"dns": "entry1.internal.tld", "target": "10.10.10.10"}, - }, - { - name: "slice of maps", - input: `[{"dns":"entry1.internal.tld","target":"10.10.10.10"},{"dns":"entry2.example.tld","target":"my.cluster.local"}]`, - expected: []any{ - map[string]any{"dns": "entry1.internal.tld", "target": "10.10.10.10"}, - map[string]any{"dns": "entry2.example.tld", "target": "my.cluster.local"}, - }, - }, - { - name: "null input", - input: "null", - expected: nil, - }, - { - name: "empty object", - input: "{}", - expected: map[string]any{}, - }, - { - name: "string value", - input: `"hello"`, - expected: "hello", - }, - { - name: "invalid json", - input: "not valid json", - expected: nil, - }, - } { - t.Run(tt.name, func(t *testing.T) { - result := fromJson(tt.input) - assert.Equal(t, tt.expected, result) - }) - } -} - -type testObject struct { - metav1.TypeMeta - metav1.ObjectMeta -} - -func (t *testObject) DeepCopyObject() runtime.Object { - return &testObject{ - TypeMeta: t.TypeMeta, - ObjectMeta: *t.ObjectMeta.DeepCopy(), - } -} - -func TestExecTemplateExecutionError(t *testing.T) { - tmpl, err := ParseTemplate("{{ call .Name }}") +func TestExecFQDNExecutionError(t *testing.T) { + engine, err := NewEngine("{{ call .Name }}", "", "", false) require.NoError(t, err) obj := &metav1.PartialObjectMetadata{ @@ -680,14 +421,18 @@ func TestExecTemplateExecutionError(t *testing.T) { }, } - _, err = ExecTemplate(tmpl, obj) + _, err = engine.ExecFQDN(obj) require.Error(t, err) assert.Contains(t, err.Error(), "failed to apply template on TestKind default/test-name") } -func TestCombineWithTemplatedEndpoints(t *testing.T) { - // Create a dummy template for tests that need one - dummyTemplate := template.Must(template.New("test").Parse("{{.Name}}")) +func TestCombineWithEndpoints(t *testing.T) { + configured, err := NewEngine("{{.Name}}", "", "", false) + require.NoError(t, err) + configuredCombine, err := NewEngine("{{.Name}}", "", "", true) + require.NoError(t, err) + unconfigured, err := NewEngine("", "", "", false) + require.NoError(t, err) annotationEndpoints := []*endpoint.Endpoint{ endpoint.NewEndpoint("annotation.example.com", endpoint.RecordTypeA, "1.2.3.4"), @@ -704,55 +449,52 @@ func TestCombineWithTemplatedEndpoints(t *testing.T) { } tests := []struct { - name string - endpoints []*endpoint.Endpoint - fqdnTemplate *template.Template - combineFQDNAnnotation bool - templateFunc func() ([]*endpoint.Endpoint, error) - want []*endpoint.Endpoint - wantErr bool + name string + endpoints []*endpoint.Endpoint + engine Engine + templateFunc func() ([]*endpoint.Endpoint, error) + want []*endpoint.Endpoint + wantErr bool }{ { - name: "nil template returns original endpoints", + name: "unconfigured engine returns original endpoints", endpoints: annotationEndpoints, - fqdnTemplate: nil, + engine: unconfigured, templateFunc: successTemplateFunc, want: annotationEndpoints, }, { name: "combine=false with existing endpoints returns original", endpoints: annotationEndpoints, - fqdnTemplate: dummyTemplate, + engine: configured, templateFunc: successTemplateFunc, want: annotationEndpoints, }, { name: "combine=false with empty endpoints returns templated", endpoints: []*endpoint.Endpoint{}, - fqdnTemplate: dummyTemplate, + engine: configured, templateFunc: successTemplateFunc, want: templatedEndpoints, }, { - name: "combine=true appends templated to existing", - endpoints: annotationEndpoints, - fqdnTemplate: dummyTemplate, - combineFQDNAnnotation: true, - templateFunc: successTemplateFunc, - want: append(annotationEndpoints, templatedEndpoints...), + name: "combine=true appends templated to existing", + endpoints: annotationEndpoints, + engine: configuredCombine, + templateFunc: successTemplateFunc, + want: append(annotationEndpoints, templatedEndpoints...), }, { - name: "combine=true with empty endpoints returns templated", - endpoints: []*endpoint.Endpoint{}, - fqdnTemplate: dummyTemplate, - combineFQDNAnnotation: true, - templateFunc: successTemplateFunc, - want: templatedEndpoints, + name: "combine=true with empty endpoints returns templated", + endpoints: []*endpoint.Endpoint{}, + engine: configuredCombine, + templateFunc: successTemplateFunc, + want: templatedEndpoints, }, { name: "template error is propagated", endpoints: []*endpoint.Endpoint{}, - fqdnTemplate: dummyTemplate, + engine: configured, templateFunc: errorTemplateFunc, want: nil, wantErr: true, @@ -760,7 +502,7 @@ func TestCombineWithTemplatedEndpoints(t *testing.T) { { name: "nil endpoints with combine=false returns templated", endpoints: nil, - fqdnTemplate: dummyTemplate, + engine: configured, templateFunc: successTemplateFunc, want: templatedEndpoints, }, @@ -768,10 +510,8 @@ func TestCombineWithTemplatedEndpoints(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := CombineWithTemplatedEndpoints( + got, err := tt.engine.CombineWithEndpoints( tt.endpoints, - tt.fqdnTemplate, - tt.combineFQDNAnnotation, tt.templateFunc, ) if tt.wantErr { @@ -784,3 +524,15 @@ func TestCombineWithTemplatedEndpoints(t *testing.T) { }) } } + +type testObject struct { + metav1.TypeMeta + metav1.ObjectMeta +} + +func (t *testObject) DeepCopyObject() runtime.Object { + return &testObject{ + TypeMeta: t.TypeMeta, + ObjectMeta: *t.ObjectMeta.DeepCopy(), + } +} diff --git a/source/template/functions.go b/source/template/functions.go new file mode 100644 index 000000000..8cec5eb4e --- /dev/null +++ b/source/template/functions.go @@ -0,0 +1,85 @@ +/* +Copyright 2026 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package template + +import ( + "encoding/json" + "strings" + "text/template" + + "sigs.k8s.io/external-dns/endpoint" +) + +var ( + // baseTemplate is the shared base with all template functions pre-registered. + // It is package-level so functions are registered once rather than on every parse call. + // Each call to parseTemplate clones it before adding new content. + baseTemplate = template.Must( + template.New("endpoint").Funcs(template.FuncMap{ + "contains": strings.Contains, + "trimPrefix": strings.TrimPrefix, + "trimSuffix": strings.TrimSuffix, + "trim": strings.TrimSpace, + "toLower": strings.ToLower, + "replace": replace, + "isIPv6": isIPv6, + "isIPv4": isIPv4, + "hasKey": hasKey, + "fromJson": fromJson, + }).Parse(""), + ) +) + +// replace all instances of oldValue with newValue in target string. +// adheres to syntax from https://masterminds.github.io/sprig/strings.html. +func replace(oldValue, newValue, target string) string { + return strings.ReplaceAll(target, oldValue, newValue) +} + +// isIPv6 reports whether the target string is an IPv6 address, +// including IPv4-mapped IPv6 addresses. +func isIPv6(target string) bool { + return endpoint.SuitableType(target) == endpoint.RecordTypeAAAA +} + +// isIPv4 reports whether the target string is an IPv4 address. +func isIPv4(target string) bool { + return endpoint.SuitableType(target) == endpoint.RecordTypeA +} + +// hasKey checks if a key exists in a map. This is needed because Go templates' +// `index` function returns the zero value ("") for missing keys, which is +// indistinguishable from keys with empty values. Kubernetes uses empty-value +// labels for markers (e.g., `service.kubernetes.io/headless: ""`), so we need +// explicit key existence checking. +func hasKey(m map[string]string, key string) bool { + _, ok := m[key] + return ok +} + +// fromJson decodes a JSON string into a Go value (map, slice, etc.). +// This enables templates to work with structured data stored as JSON strings +// in complex labels or annotations or Configmap data fields, e.g. ranging over a list of entries: +// +// {{ range $entry := (index .Data "entries" | fromJson) }}{{ index $entry "dns" }},{{ end }} +// +// Returns nil if the input is not valid JSON. +func fromJson(v string) any { + var output any + _ = json.Unmarshal([]byte(v), &output) + return output +} diff --git a/source/template/functions_test.go b/source/template/functions_test.go new file mode 100644 index 000000000..4e2b98ec1 --- /dev/null +++ b/source/template/functions_test.go @@ -0,0 +1,242 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package template + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestReplace(t *testing.T) { + for _, tt := range []struct { + name string + oldValue string + newValue string + target string + expected string + }{ + { + name: "simple replacement", + oldValue: "old", + newValue: "new", + target: "old-value", + expected: "new-value", + }, + { + name: "multiple replacements", + oldValue: ".", + newValue: "-", + target: "hello.world.com", + expected: "hello-world-com", + }, + { + name: "no replacement needed", + oldValue: "x", + newValue: "y", + target: "hello-world", + expected: "hello-world", + }, + { + name: "empty strings", + oldValue: "", + newValue: "", + target: "test", + expected: "test", + }, + } { + t.Run(tt.name, func(t *testing.T) { + result := replace(tt.oldValue, tt.newValue, tt.target) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestIsIPv6String(t *testing.T) { + for _, tt := range []struct { + name string + input string + expected bool + }{ + { + name: "valid IPv6", + input: "2001:db8::1", + expected: true, + }, + { + name: "valid IPv6 with multiple segments", + input: "2001:0db8:85a3:0000:0000:8a2e:0370:7334", + expected: true, + }, + { + name: "valid IPv4-mapped IPv6", + input: "::ffff:192.168.1.1", + expected: true, + }, + { + name: "invalid IPv6", + input: "not:ipv6:addr", + expected: false, + }, + { + name: "IPv4 address", + input: "192.168.1.1", + expected: false, + }, + { + name: "empty string", + input: "", + expected: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + result := isIPv6(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestIsIPv4String(t *testing.T) { + for _, tt := range []struct { + name string + input string + expected bool + }{ + { + name: "valid IPv4", + input: "192.168.1.1", + expected: true, + }, + { + name: "invalid IPv4", + input: "256.256.256.256", + expected: false, + }, + { + name: "IPv6 address", + input: "2001:db8::1", + expected: false, + }, + { + name: "invalid format", + input: "not.an.ip", + expected: false, + }, + { + name: "empty string", + input: "", + expected: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + result := isIPv4(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestHasKey(t *testing.T) { + for _, tt := range []struct { + name string + m map[string]string + key string + expected bool + }{ + { + name: "key exists with non-empty value", + m: map[string]string{"foo": "bar"}, + key: "foo", + expected: true, + }, + { + name: "key exists with empty value", + m: map[string]string{"service.kubernetes.io/headless": ""}, + key: "service.kubernetes.io/headless", + expected: true, + }, + { + name: "key does not exist", + m: map[string]string{"foo": "bar"}, + key: "baz", + expected: false, + }, + { + name: "nil map", + m: nil, + key: "foo", + expected: false, + }, + { + name: "empty map", + m: map[string]string{}, + key: "foo", + expected: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + result := hasKey(tt.m, tt.key) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestFromJson(t *testing.T) { + for _, tt := range []struct { + name string + input string + expected any + }{ + { + name: "map of strings", + input: `{"dns":"entry1.internal.tld","target":"10.10.10.10"}`, + expected: map[string]any{"dns": "entry1.internal.tld", "target": "10.10.10.10"}, + }, + { + name: "slice of maps", + input: `[{"dns":"entry1.internal.tld","target":"10.10.10.10"},{"dns":"entry2.example.tld","target":"my.cluster.local"}]`, + expected: []any{ + map[string]any{"dns": "entry1.internal.tld", "target": "10.10.10.10"}, + map[string]any{"dns": "entry2.example.tld", "target": "my.cluster.local"}, + }, + }, + { + name: "null input", + input: "null", + expected: nil, + }, + { + name: "empty object", + input: "{}", + expected: map[string]any{}, + }, + { + name: "string value", + input: `"hello"`, + expected: "hello", + }, + { + name: "invalid json", + input: "not valid json", + expected: nil, + }, + } { + t.Run(tt.name, func(t *testing.T) { + result := fromJson(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/source/template/testutil/testutil.go b/source/template/testutil/testutil.go new file mode 100644 index 000000000..fbe0f8773 --- /dev/null +++ b/source/template/testutil/testutil.go @@ -0,0 +1,33 @@ +/* +Copyright 2026 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testutil + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "sigs.k8s.io/external-dns/source/template" +) + +// MustEngine creates an Engine with all three templates and combine flag, failing the test on error. +func MustEngine(t testing.TB, fqdnStr, targetStr, fqdnTargetStr string, combine bool) template.Engine { + t.Helper() + engine, err := template.NewEngine(fqdnStr, targetStr, fqdnTargetStr, combine) + require.NoError(t, err) + return engine +} diff --git a/source/unstructured.go b/source/unstructured.go index f8dcf172e..7964f1b42 100644 --- a/source/unstructured.go +++ b/source/unstructured.go @@ -22,7 +22,6 @@ import ( "maps" "slices" "strings" - "text/template" log "github.com/sirupsen/logrus" @@ -39,8 +38,8 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/pkg/events" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" "sigs.k8s.io/external-dns/source/informers" + "sigs.k8s.io/external-dns/source/template" "sigs.k8s.io/external-dns/source/types" ) @@ -56,11 +55,8 @@ import ( // +externaldns:source:provider-specific=false // +externaldns:source:events=false type unstructuredSource struct { - combineFqdnAnnotation bool - fqdnTemplate *template.Template - targetTemplate *template.Template - fqdnTargetTemplate *template.Template - informers []kubeinformers.GenericInformer + templateEngine template.Engine + informers []kubeinformers.GenericInformer } // NewUnstructuredFQDNSource creates a new unstructuredSource. @@ -70,21 +66,6 @@ func NewUnstructuredFQDNSource( kubeClient kubernetes.Interface, cfg *Config, ) (Source, error) { - fqdnTmpl, err := fqdn.ParseTemplate(cfg.FQDNTemplate) - if err != nil { - return nil, err - } - - targetTmpl, err := fqdn.ParseTemplate(cfg.TargetTemplate) - if err != nil { - return nil, err - } - - fqdnTargetTmpl, err := fqdn.ParseTemplate(cfg.FQDNTargetTemplate) - if err != nil { - return nil, err - } - gvrs, err := discoverResources(kubeClient, cfg.UnstructuredResources) if err != nil { return nil, err @@ -123,11 +104,8 @@ func NewUnstructuredFQDNSource( } return &unstructuredSource{ - fqdnTemplate: fqdnTmpl, - targetTemplate: targetTmpl, - fqdnTargetTemplate: fqdnTargetTmpl, - informers: resourceInformers, - combineFqdnAnnotation: cfg.CombineFQDNAndAnnotation, + templateEngine: cfg.TemplateEngine, + informers: resourceInformers, }, nil } @@ -171,8 +149,8 @@ func (us *unstructuredSource) endpointsFromInformer(informer kubeinformers.Gener addrs := annotations.TargetsFromTargetAnnotation(el.GetAnnotations()) annotationEdps := EndpointsForHostsAndTargets(hosts, addrs) - fqdnTargetEdps, err := fqdn.CombineWithTemplatedEndpoints( - annotationEdps, us.fqdnTargetTemplate, us.combineFqdnAnnotation, + fqdnTargetEdps, err := us.templateEngine.CombineWithEndpoints( + annotationEdps, func() ([]*endpoint.Endpoint, error) { return us.endpointsFromFQDNTargetTemplate(el) }, @@ -181,8 +159,8 @@ func (us *unstructuredSource) endpointsFromInformer(informer kubeinformers.Gener return nil, err } - edps, err := fqdn.CombineWithTemplatedEndpoints( - fqdnTargetEdps, us.fqdnTemplate, us.combineFqdnAnnotation, + edps, err := us.templateEngine.CombineWithEndpoints( + fqdnTargetEdps, func() ([]*endpoint.Endpoint, error) { return us.endpointsFromTemplate(el) }, @@ -209,7 +187,7 @@ func (us *unstructuredSource) endpointsFromInformer(informer kubeinformers.Gener // endpointsFromTemplate creates endpoints using DNS names from the FQDN template. func (us *unstructuredSource) endpointsFromTemplate(el *unstructuredWrapper) ([]*endpoint.Endpoint, error) { - hostnames, err := fqdn.ExecTemplate(us.fqdnTemplate, el) + hostnames, err := us.templateEngine.ExecFQDN(el) if err != nil { return nil, err } @@ -217,12 +195,9 @@ func (us *unstructuredSource) endpointsFromTemplate(el *unstructuredWrapper) ([] return nil, nil } - var targets []string - if us.targetTemplate != nil { - targets, err = fqdn.ExecTemplate(us.targetTemplate, el) - if err != nil { - return nil, err - } + targets, err := us.templateEngine.ExecTarget(el) + if err != nil { + return nil, err } return EndpointsForHostsAndTargets(hostnames, targets), nil @@ -231,7 +206,7 @@ func (us *unstructuredSource) endpointsFromTemplate(el *unstructuredWrapper) ([] // endpointsFromFQDNTargetTemplate creates endpoints from a template that returns host:target pairs. // Each pair creates a single endpoint with 1:1 mapping between host and target. func (us *unstructuredSource) endpointsFromFQDNTargetTemplate(el *unstructuredWrapper) ([]*endpoint.Endpoint, error) { - pairs, err := fqdn.ExecTemplate(us.fqdnTargetTemplate, el) + pairs, err := us.templateEngine.ExecFQDNTarget(el) if err != nil { return nil, err } @@ -297,7 +272,7 @@ func (u *unstructuredWrapper) GetObjectMeta() metav1.Object { } // newUnstructuredWrapper creates a wrapper around an *unstructured.Unstructured, -// exposing typed convenience fields for templates alongside raw map sections. +// exposing typed convenience fields for templateEngine alongside raw map sections. func newUnstructuredWrapper(u *unstructured.Unstructured) *unstructuredWrapper { w := &unstructuredWrapper{ Unstructured: u, diff --git a/source/unstructured_fqdn_test.go b/source/unstructured_fqdn_test.go index b3b2a51f7..df5cc2396 100644 --- a/source/unstructured_fqdn_test.go +++ b/source/unstructured_fqdn_test.go @@ -28,7 +28,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" - "sigs.k8s.io/external-dns/source/fqdn" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func TestUnstructuredFqdnTemplatingExamples(t *testing.T) { @@ -838,12 +838,9 @@ func TestUnstructuredFqdnTemplatingExamples(t *testing.T) { dynamicClient, kubeClient, &Config{ - LabelFilter: selector, - UnstructuredResources: tt.cfg.resources, - FQDNTemplate: tt.cfg.fqdnTemplate, - TargetTemplate: tt.cfg.targetTemplate, - FQDNTargetTemplate: tt.cfg.fqdnTargetTemplate, - CombineFQDNAndAnnotation: tt.cfg.combine, + LabelFilter: selector, + UnstructuredResources: tt.cfg.resources, + TemplateEngine: templatetest.MustEngine(t, tt.cfg.fqdnTemplate, tt.cfg.targetTemplate, tt.cfg.fqdnTargetTemplate, tt.cfg.combine), }, ) require.NoError(t, err) @@ -996,11 +993,10 @@ func TestUnstructuredWrapper_Templating(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tmpl, err := fqdn.ParseTemplate(tt.tmpl) - require.NoError(t, err) + engine := templatetest.MustEngine(t, tt.tmpl, "", "", false) wrapped := newUnstructuredWrapper(tt.obj) - got, err := fqdn.ExecTemplate(tmpl, wrapped) + got, err := engine.ExecFQDN(wrapped) if tt.wantErr { require.Error(t, err) return diff --git a/source/unstructured_test.go b/source/unstructured_test.go index 571216a19..59b88f96e 100644 --- a/source/unstructured_test.go +++ b/source/unstructured_test.go @@ -38,6 +38,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" + templatetest "sigs.k8s.io/external-dns/source/template/testutil" ) func TestUnstructuredWrapperImplementsKubeObject(t *testing.T) { @@ -459,10 +460,10 @@ func TestUnstructured_DifferentScenarios(t *testing.T) { dynamicClient, kubeClient, &Config{ - AnnotationFilter: tt.cfg.annotationFilter, - LabelFilter: labelSelector, - UnstructuredResources: tt.cfg.resources, - CombineFQDNAndAnnotation: tt.cfg.combine, + AnnotationFilter: tt.cfg.annotationFilter, + LabelFilter: labelSelector, + UnstructuredResources: tt.cfg.resources, + TemplateEngine: templatetest.MustEngine(t, "", "", "", tt.cfg.combine), }, ) require.NoError(t, err) diff --git a/source/wrappers/build_test.go b/source/wrappers/build_test.go index 14726b053..0997d90aa 100644 --- a/source/wrappers/build_test.go +++ b/source/wrappers/build_test.go @@ -29,8 +29,11 @@ import ( "sigs.k8s.io/external-dns/source/types" ) -func stubConfig(extCfg *externaldns.Config) *source.Config { - return source.NewSourceConfig(extCfg, source.WithClientGenerator(testutils.StubClientGenerator{})) +func stubConfig(t *testing.T, extCfg *externaldns.Config) *source.Config { + t.Helper() + cfg, err := source.NewSourceConfig(extCfg, source.WithClientGenerator(testutils.StubClientGenerator{})) + require.NoError(t, err) + return cfg } func TestBuildWrappedSource(t *testing.T) { @@ -41,25 +44,25 @@ func TestBuildWrappedSource(t *testing.T) { }{ { name: "fake source with no extra wrappers", - cfg: stubConfig(&externaldns.Config{Sources: []string{types.Fake}}), + cfg: stubConfig(t, &externaldns.Config{Sources: []string{types.Fake}}), }, { name: "fake source with target filter wrapper", - cfg: stubConfig(&externaldns.Config{ + cfg: stubConfig(t, &externaldns.Config{ Sources: []string{types.Fake}, TargetNetFilter: []string{"10.0.0.0/8"}, }), }, { name: "fake source with NAT64 networks", - cfg: stubConfig(&externaldns.Config{ + cfg: stubConfig(t, &externaldns.Config{ Sources: []string{types.Fake}, NAT64Networks: []string{"2001:db8::/96"}, }), }, { name: "fake source with minTTL, provider, and preferAlias", - cfg: stubConfig(&externaldns.Config{ + cfg: stubConfig(t, &externaldns.Config{ Sources: []string{types.Fake}, MinTTL: 300 * time.Second, Provider: "aws", @@ -68,7 +71,7 @@ func TestBuildWrappedSource(t *testing.T) { }, { name: "fake source with exclude target nets", - cfg: stubConfig(&externaldns.Config{ + cfg: stubConfig(t, &externaldns.Config{ Sources: []string{types.Fake}, TargetNetFilter: []string{"10.0.0.0/8"}, ExcludeTargetNets: []string{"10.1.0.0/16"}, @@ -76,12 +79,12 @@ func TestBuildWrappedSource(t *testing.T) { }, { name: "unknown source returns error", - cfg: stubConfig(&externaldns.Config{Sources: []string{"does-not-exist"}}), + cfg: stubConfig(t, &externaldns.Config{Sources: []string{"does-not-exist"}}), wantErr: true, }, { name: "invalid NAT64 network returns error", - cfg: stubConfig(&externaldns.Config{ + cfg: stubConfig(t, &externaldns.Config{ Sources: []string{types.Fake}, NAT64Networks: []string{"not-a-cidr"}, }), diff --git a/tests/integration/toolkit/toolkit.go b/tests/integration/toolkit/toolkit.go index 0b543b15f..6687aa459 100644 --- a/tests/integration/toolkit/toolkit.go +++ b/tests/integration/toolkit/toolkit.go @@ -233,7 +233,7 @@ func LoadResources(ctx context.Context, scenario Scenario) (*fake.Clientset, err } // scenarioToConfig creates a source.Config for testing with the scenario config. -func scenarioToConfig(scenarioCfg ScenarioConfig, opts ...source.OverrideConfigOption) *source.Config { +func scenarioToConfig(scenarioCfg ScenarioConfig, opts ...source.OverrideConfigOption) (*source.Config, error) { return source.NewSourceConfig(&externaldns.Config{ Sources: scenarioCfg.Sources, ServiceTypeFilter: scenarioCfg.ServiceTypeFilter, @@ -253,6 +253,9 @@ func CreateWrappedSource( ctx context.Context, client *fake.Clientset, scenarioCfg ScenarioConfig) (source.Source, error) { - cfg := scenarioToConfig(scenarioCfg, source.WithClientGenerator(newMockClientGenerator(client))) + cfg, err := scenarioToConfig(scenarioCfg, source.WithClientGenerator(newMockClientGenerator(client))) + if err != nil { + return nil, err + } return wrappers.Build(ctx, cfg) }