*Route DNS entry uses longest matching suffix of all attached gateways

[gganley]

Description

Currently if an HTTPRoute is attached to multiple Gateways whose hostnames overlap with each of the hostnames in the HTTPRoute, but at different lengths, it returns a Multivalue of the sorted gateway addresses. In my opinion in cases like this where the DNS entries will be CNAMEs or Alias records where Multivalue doesn't make sense (AFAIA) if there is a "more correct" answer that one should be used. So in this case the gateway that has the longer overlap with a given httproute hostname is used as the value for the DNS record.

Checklist

• Unit tests updated
• End user documentation updated

[linux-foundation-easycla]

• ✅login: gganley / (8dec057)
• ✅login: gganley / (8dec057, 63b8406)

The committers listed above are authorized under a signed CLA.

[k8s-ci-robot]

Welcome @gganley!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @gganley. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign szuecs for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

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

[mloiseleur]

@gganley Would you please rename your PR following conventional commits ?

cc @abursavich for a first review

[abursavich]

I understand your reasoning and agree that makes sense in most cases, but the user has explicitly attached the route to multiple gateways and listeners and we should respect that. I don't think what we decide is probably better in most cases should overrule what a user explicitly configured. That's a very high bar to clear.

[szuecs]

In general we should wait on gateway-api people before we change anything on this right now.

[gganley]

@gganley Would you please rename your PR following conventional commits ?

cc @abursavich for a first review

Do you mean Conventional Commits? I was confused because I tried to read the contributing guide to the best of my abilities and didn't see anything referencing that but when I searched I found the issue you posted. Is this for automation reasons or just because it's something you/the team prefers?

Would that be something like BREAKING CHANGE: define preference order when multiple gateway hostnames match multiple route hostnames

[gganley]

I understand your reasoning and agree that makes sense in most cases, but the user has explicitly attached the route to multiple gateways and listeners and we should respect that. I don't think what we decide is probably better in most cases should overrule what a user explicitly configured. That's a very high bar to clear.

I think my logic changes can be updated to have the same IPs but in a different order based on the metrics I've laid out. In the specific case I use of k8s gateway API + AWS Route 53 (which creates alias) only the first entry in the multi answer is actually committed to DNS (I may have poor phrasing, sorry for that).

I could understand not wanting to break behavior even if the change "would make more sense", so would it make more sense to put this "new behavior" behind a major version or a configuration flag?

[gganley]

In general we should wait on gateway-api people before we change anything on this right now.

I've tried my best to read the gateway api spec and it doesn't seem like this is covered in the spec. The closest is the logic that decides between two HTTPRoutes, not the Gateways attached to a single HTTPRoute

In the event that multiple HTTPRoutes specify intersecting hostnames (e.g. overlapping wildcard matching and exact matching hostnames), precedence must be given to rules from the HTTPRoute with the largest number of: Characters in a matching non-wildcard hostname. Characters in a matching hostname.

[mloiseleur]

Would that be something like BREAKING CHANGE: define preference order when multiple gateway hostnames match multiple route hostnames

Something like

feat(gateway api)!:  preference order when multiple gateway hostnames match multiple route hostnames

[mloiseleur]

I could understand not wanting to break behavior even if the change "would make more sense", so would it make more sense to put this "new behavior" behind a major version or a configuration flag?

Nope. We will follow what the user explicitly configured and the Gateway API spec.
If this kind of change is accepted on Gateway API, then we can support it.

[gganley]

I could understand not wanting to break behavior even if the change "would make more sense", so would it make more sense to put this "new behavior" behind a major version or a configuration flag?

Nope. We will follow what the user explicitly configured and the Gateway API spec. If this kind of change is accepted on Gateway API, then we can support it.

Would reordering the gateway addresses rather than removing addresses be acceptable to accept without running by the spec team since the spec mentions nothing about order.

[k8s-ci-robot]

PR needs rebase.

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-sigs/prow repository.

[mloiseleur]

If this kind of change is accepted on Gateway API, then we can support it.
Following this consensus among external-dns maintainers, I close this PR.

Feel free to re-open it or open a new one if you have a change accepted upstream, by Gateway API maintainers.
/close

[k8s-ci-robot]

@mloiseleur: Closed this PR.

In response to this:

If this kind of change is accepted on Gateway API, then we can support it.
Following this consensus among the maintainers, I close this PR.

Feel free to re-open it or open a new one if you have a change accepted upstream, by Gateway API maintainers.
/close

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-sigs/prow repository.