external-dns
external-dns copied to clipboard
Don't create endpoint if attempting to create one with invalid dns name
Description
We should expect that 63 char labels works also with the txt registry. But as can be seen from https://github.com/kubernetes-sigs/external-dns/issues/2839 and others, this is not the case after introducing the new format.
This PR doesn't fix the underlying problem in any way, but it maintains backwards compatibility by ignoring the new format if it results in endpoint with too long name. This way, e.g route53 batch changes will keep working with the consequence of not migrating to the new format.
Note: This will also prevent subtle breakage in case e.g the service annotation contains invalid DNS labels.
Checklist
- [x] Unit tests updated
@Raffo / @szuecs would appreciate a review on this one. The issue with the record length is hitting us bad in production,
@olemarkus is this AWS specific? If so it's likely not a good idea to do this change. https://github.com/kubernetes-sigs/external-dns/issues/2839#issuecomment-1205025159 sounds also like a valid workaround.
The 63 char limit is per https://www.ietf.org/rfc/rfc1034.txt. E.g coredns will also refuse doing lookups if the label is longer than 63.
If we had new installations, we'd probably use a different prefix as that workaround, but setting it on our existing services will make external-dns lose ownership of all existing records. It would be a ton of work getting that cleaned up.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: olemarkus, szuecs
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [szuecs]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment