-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat: add --txt-prefix-override flag to support apex domains safely #6026
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: master
Are you sure you want to change the base?
feat: add --txt-prefix-override flag to support apex domains safely #6026
Conversation
Signed-off-by: u-kai <[email protected]>
Signed-off-by: u-kai <[email protected]>
…tomization Signed-off-by: u-kai <[email protected]>
Signed-off-by: u-kai <[email protected]>
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @u-kai. Thanks for your PR. I'm waiting for a github.com 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. DetailsInstructions 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. |
|
|
||
| // normalizeDNSName converts a DNS name to a canonical form, so that we can use string equality | ||
| // it: removes space, get ASCII version of dnsName complient with Section 5 of RFC 5891, ensures there is a trailing dot | ||
| func normalizeDNSName(dnsName string) string { |
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.
This should be in it's own PR.
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 may explain a bit more - some changes, like refactoring are easy to revew/approve/merge. While adding new capabilities or fixing certain behaviour has different SDLC
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.
Understood, thanks for the clarification.
I’ve split this into a separate PR.
|
Conceptually the change aligns with ExternalDNS’s goal of offering pragmatic knobs for tricky DNS layouts like apex records (this is just single opinion) What I'm unsure
We need to review pros-cons of adding flag complex vs annotation Even then the change may still be reviewed and rejrected by project owners |
Thanks — I’m not sure I fully understand this concern yet. If so, the current design supports this by allowing the flag to be specified multiple times at runtime.
I’ve already added a detailed description to the flag itself, including the intended use case for apex records and support for multiple domains. Would you prefer:
My current view is: Annotation approach:
Flag approach (this PR):
From a maintenance and scope perspective, I chose the flag-based approach for now. |
|
Let's w8 for other parties to review and consider which approach should we choose.
This PR slowly slips into area of #3757 Relates |
|
PR needs rebase. DetailsInstructions 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. |
What does it do ?
This PR introduces a new flag,
--txt-prefix-override, as a safe and flexible workaroundto support apex domains when using the TXT registry.
The flag allows users to override the TXT record prefix per domain by specifying
one or more mappings in the following form:
Multiple overrides can be provided, and each mapping applies only to the explicitly
specified domain.
Motivation
In #5864, we discussed limitations and edge cases around managing apex domains with the
TXT registry.
While a flag such as
--apex-domainswas considered, it was intentionally avoided forthe following reasons:
--apex-domainscould be misunderstood as applying special behavior toall apex domains automatically.
domain-specific TXT prefix customization.
surprising users.
By using
--txt-prefix-override, users must opt in per domain, making the behaviorclear and predictable.
More