[TT-10909]: fix issue with missing upstream headers in graphql proxy …

…only (#6166)

<!-- Provide a general summary of your changes in the Title above -->

This PR fixes an issue where the requests from the client were not sent
upstream. This was due to an edge case cause by the open telemetry
context modification

[TT-10909](https://tyktech.atlassian.net/browse/TT-10909)

This PR also fixes a situation where the requested content-encoding by
the client is ignored

<!-- Describe your changes in detail -->

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

<!-- Why is this change required? What problem does it solve? -->

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

[TT-10909]:
https://tyktech.atlassian.net/browse/TT-10909?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

___

bug_fix, enhancement

___

- Added a new test to ensure headers are correctly passed to the
upstream when OpenTelemetry is enabled.
- Introduced a new context management strategy for GraphQL proxy-only
mode to correctly forward headers.
- Updated various components within the internal GraphQL engine to
utilize the new context structure for header forwarding.

___

<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Tests</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>reverse_proxy_test.go</strong><dd><code>Add Test for
GraphQL Proxy with OpenTelemetry</code>&nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

gateway/reverse_proxy_test.go
<li>Added a new test
<code>TestGraphQL_ProxyOnlyPassHeadersWithOTel</code> to ensure
<br>headers are passed upstream when OpenTelemetry is enabled.

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6166/files#diff-ce040f6555143f760fba6059744bc600b6954f0966dfb0fa2832b5eabf7a3c3f">+38/-0</a>&nbsp;
&nbsp; </td>
</tr>
</table></td></tr><tr><td><strong>Enhancement</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>context.go</strong><dd><code>Manage GraphQL Proxy
Context for Headers Forwarding</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; </dd></summary>
<hr>

internal/graphengine/context.go
<li>Introduced <code>GraphQLProxyOnlyContextValues</code> struct to
store request and <br>response details.<br> <li> Added functions
<code>SetProxyOnlyContextValue</code> and
<code>GetProxyOnlyContextValue</code> <br>for managing GraphQL proxy
context.

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6166/files#diff-59c1392237cf0565e56acd0f46f7500043ec66fff078bf211ceefbb983baaf94">+31/-0</a>&nbsp;
&nbsp; </td>
</tr>

<tr>
  <td>
    <details>
<summary><strong>engine_v2.go</strong><dd><code>Integrate New Context
Management in Engine V2</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

internal/graphengine/engine_v2.go
<li>Modified to use <code>SetProxyOnlyContextValue</code> for setting
proxy context.<br> <li> Updated to retrieve proxy context using
<code>GetProxyOnlyContextValue</code>.

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6166/files#diff-b1eaa954c9836f395e1d49090e85c739e3878747c8bd748f556fc5a53ff7b191">+2/-2</a>&nbsp;
&nbsp; &nbsp; </td>
</tr>

<tr>
  <td>
    <details>
<summary><strong>graphql_go_tools_v1.go</strong><dd><code>Update Error
Handling with New Context Structure</code>&nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

internal/graphengine/graphql_go_tools_v1.go
<li>Updated <code>returnErrorsFromUpstream</code> to use
<code>GraphQLProxyOnlyContextValues</code>.

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6166/files#diff-e592cc8ca6ac39e7574765d7f2bbf19193f173791a1b0930d4dde7f9412dc882">+1/-1</a>&nbsp;
&nbsp; &nbsp; </td>
</tr>

<tr>
  <td>
    <details>
<summary><strong>transport.go</strong><dd><code>Refactor Transport Logic
for GraphQL Proxy</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

internal/graphengine/transport.go
<li>Refactored to use <code>GraphQLProxyOnlyContextValues</code> for
handling <br>proxy-only requests.<br> <li> Adjusted header forwarding
logic to accommodate new context structure.

</details>

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6166/files#diff-564061c9b29366529eb1f6f10fe39671d2ac738a4731ffd2c8b04dcc0a8cd610">+10/-10</a>&nbsp;
</td>
</tr>
</table></td></tr></tr></tbody></table>

___

> ✨ **PR-Agent usage**:
>Comment `/help` on the PR to get a list of all available PR-Agent tools
and their descriptions

(cherry picked from commit 6ab2b56)

ci/tests/tracing/scenarios/tyk_test-graphql-tracing-invalid_404.yml

@@ -6,7 +6,7 @@ spec:
   trigger:
     type: http
     httpRequest:
-      method: GET
+      method: POST
       url: tyk:8080/test-graphql-tracing-invalid/test-graphql-tracing-invalid
       body: "{\n  \"query\": \"{\\n  country(code: \\\"NG\\\"){\\n    name\\n  }\\n}\"\n}"
       headers:

gateway/handler_success_test.go

@@ -5,6 +5,8 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/TykTechnologies/graphql-go-tools/pkg/engine/datasource/httpclient"
+
 	"github.com/TykTechnologies/graphql-go-tools/pkg/graphql"
 	"github.com/TykTechnologies/tyk-pump/analytics"
 	"github.com/TykTechnologies/tyk/test"
@@ -111,6 +113,135 @@ func TestRecordDetail(t *testing.T) {
 	}
 }
 
+<<<<<<< HEAD
+=======
+func TestAnalyticRecord_GraphStats(t *testing.T) {
+
+	apiDef := BuildAPI(func(spec *APISpec) {
+		spec.Name = "graphql API"
+		spec.APIID = "graphql-api"
+		spec.Proxy.TargetURL = testGraphQLProxyUpstream
+		spec.Proxy.ListenPath = "/"
+		spec.GraphQL = apidef.GraphQLConfig{
+			Enabled:       true,
+			ExecutionMode: apidef.GraphQLExecutionModeProxyOnly,
+			Version:       apidef.GraphQLConfigVersion2,
+			Schema:        gqlProxyUpstreamSchema,
+		}
+	})[0]
+
+	testCases := []struct {
+		name      string
+		code      int
+		request   graphql.Request
+		checkFunc func(*testing.T, *analytics.AnalyticsRecord)
+		reloadAPI func(*APISpec)
+	}{
+		{
+			name: "successfully generate stats",
+			code: http.StatusOK,
+			request: graphql.Request{
+				Query: `{ hello(name: "World") httpMethod }`,
+			},
+			checkFunc: func(t *testing.T, record *analytics.AnalyticsRecord) {
+				assert.True(t, record.GraphQLStats.IsGraphQL)
+				assert.False(t, record.GraphQLStats.HasErrors)
+				assert.ElementsMatch(t, []string{"hello", "httpMethod"}, record.GraphQLStats.RootFields)
+				assert.Equal(t, map[string][]string{}, record.GraphQLStats.Types)
+				assert.Equal(t, analytics.OperationQuery, record.GraphQLStats.OperationType)
+			},
+		},
+		{
+			name: "should have variables",
+			code: http.StatusOK,
+			request: graphql.Request{
+				Query:     `{ hello(name: "World") httpMethod }`,
+				Variables: []byte(`{"in":"hello"}`),
+			},
+			checkFunc: func(t *testing.T, record *analytics.AnalyticsRecord) {
+				assert.True(t, record.GraphQLStats.IsGraphQL)
+				assert.False(t, record.GraphQLStats.HasErrors)
+				assert.ElementsMatch(t, []string{"httpMethod", "hello"}, record.GraphQLStats.RootFields)
+				assert.Equal(t, map[string][]string{}, record.GraphQLStats.Types)
+				assert.Equal(t, analytics.OperationQuery, record.GraphQLStats.OperationType)
+				assert.Equal(t, `{"in":"hello"}`, record.GraphQLStats.Variables)
+			},
+		},
+		{
+			name: "should read response and error response request with detailed recording",
+			code: http.StatusInternalServerError,
+			request: graphql.Request{
+				Query:     `{ hello(name: "World") httpMethod }`,
+				Variables: []byte(`{"in":"hello"}`),
+			},
+			reloadAPI: func(spec *APISpec) {
+				spec.Proxy.TargetURL = testGraphQLProxyUpstreamError
+				spec.EnableDetailedRecording = true
+			},
+			checkFunc: func(t *testing.T, record *analytics.AnalyticsRecord) {
+				assert.True(t, record.GraphQLStats.IsGraphQL)
+				assert.True(t, record.GraphQLStats.HasErrors)
+				assert.ElementsMatch(t, []string{"hello", "httpMethod"}, record.GraphQLStats.RootFields)
+				assert.Equal(t, map[string][]string{}, record.GraphQLStats.Types)
+				assert.Equal(t, analytics.OperationQuery, record.GraphQLStats.OperationType)
+				assert.Equal(t, `{"in":"hello"}`, record.GraphQLStats.Variables)
+				assert.Equal(t, []analytics.GraphError{
+					{Message: "unable to resolve"},
+				}, record.GraphQLStats.Errors)
+			},
+		},
+		{
+			name: "should read response request without detailed recording",
+			code: http.StatusInternalServerError,
+			request: graphql.Request{
+				Query:     `{ hello(name: "World") httpMethod }`,
+				Variables: []byte(`{"in":"hello"}`),
+			},
+			reloadAPI: func(spec *APISpec) {
+				spec.Proxy.TargetURL = testGraphQLProxyUpstreamError
+			},
+			checkFunc: func(t *testing.T, record *analytics.AnalyticsRecord) {
+				assert.True(t, record.GraphQLStats.IsGraphQL)
+				assert.True(t, record.GraphQLStats.HasErrors)
+				assert.ElementsMatch(t, []string{"hello", "httpMethod"}, record.GraphQLStats.RootFields)
+				assert.Equal(t, map[string][]string{}, record.GraphQLStats.Types)
+				assert.Equal(t, analytics.OperationQuery, record.GraphQLStats.OperationType)
+				assert.Equal(t, `{"in":"hello"}`, record.GraphQLStats.Variables)
+				assert.Equal(t, []analytics.GraphError{
+					{Message: "unable to resolve"},
+				}, record.GraphQLStats.Errors)
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			spec := apiDef
+			if tc.reloadAPI != nil {
+				tc.reloadAPI(spec)
+			}
+
+			ts := StartTest(nil)
+			defer ts.Close()
+			ts.Gw.LoadAPI(spec)
+			ts.Gw.Analytics.mockEnabled = true
+			ts.Gw.Analytics.mockRecordHit = func(record *analytics.AnalyticsRecord) {
+				tc.checkFunc(t, record)
+			}
+			_, err := ts.Run(t, test.TestCase{
+				Data:   tc.request,
+				Method: http.MethodPost,
+				Code:   tc.code,
+				Headers: map[string]string{
+					httpclient.AcceptEncodingHeader: "",
+				},
+			})
+			assert.NoError(t, err)
+		})
+	}
+}
+
+>>>>>>> 6ab2b5693... [TT-10909]: fix issue with missing upstream headers in graphql proxy only (#6166)
 func TestAnalyticsIgnoreSubgraph(t *testing.T) {
 	ts := StartTest(nil)
 	defer ts.Close()

gateway/mw_graphql_transport.go

@@ -71,18 +71,18 @@ func NewGraphQLEngineTransport(transportType GraphQLEngineTransportType, origina
 func (g *GraphQLEngineTransport) RoundTrip(request *http.Request) (res *http.Response, err error) {
 	switch g.transportType {
 	case GraphQLEngineTransportTypeProxyOnly:
-		proxyOnlyCtx, ok := request.Context().(*GraphQLProxyOnlyContext)
-		if ok {
-			return g.handleProxyOnly(proxyOnlyCtx, request)
+		val := GetProxyOnlyContextValue(request.Context())
+		if val != nil {
+			return g.handleProxyOnly(val, request)
 		}
 	}
 
 	return g.originalTransport.RoundTrip(request)
 }
 
-func (g *GraphQLEngineTransport) handleProxyOnly(proxyOnlyCtx *GraphQLProxyOnlyContext, request *http.Request) (*http.Response, error) {
-	request.Method = proxyOnlyCtx.forwardedRequest.Method
-	g.setProxyOnlyHeaders(proxyOnlyCtx, request)
+func (g *GraphQLEngineTransport) handleProxyOnly(proxyOnlyValues *GraphQLProxyOnlyContextValues, request *http.Request) (*http.Response, error) {
+	request.Method = proxyOnlyValues.forwardedRequest.Method
+	g.setProxyOnlyHeaders(proxyOnlyValues, request)
 
 	response, err := g.originalTransport.RoundTrip(request)
 	if err != nil {
@@ -109,13 +109,13 @@ func (g *GraphQLEngineTransport) handleProxyOnly(proxyOnlyCtx *GraphQLProxyOnlyC
 		}
 		response.Body = reusableBody
 	}
-	proxyOnlyCtx.upstreamResponse = response
+	proxyOnlyValues.upstreamResponse = response
 	return response, err
 }
 
-func (g *GraphQLEngineTransport) setProxyOnlyHeaders(proxyOnlyCtx *GraphQLProxyOnlyContext, r *http.Request) {
-	for forwardedHeaderKey, forwardedHeaderValues := range proxyOnlyCtx.forwardedRequest.Header {
-		if proxyOnlyCtx.ignoreForwardedHeaders[forwardedHeaderKey] {
+func (g *GraphQLEngineTransport) setProxyOnlyHeaders(proxyOnlyValues *GraphQLProxyOnlyContextValues, r *http.Request) {
+	for forwardedHeaderKey, forwardedHeaderValues := range proxyOnlyValues.forwardedRequest.Header {
+		if proxyOnlyValues.ignoreForwardedHeaders[forwardedHeaderKey] {
 			continue
 		}
 

gateway/reverse_proxy_test.go

@@ -1000,6 +1000,44 @@ func TestGraphQL_ProxyOnlyHeaders(t *testing.T) {
 	})
 }
 
+func TestGraphQL_ProxyOnlyPassHeadersWithOTel(t *testing.T) {
+	g := StartTest(func(globalConf *config.Config) {
+		globalConf.OpenTelemetry.Enabled = true
+	})
+	defer g.Close()
+
+	spec := BuildAPI(func(spec *APISpec) {
+		spec.Name = "tyk-api"
+		spec.APIID = "tyk-api"
+		spec.GraphQL.Enabled = true
+		spec.GraphQL.ExecutionMode = apidef.GraphQLExecutionModeProxyOnly
+		spec.GraphQL.Schema = gqlCountriesSchema
+		spec.GraphQL.Version = apidef.GraphQLConfigVersion2
+		spec.Proxy.TargetURL = TestHttpAny + "/dynamic"
+		spec.Proxy.ListenPath = "/"
+	})[0]
+
+	g.Gw.LoadAPI(spec)
+	g.AddDynamicHandler("/dynamic", func(writer http.ResponseWriter, r *http.Request) {
+		if gotten := r.Header.Get("custom-client-header"); gotten != "custom-value" {
+			t.Errorf("expected upstream to recieve header `custom-client-header` with value of `custom-value`, instead got %s", gotten)
+		}
+	})
+
+	_, err := g.Run(t, test.TestCase{
+		Path: "/",
+		Headers: map[string]string{
+			"custom-client-header": "custom-value",
+		},
+		Method: http.MethodPost,
+		Data: graphql.Request{
+			Query: gqlContinentQuery,
+		},
+	})
+
+	assert.NoError(t, err)
+}
+
 func TestGraphQL_InternalDataSource(t *testing.T) {
 	g := StartTest(nil)
 	defer g.Close()

internal/graphengine/context.go

@@ -0,0 +1,80 @@
+package graphengine
+
+import (
+	"context"
+	"net/http"
+
+	"github.com/TykTechnologies/tyk/apidef"
+)
+
+type GraphQLEngineTransportType int
+
+const (
+	GraphQLEngineTransportTypeProxyOnly GraphQLEngineTransportType = iota
+	GraphQLEngineTransportTypeMultiUpstream
+)
+
+type contextKey struct{}
+
+var graphqlProxyContextInfo = contextKey{}
+
+type GraphQLProxyOnlyContextValues struct {
+	forwardedRequest       *http.Request
+	upstreamResponse       *http.Response
+	ignoreForwardedHeaders map[string]bool
+}
+
+func SetProxyOnlyContextValue(ctx context.Context, req *http.Request) context.Context {
+	value := &GraphQLProxyOnlyContextValues{
+		forwardedRequest: req,
+		ignoreForwardedHeaders: map[string]bool{
+			http.CanonicalHeaderKey("date"):           true,
+			http.CanonicalHeaderKey("content-type"):   true,
+			http.CanonicalHeaderKey("content-length"): true,
+		},
+	}
+
+	return context.WithValue(ctx, graphqlProxyContextInfo, value)
+}
+
+func GetProxyOnlyContextValue(ctx context.Context) *GraphQLProxyOnlyContextValues {
+	val, ok := ctx.Value(graphqlProxyContextInfo).(*GraphQLProxyOnlyContextValues)
+	if !ok {
+		return nil
+	}
+	return val
+}
+
+func DetermineGraphQLEngineTransportType(apiDefinition *apidef.APIDefinition) GraphQLEngineTransportType {
+	switch apiDefinition.GraphQL.ExecutionMode {
+	case apidef.GraphQLExecutionModeSubgraph:
+		fallthrough
+	case apidef.GraphQLExecutionModeProxyOnly:
+		return GraphQLEngineTransportTypeProxyOnly
+	}
+
+	return GraphQLEngineTransportTypeMultiUpstream
+}
+
+type GraphQLProxyOnlyContext struct {
+	context.Context
+	forwardedRequest       *http.Request
+	upstreamResponse       *http.Response
+	ignoreForwardedHeaders map[string]bool
+}
+
+func NewGraphQLProxyOnlyContext(ctx context.Context, forwardedRequest *http.Request) *GraphQLProxyOnlyContext {
+	return &GraphQLProxyOnlyContext{
+		Context:          ctx,
+		forwardedRequest: forwardedRequest,
+		ignoreForwardedHeaders: map[string]bool{
+			http.CanonicalHeaderKey("date"):           true,
+			http.CanonicalHeaderKey("content-type"):   true,
+			http.CanonicalHeaderKey("content-length"): true,
+		},
+	}
+}
+
+func (g *GraphQLProxyOnlyContext) Response() *http.Response {
+	return g.upstreamResponse
+}