-
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
feat: add option to use only new format TXT records #4946
base: master
Are you sure you want to change the base?
feat: add option to use only new format TXT records #4946
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Welcome @malpou! |
Hi @malpou. 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 Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
@@ -3,6 +3,24 @@ | |||
The TXT registry is the default registry. | |||
It stores DNS record metadata in TXT records, using the same provider. | |||
|
|||
## Record Format Options |
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.
Could you include a subsection on how to handle the migration from two records to a single record? Specifically, what are the steps involved in this consolidation, and will the external DNS automatically delete the old records or will manual cleanup be necessary?
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.
Right now i haven't thought of any automatic cleanup, so for now it would be a manual cleanup for the old format TXT
records.
Is that an okay approach, or would you like it to happen automatically?
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.
Mentioning this in the documentation likely is enough.
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.
Added an extra section describing a manual migration flow.
docs/registry/txt.md
Outdated
external-dns | ||
|
||
# Only create new format records | ||
external-dns --txt-new-format-only |
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.
Is this required - --managed-record-types=TXT
or with that flags we imply implicitly that TXT records should be handled?
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.
The flag, is the initial implementation thought as an addition to the flags you would already start external-dns
with.
So no it doesn't implicitly tell that we are using TXT records.
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.
The usage of the flag should be more clear now.
- Legacy format: Creates a TXT record without record type information | ||
- New format: Creates a TXT record with record type information (e.g., 'a-' prefix for A records) | ||
|
||
By default, the TXT registry creates records in both formats for backwards compatibility. You can configure it to use only the new format by using the `--txt-new-format-only` flag. This reduces the number of TXT records created, which can be helpful when working with provider-specific record limits. |
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.
Can we add a bit more information about this behaviour as well? e.g. why external-dns creates two records
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.
I don't have the history of why there are 2 formats, but would be glad to add some more text about it.
We just reached a point in my team at work where we would like to get rid of our duplicate TXT records in all of our DNS zones.
@@ -138,6 +138,7 @@ type Config struct { | |||
TXTSuffix string | |||
TXTEncryptEnabled bool | |||
TXTEncryptAESKey string `secure:"yes"` | |||
TXTNewFormatOnly bool |
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.
would it be possible to update types_test.go
as well?
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.
Added tests for the new flag now
Is this flag |
I'd like for the new format to be the default thing, but i don't have enough insight to know if that's currently possible?? This PR, focuses on giving people the option to move over to the new format without breaking anything for people that don't want that at this point in time. It seems like there are multiple persons that would like this option (myself included), without having to move over to encrypted records to get rid of the extra records. |
7ad3deb
to
8937026
Compare
Description
Adds a new flag
--txt-new-format-only
that allows configuring external-dns to only create TXT records in the new format, containing record type information. This helps reduce the number of DNS records created when using external-dns at scale, addressing provider-specific record limits.By default, the TXT registry creates both old and new format records for backwards compatibility. The new format includes the record type in the name (e.g., "a-" prefix for A records). With this change, users can opt-in to using only the new format when all their external-dns instances support it.
Key changes:
--txt-new-format-only
flag (defaults to false for backwards compatibility)Example usage:
Note: AAAA records already only create new format records regardless of this setting, as they have special handling.
Fixes #3167
Fixes #4636
Checklist