From 1dd1683ed97814f84df53b6e7ffcabf7b6b0015a Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Tue, 13 Jul 2021 19:49:14 +0100 Subject: [PATCH] Header manip for split legs plumbing --- agent/consul/discoverychain/compile.go | 19 ++++-- agent/proxycfg/testing.go | 33 +++++++++- agent/structs/config_entry_discoverychain.go | 9 ++- agent/structs/discovery_chain.go | 7 +++ agent/xds/routes.go | 39 +++++++++++- ...ith-chain-and-splitter.envoy-1-18-x.golden | 60 ++++++++++++++++++- ...-and-splitter.v2compat.envoy-1-16-x.golden | 60 ++++++++++++++++++- ...ith-chain-and-splitter.envoy-1-18-x.golden | 60 ++++++++++++++++++- ...-and-splitter.v2compat.envoy-1-16-x.golden | 60 ++++++++++++++++++- 9 files changed, 325 insertions(+), 22 deletions(-) diff --git a/agent/consul/discoverychain/compile.go b/agent/consul/discoverychain/compile.go index bd385abdf..007f61d31 100644 --- a/agent/consul/discoverychain/compile.go +++ b/agent/consul/discoverychain/compile.go @@ -417,8 +417,12 @@ func (c *compiler) flattenAdjacentSplitterNodes() { effectiveWeight := split.Weight * innerSplit.Weight / 100 newDiscoverySplit := &structs.DiscoverySplit{ - Weight: structs.NormalizeServiceSplitWeight(effectiveWeight), - NextNode: innerSplit.NextNode, + // Copy the definition from the inner node so any extra config (e.g. + // header manipulation) will be applied to requests taking this + // path. + Definition: innerSplit.Definition, + Weight: structs.NormalizeServiceSplitWeight(effectiveWeight), + NextNode: innerSplit.NextNode, } fixedSplits = append(fixedSplits, newDiscoverySplit) @@ -723,9 +727,16 @@ func (c *compiler) getSplitterNode(sid structs.ServiceID) (*structs.DiscoveryGra c.recordNode(splitNode) var hasLB bool - for _, split := range splitter.Splits { + for i := range splitter.Splits { + // We don't use range variables here because we'll take the address of + // this split and store that in a DiscoveryGraphNode and the range + // variables share memory addresses between iterations which is exactly + // wrong for us here. + split := splitter.Splits[i] + compiledSplit := &structs.DiscoverySplit{ - Weight: split.Weight, + Definition: &split, + Weight: split.Weight, } splitNode.Splits = append(splitNode.Splits, compiledSplit) diff --git a/agent/proxycfg/testing.go b/agent/proxycfg/testing.go index a39acfd67..e035bd04d 100644 --- a/agent/proxycfg/testing.go +++ b/agent/proxycfg/testing.go @@ -1040,9 +1040,36 @@ func setupTestVariationConfigEntriesAndSnapshot( Kind: structs.ServiceSplitter, Name: "db", Splits: []structs.ServiceSplit{ - {Weight: 95.5, Service: "big-side"}, - {Weight: 4, Service: "goldilocks-side"}, - {Weight: 0.5, Service: "lil-bit-side"}, + { + Weight: 95.5, + Service: "big-side", + RequestHeaders: &structs.HTTPHeaderModifiers{ + Set: map[string]string{"x-split-leg": "big"}, + }, + ResponseHeaders: &structs.HTTPHeaderModifiers{ + Set: map[string]string{"x-split-leg": "big"}, + }, + }, + { + Weight: 4, + Service: "goldilocks-side", + RequestHeaders: &structs.HTTPHeaderModifiers{ + Set: map[string]string{"x-split-leg": "goldilocks"}, + }, + ResponseHeaders: &structs.HTTPHeaderModifiers{ + Set: map[string]string{"x-split-leg": "goldilocks"}, + }, + }, + { + Weight: 0.5, + Service: "lil-bit-side", + RequestHeaders: &structs.HTTPHeaderModifiers{ + Set: map[string]string{"x-split-leg": "small"}, + }, + ResponseHeaders: &structs.HTTPHeaderModifiers{ + Set: map[string]string{"x-split-leg": "small"}, + }, + }, }, }, ) diff --git a/agent/structs/config_entry_discoverychain.go b/agent/structs/config_entry_discoverychain.go index cb6ed6319..4af2fecd2 100644 --- a/agent/structs/config_entry_discoverychain.go +++ b/agent/structs/config_entry_discoverychain.go @@ -1490,12 +1490,19 @@ type HTTPHeaderModifiers struct { Remove []string `json:",omitempty"` } +func (m *HTTPHeaderModifiers) IsZero() bool { + if m == nil { + return true + } + return len(m.Add) == 0 && len(m.Set) == 0 && len(m.Remove) == 0 +} + func (m *HTTPHeaderModifiers) Validate(protocol string) error { if m == nil { // Empty is always valid return nil } - if len(m.Add) == 0 && len(m.Set) == 0 && len(m.Remove) == 0 { + if m.IsZero() { return nil } if !IsProtocolHTTPLike(protocol) { diff --git a/agent/structs/discovery_chain.go b/agent/structs/discovery_chain.go index 8ff02b197..86c24515d 100644 --- a/agent/structs/discovery_chain.go +++ b/agent/structs/discovery_chain.go @@ -192,6 +192,13 @@ type DiscoveryRoute struct { // compiled form of ServiceSplit type DiscoverySplit struct { + Definition *ServiceSplit `json:",omitempty"` + // Weight is not necessarily a duplicate of Definition.Weight since when + // multiple splits are compiled down to a single set of splits the effective + // weight of a split leg might not be the same as in the original definition. + // Proxies should use this compiled weight. The Definition is provided above + // for any other significant configuration that the proxy might need to apply + // to that leg of the split. Weight float32 `json:",omitempty"` NextNode string `json:",omitempty"` } diff --git a/agent/xds/routes.go b/agent/xds/routes.go index 00fa44d8b..1a5d6403b 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -633,6 +633,9 @@ func makeRouteActionForSplitter(splits []*structs.DiscoverySplit, chain *structs Weight: makeUint32Value(int(split.Weight * 100)), Name: clusterName, } + if err := injectHeaderManipToWeightedCluster(split.Definition, cw); err != nil { + return nil, err + } clusters = append(clusters, cw) } @@ -719,7 +722,7 @@ func injectLBToRouteAction(lb *structs.LoadBalancer, action *envoy_route_v3.Rout } func injectHeaderManipToRoute(dest *structs.ServiceRouteDestination, r *envoy_route_v3.Route) error { - if dest.RequestHeaders != nil { + if !dest.RequestHeaders.IsZero() { r.RequestHeadersToAdd = append( r.RequestHeadersToAdd, makeHeadersValueOptions(dest.RequestHeaders.Add, true)..., @@ -733,7 +736,7 @@ func injectHeaderManipToRoute(dest *structs.ServiceRouteDestination, r *envoy_ro dest.RequestHeaders.Remove..., ) } - if dest.ResponseHeaders != nil { + if !dest.ResponseHeaders.IsZero() { r.ResponseHeadersToAdd = append( r.ResponseHeadersToAdd, makeHeadersValueOptions(dest.ResponseHeaders.Add, true)..., @@ -749,3 +752,35 @@ func injectHeaderManipToRoute(dest *structs.ServiceRouteDestination, r *envoy_ro } return nil } + +func injectHeaderManipToWeightedCluster(split *structs.ServiceSplit, c *envoy_route_v3.WeightedCluster_ClusterWeight) error { + if !split.RequestHeaders.IsZero() { + c.RequestHeadersToAdd = append( + c.RequestHeadersToAdd, + makeHeadersValueOptions(split.RequestHeaders.Add, true)..., + ) + c.RequestHeadersToAdd = append( + c.RequestHeadersToAdd, + makeHeadersValueOptions(split.RequestHeaders.Set, false)..., + ) + c.RequestHeadersToRemove = append( + c.RequestHeadersToRemove, + split.RequestHeaders.Remove..., + ) + } + if !split.ResponseHeaders.IsZero() { + c.ResponseHeadersToAdd = append( + c.ResponseHeadersToAdd, + makeHeadersValueOptions(split.ResponseHeaders.Add, true)..., + ) + c.ResponseHeadersToAdd = append( + c.ResponseHeadersToAdd, + makeHeadersValueOptions(split.ResponseHeaders.Set, false)..., + ) + c.ResponseHeadersToRemove = append( + c.ResponseHeadersToRemove, + split.ResponseHeaders.Remove..., + ) + } + return nil +} diff --git a/agent/xds/testdata/routes/connect-proxy-with-chain-and-splitter.envoy-1-18-x.golden b/agent/xds/testdata/routes/connect-proxy-with-chain-and-splitter.envoy-1-18-x.golden index 5da88c61d..9dde10444 100644 --- a/agent/xds/testdata/routes/connect-proxy-with-chain-and-splitter.envoy-1-18-x.golden +++ b/agent/xds/testdata/routes/connect-proxy-with-chain-and-splitter.envoy-1-18-x.golden @@ -20,15 +20,69 @@ "clusters": [ { "name": "big-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 9550 + "weight": 9550, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "big" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "big" + }, + "append": false + } + ] }, { "name": "goldilocks-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 400 + "weight": 400, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "goldilocks" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "goldilocks" + }, + "append": false + } + ] }, { "name": "lil-bit-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 50 + "weight": 50, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "small" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "small" + }, + "append": false + } + ] } ], "totalWeight": 10000 diff --git a/agent/xds/testdata/routes/connect-proxy-with-chain-and-splitter.v2compat.envoy-1-16-x.golden b/agent/xds/testdata/routes/connect-proxy-with-chain-and-splitter.v2compat.envoy-1-16-x.golden index efe364bad..e3674f539 100644 --- a/agent/xds/testdata/routes/connect-proxy-with-chain-and-splitter.v2compat.envoy-1-16-x.golden +++ b/agent/xds/testdata/routes/connect-proxy-with-chain-and-splitter.v2compat.envoy-1-16-x.golden @@ -20,15 +20,69 @@ "clusters": [ { "name": "big-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 9550 + "weight": 9550, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "big" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "big" + }, + "append": false + } + ] }, { "name": "goldilocks-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 400 + "weight": 400, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "goldilocks" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "goldilocks" + }, + "append": false + } + ] }, { "name": "lil-bit-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 50 + "weight": 50, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "small" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "small" + }, + "append": false + } + ] } ], "totalWeight": 10000 diff --git a/agent/xds/testdata/routes/ingress-with-chain-and-splitter.envoy-1-18-x.golden b/agent/xds/testdata/routes/ingress-with-chain-and-splitter.envoy-1-18-x.golden index 87b3422e4..225d4ab91 100644 --- a/agent/xds/testdata/routes/ingress-with-chain-and-splitter.envoy-1-18-x.golden +++ b/agent/xds/testdata/routes/ingress-with-chain-and-splitter.envoy-1-18-x.golden @@ -21,15 +21,69 @@ "clusters": [ { "name": "big-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 9550 + "weight": 9550, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "big" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "big" + }, + "append": false + } + ] }, { "name": "goldilocks-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 400 + "weight": 400, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "goldilocks" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "goldilocks" + }, + "append": false + } + ] }, { "name": "lil-bit-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 50 + "weight": 50, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "small" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "small" + }, + "append": false + } + ] } ], "totalWeight": 10000 diff --git a/agent/xds/testdata/routes/ingress-with-chain-and-splitter.v2compat.envoy-1-16-x.golden b/agent/xds/testdata/routes/ingress-with-chain-and-splitter.v2compat.envoy-1-16-x.golden index 4601126e1..ab3e6ac47 100644 --- a/agent/xds/testdata/routes/ingress-with-chain-and-splitter.v2compat.envoy-1-16-x.golden +++ b/agent/xds/testdata/routes/ingress-with-chain-and-splitter.v2compat.envoy-1-16-x.golden @@ -21,15 +21,69 @@ "clusters": [ { "name": "big-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 9550 + "weight": 9550, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "big" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "big" + }, + "append": false + } + ] }, { "name": "goldilocks-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 400 + "weight": 400, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "goldilocks" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "goldilocks" + }, + "append": false + } + ] }, { "name": "lil-bit-side.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "weight": 50 + "weight": 50, + "requestHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "small" + }, + "append": false + } + ], + "responseHeadersToAdd": [ + { + "header": { + "key": "x-split-leg", + "value": "small" + }, + "append": false + } + ] } ], "totalWeight": 10000