Correct format of SRV record for NodePort

[vanekjar]

Description

Currently, the NodePort extraction results in an SRV record with the DNS name in the following format:

_31569._tcp.nginx.my-external-dns-test.com 0 IN SRV  0 50 31569 nginx.my-external-dns-test.com

Please note that the first part of the DNS name is the port. That defeats the purpose of the SRV record since the client needs to know the port number upfront to query for the record to "learn" the port again from the DNS server.

DNS RFC specifies that the SRV record should be in the following format (see https://en.wikipedia.org/wiki/SRV_record):

_service._proto.name. TTL class SRV priority weight port target.

For that reason I replaced the port in the DNS name with the K8s Service name resulting in records with this format:

_nginx._tcp.nginx.my-external-dns-test.com 0 IN SRV  0 50 31569 nginx.my-external-dns-test.com

Checklist

• Unit tests updated
• [ no related doc ] End user documentation updated

[vanekjar]

@grimmy, please can you take a look if this change doesn't brake anything regarding NodePort support?

[grimmy]

@vanekjar as previously stated, I no longer have access to the cluster where I originally added the node port srv record support for. That said this looks fine, but I'm sure a bunch of people would appreciate if you could fix #637 in this as well.

[njuettner]

This would make sense to me, however I don't know if this breaks current usage? What if you have already existing SRV records with port usage for the record?

It might be worth mentioning that people might be surprised upgrading external-dns at some point.

I'd like to hear the opinion of the community if this would be ok or if we need to have a workaround when updating external-dns.

[vanekjar]

I believe we're safe in terms of backwards compatibility. SRV records don't work so far, so there shouldn't be an issue with existing records (covered in PR #1890)

[njuettner]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: njuettner, vanekjar

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 [njuettner]

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