OCPBUGS-819: Azure provider - add wildcard replacement for TXT records

[alebedev87]

Since the addition of the new format of TXT records in external-dns 0.12.0 the TXT records are prepended with the type of the DNS record which doesn't comply with the standards of Azure DNS in case of the wildcards: the wildcard symbol (*) has to be the leftmost one.

Related issue: kubernetes-sigs/external-dns#2922

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alebedev87

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:

• OWNERS [alebedev87]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[alebedev87]

Infoblox temporary licences had to be renewed

/retest

[gcs278]

/assign

[candita]

@alebedev87 please add a JIRA so this can be tracked and tested.

[candita]

/assign @Miciah

[openshift-ci-robot]

@alebedev87: This pull request references Jira Issue OCPBUGS-819, which is invalid:

• expected the bug to target the "4.12.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Since the addition of the new format of TXT records in external-dns 0.12.0 the TXT records are prepended with the type of the DNS record which doesn't comply with the standards of Azure DNS in case of the wildcards: the wildcard symbol (*) has to be the leftmost one.

Related issue: kubernetes-sigs/external-dns#2922

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/test-infra repository.

[alebedev87]

/jira refresh

[openshift-ci-robot]

@alebedev87: This pull request references Jira Issue OCPBUGS-819, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.12.0) matches configured target version for branch (4.12.0)
• bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

In response to this:

/jira refresh

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/test-infra repository.

[Miciah]

What happens when a route or service with the host name "*.foo" exists and another route or service with the host name "ws.foo" is created? What happens in the reverse case: something with "ws.foo" exists and something with "*.foo" is created?

CoreDNS and SkyDNS treat "any" the same as "*", so we could use that instead of "ws", but "any" isn't standard. It doesn't seem like there is any value we can use in lieu of "ws" that wouldn't risk causing conflicts.

[alebedev87]

What happens when a route or service with the host name ".foo" exists and another route or service with the host name "ws.foo" is created? What happens in the reverse case: something with "ws.foo" exists and something with ".foo" is created?

A conflict of the TXT record name would happen, in both cases. So, external-dns would reject the creation of the conflicting TXT record which would prevent the creation of the "target" DNS record too.

CoreDNS and SkyDNS treat "any" the same as "*", so we could use that instead of "ws", but "any" isn't standard. It doesn't seem like there is any value we can use in lieu of "ws" that wouldn't risk causing conflicts.

Even if it's not standard, any is used in at least 1 implementation and overall looks more educated than just the acronym (wc stands for wildcard). This won't help with the conflict described above but may be considered more intuitive at least.

[openshift-ci]

@alebedev87: all tests passed!

Full PR test history. Your PR dashboard.

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/test-infra repository. I understand the commands that are listed here.

[gcs278]

Do you think it would be better to limit this option only for configv1.AzurePlatformType? Smaller blast radius? Or do you think it's better to be consistent for all platforms?

[gcs278]

Whoops, cancel that, I didn't realize the function was specific to the Azure provider already.

[alebedev87]

Yes, this PR aims at fixing the problem which is seen only in Azure DNS as it doesn't allow wildcard records (TXT included) which don't start with the asterisk.

[gcs278]

Not sure if I completely understand this one. Say I want to make a wildcard TXT record for grant-*.example.com in azure, so that I can query grant-a.example.com and grant-b.example.com.

How can I still query grant-a.example.com if the --txt-wildcard-replacement changes it to grant-any.example.com? Am I misunderstanding what field --txt-wildcard-replacement is operating on? Does the wildcard query still work?

[alebedev87]

@gcs278 : Thanks for having a look!

Does the wildcard query still work?

Yes, the wildcard itself is not impacted by --txt-wildcard-replacement. It would still remain with the asterisk.

--txt-wildcard-replacement replaces the asterisk character for the corresponding TXT record. ExternalDNS Operator uses the TXT registry of the upstream ExternalDNS to track the ownership of the DNS records (A, CNAME for the moment). So each A or CNAME record is paired with a TXT record which contains the owner ID (unique name of externaldns instance). The --txt-wildcard-replacement flag impact only these TXT records.

[Miciah]

Not sure if I completely understand this one. Say I want to make a wildcard TXT record for grant-*.example.com in azure, so that I can query grant-a.example.com and grant-b.example.com.

