Add support for AAAA records #2461
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
It's sad that you twisted my words, because that's not what I said. |
k8s-ci-robot
added
the
needs-rebase
k8s-ci-robot
removed
the
needs-rebase
|
Hm I know this is fairly early, but it seems the current code does something weird for the txt records. It fails with |
|
@MTRNord could you provide reproduction steps? |
|
Oh I see what happens here. A helm chart I am using is setting the tls hosts with that postfix. Sorry. I only just realized why I wanted to send you an ingress that fail. So the issue is in the helm chart used, not in external dns. |
|
Is there anything left on this besides review? @johngmyers do you have an image availble to test? |
|
@anthr76 (any anyone else in need) – I've built this image and pushed it to docker hub. It is: Looking forward to this being in an official release :-) |
|
I'm not aware of anything other than review being needed. I tested on a frankenimage including commits from #2419 because I had difficulty creating IPv6-capable Ingresses due to AWS LBC not supporting k8s > 1.21. |
|
There are going to be followup PRs extending AAAA support to other resources and possibly the AWS SD registry. |
k8s-ci-robot
added
the
needs-rebase
|
What's the reason for the extra registry TXT record? is that in case there may be different owners for A and AAAA? |
|
@olemarkus The record type is a field of
This PR takes approach (1). |
k8s-ci-robot
removed
the
needs-rebase
|
Just for cross-reference, this will most likely also help fix: #1877 |
| DNSName: "foo.bar", | ||
| Targets: endpoint.Targets{"8.8.8.8"}, | ||
| DNSName: "foo.bar", | ||
| RecordType: endpoint.RecordTypeA, |
There was a problem hiding this comment.
what about adding some tests for endpoint.RecordTypeAAAA? I feel like we only slightly adjust the old tests being passed but not adding new tests for testing AAAA
There was a problem hiding this comment.
I added a test case for extracting an AAAA from an Ingress status. Are there any other areas where you think this is lacking coverage?
There was a problem hiding this comment.
Looks like it is tested in https://github.com/kubernetes-sigs/external-dns/pull/2461/files#diff-7ec1f21be9321a28b8532f54f2da218bb0d7541c9af60caa3e6b6dd10b89d73cR426 , so lgtm
There was a problem hiding this comment.
First of all thank you for addressing support for AAAA records, this is huge change and I might need you to add some documentation how people can use the new records so I'd appreciate if you could add some sentences how it works 🙏🏻.
Again thank you for you work and picking it up again.
|
My understanding is that this PR only adds picking up AAAA from Ingress. Picking up AAAA from other resource types is for followup PRs. I plan on submitting said followup PRs. I'm not sure what to add to the documentation about how to use this. One doesn't do anything different than what the existing documentation describes; it's just that AAAA records now get registered in DNS whereas before they were omitted. And I think it's outside the scope of this package's documentation to crib RFC 3596. How it works: the plan separates the two types of addresses into two different So could you give a little more detail on what/where you want additional documentation? An addition to the "Ownership" sections of docs/initial-design.md? Something about the planner, which doesn't appear to be currently documented? |
|
@szuecs does this need your /lgtm or is @njuettner the one who needs to give that since he requested charges? |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/lgtm |
|
@olemarkus that was not fine to lgtm, sorry to say this. I reached out to other maintainers to review. Please don't do this again, thanks! |
|
Sorry about that. But lgtm means community members has reviewed and find the code good. There are thousands with that privilege. Approvals is the method for ensuring actual owners do final reviews and confirm all is good for merge. Typically code owners would just lgtm if they want other owners to do second reviews. Or use holds. |
|
@olemarkus ok thanks, then I just missed the idea of lgtm/approve split. |
|
The roles and responsibilites of Kubernetes contributors are well-established and documented in https://github.com/kubernetes/community/blob/master/community-membership.md. As a Member, @olemarkus was working entirely within expectations to give lgtm. |
|
@johngmyers yes my bad because approve is maintainer (owner) responsibility, lgtm everyone in the org. A bit weird to me tbh. |
When AAAA multi-target / dual stack support was added via kubernetes-sigs#2461 it broke ownership of domains across different clusters with different ingress records types. For example if 2 clusters manage the same zone, 1 cluster uses A records and the other uses CNAME records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted. This change introduces a new configuration point that which by default will only cause AAAA records to be managed independently of A and CNAME records. This should preserve the dual stack / multi-target use case while preserving ownership of across different clusters. This configuration can be overridden using the new option `--dual-stack-record-types`. For example to revert back to the current behavior where all record types are treated as independent the configuration would be: `--dual-stack-record-types AAAA --dual-stack-record-types A --dual-stack-record-types CNAME`
When AAAA multi-target / dual stack support was added via kubernetes-sigs#2461 it broke ownership of domains across different clusters with different ingress records types. For example if 2 clusters manage the same zone, 1 cluster uses A records and the other uses CNAME records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted. This change introduces a new configuration point that which by default will only cause AAAA records to be managed independently of A and CNAME records. This should preserve the dual stack / multi-target use case while preserving ownership of across different clusters. This configuration can be overridden using the new option `--dual-stack-record-types`. For example to revert back to the current behavior where all record types are treated as independent the configuration would be: `--dual-stack-record-types AAAA --dual-stack-record-types A --dual-stack-record-types CNAME`
When AAAA multi-target / dual stack support was added via kubernetes-sigs#2461 it broke ownership of domains across different clusters with different ingress records types. For example if 2 clusters manage the same zone, 1 cluster uses A records and the other uses CNAME records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted. This change updates the planner to keep track of multiple current records for a domain. This allows for A and AAAA records to exist for a domain while allowing record type changes. Similarly candidate records are grouped by record type to support dual stack records.
When AAAA multi-target / dual stack support was added via kubernetes-sigs#2461 it broke ownership of domains across different clusters with different ingress records types. For example if 2 clusters manage the same zone, 1 cluster uses A records and the other uses CNAME records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted. This change updates the planner to keep track of multiple current records for a domain. This allows for A and AAAA records to exist for a domain while allowing record type changes. The planner will ignore desired records for a domain that represent conflicting record types allowed by RFC 1034 3.6.2. For example if the desired records for a domain contains a CNAME record plus any other record type no changes for that domain will be planned. The planner now contains an owned record filter provided by the registry. This allows the planner to accurately plan create updates when there are record type changes between the current and desired endpoints. Without this filter the planner could add create changes for domains not owned by the controller.
When AAAA multi-target / dual stack support was added via kubernetes-sigs#2461 it broke ownership of domains across different clusters with different ingress records types. For example if 2 clusters manage the same zone, 1 cluster uses A records and the other uses CNAME records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted. This change updates the planner to keep track of multiple current records for a domain. This allows for A and AAAA records to exist for a domain while allowing record type changes. The planner will ignore desired records for a domain that represent conflicting record types allowed by RFC 1034 3.6.2. For example if the desired records for a domain contains a CNAME record plus any other record type no changes for that domain will be planned. The planner now contains an owned record filter provided by the registry. This allows the planner to accurately plan create updates when there are record type changes between the current and desired endpoints. Without this filter the planner could add create changes for domains not owned by the controller.
When AAAA multi-target / dual stack support was added via kubernetes-sigs#2461 it broke ownership of domains across different clusters with different ingress records types. For example if 2 clusters manage the same zone, 1 cluster uses A records and the other uses CNAME records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted. This change updates the planner to keep track of multiple current records for a domain. This allows for A and AAAA records to exist for a domain while allowing record type changes. The planner will ignore desired records for a domain that represent conflicting record types allowed by RFC 1034 3.6.2. For example if the desired records for a domain contains a CNAME record plus any other record type no changes for that domain will be planned. The planner now contains an owned record filter provided by the registry. This allows the planner to accurately plan create updates when there are record type changes between the current and desired endpoints. Without this filter the planner could add create changes for domains not owned by the controller.
When AAAA multi-target / dual stack support was added via kubernetes-sigs#2461 it broke ownership of domains across different clusters with different ingress records types. For example if 2 clusters manage the same zone, 1 cluster uses A records and the other uses CNAME records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted. This change updates the planner to keep track of multiple current records for a domain. This allows for A and AAAA records to exist for a domain while allowing record type changes. The planner will ignore desired records for a domain that represent conflicting record types allowed by RFC 1034 3.6.2. For example if the desired records for a domain contains a CNAME record plus any other record type no changes for that domain will be planned. The planner now contains an owned record filter provided by the registry. This allows the planner to accurately plan create updates when there are record type changes between the current and desired endpoints. Without this filter the planner could add create changes for domains not owned by the controller.
When AAAA multi-target / dual stack support was added via kubernetes-sigs#2461 it broke ownership of domains across different clusters with different ingress records types. For example if 2 clusters manage the same zone, 1 cluster uses A records and the other uses CNAME records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted. This change updates the planner to keep track of multiple current records for a domain. This allows for A and AAAA records to exist for a domain while allowing record type changes. The planner will ignore desired records for a domain that represent conflicting record types allowed by RFC 1034 3.6.2. For example if the desired records for a domain contains a CNAME record plus any other record type no changes for that domain will be planned. The planner now contains an owned record filter provided by the registry. This allows the planner to accurately plan create updates when there are record type changes between the current and desired endpoints. Without this filter the planner could add create changes for domains not owned by the controller.
When AAAA multi-target / dual stack support was added via kubernetes-sigs#2461 it broke ownership of domains across different clusters with different ingress records types. For example if 2 clusters manage the same zone, 1 cluster uses A records and the other uses CNAME records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted. This change updates the planner to keep track of multiple current records for a domain. This allows for A and AAAA records to exist for a domain while allowing record type changes. The planner will ignore desired records for a domain that represent conflicting record types allowed by RFC 1034 3.6.2. For example if the desired records for a domain contains a CNAME record plus any other record type no changes for that domain will be planned. The planner now contains an owned record filter provided by the registry. This allows the planner to accurately plan create updates when there are record type changes between the current and desired endpoints. Without this filter the planner could add create changes for domains not owned by the controller.
When AAAA multi-target / dual stack support was added via kubernetes-sigs#2461 it broke ownership of domains across different clusters with different ingress records types. For example if 2 clusters manage the same zone, 1 cluster uses A records and the other uses CNAME records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted. This change updates the planner to keep track of multiple current records for a domain. This allows for A and AAAA records to exist for a domain while allowing record type changes. The planner will ignore desired records for a domain that represent conflicting record types allowed by RFC 1034 3.6.2. For example if the desired records for a domain contains a CNAME record plus any other record type no changes for that domain will be planned. The planner now contains an owned record filter provided by the registry. This allows the planner to accurately plan create updates when there are record type changes between the current and desired endpoints. Without this filter the planner could add create changes for domains not owned by the controller.
Adds support for managing AAAA records.
Fixes #2300
Fixes #1812
Fixes #2051
Checklist
Bringing work started in #2309 to completion, as that PR's author has expressed that they will not be moving the work forward at this time.
Tested against the Route53 provider and the TXT registry. The external-dns AWS SD registry code likely does not support AAAA records.
This touches on multi-target, since dual-stack targets need both an A and an AAAA record. This PR solves the dual-stack problem by creating separate
Endpointobjects for the A and AAAA record and extending the TXT registry to store ownership records for AAAA Endpoints under a different domain.