-
Notifications
You must be signed in to change notification settings - Fork 2k
Refactor getValidTarget to return a list of items #8869
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: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the getValidTargets function to return a list of DNSTarget objects instead of a simple string slice and a separate record type. The new structure combines the target address with its record type (A, AAAA, or CNAME), allowing the system to handle multiple record types for a single DNS endpoint.
Changes:
- Modified
getValidTargetsto returnextdnsapi.Targets(slice ofDNSTargetstructs) instead of a string slice and separate record type - Updated
buildDNSEndpointto group targets by record type and create separate endpoints for each type - Introduced new
DNSTargettype withTypeandAddressfields to replace the simple string-basedTargetstype
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/apis/externaldns/v1/types.go | Introduced DNSTarget struct and changed Targets type from []string to []DNSTarget |
| pkg/apis/externaldns/v1/zz_generated.deepcopy.go | Added auto-generated deepcopy methods for new DNSTarget type |
| pkg/apis/externaldns/validation/externaldns.go | Updated validateTargets and isUnique to accept []string instead of v1.Targets |
| pkg/apis/externaldns/validation/externaldns_test.go | Updated test cases to use []string instead of v1.Targets for target values |
| internal/externaldns/sync.go | Refactored getValidTargets to return typed targets and updated buildDNSEndpoint to collate targets by type |
| internal/externaldns/sync_test.go | Updated tests to expect new DNSTarget structure and removed record type assertions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8869 +/- ##
==========================================
- Coverage 53.93% 53.88% -0.05%
==========================================
Files 91 91
Lines 18627 18628 +1
==========================================
- Hits 10046 10038 -8
- Misses 8049 8059 +10
+ Partials 532 531 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Proposed changes
Closes #8708
Checklist
Before creating a PR, run through this checklist and mark each as complete.