Skip to content

fix(domain-exclusion): domain exclusion filter fix#6050

Merged
k8s-ci-robot merged 3 commits intokubernetes-sigs:masterfrom
gofogo:fix-regex-domain-5674
Dec 27, 2025
Merged

fix(domain-exclusion): domain exclusion filter fix#6050
k8s-ci-robot merged 3 commits intokubernetes-sigs:masterfrom
gofogo:fix-regex-domain-5674

Conversation

@ivankatliarchuk
Copy link
Copy Markdown
Member

@ivankatliarchuk ivankatliarchuk commented Dec 22, 2025

What does it do ?

Finally found a problem.

  • --regex-domain-exclusion is silently ignored unless --regex-domain-filter is set, causing confusion and unexpected DNS records being managed.
  • Add unit coverage to make the behavior explicit and prevent regressions.

The endpoint/domain_filter.go#matchRegex function had a bug where it would accept domains that didn't match the inclusion filter, as long as they also didn't match the exclusion filter.
Changes made:

  1. Fixed the logic to check exclusion first, then inclusion
  2. If both filters are set: domain must match inclusion AND not match exclusion
  3. If only exclusion is set: domain must not match exclusion
  4. Updated the method comment to clarify the behaviour

follow-up PR #6055

Motivation

Fixes #5674
Fixes #2255

More

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

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
@k8s-ci-robot k8s-ci-robot requested a review from szuecs December 22, 2025 10:38
@k8s-ci-robot k8s-ci-robot added the apis Issues or PRs related to API change label Dec 22, 2025
@k8s-ci-robot k8s-ci-robot requested a review from vflaux December 22, 2025 10:38
@k8s-ci-robot k8s-ci-robot added controller Issues or PRs related to the controller docs size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 22, 2025
@coveralls
Copy link
Copy Markdown

coveralls commented Dec 22, 2025

Pull Request Test Coverage Report for Build 20430783058

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 13 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.001%) to 78.698%

Files with Coverage Reduction New Missed Lines %
execute.go 2 64.72%
domain_filter.go 11 83.67%
Totals Coverage Status
Change from base Build 20412655513: 0.001%
Covered Lines: 16026
Relevant Lines: 20364

💛 - Coveralls

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
@mloiseleur
Copy link
Copy Markdown
Collaborator

@tatyana12345 @asahnovskiy-deloitte @gzenrique Do you think you can test this PR and confirm if it fixes your issue?

See how to test a PR if needed.

Copy link
Copy Markdown
Collaborator

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 27, 2025
@ivankatliarchuk
Copy link
Copy Markdown
Member Author

/approve

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ivankatliarchuk

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

The pull request process is described 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 27, 2025
@k8s-ci-robot k8s-ci-robot merged commit 72425c8 into kubernetes-sigs:master Dec 27, 2025
18 checks passed
@ivankatliarchuk ivankatliarchuk deleted the fix-regex-domain-5674 branch December 28, 2025 08:03
ivankatliarchuk added a commit to gofogo/k8s-sigs-external-dns-fork that referenced this pull request Dec 28, 2025
* master:
  somehow single test was not fixed (kubernetes-sigs#6059)
  refactore(registry): move registry selector to registry package (kubernetes-sigs#6047)
  fix(domain-exclusion): domain exclusion filter fix (kubernetes-sigs#6050)
  chore(wrapper): centralized endpoint validation (kubernetes-sigs#6041)
  test(coverage): improve code coverage for different files (kubernetes-sigs#6045)
  chore(lint): configure modernize linter (kubernetes-sigs#6035)
  docs(proposal): Gateway API annotation placement clarity proposal (kubernetes-sigs#5919)
  fix(scripts): helm plugins install disable verify (kubernetes-sigs#6057)
  feat(cli): remove cobra cli support (kubernetes-sigs#6034)
  fix(chart): ptsc indentation (kubernetes-sigs#6054)
  chore(deps): bump renovatebot/github-action (kubernetes-sigs#6051)
  feat(aws): enable support for NAPTR records (kubernetes-sigs#6022)
  refactor: extract normalizeDNSName to idna package for reuse (kubernetes-sigs#6043)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

apis Issues or PRs related to API change approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. controller Issues or PRs related to the controller docs lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

regex-domain-exclusion it seems doesn't work regex-domain-exclusion it seems doesn't work

4 participants