Skip to content

fix(source): scope CNAME conflict warnings to the configured domain filter#6579

Open
jojinkb wants to merge 1 commit into
kubernetes-sigs:masterfrom
jojinkb:fix-cname-conflict-warning
Open

fix(source): scope CNAME conflict warnings to the configured domain filter#6579
jojinkb wants to merge 1 commit into
kubernetes-sigs:masterfrom
jojinkb:fix-cname-conflict-warning

Conversation

@jojinkb

@jojinkb jojinkb commented Jul 25, 2026

Copy link
Copy Markdown

What does it do ?

Stops the Only one CNAME per name warning from firing for DNS names that are outside the configured --domain-filter (and related --exclude-domains / --regex-domain-filter / --regex-domain-exclusion flags), while keeping the exact same warn-level message for names this instance actually manages.

Since #6174, sources call endpoint.MergeEndpoints(), which emitted the warning before the planner applies the domain filter — so hosts that exist purely for in-cluster routing (Istio Gateways/VirtualServices, Ingress hosts used by a CDN, ...) produced a misleading warning every sync interval even though ExternalDNS will never manage them.

Following the reviewer guidance on #6551 (fix it generically for all sources, gate the warning instead of pre-filtering endpoints, and use the source-wrapper pattern like nat64source), this PR:

  • adds a new CNAMEConflictSource wrapper (source/wrappers/cnameconflictsource.go) installed at the end of the standard wrapper pipeline. It detects CNAME endpoints sharing the same DNS name + set identifier with different targets and warns only when the name matches the domain filter; out-of-scope conflicts are logged at debug level. Endpoints are returned unmodified — conflicting records are still resolved by the planner's conflict resolver, exactly as before.
  • demotes the merge-time log in endpoint.MergeEndpoints() to debug level (same message text, still useful to trace which source produced a conflict). Because the wrapper runs on the combined output of all sources, conflicts across sources (e.g. istio-gateway vs istio-virtualservice, the exact repro in the issue) are now detected too, which the per-source merge could miss.
  • builds the DomainFilter once in source.NewSourceConfig() and shares it between the wrapper pipeline and the provider (controller/execute.go previously constructed it separately).

Behavior summary:

Scenario Before After
CNAME conflict, name matches domain filter warn (per source) warn (combined sources, same message)
CNAME conflict, name outside domain filter warn debug
No domain filter configured warn warn
Conflict across two sources not detected warn/debug per filter

Fixes #6529

Motivation

Users running --source=istio-gateway --source=istio-virtualservice (or plain Ingress, see the issue comments) with a --domain-filter get warnings every minute about "invalid DNS" for hosts that are intentionally not managed by ExternalDNS. The warning is legitimate signal for in-scope conflicts — per the maintainer feedback on #6551 it must not simply be hidden — but for out-of-scope names it is pure noise that looks like a real DNS problem.

Previous attempts #6546 and #6551 were closed after review feedback; this implements the design suggested there: keep the warning, gate it by the domain filter, apply it once for all sources via a wrapper.

Note on scope: the warning is gated by the --domain-filter family of flags only. Provider zone filters (e.g. --zone-id-filter) are not consulted, because zones are only known to the provider at plan time; if reviewers prefer gating on the registry/provider filter as well, the detection could instead move to plan.Calculate() — happy to adjust.

More

  • Yes, this PR title follows Conventional Commits
  • Yes, I added unit tests
  • Yes, I updated end user documentation accordingly

This change was developed with AI assistance (Claude Code); I have reviewed and tested it.

…ilter

The "Only one CNAME per name" warning was emitted inside
endpoint.MergeEndpoints(), before the configured domain filter is
applied, so it fired for hosts (e.g. Istio or Ingress hosts used purely
for in-cluster routing) that this ExternalDNS instance will never
manage.

Move the user-facing warning into a new CNAMEConflictSource wrapper
that runs after all sources are combined and only warns for DNS names
matching the domain filter. Conflicts on out-of-scope names, and the
per-source detection in MergeEndpoints, are logged at debug level
instead. In-scope conflicts keep the exact same warn-level message, and
cross-source conflicts are now detected as well.

Signed-off-by: Jojin <jojin.kb@gmail.com>
@kubernetes-prow kubernetes-prow Bot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 25, 2026
@kubernetes-prow

Copy link
Copy Markdown

Welcome @jojinkb!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@kubernetes-prow kubernetes-prow Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 25, 2026
@kubernetes-prow

Copy link
Copy Markdown

Hi @jojinkb. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@kubernetes-prow kubernetes-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 25, 2026
@kubernetes-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign mloiseleur for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow
kubernetes-prow Bot requested a review from u-kai July 25, 2026 10:17
@kubernetes-prow kubernetes-prow Bot added the controller Issues or PRs related to the controller label Jul 25, 2026
@kubernetes-prow
kubernetes-prow Bot requested a review from vflaux July 25, 2026 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. controller Issues or PRs related to the controller docs needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CNAME conflict warning is emitted for Istio hosts outside domain-filter

1 participant