Support wildcard records - Optional ability to replace the asterisk in generated registry TXT records with another string #1695
Conversation
…n generated registry TXT records with another string
k8s-ci-robot
added
cncf-cla: yes
|
/assign @Raffo |
|
Seems like a good idea to me. |
|
/kind feature |
k8s-ci-robot
added
the
kind/feature
k8s-ci-robot
added
the
needs-rebase
|
@dansimone please rebase and fix merge conflicts and we will try to get this reviewed. Thanks. |
…wildcard-records # Conflicts: # registry/txt.go # registry/txt_test.go
k8s-ci-robot
removed
the
needs-rebase
|
/assign |
registry/txt.go
Outdated
| func newaffixNameMapper(prefix string, suffix string) affixNameMapper { | ||
| return affixNameMapper{prefix: strings.ToLower(prefix), suffix: strings.ToLower(suffix)} | ||
| func newaffixNameMapper(prefix string, suffix string, wildcardReplacement string) affixNameMapper { | ||
| return affixNameMapper{prefix: strings.ToLower(prefix), suffix: strings.ToLower(suffix), wildcardReplacement: wildcardReplacement} |
There was a problem hiding this comment.
I'm curious ... why not also call strings.ToLower() for wildcardReplacement?
There was a problem hiding this comment.
@dansimone please let me know what you think when you have some time. Thanks!
There was a problem hiding this comment.
@seanmalloy sorry for the lag. Yes, I agree. Done in the latest commit.
|
@dansimone I left one inline comment. Also, you mentioned that some providers(i.e. AWS) have code to already handle this case, but not all providers do. It would be good to clean up all of the providers in the future, so the code is cleaner. But I'm not saying that this needs to be done in this PR. We should probably create a list of providers that need updating, so we don't lose track of this. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
@Raffo @njuettner please let me know when one of you has a chance to look at this. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dansimone, njuettner, seanmalloy 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 |
k8s-ci-robot
added
the
approved
When using the txt registry, external-dns is not able to create wildcard records for many providers because the generated TXT records corresponding to the wildcard records are malformed: they contain an asterisk, but are not of the form "*.foo.bar". For example, this is the case with the Dyn provider.
A couple providers add special handling to deal with this, like aws.go. But rather than attempt to fix this in all providers, this PR adds an optional command line parameter that allows the asterisk in generated TXT records to be replaced with some arbitrary string. Setting this parameter (to any value) allows wildcard records to be created successfully for providers (like Dyn) where it doesn't work currently. This optional parameter is unset by default, meaning existing behavior is unchanged if it is not specified.
A couple of unit tests in txt_test.go has been updated to verify the new parameter.