From 79ae5b194802f71117e6c0fedc8c48d8efded11a Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Tue, 24 Mar 2026 23:37:59 +0530 Subject: [PATCH 01/11] fix: prevent set from being forwarded to client --- .../adjusting-cache-control.mdx | 4 +- .../subgraph-response-header-operations.mdx | 42 +++++++++++---- .../protocol/header_propagation_race_test.go | 10 ++-- router-tests/protocol/header_set_test.go | 8 +-- router/core/header_rule_engine.go | 5 +- router/core/header_rule_engine_test.go | 53 +++++++++++++++++++ 6 files changed, 98 insertions(+), 24 deletions(-) diff --git a/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx b/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx index f2e7317646..56b6fa3492 100644 --- a/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx +++ b/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx @@ -133,7 +133,7 @@ By combining these mechanisms, the algorithm ensures that data handling adheres ### Overriding Cache Control Policy - By using the `set` operation in their header propagation rules, users can overwrite the cache control policy if necessary. + By using the `set` operation in their header propagation rules, users can override the cache control algorithm with an explicit value. When a `set` rule targets the `Cache-Control` header, the restrictive algorithm is skipped and the set value is forwarded to the client instead. For example, a configuration can be set like: @@ -152,4 +152,4 @@ headers: value: "max-age=5400" ``` -For this configuration, any request which hits the `specific-subgraph` will have the desired subgraph cache control value set (`max-age=5400`). +For this configuration, any request which hits the `specific-subgraph` will have `Cache-Control: max-age=5400` set on the client response, bypassing the restrictive cache control algorithm. diff --git a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx index c2ad93ebfe..51089ad1ed 100644 --- a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx +++ b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx @@ -53,10 +53,6 @@ headers: default: "123" # Set the value when the header was not set algorithm: "last_write" - - op: "set" - name: "X-Custom-Header" - value: "my-required-key" - subgraphs: specific-subgraph: # Will only affect this subgraph response: @@ -68,7 +64,7 @@ headers: ### What does the snippet do? -With `all` we address all subgraph requests. Next, we can define several rules on the client's request. The operation `propagate` forwards all matching client request headers to the subgraphs. The operation `set` sets a new header which is forward to the subgraphs. +With `all` we address all subgraph responses. Next, we can define several rules on the response headers. The operation `propagate` forwards matching subgraph response headers to the client. The operation `set` stores a header value internally for use by the router (e.g., to override cache control behavior) but does not forward it to the client. The `subgraphs` section allows to propagate headers for specific subgraphs. The name must match with the subgraph name in the Studio. @@ -90,12 +86,36 @@ Currently, we support the following header rules: * `default` - Fallback to this value when the `named`, `matching` or `rename` header could not be found. -* `set` - Sets a header on the request forward to the subgraph. You must set the following values: - - * `name` - The name of the header to set - - * `value` - The value to set for the header - Go canonicalizes headers by default e.g. `x-my-header` to `X-My-Header.` Write your rule accordingly or use `(?i)``^X-Test-.*` flags to make your regex case insensitive. + +## Response Header Set + +The `set` operation on response rules behaves differently depending on the header: + +- **`Cache-Control`**: The value is forwarded to the client response and the restrictive cache control algorithm is disabled. This lets you override computed cache control with an explicit value. +- **All other headers**: The value is **not** forwarded to the client response. If you need to add custom headers to client responses, use [Router Response Header Operations](/router/proxy-capabilities/router-response-header-operations) instead. + +### Configuration + +* `name` - The name of the header to set +* `value` - The value to set for the header + +### Example: Overriding Cache Control for a Subgraph + +```yaml +cache_control_policy: + enabled: true + value: "max-age=180, public" + +headers: + subgraphs: + specific-subgraph: + response: + - op: "set" + name: "Cache-Control" + value: "max-age=5400" +``` + +In this example, any request that hits `specific-subgraph` will have `Cache-Control: max-age=5400` set on the client response, overriding the restrictive cache control algorithm. diff --git a/router-tests/protocol/header_propagation_race_test.go b/router-tests/protocol/header_propagation_race_test.go index 70663f2291..1576cc48dc 100644 --- a/router-tests/protocol/header_propagation_race_test.go +++ b/router-tests/protocol/header_propagation_race_test.go @@ -31,7 +31,7 @@ func TestHeaderPropagationConcurrentMapWrites(t *testing.T) { const expectedResponse = `{"data":{"employees":[{"id":1,"isAvailable":false,"hobbies":[{},{"name":"Counter Strike"},{},{},{}]},{"id":2,"isAvailable":false,"hobbies":[{},{"name":"Counter Strike"},{}]},{"id":3,"isAvailable":false,"hobbies":[{},{},{},{}]},{"id":4,"isAvailable":false,"hobbies":[{},{},{}]},{"id":5,"isAvailable":false,"hobbies":[{},{},{}]},{"id":7,"isAvailable":false,"hobbies":[{"name":"Chess"},{}]},{"id":8,"isAvailable":false,"hobbies":[{},{"name":"Miscellaneous"},{}]},{"id":10,"isAvailable":false,"hobbies":[{},{},{},{},{},{}]},{"id":11,"isAvailable":false,"hobbies":[{}]},{"id":12,"isAvailable":false,"hobbies":[{},{},{"name":"Miscellaneous"},{}]}]}}` - t.Run("response set rule with parallel subgraph fetches", func(t *testing.T) { + t.Run("response set rule with parallel subgraph fetches is internal only", func(t *testing.T) { testenv.Run(t, &testenv.Config{ RouterOptions: []core.Option{ core.WithHeaderRules(config.HeaderRules{ @@ -53,7 +53,7 @@ func TestHeaderPropagationConcurrentMapWrites(t *testing.T) { require.Equal(t, http.StatusOK, res.Response.StatusCode) require.Equal(t, expectedResponse, res.Body) - require.Equal(t, "test-value", res.Response.Header.Get("X-Custom-Header"), "single request failed") + require.Equal(t, "", res.Response.Header.Get("X-Custom-Header"), "set response headers should not be forwarded to the client") }) }) @@ -92,7 +92,7 @@ func TestHeaderPropagationConcurrentMapWrites(t *testing.T) { }) }) - t.Run("multiple response set rules with parallel subgraph fetches", func(t *testing.T) { + t.Run("multiple response set rules with parallel subgraph fetches are internal only", func(t *testing.T) { testenv.Run(t, &testenv.Config{ RouterOptions: []core.Option{ core.WithHeaderRules(config.HeaderRules{ @@ -119,8 +119,8 @@ func TestHeaderPropagationConcurrentMapWrites(t *testing.T) { require.Equal(t, http.StatusOK, res.Response.StatusCode) require.Equal(t, expectedResponse, res.Body) - require.Equal(t, "value-a", res.Response.Header.Get("X-Header-A"), "single request failed") - require.Equal(t, "value-b", res.Response.Header.Get("X-Header-B"), "single request failed") + require.Equal(t, "", res.Response.Header.Get("X-Header-A"), "set response headers should not be forwarded to the client") + require.Equal(t, "", res.Response.Header.Get("X-Header-B"), "set response headers should not be forwarded to the client") }) }) } diff --git a/router-tests/protocol/header_set_test.go b/router-tests/protocol/header_set_test.go index 0b973910d6..dfd8810b23 100644 --- a/router-tests/protocol/header_set_test.go +++ b/router-tests/protocol/header_set_test.go @@ -128,7 +128,7 @@ func TestHeaderSet(t *testing.T) { }) }) - t.Run("global set works", func(t *testing.T) { + t.Run("global set is internal only", func(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{ RouterOptions: global(customHeader, hobbyVal), @@ -137,12 +137,12 @@ func TestHeaderSet(t *testing.T) { Query: queryEmployeeWithHobby, }) ch := strings.Join(res.Response.Header.Values(customHeader), ",") - require.Equal(t, hobbyVal, ch) + require.Equal(t, "", ch, "set response headers should not be forwarded to the client") require.Equal(t, `{"data":{"employee":{"id":1,"hobbies":[{},{"name":"Counter Strike"},{},{},{}]}}}`, res.Body) }) }) - t.Run("subgraph set works", func(t *testing.T) { + t.Run("subgraph set is internal only", func(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{ RouterOptions: partial(customHeader, employeeVal), @@ -151,7 +151,7 @@ func TestHeaderSet(t *testing.T) { Query: queryEmployeeWithHobby, }) ch := strings.Join(res.Response.Header.Values(customHeader), ",") - require.Equal(t, employeeVal, ch) + require.Equal(t, "", ch, "set response headers should not be forwarded to the client") require.Equal(t, `{"data":{"employee":{"id":1,"hobbies":[{},{"name":"Counter Strike"},{},{},{}]}}}`, res.Body) }) }) diff --git a/router/core/header_rule_engine.go b/router/core/header_rule_engine.go index 784411b75d..e96e0bd20e 100644 --- a/router/core/header_rule_engine.go +++ b/router/core/header_rule_engine.go @@ -457,9 +457,10 @@ func (h *HeaderPropagation) OnOriginResponse(resp *http.Response, ctx RequestCon func (h *HeaderPropagation) applyResponseRule(propagation *responseHeaderPropagation, res *http.Response, rule *config.ResponseHeaderRule) { if rule.Operation == config.HeaderRuleOperationSet { propagation.m.Lock() - propagation.header.Set(rule.Name, rule.Value) if rule.Name == cacheControlKey { - // Handle the case where the cache control header is set explicitly + // Cache-Control set rules are forwarded to the client and disable the + // restrictive cache control algorithm. + propagation.header.Set(rule.Name, rule.Value) propagation.setCacheControl = true } propagation.m.Unlock() diff --git a/router/core/header_rule_engine_test.go b/router/core/header_rule_engine_test.go index 6a8f2711f5..42582832b0 100644 --- a/router/core/header_rule_engine_test.go +++ b/router/core/header_rule_engine_test.go @@ -258,6 +258,59 @@ func TestApplyResponseRuleKeyValue(t *testing.T) { }) } +func TestApplyResponseRuleSetNotForwarded(t *testing.T) { + t.Parallel() + + newPropagation := func() *responseHeaderPropagation { + return &responseHeaderPropagation{ + header: make(http.Header), + m: &sync.Mutex{}, + } + } + + hp := &HeaderPropagation{} + + t.Run("set does not write to client header", func(t *testing.T) { + t.Parallel() + prop := newPropagation() + rule := &config.ResponseHeaderRule{ + Operation: config.HeaderRuleOperationSet, + Name: "X-Custom", + Value: "test-value", + } + hp.applyResponseRule(prop, &http.Response{}, rule) + require.Equal(t, "", prop.header.Get("X-Custom")) + }) + + t.Run("set Cache-Control writes to client header and sets flag", func(t *testing.T) { + t.Parallel() + prop := newPropagation() + rule := &config.ResponseHeaderRule{ + Operation: config.HeaderRuleOperationSet, + Name: "Cache-Control", + Value: "max-age=300", + } + hp.applyResponseRule(prop, &http.Response{}, rule) + require.Equal(t, "max-age=300", prop.header.Get("Cache-Control")) + require.True(t, prop.setCacheControl) + }) + + t.Run("propagate still writes to client header", func(t *testing.T) { + t.Parallel() + prop := newPropagation() + rule := &config.ResponseHeaderRule{ + Operation: config.HeaderRuleOperationPropagate, + Named: "X-Custom", + Algorithm: config.ResponseHeaderRuleAlgorithmFirstWrite, + } + res := &http.Response{ + Header: http.Header{"X-Custom": []string{"from-subgraph"}}, + } + hp.applyResponseRule(prop, res, rule) + require.Equal(t, "from-subgraph", prop.header.Get("X-Custom")) + }) +} + func TestPropagatedHeaders(t *testing.T) { t.Parallel() From 874bce1e1df2c1e63b0b8d28118e1cd504249dca Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Thu, 26 Mar 2026 23:35:56 +0530 Subject: [PATCH 02/11] fix: cache updates --- .../adjusting-cache-control.mdx | 16 ++- .../subgraph-response-header-operations.mdx | 35 +++++- router-tests/operations/singleflight_test.go | 48 ++++---- .../protocol/header_propagation_test.go | 27 +++-- router-tests/protocol/header_set_test.go | 107 ++++++++++++++++++ router/core/header_rule_engine.go | 45 ++++---- router/core/header_rule_engine_test.go | 83 +++++++------- router/pkg/config/config.go | 4 + 8 files changed, 254 insertions(+), 111 deletions(-) diff --git a/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx b/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx index 56b6fa3492..c977bcc378 100644 --- a/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx +++ b/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx @@ -130,10 +130,10 @@ The algorithm evaluates the following in order: By combining these mechanisms, the algorithm ensures that data handling adheres to the strictest cache control settings from all subgraph responses, promoting both security and performance integrity. Users can define global defaults to enforce a baseline cache policy, and can rely on `no-cache` or `no-store` directives for security sensitive subgraphs. -### Overriding Cache Control Policy +### Influencing Cache Control via Set Rules - By using the `set` operation in their header propagation rules, users can override the cache control algorithm with an explicit value. When a `set` rule targets the `Cache-Control` header, the restrictive algorithm is skipped and the set value is forwarded to the client instead. + By using the `set` operation in their header propagation rules, users can inject a `Cache-Control` value into a subgraph's response. The restrictive algorithm then includes this value when computing the most restrictive policy across all subgraph responses. For example, a configuration can be set like: @@ -152,4 +152,14 @@ headers: value: "max-age=5400" ``` -For this configuration, any request which hits the `specific-subgraph` will have `Cache-Control: max-age=5400` set on the client response, bypassing the restrictive cache control algorithm. +For this configuration, any request which hits the `specific-subgraph` will have `Cache-Control: max-age=5400` injected into the subgraph response. The restrictive cache control algorithm then considers this value alongside other subgraph responses to compute the final `Cache-Control` header sent to the client. + +This is however equivalent to the following + +``` +cache_control_policy: + enabled: true + value: "max-age=180, public" + - name: "specific-subgraph" + value: "max-age=5400, public" +``` diff --git a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx index 51089ad1ed..b91d1928d1 100644 --- a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx +++ b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx @@ -64,7 +64,7 @@ headers: ### What does the snippet do? -With `all` we address all subgraph responses. Next, we can define several rules on the response headers. The operation `propagate` forwards matching subgraph response headers to the client. The operation `set` stores a header value internally for use by the router (e.g., to override cache control behavior) but does not forward it to the client. +With `all` we address all subgraph responses. Next, we can define several rules on the response headers. The operation `propagate` forwards matching subgraph response headers to the client. The operation `set` injects a header value into the subgraph response so it can be processed by downstream rules (e.g., the cache control algorithm or propagate rules). The `subgraphs` section allows to propagate headers for specific subgraphs. The name must match with the subgraph name in the Studio. @@ -90,19 +90,35 @@ Currently, we support the following header rules: Go canonicalizes headers by default e.g. `x-my-header` to `X-My-Header.` Write your rule accordingly or use `(?i)``^X-Test-.*` flags to make your regex case insensitive. +## Order of Execution + +Response header rules are applied per subgraph fetch in the following order: + +1. **`all` rules** — Rules defined under `headers.all.response` are applied first, in the order they are defined. +2. **Subgraph-specific rules** — Rules defined under `headers.subgraphs..response` are applied next, in the order they are defined. +3. **Cache control policy** — The `cache_control_policy` rules run last. This ensures that all `set` rules (both global and subgraph-specific) have already injected their values into the subgraph response before the [restrictive cache control algorithm](/router/proxy-capabilities/adjusting-cache-control) reads them. + +Within each scope, rules execute in definition order. This means a `set` rule defined before a `propagate` rule in the same scope will inject the value into the subgraph response before the `propagate` rule evaluates it. + ## Response Header Set -The `set` operation on response rules behaves differently depending on the header: +The `set` operation injects a header value into the subgraph response, making it appear as if the subgraph returned it. This value is then processed by downstream rules (e.g., `propagate` rules or the [restrictive cache control algorithm](/router/proxy-capabilities/adjusting-cache-control)). + + +The `set` value is **not** forwarded to the client response. + + -- **`Cache-Control`**: The value is forwarded to the client response and the restrictive cache control algorithm is disabled. This lets you override computed cache control with an explicit value. -- **All other headers**: The value is **not** forwarded to the client response. If you need to add custom headers to client responses, use [Router Response Header Operations](/router/proxy-capabilities/router-response-header-operations) instead. + +You can use `set`, to set a header value and then use a `propagate` rule to propagate that same header to the client. Howevever we would caution against doing this, as no client header will be propagated in case no subgraph was actually called. + ### Configuration * `name` - The name of the header to set * `value` - The value to set for the header -### Example: Overriding Cache Control for a Subgraph +### Example: Setting Cache Control on a Subgraph Response ```yaml cache_control_policy: @@ -118,4 +134,11 @@ headers: value: "max-age=5400" ``` -In this example, any request that hits `specific-subgraph` will have `Cache-Control: max-age=5400` set on the client response, overriding the restrictive cache control algorithm. +In this example, when a request hits `specific-subgraph`, the `Cache-Control: max-age=5400` value is injected into the subgraph response. The restrictive cache control algorithm then processes this value alongside other subgraph responses to compute the final `Cache-Control` header sent to the client. This is equivalent to doing + +``` +cache_control_policy: + enabled: true + value: "max-age=180, public" + +``` \ No newline at end of file diff --git a/router-tests/operations/singleflight_test.go b/router-tests/operations/singleflight_test.go index 29fb64f583..e30ce6ab55 100644 --- a/router-tests/operations/singleflight_test.go +++ b/router-tests/operations/singleflight_test.go @@ -958,7 +958,7 @@ func TestSingleFlight(t *testing.T) { } }) }) - t.Run("response header set rule with singleflight followers", func(t *testing.T) { + t.Run("response header set rule with singleflight followers is internal only", func(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{ Subgraphs: testenv.SubgraphsConfig{ @@ -985,8 +985,8 @@ func TestSingleFlight(t *testing.T) { responses := runConcurrentSingleflightRequests(t, xEnv, `{ employee(id: 1) { id } }`, 5) for i, res := range responses { require.Equal(t, `{"data":{"employee":{"id":1}}}`, res.Body) - require.Equal(t, "test-value", res.Response.Header.Get("X-Custom-Header"), - "response %d missing X-Custom-Header", i) + require.Equal(t, "", res.Response.Header.Get("X-Custom-Header"), + "response %d: set response headers should not be forwarded to the client", i) } }) }) @@ -1028,7 +1028,7 @@ func TestSingleFlight(t *testing.T) { } }) }) - t.Run("multiple response set rules with singleflight followers", func(t *testing.T) { + t.Run("multiple response set rules with singleflight followers are internal only", func(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{ Subgraphs: testenv.SubgraphsConfig{ @@ -1057,23 +1057,23 @@ func TestSingleFlight(t *testing.T) { }), }, }, func(t *testing.T, xEnv *testenv.Environment) { - // Verify single request works + // Verify single request works — set headers are internal only res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ Query: `{ employee(id: 1) { id } }`, }) - require.Equal(t, "value-a", res.Response.Header.Get("X-Header-A"), "single request should have X-Header-A") - require.Equal(t, "value-b", res.Response.Header.Get("X-Header-B"), "single request should have X-Header-B") + require.Equal(t, "", res.Response.Header.Get("X-Header-A"), "set response headers should not be forwarded to the client") + require.Equal(t, "", res.Response.Header.Get("X-Header-B"), "set response headers should not be forwarded to the client") responses := runConcurrentSingleflightRequests(t, xEnv, `{ employee(id: 1) { id } }`, 5) for i, res := range responses { - require.Equal(t, "value-a", res.Response.Header.Get("X-Header-A"), - "response %d missing X-Header-A", i) - require.Equal(t, "value-b", res.Response.Header.Get("X-Header-B"), - "response %d missing X-Header-B", i) + require.Equal(t, "", res.Response.Header.Get("X-Header-A"), + "response %d: set response headers should not be forwarded to the client", i) + require.Equal(t, "", res.Response.Header.Get("X-Header-B"), + "response %d: set response headers should not be forwarded to the client", i) } }) }) - t.Run("multi-subgraph response header propagation with singleflight", func(t *testing.T) { + t.Run("multi-subgraph response set with singleflight is internal only", func(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{ Subgraphs: testenv.SubgraphsConfig{ @@ -1103,12 +1103,12 @@ func TestSingleFlight(t *testing.T) { responses := runConcurrentSingleflightRequests(t, xEnv, query, 5) for i, res := range responses { require.Contains(t, res.Body, `"employee"`) - require.Equal(t, "multi-subgraph-value", res.Response.Header.Get("X-Custom-Header"), - "response %d missing X-Custom-Header from multi-subgraph query", i) + require.Equal(t, "", res.Response.Header.Get("X-Custom-Header"), + "response %d: set response headers should not be forwarded to the client", i) } }) }) - t.Run("subgraph-specific response header rule with singleflight", func(t *testing.T) { + t.Run("subgraph-specific response set rule with singleflight is internal only", func(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{ Subgraphs: testenv.SubgraphsConfig{ @@ -1138,12 +1138,12 @@ func TestSingleFlight(t *testing.T) { responses := runConcurrentSingleflightRequests(t, xEnv, `{ employees { id } }`, 5) for i, res := range responses { require.Equal(t, `{"data":{"employees":[{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":7},{"id":8},{"id":10},{"id":11},{"id":12}]}}`, res.Body) - require.Equal(t, "employees-value", res.Response.Header.Get("X-Subgraph-Header"), - "response %d missing subgraph-specific X-Subgraph-Header", i) + require.Equal(t, "", res.Response.Header.Get("X-Subgraph-Header"), + "response %d: set response headers should not be forwarded to the client", i) } }) }) - t.Run("mixed global and subgraph-specific response header rules with singleflight", func(t *testing.T) { + t.Run("mixed global and subgraph-specific response set rules with singleflight are internal only", func(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{ Subgraphs: testenv.SubgraphsConfig{ @@ -1193,12 +1193,12 @@ func TestSingleFlight(t *testing.T) { responses := runConcurrentSingleflightRequests(t, xEnv, query, 5) for i, res := range responses { require.Contains(t, res.Body, `"employee"`) - require.Equal(t, "global-value", res.Response.Header.Get("X-Global-Header"), - "response %d missing global X-Global-Header", i) - require.Equal(t, "employees-value", res.Response.Header.Get("X-Employees-Header"), - "response %d missing subgraph-specific X-Employees-Header", i) - require.Equal(t, "family-value", res.Response.Header.Get("X-Family-Header"), - "response %d missing subgraph-specific X-Family-Header", i) + require.Equal(t, "", res.Response.Header.Get("X-Global-Header"), + "response %d: set response headers should not be forwarded to the client", i) + require.Equal(t, "", res.Response.Header.Get("X-Employees-Header"), + "response %d: set response headers should not be forwarded to the client", i) + require.Equal(t, "", res.Response.Header.Get("X-Family-Header"), + "response %d: set response headers should not be forwarded to the client", i) } }) }) diff --git a/router-tests/protocol/header_propagation_test.go b/router-tests/protocol/header_propagation_test.go index 0c139e47a0..fd607ddc10 100644 --- a/router-tests/protocol/header_propagation_test.go +++ b/router-tests/protocol/header_propagation_test.go @@ -1122,10 +1122,15 @@ func TestHeaderPropagation(t *testing.T) { }) }) - t.Run("set operation can override cache control policies", func(t *testing.T) { + t.Run("set operation feeds into cache control algorithm", func(t *testing.T) { t.Parallel() - t.Run("global set operation", func(t *testing.T) { + // The set operation injects into res.Header, so the cache control + // algorithm sees the set value as if the subgraph returned it. + t.Run("global set injects value for algorithm", func(t *testing.T) { t.Parallel() + // set in All.Response runs before the cache control algorithm. + // It overwrites the subgraph's real CC header, so the algorithm + // sees the injected value for every subgraph. testenv.Run(t, &testenv.Config{ CacheControlPolicy: config.CacheControlPolicy{ Enabled: true, @@ -1138,7 +1143,7 @@ func TestHeaderPropagation(t *testing.T) { { Operation: config.HeaderRuleOperationSet, Name: "Cache-Control", - Value: "my-fake-value", + Value: "max-age=60", }, }, }, @@ -1148,13 +1153,19 @@ func TestHeaderPropagation(t *testing.T) { Query: queryEmployeeWithHobby, }) cc := res.Response.Header.Get("Cache-Control") - require.Equal(t, "my-fake-value", cc) + // The set value (max-age=60) is injected into every subgraph + // response, overwriting the real headers. The algorithm picks + // max-age=60 as the most restrictive. + require.Equal(t, "max-age=60", cc) require.Equal(t, `{"data":{"employee":{"id":1,"hobbies":[{},{"name":"Counter Strike"},{},{},{}]}}}`, res.Body) }) }) - t.Run("local subgraph set operation", func(t *testing.T) { + t.Run("subgraph set feeds into algorithm", func(t *testing.T) { t.Parallel() + // The cache control algorithm runs after all rules (including + // subgraph-specific set rules), so the set value is visible to + // the algorithm. testenv.Run(t, &testenv.Config{ CacheControlPolicy: config.CacheControlPolicy{ Enabled: true, @@ -1168,7 +1179,7 @@ func TestHeaderPropagation(t *testing.T) { { Operation: config.HeaderRuleOperationSet, Name: "Cache-Control", - Value: "my-fake-value", + Value: "max-age=10", }, }, }, @@ -1179,7 +1190,9 @@ func TestHeaderPropagation(t *testing.T) { Query: queryEmployeeWithHobby, }) cc := res.Response.Header.Get("Cache-Control") - require.Equal(t, "my-fake-value", cc) + // The set overwrites employees' max-age=180 with max-age=10. + // The algorithm sees employees=10 and hobbies=250, picks 10. + require.Equal(t, "max-age=10", cc) require.Equal(t, `{"data":{"employee":{"id":1,"hobbies":[{},{"name":"Counter Strike"},{},{},{}]}}}`, res.Body) }) }) diff --git a/router-tests/protocol/header_set_test.go b/router-tests/protocol/header_set_test.go index dfd8810b23..ffe75d72be 100644 --- a/router-tests/protocol/header_set_test.go +++ b/router-tests/protocol/header_set_test.go @@ -155,6 +155,113 @@ func TestHeaderSet(t *testing.T) { require.Equal(t, `{"data":{"employee":{"id":1,"hobbies":[{},{"name":"Counter Strike"},{},{},{}]}}}`, res.Body) }) }) + + t.Run("set followed by propagate forwards to client", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + RouterOptions: []core.Option{ + core.WithHeaderRules(config.HeaderRules{ + All: &config.GlobalHeaderRule{ + Response: []*config.ResponseHeaderRule{ + { + Operation: config.HeaderRuleOperationSet, + Name: customHeader, + Value: "injected-value", + }, + { + Operation: config.HeaderRuleOperationPropagate, + Named: customHeader, + Algorithm: config.ResponseHeaderRuleAlgorithmLastWrite, + }, + }, + }, + }), + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ + Query: queryEmployeeWithHobby, + }) + ch := strings.Join(res.Response.Header.Values(customHeader), ",") + require.Equal(t, "injected-value", ch, "set + propagate should forward the header to the client") + require.Equal(t, `{"data":{"employee":{"id":1,"hobbies":[{},{"name":"Counter Strike"},{},{},{}]}}}`, res.Body) + }) + }) + + t.Run("set Cache-Control feeds into restrictive algorithm via All rules", func(t *testing.T) { + t.Parallel() + // The set rule in All.Response runs before the cache control + // algorithm (also in All.Response), injecting a Cache-Control + // value that the algorithm then processes as if the subgraph + // returned it. + testenv.Run(t, &testenv.Config{ + CacheControlPolicy: config.CacheControlPolicy{ + Enabled: true, + Value: "max-age=300, public", + }, + RouterOptions: []core.Option{ + core.WithHeaderRules(config.HeaderRules{ + All: &config.GlobalHeaderRule{ + Response: []*config.ResponseHeaderRule{ + { + Operation: config.HeaderRuleOperationSet, + Name: "Cache-Control", + Value: "max-age=60, public", + }, + }, + }, + }), + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ + Query: queryEmployeeWithHobby, + }) + cc := res.Response.Header.Get("Cache-Control") + require.Equal(t, "max-age=60, public", cc, "set Cache-Control should feed into the restrictive algorithm") + }) + }) + + t.Run("set overwrites existing subgraph response header", func(t *testing.T) { + t.Parallel() + // The employees subgraph returns X-Custom-Header: original-value. + // A set rule in All.Response overwrites it before the propagate + // rule sees it, so the client receives the overwritten value. + testenv.Run(t, &testenv.Config{ + Subgraphs: testenv.SubgraphsConfig{ + Employees: testenv.SubgraphConfig{ + Middleware: func(handler http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set(customHeader, "original-value") + handler.ServeHTTP(w, r) + }) + }, + }, + }, + RouterOptions: []core.Option{ + core.WithHeaderRules(config.HeaderRules{ + All: &config.GlobalHeaderRule{ + Response: []*config.ResponseHeaderRule{ + { + Operation: config.HeaderRuleOperationSet, + Name: customHeader, + Value: "overwritten-value", + }, + { + Operation: config.HeaderRuleOperationPropagate, + Named: customHeader, + Algorithm: config.ResponseHeaderRuleAlgorithmLastWrite, + }, + }, + }, + }), + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ + Query: queryEmployeeWithHobby, + }) + ch := strings.Join(res.Response.Header.Values(customHeader), ",") + require.Equal(t, "overwritten-value", ch, "set should overwrite the subgraph's original header before propagate sees it") + }) + }) }) } diff --git a/router/core/header_rule_engine.go b/router/core/header_rule_engine.go index e96e0bd20e..b26894ef72 100644 --- a/router/core/header_rule_engine.go +++ b/router/core/header_rule_engine.go @@ -65,7 +65,6 @@ type responseHeaderPropagation struct { header http.Header m *sync.Mutex previousCacheControl *cachedirective.Object - setCacheControl bool } func WithResponseHeaderPropagation(ctx *resolve.Context) *resolve.Context { @@ -181,7 +180,7 @@ func NewHeaderPropagation(rules *config.HeaderRules) (*HeaderPropagation, error) rhrs, rhrrs, rrs := hf.getAllRules() hf.hasRequestRules = len(rhrs) > 0 - hf.hasResponseRules = len(rhrrs) > 0 + hf.hasResponseRules = len(rhrrs) > 0 || len(hf.rules.AfterSubgraphResponse) > 0 // Pre-compute request rule presence hf.hasAllRequestRules = len(hf.rules.All.Request) > 0 @@ -206,6 +205,10 @@ func NewHeaderPropagation(rules *config.HeaderRules) (*HeaderPropagation, error) return &hf, nil } +// AddCacheControlPolicyToRules adds cache control rules to +// rules.AfterSubgraphResponse so they run after all user-defined response +// rules. This ensures set rules have already injected values into res.Header +// before the cache control algorithm reads them. func AddCacheControlPolicyToRules(rules *config.HeaderRules, cacheControl config.CacheControlPolicy) *config.HeaderRules { if rules == nil { rules = &config.HeaderRules{} @@ -216,7 +219,7 @@ func AddCacheControlPolicyToRules(rules *config.HeaderRules, cacheControl config initHeaderRules(rules) if cacheControl.Enabled { - rules.All.Response = append(rules.All.Response, &config.ResponseHeaderRule{ + rules.AfterSubgraphResponse = append(rules.AfterSubgraphResponse, &config.ResponseHeaderRule{ Operation: config.HeaderRuleOperationPropagate, Algorithm: config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, Default: cacheControl.Value, @@ -252,6 +255,8 @@ func (hf *HeaderPropagation) getAllRules() ([]*config.RequestHeaderRule, []*conf rhrrs = append(rhrrs, subgraph.Response...) } + rhrrs = append(rhrrs, hf.rules.AfterSubgraphResponse...) + return rhrs, rhrrs, hf.rules.Router.Response } @@ -445,6 +450,13 @@ func (h *HeaderPropagation) ApplyResponseHeaderRules(ctx context.Context, header } } } + + // AfterSubgraphResponse rules run last so that set rules (both global and + // subgraph-specific) have already injected values into res.Header before + // these rules (e.g. cache control algorithm) read them. + for _, rule := range h.rules.AfterSubgraphResponse { + h.applyResponseRule(propagation, resp, rule) + } } func (h *HeaderPropagation) OnOriginResponse(resp *http.Response, ctx RequestContext) *http.Response { @@ -456,14 +468,10 @@ func (h *HeaderPropagation) OnOriginResponse(resp *http.Response, ctx RequestCon func (h *HeaderPropagation) applyResponseRule(propagation *responseHeaderPropagation, res *http.Response, rule *config.ResponseHeaderRule) { if rule.Operation == config.HeaderRuleOperationSet { - propagation.m.Lock() - if rule.Name == cacheControlKey { - // Cache-Control set rules are forwarded to the client and disable the - // restrictive cache control algorithm. - propagation.header.Set(rule.Name, rule.Value) - propagation.setCacheControl = true - } - propagation.m.Unlock() + // Inject the value into the subgraph response headers so it looks like it + // came from the subgraph. Downstream rules (propagate, cache control + // algorithm, etc.) will process it naturally. + res.Header.Set(rule.Name, rule.Value) return } @@ -658,11 +666,6 @@ func (h *HeaderPropagation) applyRequestRuleToHeader(ctx *requestContext, header func (h *HeaderPropagation) applyResponseRuleMostRestrictiveCacheControl(res *http.Response, propagation *responseHeaderPropagation, rule *config.ResponseHeaderRule) { propagation.m.Lock() - if propagation.setCacheControl { - propagation.m.Unlock() - // Handle the case where the cache control header is set explicitly using the set propagation rule - return - } previousCacheControl := propagation.previousCacheControl propagation.m.Unlock() @@ -681,10 +684,6 @@ func (h *HeaderPropagation) applyResponseRuleMostRestrictiveCacheControl(res *ht // Set no-cache for all mutations, to ensure that requests to mutate data always work as expected (without returning cached data) if resolve.GetOperationTypeFromContext(ctx) == ast.OperationTypeMutation { propagation.m.Lock() - if propagation.setCacheControl { - propagation.m.Unlock() - return - } propagation.header.Set(cacheControlKey, noCache) propagation.m.Unlock() return @@ -732,12 +731,6 @@ func (h *HeaderPropagation) applyResponseRuleMostRestrictiveCacheControl(res *ht propagation.m.Lock() defer propagation.m.Unlock() - if propagation.setCacheControl { - // We compute restrictivePolicy outside the lock. If a concurrent - // response applied an explicit `set` Cache-Control rule in the meantime, - // that explicit value must win; drop this computed result. - return - } // Merge with the current shared state under lock to avoid lost updates when // multiple subgraph responses compute policies concurrently. policies := []*cachedirective.Object{obj} diff --git a/router/core/header_rule_engine_test.go b/router/core/header_rule_engine_test.go index 42582832b0..3ee747df2d 100644 --- a/router/core/header_rule_engine_test.go +++ b/router/core/header_rule_engine_test.go @@ -114,7 +114,7 @@ func TestCreateMostRestrictivePolicy(t *testing.T) { func TestAddCacheControlPolicyToRules(t *testing.T) { t.Parallel() - t.Run("nil rules and disabled cache returns nil", func(t *testing.T) { + t.Run("disabled cache returns rules unchanged", func(t *testing.T) { t.Parallel() result := AddCacheControlPolicyToRules(nil, config.CacheControlPolicy{ Enabled: false, @@ -122,72 +122,62 @@ func TestAddCacheControlPolicyToRules(t *testing.T) { assert.Nil(t, result) }) - t.Run("nil rules and enabled cache creates rules", func(t *testing.T) { + t.Run("enabled cache populates AfterSubgraphResponse", func(t *testing.T) { t.Parallel() result := AddCacheControlPolicyToRules(nil, config.CacheControlPolicy{ Enabled: true, Value: "max-age=300", }) require.NotNil(t, result) - require.NotNil(t, result.All) - require.Len(t, result.All.Response, 1) - assert.Equal(t, config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, result.All.Response[0].Algorithm) - assert.Equal(t, "max-age=300", result.All.Response[0].Default) + require.Len(t, result.AfterSubgraphResponse, 1) + assert.Equal(t, config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, result.AfterSubgraphResponse[0].Algorithm) + assert.Equal(t, "max-age=300", result.AfterSubgraphResponse[0].Default) }) - t.Run("existing rules with enabled cache appends", func(t *testing.T) { + t.Run("subgraph-specific cache adds to AfterSubgraphResponse", func(t *testing.T) { t.Parallel() - existing := &config.HeaderRules{ - All: &config.GlobalHeaderRule{ - Response: []*config.ResponseHeaderRule{ - {Operation: config.HeaderRuleOperationPropagate, Named: "X-Existing"}, - }, + result := AddCacheControlPolicyToRules(nil, config.CacheControlPolicy{ + Subgraphs: []config.SubgraphCacheControlRule{ + {Name: "sg1", Value: "max-age=60"}, }, - } - result := AddCacheControlPolicyToRules(existing, config.CacheControlPolicy{ - Enabled: true, - Value: "max-age=300", }) require.NotNil(t, result) - require.Len(t, result.All.Response, 2) - assert.Equal(t, "X-Existing", result.All.Response[0].Named) - assert.Equal(t, config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, result.All.Response[1].Algorithm) + require.Len(t, result.AfterSubgraphResponse, 1) + assert.Equal(t, "max-age=60", result.AfterSubgraphResponse[0].Default) }) - t.Run("subgraph-specific cache creates per-subgraph response rule", func(t *testing.T) { + t.Run("global and subgraph rules coexist", func(t *testing.T) { t.Parallel() result := AddCacheControlPolicyToRules(nil, config.CacheControlPolicy{ + Enabled: true, + Value: "max-age=300", Subgraphs: []config.SubgraphCacheControlRule{ {Name: "sg1", Value: "max-age=60"}, }, }) require.NotNil(t, result) - require.Contains(t, result.Subgraphs, "sg1") - require.Len(t, result.Subgraphs["sg1"].Response, 1) - assert.Equal(t, "max-age=60", result.Subgraphs["sg1"].Response[0].Default) + require.Len(t, result.AfterSubgraphResponse, 2) + assert.Equal(t, "max-age=300", result.AfterSubgraphResponse[0].Default) + assert.Equal(t, "max-age=60", result.AfterSubgraphResponse[1].Default) }) - t.Run("existing subgraph gets cache rule appended", func(t *testing.T) { + t.Run("preserves existing user rules", func(t *testing.T) { t.Parallel() existing := &config.HeaderRules{ - All: &config.GlobalHeaderRule{}, - Subgraphs: map[string]*config.GlobalHeaderRule{ - "sg1": { - Response: []*config.ResponseHeaderRule{ - {Operation: config.HeaderRuleOperationPropagate, Named: "X-Existing"}, - }, + All: &config.GlobalHeaderRule{ + Response: []*config.ResponseHeaderRule{ + {Operation: config.HeaderRuleOperationPropagate, Named: "X-Existing"}, }, }, } result := AddCacheControlPolicyToRules(existing, config.CacheControlPolicy{ - Subgraphs: []config.SubgraphCacheControlRule{ - {Name: "sg1", Value: "max-age=60"}, - }, + Enabled: true, + Value: "max-age=300", }) require.NotNil(t, result) - require.Len(t, result.Subgraphs["sg1"].Response, 2) - assert.Equal(t, "X-Existing", result.Subgraphs["sg1"].Response[0].Named) - assert.Equal(t, config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, result.Subgraphs["sg1"].Response[1].Algorithm) + require.Len(t, result.All.Response, 1) + assert.Equal(t, "X-Existing", result.All.Response[0].Named) + require.Len(t, result.AfterSubgraphResponse, 1) }) } @@ -258,7 +248,7 @@ func TestApplyResponseRuleKeyValue(t *testing.T) { }) } -func TestApplyResponseRuleSetNotForwarded(t *testing.T) { +func TestApplyResponseRuleSetWritesToSubgraphResponse(t *testing.T) { t.Parallel() newPropagation := func() *responseHeaderPropagation { @@ -270,32 +260,35 @@ func TestApplyResponseRuleSetNotForwarded(t *testing.T) { hp := &HeaderPropagation{} - t.Run("set does not write to client header", func(t *testing.T) { + t.Run("set writes to subgraph response header, not propagation header", func(t *testing.T) { t.Parallel() prop := newPropagation() + res := &http.Response{Header: make(http.Header)} rule := &config.ResponseHeaderRule{ Operation: config.HeaderRuleOperationSet, Name: "X-Custom", Value: "test-value", } - hp.applyResponseRule(prop, &http.Response{}, rule) - require.Equal(t, "", prop.header.Get("X-Custom")) + hp.applyResponseRule(prop, res, rule) + require.Equal(t, "", prop.header.Get("X-Custom"), "set should not write to propagation header") + require.Equal(t, "test-value", res.Header.Get("X-Custom"), "set should write to subgraph response header") }) - t.Run("set Cache-Control writes to client header and sets flag", func(t *testing.T) { + t.Run("set Cache-Control writes to subgraph response header", func(t *testing.T) { t.Parallel() prop := newPropagation() + res := &http.Response{Header: make(http.Header)} rule := &config.ResponseHeaderRule{ Operation: config.HeaderRuleOperationSet, Name: "Cache-Control", Value: "max-age=300", } - hp.applyResponseRule(prop, &http.Response{}, rule) - require.Equal(t, "max-age=300", prop.header.Get("Cache-Control")) - require.True(t, prop.setCacheControl) + hp.applyResponseRule(prop, res, rule) + require.Equal(t, "", prop.header.Get("Cache-Control"), "set should not write to propagation header") + require.Equal(t, "max-age=300", res.Header.Get("Cache-Control"), "set should write to subgraph response header") }) - t.Run("propagate still writes to client header", func(t *testing.T) { + t.Run("propagate still writes to propagation header", func(t *testing.T) { t.Parallel() prop := newPropagation() rule := &config.ResponseHeaderRule{ diff --git a/router/pkg/config/config.go b/router/pkg/config/config.go index f9715bfe97..945fdc87c1 100644 --- a/router/pkg/config/config.go +++ b/router/pkg/config/config.go @@ -284,6 +284,10 @@ type HeaderRules struct { Subgraphs map[string]*GlobalHeaderRule `yaml:"subgraphs,omitempty"` CookieWhitelist []string `yaml:"cookie_whitelist,omitempty"` Router RouterHeaderRules `yaml:"router,omitempty"` + // AfterSubgraphResponse runs after all other response rules (All + Subgraph-specific). + // Set programmatically, not via YAML. Used by cache_control_policy so that set + // rules have already injected values before the algorithm reads them. + AfterSubgraphResponse []*ResponseHeaderRule `yaml:"-"` } type RouterHeaderRules struct { From f886c7ae7d1aecca9f3894a7146492dab4940c37 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 27 Mar 2026 00:10:28 +0530 Subject: [PATCH 03/11] fix: changes --- router/core/graph_server.go | 2 +- router/core/header_rule_engine.go | 70 +++++++++++-------- .../header_rule_engine_buildheader_test.go | 16 ++--- router/core/header_rule_engine_test.go | 65 ++++++++--------- router/core/router.go | 5 +- router/pkg/config/config.go | 4 -- 6 files changed, 79 insertions(+), 83 deletions(-) diff --git a/router/core/graph_server.go b/router/core/graph_server.go index d9e73be51f..871294000b 100644 --- a/router/core/graph_server.go +++ b/router/core/graph_server.go @@ -1474,7 +1474,7 @@ func (s *graphServer) buildGraphMux( Executor: executor, Log: s.logger, EnableCacheResponseHeaders: s.engineExecutionConfiguration.Debug.EnableCacheResponseHeaders, - EnableResponseHeaderPropagation: s.headerRules != nil, + EnableResponseHeaderPropagation: s.headerPropagation.HasResponseRules(), EnableCostResponseHeaders: s.securityConfiguration.CostControl != nil && s.securityConfiguration.CostControl.ExposeHeaders, EngineStats: s.engineStats, TracerProvider: s.tracerProvider, diff --git a/router/core/header_rule_engine.go b/router/core/header_rule_engine.go index b26894ef72..cf789367e4 100644 --- a/router/core/header_rule_engine.go +++ b/router/core/header_rule_engine.go @@ -154,6 +154,12 @@ type HeaderPropagation struct { // Precomputed request rule presence for fast-path checks hasAllRequestRules bool subgraphHasRequestRules map[string]bool + // afterSubgraphResponse* runs after all other response rules (All + Subgraph-specific). + // Used by cache_control_policy so that set rules have already injected + // values into res.Header before the algorithm reads them. + // Execution order: afterSubgraphResponseAll (every fetch), then afterSubgraphResponseSubgraphs[name]. + afterSubgraphResponseAll []*config.ResponseHeaderRule + afterSubgraphResponseSubgraphs map[string][]*config.ResponseHeaderRule } func initHeaderRules(rules *config.HeaderRules) { @@ -165,22 +171,27 @@ func initHeaderRules(rules *config.HeaderRules) { } } -func NewHeaderPropagation(rules *config.HeaderRules) (*HeaderPropagation, error) { - if rules == nil { +func NewHeaderPropagation(rules *config.HeaderRules, afterAll []*config.ResponseHeaderRule, afterSubgraphs map[string][]*config.ResponseHeaderRule) (*HeaderPropagation, error) { + if rules == nil && len(afterAll) == 0 && len(afterSubgraphs) == 0 { return nil, nil } + if rules == nil { + rules = &config.HeaderRules{} + } initHeaderRules(rules) hf := HeaderPropagation{ - rules: rules, - regex: map[string]*regexp.Regexp{}, - compiledRules: map[string]*vm.Program{}, - compiledRouterResponseRules: map[string]*vm.Program{}, + rules: rules, + regex: map[string]*regexp.Regexp{}, + compiledRules: map[string]*vm.Program{}, + compiledRouterResponseRules: map[string]*vm.Program{}, + afterSubgraphResponseAll: afterAll, + afterSubgraphResponseSubgraphs: afterSubgraphs, } rhrs, rhrrs, rrs := hf.getAllRules() hf.hasRequestRules = len(rhrs) > 0 - hf.hasResponseRules = len(rhrrs) > 0 || len(hf.rules.AfterSubgraphResponse) > 0 + hf.hasResponseRules = len(rhrrs) > 0 || len(hf.afterSubgraphResponseAll) > 0 || len(hf.afterSubgraphResponseSubgraphs) > 0 // Pre-compute request rule presence hf.hasAllRequestRules = len(hf.rules.All.Request) > 0 @@ -205,43 +216,32 @@ func NewHeaderPropagation(rules *config.HeaderRules) (*HeaderPropagation, error) return &hf, nil } -// AddCacheControlPolicyToRules adds cache control rules to -// rules.AfterSubgraphResponse so they run after all user-defined response -// rules. This ensures set rules have already injected values into res.Header -// before the cache control algorithm reads them. -func AddCacheControlPolicyToRules(rules *config.HeaderRules, cacheControl config.CacheControlPolicy) *config.HeaderRules { - if rules == nil { - rules = &config.HeaderRules{} - if !cacheControl.Enabled && cacheControl.Subgraphs == nil { - return nil - } - } - - initHeaderRules(rules) +// AddCacheControlPolicyToRules builds cache control rules that run after all +// user-defined response rules. Returns the global after-rules (run for every +// fetch) and per-subgraph after-rules. +func AddCacheControlPolicyToRules(cacheControl config.CacheControlPolicy) ([]*config.ResponseHeaderRule, map[string][]*config.ResponseHeaderRule) { + var afterAll []*config.ResponseHeaderRule if cacheControl.Enabled { - rules.AfterSubgraphResponse = append(rules.AfterSubgraphResponse, &config.ResponseHeaderRule{ + afterAll = append(afterAll, &config.ResponseHeaderRule{ Operation: config.HeaderRuleOperationPropagate, Algorithm: config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, Default: cacheControl.Value, }) } + var afterSubgraphs map[string][]*config.ResponseHeaderRule for _, graph := range cacheControl.Subgraphs { - subgraphRules, ok := rules.Subgraphs[graph.Name] - if !ok { - subgraphRules = &config.GlobalHeaderRule{Response: make([]*config.ResponseHeaderRule, 0)} + if afterSubgraphs == nil { + afterSubgraphs = make(map[string][]*config.ResponseHeaderRule) } - - subgraphRules.Response = append(subgraphRules.Response, &config.ResponseHeaderRule{ + afterSubgraphs[graph.Name] = append(afterSubgraphs[graph.Name], &config.ResponseHeaderRule{ Operation: config.HeaderRuleOperationPropagate, Algorithm: config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, Default: graph.Value, }) - - rules.Subgraphs[graph.Name] = subgraphRules } - return rules + return afterAll, afterSubgraphs } func (hf *HeaderPropagation) getAllRules() ([]*config.RequestHeaderRule, []*config.ResponseHeaderRule, []*config.RouterResponseHeaderRule) { @@ -255,7 +255,10 @@ func (hf *HeaderPropagation) getAllRules() ([]*config.RequestHeaderRule, []*conf rhrrs = append(rhrrs, subgraph.Response...) } - rhrrs = append(rhrrs, hf.rules.AfterSubgraphResponse...) + rhrrs = append(rhrrs, hf.afterSubgraphResponseAll...) + for _, sgRules := range hf.afterSubgraphResponseSubgraphs { + rhrrs = append(rhrrs, sgRules...) + } return rhrs, rhrrs, hf.rules.Router.Response } @@ -454,9 +457,14 @@ func (h *HeaderPropagation) ApplyResponseHeaderRules(ctx context.Context, header // AfterSubgraphResponse rules run last so that set rules (both global and // subgraph-specific) have already injected values into res.Header before // these rules (e.g. cache control algorithm) read them. - for _, rule := range h.rules.AfterSubgraphResponse { + for _, rule := range h.afterSubgraphResponseAll { h.applyResponseRule(propagation, resp, rule) } + if subgraphName != "" { + for _, rule := range h.afterSubgraphResponseSubgraphs[subgraphName] { + h.applyResponseRule(propagation, resp, rule) + } + } } func (h *HeaderPropagation) OnOriginResponse(resp *http.Response, ctx RequestContext) *http.Response { diff --git a/router/core/header_rule_engine_buildheader_test.go b/router/core/header_rule_engine_buildheader_test.go index c652deeed4..3a2703e1cd 100644 --- a/router/core/header_rule_engine_buildheader_test.go +++ b/router/core/header_rule_engine_buildheader_test.go @@ -22,7 +22,7 @@ func TestBuildRequestHeaderForSubgraph_GlobalRulesAndHashStable(t *testing.T) { {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }) + }, nil, nil) require.NoError(t, err) require.NotNil(t, ht) @@ -66,7 +66,7 @@ func TestBuildRequestHeaderForSubgraph_SubgraphSpecificRules(t *testing.T) { }, }, }, - }) + }, nil, nil) require.NoError(t, err) rr := httptest.NewRecorder() @@ -118,7 +118,7 @@ func BenchmarkBuildRequestHeaderForSubgraph(b *testing.B) { {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }) + }, nil, nil) if err != nil { b.Fatal(err) } @@ -153,7 +153,7 @@ func TestSubgraphHeadersBuilder_PrePopulatesAndClones_SyncPlan(t *testing.T) { {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }) + }, nil, nil) require.NoError(t, err) // Prepare client request @@ -212,7 +212,7 @@ func TestSubgraphHeadersBuilder_IgnoresClientHeaderChangesAfterPrepopulate(t *te {Operation: "propagate", Named: "X-A"}, }, }, - }) + }, nil, nil) require.NoError(t, err) rr := httptest.NewRecorder() @@ -257,7 +257,7 @@ func TestSubgraphHeadersBuilder_SubscriptionPlan_IncludesTriggerAndResponse(t *t {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }) + }, nil, nil) require.NoError(t, err) rr := httptest.NewRecorder() @@ -330,7 +330,7 @@ func TestSubgraphHeadersBuilder_MissingPrePopulatedCache(t *testing.T) { {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }) + }, nil, nil) require.NoError(t, err) rr := httptest.NewRecorder() @@ -388,7 +388,7 @@ func TestSubgraphHeadersBuilder_ConcurrentAccessSameSubgraph(t *testing.T) { {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }) + }, nil, nil) require.NoError(t, err) rr := httptest.NewRecorder() diff --git a/router/core/header_rule_engine_test.go b/router/core/header_rule_engine_test.go index 3ee747df2d..1836bc87c9 100644 --- a/router/core/header_rule_engine_test.go +++ b/router/core/header_rule_engine_test.go @@ -114,70 +114,63 @@ func TestCreateMostRestrictivePolicy(t *testing.T) { func TestAddCacheControlPolicyToRules(t *testing.T) { t.Parallel() - t.Run("disabled cache returns rules unchanged", func(t *testing.T) { + t.Run("disabled cache returns nil after-rules", func(t *testing.T) { t.Parallel() - result := AddCacheControlPolicyToRules(nil, config.CacheControlPolicy{ + afterAll, afterSG := AddCacheControlPolicyToRules(config.CacheControlPolicy{ Enabled: false, }) - assert.Nil(t, result) + assert.Nil(t, afterAll) + assert.Nil(t, afterSG) }) - t.Run("enabled cache populates AfterSubgraphResponse", func(t *testing.T) { + t.Run("enabled cache returns global after-rule", func(t *testing.T) { t.Parallel() - result := AddCacheControlPolicyToRules(nil, config.CacheControlPolicy{ + afterAll, afterSG := AddCacheControlPolicyToRules(config.CacheControlPolicy{ Enabled: true, Value: "max-age=300", }) - require.NotNil(t, result) - require.Len(t, result.AfterSubgraphResponse, 1) - assert.Equal(t, config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, result.AfterSubgraphResponse[0].Algorithm) - assert.Equal(t, "max-age=300", result.AfterSubgraphResponse[0].Default) + require.Len(t, afterAll, 1) + assert.Equal(t, config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, afterAll[0].Algorithm) + assert.Equal(t, "max-age=300", afterAll[0].Default) + assert.Nil(t, afterSG) }) - t.Run("subgraph-specific cache adds to AfterSubgraphResponse", func(t *testing.T) { + t.Run("subgraph-specific cache returns per-subgraph after-rule", func(t *testing.T) { t.Parallel() - result := AddCacheControlPolicyToRules(nil, config.CacheControlPolicy{ + afterAll, afterSG := AddCacheControlPolicyToRules(config.CacheControlPolicy{ Subgraphs: []config.SubgraphCacheControlRule{ {Name: "sg1", Value: "max-age=60"}, }, }) - require.NotNil(t, result) - require.Len(t, result.AfterSubgraphResponse, 1) - assert.Equal(t, "max-age=60", result.AfterSubgraphResponse[0].Default) + assert.Nil(t, afterAll) + require.Contains(t, afterSG, "sg1") + require.Len(t, afterSG["sg1"], 1) + assert.Equal(t, "max-age=60", afterSG["sg1"][0].Default) }) t.Run("global and subgraph rules coexist", func(t *testing.T) { t.Parallel() - result := AddCacheControlPolicyToRules(nil, config.CacheControlPolicy{ + afterAll, afterSG := AddCacheControlPolicyToRules(config.CacheControlPolicy{ Enabled: true, Value: "max-age=300", Subgraphs: []config.SubgraphCacheControlRule{ {Name: "sg1", Value: "max-age=60"}, }, }) - require.NotNil(t, result) - require.Len(t, result.AfterSubgraphResponse, 2) - assert.Equal(t, "max-age=300", result.AfterSubgraphResponse[0].Default) - assert.Equal(t, "max-age=60", result.AfterSubgraphResponse[1].Default) + require.Len(t, afterAll, 1) + assert.Equal(t, "max-age=300", afterAll[0].Default) + require.Contains(t, afterSG, "sg1") + assert.Equal(t, "max-age=60", afterSG["sg1"][0].Default) }) - t.Run("preserves existing user rules", func(t *testing.T) { + t.Run("enabled with value returns after-rule with default", func(t *testing.T) { t.Parallel() - existing := &config.HeaderRules{ - All: &config.GlobalHeaderRule{ - Response: []*config.ResponseHeaderRule{ - {Operation: config.HeaderRuleOperationPropagate, Named: "X-Existing"}, - }, - }, - } - result := AddCacheControlPolicyToRules(existing, config.CacheControlPolicy{ + afterAll, _ := AddCacheControlPolicyToRules(config.CacheControlPolicy{ Enabled: true, Value: "max-age=300", }) - require.NotNil(t, result) - require.Len(t, result.All.Response, 1) - assert.Equal(t, "X-Existing", result.All.Response[0].Named) - require.Len(t, result.AfterSubgraphResponse, 1) + require.Len(t, afterAll, 1) + assert.Equal(t, "max-age=300", afterAll[0].Default) }) } @@ -387,14 +380,14 @@ func TestNewHeaderPropagation(t *testing.T) { t.Run("nil rules returns nil", func(t *testing.T) { t.Parallel() - hp, err := NewHeaderPropagation(nil) + hp, err := NewHeaderPropagation(nil, nil, nil) require.NoError(t, err) assert.Nil(t, hp) }) t.Run("empty rules returns valid instance", func(t *testing.T) { t.Parallel() - hp, err := NewHeaderPropagation(&config.HeaderRules{}) + hp, err := NewHeaderPropagation(&config.HeaderRules{}, nil, nil) require.NoError(t, err) require.NotNil(t, hp) }) @@ -407,7 +400,7 @@ func TestNewHeaderPropagation(t *testing.T) { {Operation: config.HeaderRuleOperationPropagate, Matching: "[invalid"}, }, }, - }) + }, nil, nil) require.Error(t, err) }) @@ -419,7 +412,7 @@ func TestNewHeaderPropagation(t *testing.T) { {Operation: config.HeaderRuleOperationPropagate, Matching: "[invalid"}, }, }, - }) + }, nil, nil) require.Error(t, err) }) diff --git a/router/core/router.go b/router/core/router.go index 34e619e0da..21d3ed9028 100644 --- a/router/core/router.go +++ b/router/core/router.go @@ -311,9 +311,9 @@ func NewRouter(opts ...Option) (*Router, error) { r.livenessCheckPath = "/health/live" } - r.headerRules = AddCacheControlPolicyToRules(r.headerRules, r.cacheControlPolicy) + afterAll, afterSubgraphs := AddCacheControlPolicyToRules(r.cacheControlPolicy) var err error - r.headerPropagation, err = NewHeaderPropagation(r.headerRules) + r.headerPropagation, err = NewHeaderPropagation(r.headerRules, afterAll, afterSubgraphs) if err != nil { return nil, err } @@ -526,7 +526,6 @@ func NewRouter(opts ...Option) (*Router, error) { r.engineExecutionConfiguration.Debug.EnableCacheResponseHeaders = true } - if r.securityConfiguration.DepthLimit != nil { r.logger.Warn("The security configuration field 'depth_limit' is deprecated, and will be removed. Use 'security.complexity_limits.depth' instead.") diff --git a/router/pkg/config/config.go b/router/pkg/config/config.go index 945fdc87c1..f9715bfe97 100644 --- a/router/pkg/config/config.go +++ b/router/pkg/config/config.go @@ -284,10 +284,6 @@ type HeaderRules struct { Subgraphs map[string]*GlobalHeaderRule `yaml:"subgraphs,omitempty"` CookieWhitelist []string `yaml:"cookie_whitelist,omitempty"` Router RouterHeaderRules `yaml:"router,omitempty"` - // AfterSubgraphResponse runs after all other response rules (All + Subgraph-specific). - // Set programmatically, not via YAML. Used by cache_control_policy so that set - // rules have already injected values before the algorithm reads them. - AfterSubgraphResponse []*ResponseHeaderRule `yaml:"-"` } type RouterHeaderRules struct { From 6c6b23a28e5608f796d3824f4a377c1aec3b2661 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 27 Mar 2026 00:25:08 +0530 Subject: [PATCH 04/11] fix: review changes --- .../adjusting-cache-control.mdx | 1 + router/core/header_rule_engine.go | 83 ++++++++++++------- .../header_rule_engine_buildheader_test.go | 16 ++-- router/core/header_rule_engine_test.go | 60 ++++++-------- router/core/router.go | 5 +- 5 files changed, 89 insertions(+), 76 deletions(-) diff --git a/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx b/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx index c977bcc378..6c27272d5b 100644 --- a/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx +++ b/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx @@ -160,6 +160,7 @@ This is however equivalent to the following cache_control_policy: enabled: true value: "max-age=180, public" + subgraphs: - name: "specific-subgraph" value: "max-age=5400, public" ``` diff --git a/router/core/header_rule_engine.go b/router/core/header_rule_engine.go index cf789367e4..69d11295d9 100644 --- a/router/core/header_rule_engine.go +++ b/router/core/header_rule_engine.go @@ -154,12 +154,25 @@ type HeaderPropagation struct { // Precomputed request rule presence for fast-path checks hasAllRequestRules bool subgraphHasRequestRules map[string]bool - // afterSubgraphResponse* runs after all other response rules (All + Subgraph-specific). + // postResponseRules runs after all other response rules (All + Subgraph-specific). // Used by cache_control_policy so that set rules have already injected // values into res.Header before the algorithm reads them. - // Execution order: afterSubgraphResponseAll (every fetch), then afterSubgraphResponseSubgraphs[name]. - afterSubgraphResponseAll []*config.ResponseHeaderRule - afterSubgraphResponseSubgraphs map[string][]*config.ResponseHeaderRule + postResponseRules *PostResponseRules +} + +// PostResponseRules holds response rules that execute after all user-defined +// response rules have been applied. Execution order: All (every fetch), then +// Subgraphs[name] (only for that subgraph). +type PostResponseRules struct { + All []*config.ResponseHeaderRule + Subgraphs map[string][]*config.ResponseHeaderRule +} + +func (p *PostResponseRules) hasRules() bool { + if p == nil { + return false + } + return len(p.All) > 0 || len(p.Subgraphs) > 0 } func initHeaderRules(rules *config.HeaderRules) { @@ -171,27 +184,27 @@ func initHeaderRules(rules *config.HeaderRules) { } } -func NewHeaderPropagation(rules *config.HeaderRules, afterAll []*config.ResponseHeaderRule, afterSubgraphs map[string][]*config.ResponseHeaderRule) (*HeaderPropagation, error) { - if rules == nil && len(afterAll) == 0 && len(afterSubgraphs) == 0 { +func NewHeaderPropagation(rules *config.HeaderRules, postRules *PostResponseRules) (*HeaderPropagation, error) { + if rules == nil && !postRules.hasRules() { return nil, nil } + if rules == nil { rules = &config.HeaderRules{} } initHeaderRules(rules) hf := HeaderPropagation{ - rules: rules, - regex: map[string]*regexp.Regexp{}, - compiledRules: map[string]*vm.Program{}, - compiledRouterResponseRules: map[string]*vm.Program{}, - afterSubgraphResponseAll: afterAll, - afterSubgraphResponseSubgraphs: afterSubgraphs, + rules: rules, + regex: map[string]*regexp.Regexp{}, + compiledRules: map[string]*vm.Program{}, + compiledRouterResponseRules: map[string]*vm.Program{}, + postResponseRules: postRules, } rhrs, rhrrs, rrs := hf.getAllRules() hf.hasRequestRules = len(rhrs) > 0 - hf.hasResponseRules = len(rhrrs) > 0 || len(hf.afterSubgraphResponseAll) > 0 || len(hf.afterSubgraphResponseSubgraphs) > 0 + hf.hasResponseRules = len(rhrrs) > 0 || postRules.hasRules() // Pre-compute request rule presence hf.hasAllRequestRules = len(hf.rules.All.Request) > 0 @@ -216,32 +229,34 @@ func NewHeaderPropagation(rules *config.HeaderRules, afterAll []*config.Response return &hf, nil } -// AddCacheControlPolicyToRules builds cache control rules that run after all -// user-defined response rules. Returns the global after-rules (run for every -// fetch) and per-subgraph after-rules. -func AddCacheControlPolicyToRules(cacheControl config.CacheControlPolicy) ([]*config.ResponseHeaderRule, map[string][]*config.ResponseHeaderRule) { - var afterAll []*config.ResponseHeaderRule +// CreateCacheControlPolicyHeaderRules builds cache control rules that run after +// all user-defined response rules. +func CreateCacheControlPolicyHeaderRules(cacheControl config.CacheControlPolicy) *PostResponseRules { + var rules PostResponseRules + if cacheControl.Enabled { - afterAll = append(afterAll, &config.ResponseHeaderRule{ + rules.All = append(rules.All, &config.ResponseHeaderRule{ Operation: config.HeaderRuleOperationPropagate, Algorithm: config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, Default: cacheControl.Value, }) } - var afterSubgraphs map[string][]*config.ResponseHeaderRule for _, graph := range cacheControl.Subgraphs { - if afterSubgraphs == nil { - afterSubgraphs = make(map[string][]*config.ResponseHeaderRule) + if rules.Subgraphs == nil { + rules.Subgraphs = make(map[string][]*config.ResponseHeaderRule) } - afterSubgraphs[graph.Name] = append(afterSubgraphs[graph.Name], &config.ResponseHeaderRule{ + rules.Subgraphs[graph.Name] = append(rules.Subgraphs[graph.Name], &config.ResponseHeaderRule{ Operation: config.HeaderRuleOperationPropagate, Algorithm: config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, Default: graph.Value, }) } - return afterAll, afterSubgraphs + if !rules.hasRules() { + return nil + } + return &rules } func (hf *HeaderPropagation) getAllRules() ([]*config.RequestHeaderRule, []*config.ResponseHeaderRule, []*config.RouterResponseHeaderRule) { @@ -255,9 +270,11 @@ func (hf *HeaderPropagation) getAllRules() ([]*config.RequestHeaderRule, []*conf rhrrs = append(rhrrs, subgraph.Response...) } - rhrrs = append(rhrrs, hf.afterSubgraphResponseAll...) - for _, sgRules := range hf.afterSubgraphResponseSubgraphs { - rhrrs = append(rhrrs, sgRules...) + if hf.postResponseRules != nil { + rhrrs = append(rhrrs, hf.postResponseRules.All...) + for _, sgRules := range hf.postResponseRules.Subgraphs { + rhrrs = append(rhrrs, sgRules...) + } } return rhrs, rhrrs, hf.rules.Router.Response @@ -457,13 +474,15 @@ func (h *HeaderPropagation) ApplyResponseHeaderRules(ctx context.Context, header // AfterSubgraphResponse rules run last so that set rules (both global and // subgraph-specific) have already injected values into res.Header before // these rules (e.g. cache control algorithm) read them. - for _, rule := range h.afterSubgraphResponseAll { - h.applyResponseRule(propagation, resp, rule) - } - if subgraphName != "" { - for _, rule := range h.afterSubgraphResponseSubgraphs[subgraphName] { + if h.postResponseRules != nil { + for _, rule := range h.postResponseRules.All { h.applyResponseRule(propagation, resp, rule) } + if subgraphName != "" { + for _, rule := range h.postResponseRules.Subgraphs[subgraphName] { + h.applyResponseRule(propagation, resp, rule) + } + } } } diff --git a/router/core/header_rule_engine_buildheader_test.go b/router/core/header_rule_engine_buildheader_test.go index 3a2703e1cd..f139b68fc3 100644 --- a/router/core/header_rule_engine_buildheader_test.go +++ b/router/core/header_rule_engine_buildheader_test.go @@ -22,7 +22,7 @@ func TestBuildRequestHeaderForSubgraph_GlobalRulesAndHashStable(t *testing.T) { {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }, nil, nil) + }, nil) require.NoError(t, err) require.NotNil(t, ht) @@ -66,7 +66,7 @@ func TestBuildRequestHeaderForSubgraph_SubgraphSpecificRules(t *testing.T) { }, }, }, - }, nil, nil) + }, nil) require.NoError(t, err) rr := httptest.NewRecorder() @@ -118,7 +118,7 @@ func BenchmarkBuildRequestHeaderForSubgraph(b *testing.B) { {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }, nil, nil) + }, nil) if err != nil { b.Fatal(err) } @@ -153,7 +153,7 @@ func TestSubgraphHeadersBuilder_PrePopulatesAndClones_SyncPlan(t *testing.T) { {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }, nil, nil) + }, nil) require.NoError(t, err) // Prepare client request @@ -212,7 +212,7 @@ func TestSubgraphHeadersBuilder_IgnoresClientHeaderChangesAfterPrepopulate(t *te {Operation: "propagate", Named: "X-A"}, }, }, - }, nil, nil) + }, nil) require.NoError(t, err) rr := httptest.NewRecorder() @@ -257,7 +257,7 @@ func TestSubgraphHeadersBuilder_SubscriptionPlan_IncludesTriggerAndResponse(t *t {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }, nil, nil) + }, nil) require.NoError(t, err) rr := httptest.NewRecorder() @@ -330,7 +330,7 @@ func TestSubgraphHeadersBuilder_MissingPrePopulatedCache(t *testing.T) { {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }, nil, nil) + }, nil) require.NoError(t, err) rr := httptest.NewRecorder() @@ -388,7 +388,7 @@ func TestSubgraphHeadersBuilder_ConcurrentAccessSameSubgraph(t *testing.T) { {Operation: "set", Name: "X-Static", Value: "static"}, }, }, - }, nil, nil) + }, nil) require.NoError(t, err) rr := httptest.NewRecorder() diff --git a/router/core/header_rule_engine_test.go b/router/core/header_rule_engine_test.go index 1836bc87c9..1bc108912e 100644 --- a/router/core/header_rule_engine_test.go +++ b/router/core/header_rule_engine_test.go @@ -111,66 +111,58 @@ func TestCreateMostRestrictivePolicy(t *testing.T) { }) } -func TestAddCacheControlPolicyToRules(t *testing.T) { +func TestCreateCacheControlPolicyHeaderRules(t *testing.T) { t.Parallel() - t.Run("disabled cache returns nil after-rules", func(t *testing.T) { + t.Run("disabled cache returns nil", func(t *testing.T) { t.Parallel() - afterAll, afterSG := AddCacheControlPolicyToRules(config.CacheControlPolicy{ + result := CreateCacheControlPolicyHeaderRules(config.CacheControlPolicy{ Enabled: false, }) - assert.Nil(t, afterAll) - assert.Nil(t, afterSG) + assert.Nil(t, result) }) t.Run("enabled cache returns global after-rule", func(t *testing.T) { t.Parallel() - afterAll, afterSG := AddCacheControlPolicyToRules(config.CacheControlPolicy{ + result := CreateCacheControlPolicyHeaderRules(config.CacheControlPolicy{ Enabled: true, Value: "max-age=300", }) - require.Len(t, afterAll, 1) - assert.Equal(t, config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, afterAll[0].Algorithm) - assert.Equal(t, "max-age=300", afterAll[0].Default) - assert.Nil(t, afterSG) + require.NotNil(t, result) + require.Len(t, result.All, 1) + assert.Equal(t, config.ResponseHeaderRuleAlgorithmMostRestrictiveCacheControl, result.All[0].Algorithm) + assert.Equal(t, "max-age=300", result.All[0].Default) + assert.Nil(t, result.Subgraphs) }) t.Run("subgraph-specific cache returns per-subgraph after-rule", func(t *testing.T) { t.Parallel() - afterAll, afterSG := AddCacheControlPolicyToRules(config.CacheControlPolicy{ + result := CreateCacheControlPolicyHeaderRules(config.CacheControlPolicy{ Subgraphs: []config.SubgraphCacheControlRule{ {Name: "sg1", Value: "max-age=60"}, }, }) - assert.Nil(t, afterAll) - require.Contains(t, afterSG, "sg1") - require.Len(t, afterSG["sg1"], 1) - assert.Equal(t, "max-age=60", afterSG["sg1"][0].Default) + require.NotNil(t, result) + assert.Nil(t, result.All) + require.Contains(t, result.Subgraphs, "sg1") + require.Len(t, result.Subgraphs["sg1"], 1) + assert.Equal(t, "max-age=60", result.Subgraphs["sg1"][0].Default) }) t.Run("global and subgraph rules coexist", func(t *testing.T) { t.Parallel() - afterAll, afterSG := AddCacheControlPolicyToRules(config.CacheControlPolicy{ + result := CreateCacheControlPolicyHeaderRules(config.CacheControlPolicy{ Enabled: true, Value: "max-age=300", Subgraphs: []config.SubgraphCacheControlRule{ {Name: "sg1", Value: "max-age=60"}, }, }) - require.Len(t, afterAll, 1) - assert.Equal(t, "max-age=300", afterAll[0].Default) - require.Contains(t, afterSG, "sg1") - assert.Equal(t, "max-age=60", afterSG["sg1"][0].Default) - }) - - t.Run("enabled with value returns after-rule with default", func(t *testing.T) { - t.Parallel() - afterAll, _ := AddCacheControlPolicyToRules(config.CacheControlPolicy{ - Enabled: true, - Value: "max-age=300", - }) - require.Len(t, afterAll, 1) - assert.Equal(t, "max-age=300", afterAll[0].Default) + require.NotNil(t, result) + require.Len(t, result.All, 1) + assert.Equal(t, "max-age=300", result.All[0].Default) + require.Contains(t, result.Subgraphs, "sg1") + assert.Equal(t, "max-age=60", result.Subgraphs["sg1"][0].Default) }) } @@ -380,14 +372,14 @@ func TestNewHeaderPropagation(t *testing.T) { t.Run("nil rules returns nil", func(t *testing.T) { t.Parallel() - hp, err := NewHeaderPropagation(nil, nil, nil) + hp, err := NewHeaderPropagation(nil, nil) require.NoError(t, err) assert.Nil(t, hp) }) t.Run("empty rules returns valid instance", func(t *testing.T) { t.Parallel() - hp, err := NewHeaderPropagation(&config.HeaderRules{}, nil, nil) + hp, err := NewHeaderPropagation(&config.HeaderRules{}, nil) require.NoError(t, err) require.NotNil(t, hp) }) @@ -400,7 +392,7 @@ func TestNewHeaderPropagation(t *testing.T) { {Operation: config.HeaderRuleOperationPropagate, Matching: "[invalid"}, }, }, - }, nil, nil) + }, nil) require.Error(t, err) }) @@ -412,7 +404,7 @@ func TestNewHeaderPropagation(t *testing.T) { {Operation: config.HeaderRuleOperationPropagate, Matching: "[invalid"}, }, }, - }, nil, nil) + }, nil) require.Error(t, err) }) diff --git a/router/core/router.go b/router/core/router.go index 21d3ed9028..c4cacc8e98 100644 --- a/router/core/router.go +++ b/router/core/router.go @@ -311,12 +311,13 @@ func NewRouter(opts ...Option) (*Router, error) { r.livenessCheckPath = "/health/live" } - afterAll, afterSubgraphs := AddCacheControlPolicyToRules(r.cacheControlPolicy) + postRules := CreateCacheControlPolicyHeaderRules(r.cacheControlPolicy) var err error - r.headerPropagation, err = NewHeaderPropagation(r.headerRules, afterAll, afterSubgraphs) + r.headerPropagation, err = NewHeaderPropagation(r.headerRules, postRules) if err != nil { return nil, err } + defaultCorsHeaders := []string{ // Common headers "authorization", From 18489eb14e962f4919c70f30536a1cb25b3da25e Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 27 Mar 2026 00:28:14 +0530 Subject: [PATCH 05/11] fix: review comments --- .../subgraph-response-header-operations.mdx | 2 +- router/core/header_rule_engine.go | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx index b91d1928d1..c0353cf779 100644 --- a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx +++ b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx @@ -64,7 +64,7 @@ headers: ### What does the snippet do? -With `all` we address all subgraph responses. Next, we can define several rules on the response headers. The operation `propagate` forwards matching subgraph response headers to the client. The operation `set` injects a header value into the subgraph response so it can be processed by downstream rules (e.g., the cache control algorithm or propagate rules). +With `all` we address all subgraph responses. Next, we can define several rules on the response headers. The operation `propagate` forwards matching subgraph response headers to the client. The operation `set` injects a header value into the subgraph response — for `Cache-Control`, this value is picked up by the cache control algorithm; for other headers, the value is not forwarded to the client unless a separate `propagate` rule matches it. The `subgraphs` section allows to propagate headers for specific subgraphs. The name must match with the subgraph name in the Studio. diff --git a/router/core/header_rule_engine.go b/router/core/header_rule_engine.go index 69d11295d9..cf1983aefc 100644 --- a/router/core/header_rule_engine.go +++ b/router/core/header_rule_engine.go @@ -154,10 +154,7 @@ type HeaderPropagation struct { // Precomputed request rule presence for fast-path checks hasAllRequestRules bool subgraphHasRequestRules map[string]bool - // postResponseRules runs after all other response rules (All + Subgraph-specific). - // Used by cache_control_policy so that set rules have already injected - // values into res.Header before the algorithm reads them. - postResponseRules *PostResponseRules + postResponseRules *PostResponseRules } // PostResponseRules holds response rules that execute after all user-defined From 098f6425e6b56b5814ecb1636b232e1136085366 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 27 Mar 2026 00:36:30 +0530 Subject: [PATCH 06/11] fix: docs --- .../router/proxy-capabilities/adjusting-cache-control.mdx | 8 ++++++++ .../subgraph-response-header-operations.mdx | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx b/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx index 6c27272d5b..e34f963f31 100644 --- a/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx +++ b/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx @@ -164,3 +164,11 @@ cache_control_policy: - name: "specific-subgraph" value: "max-age=5400, public" ``` + + +As such we recommend not using this method and simply sticking to using the `subgraph` configuration in `cache_control_policy`, as it makes things more explicit and clearer. + + + + Until router version VERSION, setting a `Cache-Control` header would do a hard override of whatever value would be computed by the `cache_control_policy` configuration. This has been changed since. + \ No newline at end of file diff --git a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx index c0353cf779..4032a875a3 100644 --- a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx +++ b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx @@ -111,7 +111,7 @@ The `set` value is **not** forwarded to the client response. You can use `set`, to set a header value and then use a `propagate` rule to propagate that same header to the client. Howevever we would caution against doing this, as no client header will be propagated in case no subgraph was actually called. - + ### Configuration From 4f2529eff0aec932ddc71ac05831401df8a4aee4 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 27 Mar 2026 00:38:00 +0530 Subject: [PATCH 07/11] fix: docs --- .../proxy-capabilities/subgraph-response-header-operations.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx index 4032a875a3..b19f029016 100644 --- a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx +++ b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx @@ -110,7 +110,7 @@ The `set` value is **not** forwarded to the client response. -You can use `set`, to set a header value and then use a `propagate` rule to propagate that same header to the client. Howevever we would caution against doing this, as no client header will be propagated in case no subgraph was actually called. +You can use `set` to set a header value and then use a `propagate` rule to forward that same header to the client. However, we would caution against this, as the header will not be propagated if no subgraph was actually called. ### Configuration From 25fa12d096085c61082061c3c1c25b580beb9e96 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 27 Mar 2026 00:46:08 +0530 Subject: [PATCH 08/11] fix: updates --- router-tests/protocol/header_set_test.go | 38 ++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/router-tests/protocol/header_set_test.go b/router-tests/protocol/header_set_test.go index ffe75d72be..82fa492992 100644 --- a/router-tests/protocol/header_set_test.go +++ b/router-tests/protocol/header_set_test.go @@ -220,6 +220,44 @@ func TestHeaderSet(t *testing.T) { }) }) + t.Run("subgraph set Cache-Control feeds into restrictive algorithm", func(t *testing.T) { + t.Parallel() + // A subgraph-level set rule injects Cache-Control into the employees + // response. The cache control algorithm (which runs after all user + // rules) sees this value and uses it in the restrictive merge. + testenv.Run(t, &testenv.Config{ + CacheControlPolicy: config.CacheControlPolicy{ + Enabled: true, + Value: "max-age=300, public", + }, + RouterOptions: []core.Option{ + core.WithHeaderRules(config.HeaderRules{ + Subgraphs: map[string]*config.GlobalHeaderRule{ + "employees": { + Response: []*config.ResponseHeaderRule{ + { + Operation: config.HeaderRuleOperationSet, + Name: "Cache-Control", + Value: "max-age=45, public", + }, + }, + }, + }, + }), + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ + Query: queryEmployeeWithHobby, + }) + cc := res.Response.Header.Get("Cache-Control") + // The set rule injects max-age=45 on the employees subgraph. + // The hobbies subgraph has no Cache-Control, so it uses the + // default (max-age=300). The algorithm picks the most + // restrictive: max-age=45. + require.Equal(t, "max-age=45, public", cc, "subgraph-level set should feed into the cache control algorithm") + }) + }) + t.Run("set overwrites existing subgraph response header", func(t *testing.T) { t.Parallel() // The employees subgraph returns X-Custom-Header: original-value. From 237f0de1af5b9d14ae385a23e9bbe88216fa0fe5 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 24 Apr 2026 02:38:29 +0530 Subject: [PATCH 09/11] fix: review comments --- .../adjusting-cache-control.mdx | 6 +--- .../subgraph-response-header-operations.mdx | 32 +++---------------- .../protocol/header_propagation_race_test.go | 4 +-- 3 files changed, 7 insertions(+), 35 deletions(-) diff --git a/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx b/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx index e34f963f31..186d78e079 100644 --- a/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx +++ b/docs-website/router/proxy-capabilities/adjusting-cache-control.mdx @@ -166,9 +166,5 @@ cache_control_policy: ``` -As such we recommend not using this method and simply sticking to using the `subgraph` configuration in `cache_control_policy`, as it makes things more explicit and clearer. + We do not recommend using this method, simply sticking to using the `subgraph` configuration in `cache_control_policy` makes things more explicit and clearer. - - - Until router version VERSION, setting a `Cache-Control` header would do a hard override of whatever value would be computed by the `cache_control_policy` configuration. This has been changed since. - \ No newline at end of file diff --git a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx index b19f029016..1d02fcb0eb 100644 --- a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx +++ b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx @@ -86,6 +86,8 @@ Currently, we support the following header rules: * `default` - Fallback to this value when the `named`, `matching` or `rename` header could not be found. +* **set** - Sets a header on the response from the subgraph to the router. + Go canonicalizes headers by default e.g. `x-my-header` to `X-My-Header.` Write your rule accordingly or use `(?i)``^X-Test-.*` flags to make your regex case insensitive. @@ -104,13 +106,8 @@ Within each scope, rules execute in definition order. This means a `set` rule de The `set` operation injects a header value into the subgraph response, making it appear as if the subgraph returned it. This value is then processed by downstream rules (e.g., `propagate` rules or the [restrictive cache control algorithm](/router/proxy-capabilities/adjusting-cache-control)). - -The `set` value is **not** forwarded to the client response. - - - -You can use `set` to set a header value and then use a `propagate` rule to forward that same header to the client. However, we would caution against this, as the header will not be propagated if no subgraph was actually called. + The `set` value is **not** forwarded to the client response unless a `propagate` rule explicitly forwards it. We caution against relying on this pattern, as the header will not be propagated if no subgraph is actually called. ### Configuration @@ -120,25 +117,4 @@ You can use `set` to set a header value and then use a `propagate` rule to forwa ### Example: Setting Cache Control on a Subgraph Response -```yaml -cache_control_policy: - enabled: true - value: "max-age=180, public" - -headers: - subgraphs: - specific-subgraph: - response: - - op: "set" - name: "Cache-Control" - value: "max-age=5400" -``` - -In this example, when a request hits `specific-subgraph`, the `Cache-Control: max-age=5400` value is injected into the subgraph response. The restrictive cache control algorithm then processes this value alongside other subgraph responses to compute the final `Cache-Control` header sent to the client. This is equivalent to doing - -``` -cache_control_policy: - enabled: true - value: "max-age=180, public" - -``` \ No newline at end of file +While users can use `set` on to set `Cache-Control` headers on subgraph responses, we instead recommend using the [`cache_control_policy` configuration](/router/proxy-capabilities/adjusting-cache-control). diff --git a/router-tests/protocol/header_propagation_race_test.go b/router-tests/protocol/header_propagation_race_test.go index 1576cc48dc..25d9ee497c 100644 --- a/router-tests/protocol/header_propagation_race_test.go +++ b/router-tests/protocol/header_propagation_race_test.go @@ -119,8 +119,8 @@ func TestHeaderPropagationConcurrentMapWrites(t *testing.T) { require.Equal(t, http.StatusOK, res.Response.StatusCode) require.Equal(t, expectedResponse, res.Body) - require.Equal(t, "", res.Response.Header.Get("X-Header-A"), "set response headers should not be forwarded to the client") - require.Equal(t, "", res.Response.Header.Get("X-Header-B"), "set response headers should not be forwarded to the client") + require.Empty(t, res.Response.Header.Get("X-Header-A"), "set response headers should not be forwarded to the client") + require.Empty(t, res.Response.Header.Get("X-Header-B"), "set response headers should not be forwarded to the client") }) }) } From 9efd406eefda7e5f8b4466f277a2749949daaf08 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 24 Apr 2026 02:39:30 +0530 Subject: [PATCH 10/11] fix: review comments --- .../proxy-capabilities/subgraph-response-header-operations.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx index 1d02fcb0eb..80281eb221 100644 --- a/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx +++ b/docs-website/router/proxy-capabilities/subgraph-response-header-operations.mdx @@ -115,6 +115,6 @@ The `set` operation injects a header value into the subgraph response, making it * `name` - The name of the header to set * `value` - The value to set for the header -### Example: Setting Cache Control on a Subgraph Response +### Setting Cache Control on a Subgraph Response While users can use `set` on to set `Cache-Control` headers on subgraph responses, we instead recommend using the [`cache_control_policy` configuration](/router/proxy-capabilities/adjusting-cache-control). From 282e6ccf3816b2ee4a9641abbd341ca2e3bb8ed9 Mon Sep 17 00:00:00 2001 From: Milinda Dias Date: Fri, 24 Apr 2026 02:55:06 +0530 Subject: [PATCH 11/11] fix: gofmt --- router/core/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/core/router.go b/router/core/router.go index d8e42e9c2f..267a8dc405 100644 --- a/router/core/router.go +++ b/router/core/router.go @@ -319,7 +319,7 @@ func NewRouter(opts ...Option) (*Router, error) { if err != nil { return nil, err } - + defaultCorsHeaders := []string{ // Common headers "authorization",