feat: add support for URLRewrite filter and refactor GWAPI translator by Xunzhuo · Pull Request #819 · envoyproxy/gateway

Xunzhuo · 2022-12-17T06:20:04Z

Resolves: #564

Signed-off-by: bitliu bitliu@tencent.com

codecov-commenter · 2022-12-17T06:43:42Z

Codecov Report

Merging #819 (7935e3b) into main (5d11bbe) will increase coverage by 0.01%.
The diff coverage is 86.47%.

❗ Current head 7935e3b differs from pull request most recent head 65ec238. Consider uploading reports for the commit 65ec238 to get more accurate results

@@            Coverage Diff             @@
##             main     #819      +/-   ##
==========================================
+ Coverage   64.33%   64.35%   +0.01%     
==========================================
  Files          46       51       +5     
  Lines        6189     6391     +202     
==========================================
+ Hits         3982     4113     +131     
- Misses       1973     2036      +63     
- Partials      234      242       +8

Impacted Files	Coverage Δ
internal/gatewayapi/translator.go	`96.77% <ø> (+7.10%)`	⬆️
internal/ir/zz_generated.deepcopy.go	`17.30% <0.00%> (-1.06%)`	⬇️
internal/gatewayapi/resource.go	`58.62% <58.62%> (ø)`
internal/gatewayapi/filters.go	`80.08% <80.08%> (ø)`
internal/gatewayapi/route.go	`89.00% <89.00%> (ø)`
internal/gatewayapi/validate.go	`92.16% <92.16%> (ø)`
internal/gatewayapi/helpers.go	`81.08% <93.87%> (+3.95%)`	⬆️
internal/gatewayapi/listener.go	`100.00% <100.00%> (ø)`
internal/ir/xds.go	`79.54% <100.00%> (+0.97%)`	⬆️
internal/xds/translator/route.go	`85.90% <100.00%> (+3.99%)`	⬆️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

qicz · 2022-12-19T08:38:52Z

hello, I am testing this feature, but an error occurred

2022-12-19T07:36:02.079Z        INFO    cache/logrwrapper.go:29 Got a new request, response_nonce 6, nodeID envoy-default-eg-63636f73-848c6454dd-xnb8v, node_version v1.24.1 {"runner": "xds-server"}
2022-12-19T07:36:02.079Z        INFO    cache/logrwrapper.go:29 handling v3 xDS resource request, response_nonce 6, nodeID envoy-default-eg-63636f73-848c6454dd-xnb8v, node_version v1.24.1, resource_names_subscribe [], resource_names_unsubscribe [], type_url type.googleapis.com/envoy.config.route.v3.RouteConfiguration, errorCode 13, errorMessage Proto constraint validation failed (RouteConfigurationValidationError.VirtualHosts[0]: embedded message failed validation | caused by VirtualHostValidationError.Routes[0]: embedded message failed validation | caused by RouteValidationError.Route: embedded message failed validation | caused by field: "cluster_specifier", reason: is required):

I fixed like this, it works

func buildXdsURLRewriteAction(rou *route.Route, routeName string, urlRewrite *ir.URLRewrite) *route.RouteAction {
	ret := &route.RouteAction{
		ClusterSpecifier: &route.RouteAction_Cluster{
			Cluster: routeName,
		}}

qicz · 2022-12-19T09:42:57Z

rewrite the hostname may be set AppendXForwardedHost to true

// Types that are assignable to HostRewriteSpecifier:
	//	*RouteAction_HostRewriteLiteral
	//	*RouteAction_AutoHostRewrite
	//	*RouteAction_HostRewriteHeader
	//	*RouteAction_HostRewritePathRegex
	HostRewriteSpecifier isRouteAction_HostRewriteSpecifier `protobuf_oneof:"host_rewrite_specifier"`
	// If set, then a host rewrite action (one of
	// :ref:`host_rewrite_literal <envoy_v3_api_field_config.route.v3.RouteAction.host_rewrite_literal>`,
	// :ref:`auto_host_rewrite <envoy_v3_api_field_config.route.v3.RouteAction.auto_host_rewrite>`,
	// :ref:`host_rewrite_header <envoy_v3_api_field_config.route.v3.RouteAction.host_rewrite_header>`, or
	// :ref:`host_rewrite_path_regex <envoy_v3_api_field_config.route.v3.RouteAction.host_rewrite_path_regex>`)
	// causes the original value of the host header, if any, to be appended to the
	// :ref:`config_http_conn_man_headers_x-forwarded-host` HTTP header.
	AppendXForwardedHost bool `protobuf:"varint,38,opt,name=append_x_forwarded_host,json=appendXForwardedHost,proto3" json:"append_x_forwarded_host,omitempty"`

Xunzhuo · 2022-12-19T10:46:55Z

Thank you @qicz, though this PR is WIP, but thanks for testing it, I will update this PR ASAP.

docs/latest/user/http-urlrewrite.md

internal/gatewayapi/translator.go

internal/xds/translator/route.go

arkodg · 2022-12-21T19:27:03Z

the PR looks good @Xunzhuo ! thanks for all attached the end user docs as part of this PR. left some comments

Signed-off-by: bitliu <bitliu@tencent.com>

arkodg

LGTM, thanks for the refactor too !

Xunzhuo changed the title ~~feat: add support for urlRewrite filter~~ feat: add support for URLRewrite filter Dec 17, 2022

Xunzhuo force-pushed the feat-urlrewrite branch from 869682c to ffb1cc1 Compare December 17, 2022 06:40

Xunzhuo force-pushed the feat-urlrewrite branch 5 times, most recently from a448a03 to 6f88a33 Compare December 17, 2022 07:23

Xunzhuo force-pushed the feat-urlrewrite branch 2 times, most recently from d9810e3 to c7e0e99 Compare December 21, 2022 11:17

Xunzhuo marked this pull request as ready for review December 21, 2022 11:17

Xunzhuo requested a review from a team as a code owner December 21, 2022 11:17

Xunzhuo force-pushed the feat-urlrewrite branch 4 times, most recently from 9149dca to be6f299 Compare December 21, 2022 11:28