How can I still query grant-a.example.com if the --txt-wildcard-replacement changes it to grant-any.example.com? Am I misunderstanding what field --txt-wildcard-replacement is operating on? Does the wildcard query still work?

Per https://www.rfc-editor.org/rfc/rfc4592#section-2.1.1, the entire first label must be an asterisk for the record to be a wildcard record. Accordingly, external-dns only substitutes (source) or matches (source) for "*" if it is equal to the entirety of the first label.

[gcs278]

Oh okay, now I'm starting to understand. External DNS is creating TXT records to track what other DNS records it owns. And azure is rejecting TXT records with * not in the left most position. This fix just substitutes * with other characters.

But one thing I'm still confused about, is how/why a record such as a-*.example.com gets created, like in the issue. Isn't that already an invalid wildcard domain? Is someone purposely creating this type of record or is External DNS creating this a-*.example.com record in it's TXT registry logic?

[Miciah]

But one thing I'm still confused about, is how/why a record such as a-*.example.com gets created, like in the issue. Isn't that already an invalid wildcard domain? Is someone purposely creating this type of record or is External DNS creating this a-*.example.com record in it's TXT registry logic?

Yeah, a-*.example.com is invalid, or at least imprudent and not preferred, per https://www.rfc-editor.org/rfc/rfc1035#section-2.3.1. DNS might allow it, but I'd expect its use as a host name to be rejected. I'm surprised that --txt-wildcard-replacement resolved that issue.

[alebedev87]

Yeah, a-*.example.com is invalid, or at least imprudent and not preferred, per https://www.rfc-editor.org/rfc/rfc1035#section-2.3.1. DNS might allow it, but I'd expect its use as a host name to be rejected. I'm surprised that --txt-wildcard-replacement resolved that issue.

a-*.example.com is the domain name for the TXT record only, the corresponding DNS record would look like *.example.com. a- is the default prefix added to TXT records by external dns since the version 0.12.0.

[gcs278]

a-*.example.com is the domain name for the TXT record only, the corresponding DNS record would look like *.example.com. a- is the default prefix added to TXT records by external dns since the version 0.12.0.

Okay - that makes more sense now. I was curious why this was even a problem (aka let's just stop making invalid records). But now I know External DNS is making them.

Okay yikes. Sounds like this is our best path forward at the moment, but @Miciah does bring up a good point about naming collisions. Correct me if I'm wrong, but you can't have *.example.com and any.example.com co-exist with External DNS on Azure with this PR.

I'm good with this, but I feel hesitant, so @Miciah could you /lgtm (since it's already approved, it'll merge after one lgtm).

[alebedev87]

Correct me if I'm wrong, but you can't have *.example.com and any.example.com co-exist with External DNS on Azure with this PR.

You are right, @gcs278. These 2 records won't be able to be served by ExternalDNS on Azure. This is a limitation (not only in this downstream but upstream too) which I think is preferable over the regression in the support of the wildcard record which we are currently having.

[Miciah]

You are right, @gcs278. These 2 records won't be able to be served by ExternalDNS on Azure. This is a limitation (not only in this downstream but upstream too) which I think is preferable over the regression in the support of the wildcard record which we are currently having.

Please make sure to address this caveat with a release note.

/lgtm

[alebedev87]

/assign @quarterpin

[alebedev87]

/assign @xenolinux

Note this comment: #171 (comment)

We need to mention the fact that wildcard TXT records get modified: asterisk is replaced with any on AzureDNS. And this may cause a conflict with records starting with any. So any prefixed records are to be avoided on Azure.

[alebedev87]

/assign @lihongan

[lihongan]

/label qe-approved

[xenolinux]

/label docs-approved

[alebedev87]

/label px-approved

[openshift-ci-robot]

@alebedev87: All pull requests linked via external trackers have merged:

• openshift/external-dns-operator#171

Jira Issue OCPBUGS-819 has been moved to the MODIFIED state.

In response to this:

Since the addition of the new format of TXT records in external-dns 0.12.0 the TXT records are prepended with the type of the DNS record which doesn't comply with the standards of Azure DNS in case of the wildcards: the wildcard symbol (*) has to be the leftmost one.

Related issue: kubernetes-sigs/external-dns#2922

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/test-infra repository.