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

NET-5530 Support response header modifiers on http-route config entry #18646

Merged
merged 13 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/18646.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:feature
api-gateway: Add support for response header modifiers on http-route configuration entry
```
21 changes: 12 additions & 9 deletions agent/consul/discoverychain/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ type GatewayChainSynthesizer struct {
}

type hostnameMatch struct {
match structs.HTTPMatch
filters structs.HTTPFilters
services []structs.HTTPService
match structs.HTTPMatch
filters structs.HTTPFilters
responseFilters structs.HTTPResponseFilters
services []structs.HTTPService
}

// NewGatewayChainSynthesizer creates a new GatewayChainSynthesizer for the
Expand Down Expand Up @@ -87,9 +88,10 @@ func initHostMatches(hostname string, route *structs.HTTPRouteConfigEntry, curre
// Add all matches for this rule to the list for this hostname
for _, match := range rule.Matches {
matches = append(matches, hostnameMatch{
match: match,
filters: rule.Filters,
services: rule.Services,
match: match,
filters: rule.Filters,
responseFilters: rule.ResponseFilters,
services: rule.Services,
})
}
}
Expand Down Expand Up @@ -226,9 +228,10 @@ func consolidateHTTPRoutes(matchesByHostname map[string][]hostnameMatch, listene
// Add all rules for this hostname
for _, rule := range rules {
route.Rules = append(route.Rules, structs.HTTPRouteRule{
Matches: []structs.HTTPMatch{rule.match},
Filters: rule.filters,
Services: rule.services,
Matches: []structs.HTTPMatch{rule.match},
Filters: rule.filters,
ResponseFilters: rule.responseFilters,
Services: rule.services,
})
}

Expand Down
29 changes: 22 additions & 7 deletions agent/consul/discoverychain/gateway_httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ func httpRouteToDiscoveryChain(route structs.HTTPRouteConfigEntry) (*structs.Ser
var defaults []*structs.ServiceConfigEntry

for idx, rule := range route.Rules {
modifier := httpRouteFiltersToServiceRouteHeaderModifier(rule.Filters.Headers)
requestModifier := httpRouteFiltersToServiceRouteHeaderModifier(rule.Filters.Headers)
responseModifier := httpRouteFiltersToServiceRouteHeaderModifier(rule.ResponseFilters.Headers)
prefixRewrite := httpRouteFiltersToDestinationPrefixRewrite(rule.Filters.URLRewrite)

var destination structs.ServiceRouteDestination
Expand All @@ -90,16 +91,29 @@ func httpRouteToDiscoveryChain(route structs.HTTPRouteConfigEntry) (*structs.Ser
if service.Filters.URLRewrite == nil {
servicePrefixRewrite = prefixRewrite
}
serviceModifier := httpRouteFiltersToServiceRouteHeaderModifier(service.Filters.Headers)
modifier.Add = mergeMaps(modifier.Add, serviceModifier.Add)
modifier.Set = mergeMaps(modifier.Set, serviceModifier.Set)
modifier.Remove = append(modifier.Remove, serviceModifier.Remove...)

// Merge service request header modifier(s) onto route rule modifiers
// Note: Removals for the same header may exist on the rule + the service and
// will result in idempotent duplicate values in the modifier w/ service coming last
Comment on lines +96 to +97
Copy link
Member Author

Choose a reason for hiding this comment

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

Calling this out as pre-existing behavior. I considered doing a merge of the Remove lists but did not want to introduce that as a side effect of this PR

serviceRequestModifier := httpRouteFiltersToServiceRouteHeaderModifier(service.Filters.Headers)
requestModifier.Add = mergeMaps(requestModifier.Add, serviceRequestModifier.Add)
requestModifier.Set = mergeMaps(requestModifier.Set, serviceRequestModifier.Set)
requestModifier.Remove = append(requestModifier.Remove, serviceRequestModifier.Remove...)

// Merge service response header modifier(s) onto route rule modifiers
// Note: Removals for the same header may exist on the rule + the service and
// will result in idempotent duplicate values in the modifier w/ service coming last
serviceResponseModifier := httpRouteFiltersToServiceRouteHeaderModifier(service.ResponseFilters.Headers)
responseModifier.Add = mergeMaps(responseModifier.Add, serviceResponseModifier.Add)
responseModifier.Set = mergeMaps(responseModifier.Set, serviceResponseModifier.Set)
responseModifier.Remove = append(responseModifier.Remove, serviceResponseModifier.Remove...)

destination.Service = service.Name
destination.Namespace = service.NamespaceOrDefault()
destination.Partition = service.PartitionOrDefault()
destination.PrefixRewrite = servicePrefixRewrite
destination.RequestHeaders = modifier
destination.RequestHeaders = requestModifier
destination.ResponseHeaders = responseModifier

// since we have already validated the protocol elsewhere, we
// create a new service defaults here to make sure we pass validation
Expand All @@ -115,7 +129,8 @@ func httpRouteToDiscoveryChain(route structs.HTTPRouteConfigEntry) (*structs.Ser
destination.Namespace = route.NamespaceOrDefault()
destination.Partition = route.PartitionOrDefault()
destination.PrefixRewrite = prefixRewrite
destination.RequestHeaders = modifier
destination.RequestHeaders = requestModifier
destination.ResponseHeaders = responseModifier

splitter := &structs.ServiceSplitterConfigEntry{
Kind: structs.ServiceSplitter,
Expand Down
106 changes: 104 additions & 2 deletions agent/consul/discoverychain/gateway_test.go
Copy link
Member Author

@nathancoleman nathancoleman Sep 7, 2023

Choose a reason for hiding this comment

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

Changes in this test:

  • add coverage for request filter merging where there was none before
  • add coverage for response filter merging added in this PR
  • verify that breaking the modifiers across multiple filter entries for both request + response works as expected
  • verify that service-level modifiers take precedence over rule-level modifiers for both request + response
  • demonstrate the duplicate remove modifier behavior described in the docstrings added above in agent/consul/discoverychain/gateway_httproute.go

Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,70 @@ func TestGatewayChainSynthesizer_Synthesize(t *testing.T) {
Kind: structs.HTTPRoute,
Name: "http-route",
Rules: []structs.HTTPRouteRule{{
Filters: structs.HTTPFilters{
Headers: []structs.HTTPHeaderFilter{
{
Add: map[string]string{"add me to the rule request": "present"},
Set: map[string]string{"set me on the rule request": "present"},
Remove: []string{"remove me from the rule request"},
},
{
Add: map[string]string{"add me to the rule and service request": "rule"},
Set: map[string]string{"set me on the rule and service request": "rule"},
},
{
Remove: []string{"remove me from the rule and service request"},
},
},
},
ResponseFilters: structs.HTTPResponseFilters{
Headers: []structs.HTTPHeaderFilter{{
Add: map[string]string{
"add me to the rule response": "present",
"add me to the rule and service response": "rule",
},
Set: map[string]string{
"set me on the rule response": "present",
"set me on the rule and service response": "rule",
},
Remove: []string{
"remove me from the rule response",
"remove me from the rule and service response",
},
}},
},
Services: []structs.HTTPService{{
Name: "foo",
Filters: structs.HTTPFilters{
Headers: []structs.HTTPHeaderFilter{
{
Add: map[string]string{"add me to the service request": "present"},
},
{
Set: map[string]string{"set me on the service request": "present"},
Remove: []string{"remove me from the service request"},
},
{
Add: map[string]string{"add me to the rule and service request": "service"},
Set: map[string]string{"set me on the rule and service request": "service"},
Remove: []string{"remove me from the rule and service request"},
},
},
},
ResponseFilters: structs.HTTPResponseFilters{
Headers: []structs.HTTPHeaderFilter{
{
Add: map[string]string{"add me to the service response": "present"},
Set: map[string]string{"set me on the service response": "present"},
Remove: []string{"remove me from the service response"},
},
{
Add: map[string]string{"add me to the rule and service response": "service"},
Set: map[string]string{"set me on the rule and service response": "service"},
Remove: []string{"remove me from the rule and service response"},
},
},
},
}},
}},
},
Expand Down Expand Up @@ -557,8 +619,40 @@ func TestGatewayChainSynthesizer_Synthesize(t *testing.T) {
Partition: "default",
Namespace: "default",
RequestHeaders: &structs.HTTPHeaderModifiers{
Add: make(map[string]string),
Set: make(map[string]string),
Add: map[string]string{
"add me to the rule request": "present",
"add me to the service request": "present",
"add me to the rule and service request": "service",
Copy link
Member Author

Choose a reason for hiding this comment

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

we expect service as the value here, indicating that the filter modifier on the service took precedence over the same modifier on the rule which specifies rule as the value.

this pattern applies to the other modifiers below as well.

},
Set: map[string]string{
"set me on the rule request": "present",
"set me on the service request": "present",
"set me on the rule and service request": "service",
},
Remove: []string{
"remove me from the rule request",
"remove me from the rule and service request",
"remove me from the service request",
"remove me from the rule and service request",
},
},
ResponseHeaders: &structs.HTTPHeaderModifiers{
Add: map[string]string{
"add me to the rule response": "present",
"add me to the service response": "present",
"add me to the rule and service response": "service",
},
Set: map[string]string{
"set me on the rule response": "present",
"set me on the service response": "present",
"set me on the rule and service response": "service",
},
Remove: []string{
"remove me from the rule response",
"remove me from the rule and service response",
"remove me from the service response",
"remove me from the rule and service response",
},
},
},
},
Expand Down Expand Up @@ -663,6 +757,10 @@ func TestGatewayChainSynthesizer_Synthesize(t *testing.T) {
Add: make(map[string]string),
Set: make(map[string]string),
},
ResponseHeaders: &structs.HTTPHeaderModifiers{
Add: make(map[string]string),
Set: make(map[string]string),
},
},
},
NextNode: "resolver:foo-2.default.default.dc2",
Expand Down Expand Up @@ -850,6 +948,10 @@ func TestGatewayChainSynthesizer_ComplexChain(t *testing.T) {
Add: make(map[string]string),
Set: make(map[string]string),
},
ResponseHeaders: &structs.HTTPHeaderModifiers{
Add: make(map[string]string),
Set: make(map[string]string),
},
},
},
NextNode: "splitter:splitter-one.default.default",
Expand Down
13 changes: 13 additions & 0 deletions agent/structs/config_entry_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,12 @@ type HTTPFilters struct {
JWT *JWTFilter
}

// HTTPResponseFilters specifies a list of filters used to modify the
// response returned by an upstream
type HTTPResponseFilters struct {
Headers []HTTPHeaderFilter
}

// HTTPHeaderFilter specifies how HTTP headers should be modified.
type HTTPHeaderFilter struct {
Add map[string]string
Expand Down Expand Up @@ -486,6 +492,9 @@ type HTTPRouteRule struct {
// Filters is a list of HTTP-based filters used to modify a request prior
// to routing it to the upstream service
Filters HTTPFilters
// ResponseFilters is a list of HTTP-based filters used to modify a response
// returned by the upstream service
ResponseFilters HTTPResponseFilters
// Matches specified the matching criteria used in the routing table. If a
// request matches the given HTTPMatch configuration, then traffic is routed
// to services specified in the Services field.
Expand All @@ -505,6 +514,10 @@ type HTTPService struct {
// to routing it to the upstream service
Filters HTTPFilters

// ResponseFilters is a list of HTTP-based filters used to modify the
// response returned from the upstream service
ResponseFilters HTTPResponseFilters

acl.EnterpriseMeta `hcl:",squash" mapstructure:",squash"`
}

Expand Down
14 changes: 14 additions & 0 deletions api/config_entry_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,12 @@ type HTTPFilters struct {
JWT *JWTFilter
}

// HTTPResponseFilters specifies a list of filters used to modify a
// response returned by an upstream
type HTTPResponseFilters struct {
Headers []HTTPHeaderFilter
}

// HTTPHeaderFilter specifies how HTTP headers should be modified.
type HTTPHeaderFilter struct {
Add map[string]string
Expand Down Expand Up @@ -238,6 +244,9 @@ type HTTPRouteRule struct {
// Filters is a list of HTTP-based filters used to modify a request prior
// to routing it to the upstream service
Filters HTTPFilters
// ResponseFilters is a list of HTTP-based filters used to modify a response
// returned by the upstream service
ResponseFilters HTTPResponseFilters
// Matches specified the matching criteria used in the routing table. If a
// request matches the given HTTPMatch configuration, then traffic is routed
// to services specified in the Services field.
Expand All @@ -253,10 +262,15 @@ type HTTPService struct {
// Weight is an arbitrary integer used in calculating how much
// traffic should be sent to the given service.
Weight int

// Filters is a list of HTTP-based filters used to modify a request prior
// to routing it to the upstream service
Filters HTTPFilters

// ResponseFilters is a list of HTTP-based filters used to modify the
// response returned from the upstream service
ResponseFilters HTTPResponseFilters

// Partition is the partition the config entry is associated with.
// Partitioning is a Consul Enterprise feature.
Partition string `json:",omitempty"`
Expand Down
44 changes: 44 additions & 0 deletions proto/private/pbconfigentry/config_entry.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading