-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat: resolve endpoint target CNAME to A/AAAA records with external-dns.alpha.kubernetes.io/resolve-target #6329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
4e3af94
6b41e0d
e327c92
8f5fc03
fadcf68
862a87d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,137 @@ | ||||
| /* | ||||
|
Apoorva64 marked this conversation as resolved.
|
||||
| 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 wrappers | ||||
|
|
||||
| import ( | ||||
| "context" | ||||
| "net" | ||||
| "sort" | ||||
|
|
||||
| log "github.com/sirupsen/logrus" | ||||
|
|
||||
| "sigs.k8s.io/external-dns/endpoint" | ||||
| "sigs.k8s.io/external-dns/source" | ||||
| ) | ||||
|
|
||||
| const resolveTargetPropertyName = "resolve-target" | ||||
|
|
||||
| // resolveTarget is a Source wrapper that resolves CNAME targets (load balancer hostnames) | ||||
| // to their underlying A/AAAA IP addresses via DNS lookup. | ||||
| // | ||||
| // Resolution is controlled per-endpoint via the "resolve-target" provider-specific property: | ||||
| // | ||||
| // "true" – resolve this endpoint's hostname targets to A/AAAA records | ||||
| // "false" – keep this endpoint's hostname targets as CNAME records | ||||
| // | ||||
| // The property is always consumed (deleted) by this wrapper so it does not leak | ||||
| // to downstream components. | ||||
| type resolveTarget struct { | ||||
| source source.Source | ||||
| lookupIP func(string) ([]net.IP, error) | ||||
| } | ||||
|
|
||||
| // resolveTargetOption is a functional option for resolveTarget. | ||||
| type resolveTargetOption func(*resolveTarget) | ||||
|
|
||||
| // WithResolveTargetLookupIP is a helper to create a resolveTargetOption that sets a custom lookupIP function for testing. | ||||
| // If fn is nil, the default net.LookupIP is preserved. | ||||
| func WithResolveTargetLookupIP(fn func(string) ([]net.IP, error)) resolveTargetOption { | ||||
| return func(rs *resolveTarget) { | ||||
| if fn != nil { | ||||
| rs.lookupIP = fn | ||||
| } | ||||
| } | ||||
| } | ||||
|
|
||||
| // NewResolveTarget creates a new resolveTarget wrapping src. | ||||
| func NewResolveTarget(src source.Source, opts ...resolveTargetOption) source.Source { | ||||
| rs := &resolveTarget{ | ||||
| source: src, | ||||
| lookupIP: net.LookupIP, | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||
| } | ||||
| for _, opt := range opts { | ||||
| opt(rs) | ||||
| } | ||||
| return rs | ||||
| } | ||||
|
|
||||
| func (rs *resolveTarget) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error) { | ||||
| endpoints, err := rs.source.Endpoints(ctx) | ||||
| if err != nil { | ||||
| return nil, err | ||||
| } | ||||
|
|
||||
| result := make([]*endpoint.Endpoint, 0, len(endpoints)) | ||||
| for _, ep := range endpoints { | ||||
| if ep == nil { | ||||
| continue | ||||
| } | ||||
|
|
||||
| // Always consume the "resolve-target" property so it never leaks downstream, | ||||
| // regardless of record type or value otherwise it won't converge with a non-empty UpdateNew in the plan. | ||||
| resolve, _ := ep.GetProviderSpecificProperty(resolveTargetPropertyName) | ||||
| ep.DeleteProviderSpecificProperty(resolveTargetPropertyName) | ||||
|
|
||||
| // Only CNAME endpoints opted in via "true" are resolved; everything else passes through. | ||||
| if ep.RecordType != endpoint.RecordTypeCNAME || resolve != "true" { | ||||
| result = append(result, ep) | ||||
| continue | ||||
| } | ||||
|
|
||||
| var ipTargets endpoint.Targets | ||||
| for _, target := range ep.Targets { | ||||
| ips, err := rs.lookupIP(target) | ||||
| if err != nil { | ||||
| log.Debugf("Unable to resolve %q, skipping target: %v", target, err) | ||||
| continue | ||||
| } | ||||
| for _, ip := range ips { | ||||
| ipTargets = append(ipTargets, ip.String()) | ||||
| } | ||||
|
mloiseleur marked this conversation as resolved.
|
||||
| } | ||||
| if len(ipTargets) == 0 { | ||||
| // All resolutions failed; skip this endpoint entirely. | ||||
| continue | ||||
| } | ||||
|
Comment on lines
+106
to
+109
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If all targets transiently fail to resolve, the endpoint vanishes from desired state → plan computes a delete → DNS record removed → outage, then recreated next sync when DNS recovers. Do you think that, on total resolution failure, it can keep the previous/CNAME endpoint (skip the change) rather than emitting nothing ?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A test like this would confirm that behavior: func TestTotalFailureKeepsCNAME(t *testing.T) {
ep := endpoint.NewEndpoint("app.example.internal", endpoint.RecordTypeCNAME, "lb.example.com")
ep.WithProviderSpecific(resolveTargetPropertyName, "true")
ms := new(testutils.MockSource)
ms.On("Endpoints").Return([]*endpoint.Endpoint{ep}, nil)
src := NewResolveTarget(ms, WithResolveTargetLookupIP(
func(string) ([]net.IP, error) { return nil, errors.New("i/o timeout") }, // transient
))
got, err := src.Endpoints(t.Context())
require.NoError(t, err)
// Expected: original CNAME preserved, not dropped.
require.Len(t, got, 1, "endpoint must be kept when resolution totally fails")
require.Equal(t, endpoint.RecordTypeCNAME, got[0].RecordType, "should fall back to the original CNAME")
require.Equal(t, endpoint.Targets{"lb.example.com"}, got[0].Targets)
// Property still consumed so it does not leak downstream.
_, ok := got[0].GetProviderSpecificProperty(resolveTargetPropertyName)
require.False(t, ok, "resolve-target property should be consumed even on the fallback path")
}
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the host returns no IPs, why would you want to preserve the record? That's a signal something is wrong - and we don't know whether the host is being reprovisioned, permanently removed, or just experiencing a transient network issue. This comes down to reconciliation interval tuning. The current service lookup is straightforward - no special logic, just binary: resolved or not. Currnet service lookup external-dns/source/service.go Line 585 in 9bb899c
There's no clean answer here. Either you keep a record that points nowhere - which is going to be painful to debug - or you remove it when it stops resolving. Both options have real downsides.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ivankatliarchuk Yes, that's true. Maybe we can improve UserXP, though. Wdyt of documenting this limitation and log a warning or a SoftError ? |
||||
|
|
||||
| // Sort targets before grouping for deterministic output. | ||||
| sort.Strings(ipTargets) | ||||
|
|
||||
| // Group targets by record type (A vs AAAA) | ||||
| byType := map[string]endpoint.Targets{} | ||||
| for _, t := range ipTargets { | ||||
| rt := endpoint.SuitableType(t) | ||||
| byType[rt] = append(byType[rt], t) | ||||
| } | ||||
|
|
||||
| // Emit one endpoint per record type with the same DNSName and provider-specific properties. | ||||
| for _, rt := range []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA} { | ||||
| if targets := byType[rt]; len(targets) > 0 { | ||||
| result = append(result, ep.WithTargets(targets)) | ||||
| } | ||||
| } | ||||
|
|
||||
| log.Debugf("resolveTarget: resolved endpoint %q into %d IP target(s)", ep.DNSName, len(ipTargets)) | ||||
| } | ||||
|
|
||||
| return result, nil | ||||
| } | ||||
|
|
||||
| func (rs *resolveTarget) AddEventHandler(ctx context.Context, handler func()) { | ||||
| log.Debug("resolveTarget: adding event handler") | ||||
| rs.source.AddEventHandler(ctx, handler) | ||||
| } | ||||
Uh oh!
There was an error while loading. Please reload this page.