Clarify port redirect behaviour and add conformance tests for the same

[gauravkghildiyal]

What type of PR is this?

/kind documentation
/area conformance
/kind test

What this PR does / why we need it:

Clarify port redirect behaviour and add conformance tests for the same:

// When empty, the redirect port MUST be derived using the following rules:
//
// * If redirect scheme is not-empty, the redirect port MUST be the well-known
//   port associated with the redirect scheme. Specifically "http" to port 80
//   and "https" to port 443. If the redirect scheme does not have a
//   well-known port, the listener port of the Gateway SHOULD be used.
// * If redirect scheme is empty, the redirect port MUST be the Gateway
//   Listener port.

Proposal doc: https://docs.google.com/document/d/1fx8kuWh2bmBge9IRGn7NlhGqTP0Kj21Cz5RZyUW6428

Which issue(s) this PR fixes:

Fixes #1805
Fixes #1909

Does this PR introduce a user-facing change?:

Port redirect when empty will depend on the configured Redirect scheme

[k8s-ci-robot]

Hi @gauravkghildiyal. 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.

[robscott]

Thanks @gauravkghildiyal!

/ok-to-test

[youngnick]

One more small change from me, then this LGTM.

[howardjohn]

As far as I can tell this cannot currently be implemented by Envoy as is, meaning most implementations cannot be conformant. Envoy has 2 modes:

• Set explicit port
• Derive port from Host

But we have a test that Host: ip:80 results in Location: http://ip. Envoy doesn't have a way to say "set no port", as far as I can tell?

I think we need either envoyproxy/envoy#25573 or envoyproxy/envoy#26516

[arkodg]

As far as I can tell this cannot currently be implemented by Envoy as is, meaning most implementations cannot be conformant. Envoy has 2 modes:

• Set explicit port
• Derive port from Host

But we have a test that Host: ip:80 results in Location: http://ip. Envoy doesn't have a way to say "set no port", as far as I can tell?

I think we need either envoyproxy/envoy#25573 or envoyproxy/envoy#26516

The simplest way for to make some progress now would be to set Host: ip instead of Host: ip:80 when sending requests for this test ?

[howardjohn]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gauravkghildiyal, howardjohn, youngnick

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

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

[robscott]

Will wait until tomorrow to remove the hold, but this LGTM as well. Thanks for all the work on this @gauravkghildiyal!

[robscott]

Thanks to everyone for the quick reviews and feedback on this and to @gauravkghildiyal for the great work here! This was discussed at both Gateway API and GAMMA meetings this week and no significant blockers came up. Going to go ahead and merge and transition to RC2 prep.

/lgtm
/hold cancel