Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gateway-api: Support all the extended features #27472

Merged
merged 4 commits into from Aug 18, 2023

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Aug 13, 2023

Description

This PR is to support the below extended feature in Gateway API, which is currently missing in profile conformance report https://github.com/kubernetes-sigs/gateway-api/blob/main/conformance/reports/v0.7.1/cilium-cilium.yaml.

  • HTTPRouteSchemeRedirect
  • HTTPRouteHostRewrite
  • HTTPRouteRequestMirror
  • HTTPRoutePathRedirect
  • HTTPRoutePathRewrite
  • HTTPRoutePortRedirect

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 13, 2023
@sayboras
Copy link
Member Author

/ci-gateway-api

@sayboras
Copy link
Member Author

/ci-ingress

@sayboras sayboras force-pushed the tam/gateway-api-extended-feature branch from 53ab756 to 2dbda6e Compare August 15, 2023 12:07
@sayboras
Copy link
Member Author

/test-ingress

@sayboras
Copy link
Member Author

/test-gateway

@sayboras
Copy link
Member Author

/test-gateway-api

@sayboras
Copy link
Member Author

/ci-gateway-api

@sayboras
Copy link
Member Author

/ci-ingress

@sayboras sayboras added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Aug 16, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 16, 2023
@sayboras sayboras force-pushed the tam/gateway-api-extended-feature branch 3 times, most recently from 6e0e633 to 4c7198c Compare August 17, 2023 05:39
@sayboras
Copy link
Member Author

/test

@sayboras sayboras marked this pull request as ready for review August 17, 2023 06:43
@sayboras sayboras requested review from a team as code owners August 17, 2023 06:43
@sayboras sayboras force-pushed the tam/gateway-api-extended-feature branch from 4c7198c to 93aff6f Compare August 17, 2023 12:23
@sayboras
Copy link
Member Author

/test

1 similar comment
@sayboras
Copy link
Member Author

/test

Copy link
Member

@nbusseneau nbusseneau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blind ack for github-sec considering the trivial changes.

Copy link
Member

@mhofstetter mhofstetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks a lot @sayboras ! 🚀 Only some non blocking input from my side (see inline comments).

btw: I didn't check all functionality in detail and hope this is covered by Gateway-API conformance tests too.

operator/pkg/model/ingestion/gateway.go Show resolved Hide resolved
operator/pkg/model/translation/envoy_listener.go Outdated Show resolved Hide resolved
operator/pkg/model/translation/envoy_virtual_host.go Outdated Show resolved Hide resolved
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Aug 18, 2023
This is to add the support for extended feature HTTPRouteHostRewrite.

Signed-off-by: Tam Mach <tam.mach@cilium.io>
For prefix rewrite to work, we need to change the prefix match based on
regex to PathSeparatedPrefix route match, so that the prefix match is
recognized correctly.

Signed-off-by: Tam Mach <tam.mach@cilium.io>
This commit is to enable HTTP redirect for port, schema and path. Small
fix for TLS filter match on server name is done to avoid the below
issue.

As part of recent changes in upstream, if scheme and port are not
specified, gateway listener port must be used.

```console
2023-08-16T11:28:20.797264246Z level=warning msg="NACK received for versions after 52 and up to 53; waiting for a version update before sending again" subsys=xds xdsAckedVersion=52 xdsClientNode="host~127.0.0.1~no-id~localdomain" xdsDetail="Error adding/updating listener(s) gateway-conformance-infra/cilium-gateway-same-namespace-with-https-listener/listener: error adding listener '127.0.0.1:10605': partial wildcards are not supported in \"server_names\"\n" xdsNonce=53 xdsStreamID=2 xdsTypeURL=type.googleapis.com/envoy.config.listener.v3.Listener
```

Signed-off-by: Tam Mach <tam.mach@cilium.io>
All validation logic in HTTPRoute is applied the same as with the normal
backend, as backend for mirror requests is consolidated and merged with
other backends in the same HTTPRoute.

Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras sayboras force-pushed the tam/gateway-api-extended-feature branch from ad757d9 to 76e9da1 Compare August 18, 2023 10:07
@sayboras sayboras removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 18, 2023
@sayboras
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 18, 2023
@sayboras sayboras merged commit cda37f6 into cilium:main Aug 18, 2023
60 checks passed
@sayboras sayboras deleted the tam/gateway-api-extended-feature branch August 18, 2023 11:15
@sayboras sayboras added needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch backport/author The backport will be carried out by the author of the PR. and removed backport/author The backport will be carried out by the author of the PR. needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Aug 21, 2023
@sayboras sayboras added backport/author The backport will be carried out by the author of the PR. needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Aug 23, 2023
@sayboras sayboras mentioned this pull request Aug 23, 2023
2 tasks
@sayboras sayboras added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Aug 23, 2023
@sayboras sayboras mentioned this pull request Aug 23, 2023
2 tasks
@sayboras sayboras added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/author The backport will be carried out by the author of the PR. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. feature/k8s-gateway-api ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants