From 83fc8723a3f02f49eea61e7dc6f49ed29c5ae65e Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Tue, 13 Jul 2021 16:15:04 +0100 Subject: [PATCH] Header manip for service-router plumbed through --- agent/proxycfg/testing.go | 26 ++++++++ agent/xds/routes.go | 59 +++++++++++++++---- ...-with-chain-and-router.envoy-1-18-x.golden | 46 +++++++++++++++ ...in-and-router.v2compat.envoy-1-16-x.golden | 46 +++++++++++++++ ...nd-router-header-manip.envoy-1-18-x.golden | 46 +++++++++++++++ ...-header-manip.v2compat.envoy-1-16-x.golden | 46 +++++++++++++++ ...-with-chain-and-router.envoy-1-18-x.golden | 46 +++++++++++++++ ...in-and-router.v2compat.envoy-1-16-x.golden | 46 +++++++++++++++ 8 files changed, 349 insertions(+), 12 deletions(-) diff --git a/agent/proxycfg/testing.go b/agent/proxycfg/testing.go index 52430feeca..a39acfd671 100644 --- a/agent/proxycfg/testing.go +++ b/agent/proxycfg/testing.go @@ -1281,6 +1281,32 @@ func setupTestVariationConfigEntriesAndSnapshot( }), Destination: toService("split-3-ways"), }, + { + Match: httpMatch(&structs.ServiceRouteHTTPMatch{ + PathExact: "/header-manip", + }), + Destination: &structs.ServiceRouteDestination{ + Service: "header-manip", + RequestHeaders: &structs.HTTPHeaderModifiers{ + Add: map[string]string{ + "request": "bar", + }, + Set: map[string]string{ + "bar": "baz", + }, + Remove: []string{"qux"}, + }, + ResponseHeaders: &structs.HTTPHeaderModifiers{ + Add: map[string]string{ + "response": "bar", + }, + Set: map[string]string{ + "bar": "baz", + }, + Remove: []string{"qux"}, + }, + }, + }, }, }, ) diff --git a/agent/xds/routes.go b/agent/xds/routes.go index 460b2b26bd..00fa44d8b6 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -355,24 +355,23 @@ func makeUpstreamRouteForDiscoveryChain( return nil, err } - if err := injectLBToRouteAction(lb, routeAction.Route); err != nil { - return nil, fmt.Errorf("failed to apply load balancer configuration to route action: %v", err) - } - case structs.DiscoveryGraphNodeTypeResolver: routeAction = makeRouteActionForChainCluster(nextNode.Resolver.Target, chain) - if err := injectLBToRouteAction(lb, routeAction.Route); err != nil { - return nil, fmt.Errorf("failed to apply load balancer configuration to route action: %v", err) - } - default: return nil, fmt.Errorf("unexpected graph node after route %q", nextNode.Type) } + if err := injectLBToRouteAction(lb, routeAction.Route); err != nil { + return nil, fmt.Errorf("failed to apply load balancer configuration to route action: %v", err) + } + // TODO(rb): Better help handle the envoy case where you need (prefix=/foo/,rewrite=/) and (exact=/foo,rewrite=/) to do a full rewrite destination := discoveryRoute.Definition.Destination + + route := &envoy_route_v3.Route{} + if destination != nil { if destination.PrefixRewrite != "" { routeAction.Route.PrefixRewrite = destination.PrefixRewrite @@ -403,12 +402,16 @@ func makeUpstreamRouteForDiscoveryChain( routeAction.Route.RetryPolicy = retryPolicy } + + if err := injectHeaderManipToRoute(destination, route); err != nil { + return nil, fmt.Errorf("failed to apply header manipulation configuration to route: %v", err) + } } - routes = append(routes, &envoy_route_v3.Route{ - Match: routeMatch, - Action: routeAction, - }) + route.Match = routeMatch + route.Action = routeAction + + routes = append(routes, route) } case structs.DiscoveryGraphNodeTypeSplitter: @@ -714,3 +717,35 @@ func injectLBToRouteAction(lb *structs.LoadBalancer, action *envoy_route_v3.Rout action.HashPolicy = result return nil } + +func injectHeaderManipToRoute(dest *structs.ServiceRouteDestination, r *envoy_route_v3.Route) error { + if dest.RequestHeaders != nil { + r.RequestHeadersToAdd = append( + r.RequestHeadersToAdd, + makeHeadersValueOptions(dest.RequestHeaders.Add, true)..., + ) + r.RequestHeadersToAdd = append( + r.RequestHeadersToAdd, + makeHeadersValueOptions(dest.RequestHeaders.Set, false)..., + ) + r.RequestHeadersToRemove = append( + r.RequestHeadersToRemove, + dest.RequestHeaders.Remove..., + ) + } + if dest.ResponseHeaders != nil { + r.ResponseHeadersToAdd = append( + r.ResponseHeadersToAdd, + makeHeadersValueOptions(dest.ResponseHeaders.Add, true)..., + ) + r.ResponseHeadersToAdd = append( + r.ResponseHeadersToAdd, + makeHeadersValueOptions(dest.ResponseHeaders.Set, false)..., + ) + r.ResponseHeadersToRemove = append( + r.ResponseHeadersToRemove, + dest.ResponseHeaders.Remove..., + ) + } + return nil +} diff --git a/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.envoy-1-18-x.golden b/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.envoy-1-18-x.golden index 7e2f58b0d2..5f48cd9720 100644 --- a/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.envoy-1-18-x.golden +++ b/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.envoy-1-18-x.golden @@ -343,6 +343,52 @@ } } }, + { + "match": { + "path": "/header-manip" + }, + "route": { + "cluster": "header-manip.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + }, + "requestHeadersToAdd": [ + { + "header": { + "key": "request", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "requestHeadersToRemove": [ + "qux" + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "response", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "responseHeadersToRemove": [ + "qux" + ] + }, { "match": { "prefix": "/" diff --git a/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.v2compat.envoy-1-16-x.golden b/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.v2compat.envoy-1-16-x.golden index 85d873ab0f..e06e74a39f 100644 --- a/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.v2compat.envoy-1-16-x.golden +++ b/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.v2compat.envoy-1-16-x.golden @@ -343,6 +343,52 @@ } } }, + { + "match": { + "path": "/header-manip" + }, + "route": { + "cluster": "header-manip.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + }, + "requestHeadersToAdd": [ + { + "header": { + "key": "request", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "requestHeadersToRemove": [ + "qux" + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "response", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "responseHeadersToRemove": [ + "qux" + ] + }, { "match": { "prefix": "/" diff --git a/agent/xds/testdata/routes/ingress-with-chain-and-router-header-manip.envoy-1-18-x.golden b/agent/xds/testdata/routes/ingress-with-chain-and-router-header-manip.envoy-1-18-x.golden index 13074bbd75..649b37240f 100644 --- a/agent/xds/testdata/routes/ingress-with-chain-and-router-header-manip.envoy-1-18-x.golden +++ b/agent/xds/testdata/routes/ingress-with-chain-and-router-header-manip.envoy-1-18-x.golden @@ -344,6 +344,52 @@ } } }, + { + "match": { + "path": "/header-manip" + }, + "route": { + "cluster": "header-manip.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + }, + "requestHeadersToAdd": [ + { + "header": { + "key": "request", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "requestHeadersToRemove": [ + "qux" + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "response", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "responseHeadersToRemove": [ + "qux" + ] + }, { "match": { "prefix": "/" diff --git a/agent/xds/testdata/routes/ingress-with-chain-and-router-header-manip.v2compat.envoy-1-16-x.golden b/agent/xds/testdata/routes/ingress-with-chain-and-router-header-manip.v2compat.envoy-1-16-x.golden index b48ded44d1..9d70b06816 100644 --- a/agent/xds/testdata/routes/ingress-with-chain-and-router-header-manip.v2compat.envoy-1-16-x.golden +++ b/agent/xds/testdata/routes/ingress-with-chain-and-router-header-manip.v2compat.envoy-1-16-x.golden @@ -344,6 +344,52 @@ } } }, + { + "match": { + "path": "/header-manip" + }, + "route": { + "cluster": "header-manip.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + }, + "requestHeadersToAdd": [ + { + "header": { + "key": "request", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "requestHeadersToRemove": [ + "qux" + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "response", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "responseHeadersToRemove": [ + "qux" + ] + }, { "match": { "prefix": "/" diff --git a/agent/xds/testdata/routes/ingress-with-chain-and-router.envoy-1-18-x.golden b/agent/xds/testdata/routes/ingress-with-chain-and-router.envoy-1-18-x.golden index 26da960516..ddd97143d8 100644 --- a/agent/xds/testdata/routes/ingress-with-chain-and-router.envoy-1-18-x.golden +++ b/agent/xds/testdata/routes/ingress-with-chain-and-router.envoy-1-18-x.golden @@ -344,6 +344,52 @@ } } }, + { + "match": { + "path": "/header-manip" + }, + "route": { + "cluster": "header-manip.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + }, + "requestHeadersToAdd": [ + { + "header": { + "key": "request", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "requestHeadersToRemove": [ + "qux" + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "response", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "responseHeadersToRemove": [ + "qux" + ] + }, { "match": { "prefix": "/" diff --git a/agent/xds/testdata/routes/ingress-with-chain-and-router.v2compat.envoy-1-16-x.golden b/agent/xds/testdata/routes/ingress-with-chain-and-router.v2compat.envoy-1-16-x.golden index 70048c7b16..787beacb4f 100644 --- a/agent/xds/testdata/routes/ingress-with-chain-and-router.v2compat.envoy-1-16-x.golden +++ b/agent/xds/testdata/routes/ingress-with-chain-and-router.v2compat.envoy-1-16-x.golden @@ -344,6 +344,52 @@ } } }, + { + "match": { + "path": "/header-manip" + }, + "route": { + "cluster": "header-manip.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + }, + "requestHeadersToAdd": [ + { + "header": { + "key": "request", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "requestHeadersToRemove": [ + "qux" + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "response", + "value": "bar" + }, + "append": true + }, + { + "header": { + "key": "bar", + "value": "baz" + }, + "append": false + } + ], + "responseHeadersToRemove": [ + "qux" + ] + }, { "match": { "prefix": "/"