Webhook: Validate unique port, protocol per hostname for gateway listeners.

[gyohuangxin]

What type of PR is this?

/kind feature

What this PR does / why we need it:

1. Add validation to validate protocol is unique per hostname for gateway listeners
2. Add validation to validate port is unique per hostname for gateway listeners

Which issue(s) this PR fixes:

Fixes #847

Does this PR introduce a user-facing change?:

NONE

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gyohuangxin Once this PR has been reviewed and has the lgtm label, please assign hbagdi for approval by writing /assign @hbagdi in a comment. For more information see:The Kubernetes 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

[k8s-ci-robot]

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

[gyohuangxin]

@robscott I make it a draft PR because I have some question:

1. As you mentioned in #847:

The validating webhook should ensure that port, protocol, and hostname are unique among Gateway listeners

but then you give an example:

"2 listeners for foo.com would be valid if they were on different ports or protocols, but not if port and protocol were also identical."

They are a bit inconsistent, but my understanding is the webhook should separately validate unique port, protocol per hostname for gateway listeners. Is it correct? Is it needed to verify unique port, protocol across hostnames? Is it needed to verify unique hostname per listener?

2. I add validations in v1beta1 apis, should I add them in v1alpha1 apis too?

[k8s-ci-robot]

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

[shaneutt]

@gyohuangxin where are we at with this one? Is there anything we can do to help the progression here? Looks like maybe we're still waiting on some feedback requested from @robscott?

[gyohuangxin]

Yes, it's on hold because I'm still confused with the first question, I'm not sure my thought is close to the reality.

@robscott I make it a draft PR because I have some question:

1. As you mentioned in Webhook: Validate unique port, protocol, and hostname per listener #847:

The validating webhook should ensure that port, protocol, and hostname are unique among Gateway listeners

but then you give an example:

"2 listeners for foo.com would be valid if they were on different ports or protocols, but not if port and protocol were also identical."

They are a bit inconsistent, but my understanding is the webhook should separately validate unique port, protocol per hostname for gateway listeners. Is it correct? Is it needed to verify unique port, protocol across hostnames? Is it needed to verify unique hostname per listener?

[robscott]

Thanks for the work on this @gyohuangxin! Sorry I lost track of this, don't hesitate to ping me if I can help explain anything better.

[gyohuangxin]

@robscott @shaneutt Can you review this PR? Thanks

[LiorLieberman]

Thanks @gyohuangxin ! This Looks good to me. We probably want to change the PR description to match the new logic - ensuring Listener's port-protocol-hostname combinations are unique within an individual Gateway.

over to @robscott for a final review and approval.

[gyohuangxin]

Thanks @gyohuangxin ! This Looks good to me. We probably want to change the PR description to match the new logic - ensuring Listener's port-protocol-hostname combinations are unique within an individual Gateway.

over to @robscott for a final review and approval.

Sorry for missing the PR description, updated. Thanks for reminding 💯

[dprotaso]

Based on this comment it seems like we should take into consideration the tls properties attached to a listener when determing uniqueness

https://github.com/kubernetes-sigs/gateway-api/issues/1842#issuecomment-1484615419

[gyohuangxin]

Thanks for your comments @dprotaso . As said in @robscott 's https://github.com/kubernetes-sigs/gateway-api/issues/1842#issuecomment-1496560365, the "Hostname" can be different for shared Port and Protocol, like the TLS example on the website: https://gateway-api.sigs.k8s.io/guides/tls/#wildcard-tls-listeners

So, I think it can cover the use cases. Please let me know if there are some use cases that use the same combination of port, protocol and hostname but use distinct tls fields.

[robscott]

Thanks @gyohuangxin!

/approve /ok-to-test

[robscott]

/lgtm /approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arkodg, gyohuangxin, robscott

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~~ [robscott]

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