-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
test(domain-filter): simple filters on domain exclusion #5064
test(domain-filter): simple filters on domain exclusion #5064
Conversation
Signed-off-by: ivan katliarchuk <[email protected]>
Signed-off-by: ivan katliarchuk <[email protected]>
/label tide/merge-method-squash |
Signed-off-by: ivan katliarchuk <[email protected]>
There are already tests on this with |
These tests are different. There is some sort of deserialisation, fixtures coupled with two tests, and handled with block like
I was actually not able to make it green with such a simple case, probably designed with a slightly different model, that I can't get my head around. So the plan was to create a simple tests. I think long term this whole domain filter need to be revisited and simplified. If you think is not required, I'll close this pull request. We could w8 for community member to review they may came out with better plan. |
Then let's rename it with a clear different name from the already existing one and we should be good to go. |
Co-authored-by: Michel Loiseleur <[email protected]>
/retitle test(domain-filer): simple filters on domain exclusion |
/retitle test(domain-filter): simple filters on domain exclusion |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mloiseleur The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Relates #3718
The work relates to a bug in domain filter logic. I've added a test that focuses only on MATCH behaviour for a domain filters with include and exclude values, and validate against provided domains. This does not fix anything. The filters itself looks like works correctly.
There is a slight design problem. We instantiate filters in main.go
external-dns/main.go
Line 183 in eb19497
Probably an option to consider is to create a composition filter or something. Move if/else filter creation logic away from main.go file. Any preferences, ideas?
Checklist