[TT-10291]: support gql-go-tools verison 2

[kofoworola]

User description

Description

This PR adds the suppport of the new graphql-go-tools v2 with the use of the version 3-preview in the graphql version of the api definition

TT-10291

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• [ ] 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)

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

---

Type

Enhancement

---

Description

• Added a series of unit tests across multiple files to ensure the robustness and reliability of the new GraphQL engine configurations and their related functionalities.
• Tests cover the new EngineV3 processing, UniversalDataGraph configurations, proxy-only setups, and utility functions in the GraphQL Go Tools V2.
• Ensures that new functionalities integrate correctly with existing systems and behave as expected under various scenarios.

---

Changes walkthrough

Relevant files

Tests

adapter_udg_test.go Add Unit Tests for UniversalDataGraph Engine Configuration apidef/adapter/gqlengineadapter/v2/adapter_udg_test.go Added comprehensive tests for the UniversalDataGraph struct's methods related to engine configuration. Tests cover scenarios for creating engine configurations, handling field configurations, and data source configurations. Utilizes mock data and assertions to ensure correct behavior of the configuration generation. +775/-0  adapter_proxy_only_test.go Implement Tests for ProxyOnly Engine Configuration              apidef/adapter/gqlengineadapter/v2/adapter_proxy_only_test.go Introduced tests for the ProxyOnly struct focusing on the engine configuration. Checks the correct handling of proxy-only mode configurations including headers and subscription setups. Ensures that the engine configuration is correctly set up with the expected properties from the API definition. +272/-0  mw_graphql_test.go Extend GraphQL Middleware Tests for Engine V3                        gateway/mw_graphql_test.go Extended the existing tests for GraphQL middleware to include scenarios for the new GraphQL engine version 3. Added test cases to verify the correct initialization and request processing by the GraphQL middleware using the new engine. Ensures middleware handles different configurations and request types correctly. +167/-13 engine_v3_test.go Add Tests for EngineV3 Request Processing                                internal/graphengine/engine_v3_test.go Created tests for the new EngineV3 struct to validate its request processing logic. Tests cover error handling, schema parsing, and request execution scenarios. Validates the integration of the new engine with expected GraphQL operations and error management. +100/-0  graphql_go_tools_v2_test.go Unit Tests for GraphQL Go Tools V2 Utilities                          internal/graphengine/graphql_go_tools_v2_test.go Introduced unit tests for the utility functions in graphql_go_tools_v2 which are used across various GraphQL engine configurations. Focuses on testing the parsing of schemas, handling of introspection, and header modifications. Ensures utility functions behave as expected when used in engine configurations. +176/-0

---

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

[github-actions[bot]]

:boom: CI tests failed :see_no_evil:

git-state

all ok

Please look at the run or in the Checks tab.

[github-actions[bot]]

PR Description updated to latest commit (https://github.com/TykTechnologies/tyk/commit/9d4c650158efcde73d12453bd21dd2579203fc4f)

[github-actions[bot]]

PR Review

⏱️ Estimated effort to review [1-5]	4, due to the extensive changes across multiple files, the introduction of new features, and the complexity of integrating GraphQL v2 and v3 configurations. The PR involves updates to core components like schema parsing, data source configuration, and engine setup which require careful review to ensure compatibility and correctness.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The PR introduces changes to the GraphQL engine configuration and handling that could potentially disrupt existing functionalities or introduce new bugs if not thoroughly tested. For example, changes in schema parsing and error handling might affect how queries are processed.
	Performance Concern: The new configurations and additional layers of processing might introduce performance overhead, especially in high-load environments. It's crucial to benchmark these changes against current performance metrics.
🔒 Security concerns	No

Code feedback:

---

relevant file	apidef/adapter/gqlengineadapter/v2/adapter_udg.go
suggestion	Consider handling errors from parseSchema directly within the EngineConfigV2 method to simplify error handling and avoid repeating the check for nil schema across different parts of the code. This change ensures that any issues with schema parsing are caught early and managed in a consistent manner. [important]
relevant line	p.Schema, err = parseSchema(p.ApiDefinition.GraphQL.Schema)

---

relevant file	apidef/adapter/gqlengineadapter/v2/adapter_proxy_only.go
suggestion	It's recommended to validate the url after it's modified by replacing "tyk://" with "http://". This validation can prevent configuration errors that might lead to runtime issues. Adding a simple URL validation check can enhance the robustness of the proxy configuration. [important]
relevant line	url = strings.ReplaceAll(url, "tyk://", "http://")

---

relevant file	gateway/mw_graphql.go
suggestion	Ensure that the new GraphQL engine (V3) initialization includes error handling similar to other engine versions. This would involve logging the error and potentially stopping further initialization if the engine setup fails, which is critical to prevent runtime panics or malfunctions. [important]
relevant line	log.Errorf("Error creating enginev3: %v", err)

---

relevant file	internal/graphengine/engine_v3.go
suggestion	For the ProcessRequest method, consider adding detailed logging for each type of error returned. This would help in debugging and maintaining the system by providing clear insights into the flow and types of issues encountered during request processing. [medium]
relevant line	return err, http.StatusBadRequest

---

---

✨ Review tool usage guide:

---

Overview: The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[github-actions[bot]]

PR Code Suggestions

Category	Suggestions
Best practice	Add error handling after calling RemoveDuplicateApiDefinitionHeaders. Consider checking for errors after calling RemoveDuplicateApiDefinitionHeaders to ensure that the headers are processed correctly without issues. apidef/adapter/gqlengineadapter/adapter_supergraph.go [69] -hdr := RemoveDuplicateApiDefinitionHeaders(apiDefSubgraphConf.Headers, s.ApiDefinition.GraphQL.Supergraph.GlobalHeaders) +hdr, err := RemoveDuplicateApiDefinitionHeaders(apiDefSubgraphConf.Headers, s.ApiDefinition.GraphQL.Supergraph.GlobalHeaders) +if err != nil { + // handle error +}
	Add error handling for header conversion to ensure robustness. Ensure that the ConvertApiDefinitionHeadersToHttpHeaders function handles potential errors or unexpected input, as it directly affects HTTP header configuration. apidef/adapter/gqlengineadapter/adapter_udg.go [106] -Header: ConvertApiDefinitionHeadersToHttpHeaders(restConfig.Headers), +Header, err := ConvertApiDefinitionHeadersToHttpHeaders(restConfig.Headers) +if err != nil { + // handle error +}
	Add validation for the URL after modifying its scheme to ensure it's valid. Validate the URL after replacing the "tyk://" scheme to ensure it results in a valid HTTP URL. apidef/adapter/gqlengineadapter/v2/adapter_proxy_only.go [36] url = strings.ReplaceAll(url, "tyk://", "http://") +if _, err := url.ParseRequestURI(url); err != nil { + // handle invalid URL +}
	Normalize HTTP header keys using http.CanonicalHeaderKey. In the function ConvertApiDefinitionHeadersToHttpHeaders, consider using http.CanonicalHeaderKey to normalize the header keys. This ensures that the headers are correctly recognized regardless of their case. apidef/adapter/gqlengineadapter/v2/utils.go [78] -engineV2Headers.Add(apiDefHeaderKey, apiDefHeaderValue) +engineV2Headers.Add(http.CanonicalHeaderKey(apiDefHeaderKey), apiDefHeaderValue)
	Add explicit handling for unexpected GraphQLConfigVersion values. In the needsGraphQLExecutionEngine function, ensure that the case for GraphQLExecutionModeProxyOnly includes a default return of false to handle unexpected GraphQLConfigVersion values explicitly. gateway/mw_graphql.go [324-326] if apiSpec.GraphQL.Version == apidef.GraphQLConfigVersion2 || apiSpec.GraphQL.Version == apidef.GraphQLConfigVersion3Preview { return true } +return false
	Replace panic with error handling in test cases. Consider handling errors from json.Unmarshal instead of using panic. This will allow for more graceful error handling and better control over the application flow. gateway/mw_graphql_test.go [467-469] if err := json.Unmarshal([]byte(testRESTDataSourceConfigurationV3), &ds); err != nil { - panic(err) + t.Fatalf("Failed to unmarshal data source configuration: %v", err) }
	Add error handling for test execution methods. Consider adding error checks for the g.Run method calls to handle potential test case failures more explicitly. gateway/mw_graphql_test.go [517] -_, _ = g.Run(t, test.TestCase{Data: request, BodyMatch: `{"kind":"OBJECT","name":"Country"`, Code: http.StatusOK}) +_, err = g.Run(t, test.TestCase{Data: request, BodyMatch: `{"kind":"OBJECT","name":"Country"`, Code: http.StatusOK}) +if err != nil { + t.Errorf("Failed to run test case: %v", err) +}
Enhancement	Modify RemoveDuplicateApiDefinitionHeaders to return an error for better error handling. Refactor RemoveDuplicateApiDefinitionHeaders to return an error along with the headers map to handle potential issues during header processing. apidef/adapter/gqlengineadapter/utils.go [71] -func RemoveDuplicateApiDefinitionHeaders(headers ...map[string]string) map[string]string { +func RemoveDuplicateApiDefinitionHeaders(headers ...map[string]string) (map[string]string, error) {
	Improve URL scheme replacement by using URL parsing. Replace the direct string replacement for URL schemes with a more robust URL parsing and modification approach to avoid potential bugs with incorrect URLs. apidef/adapter/gqlengineadapter/v2/adapter_supergraph.go [78-81] if strings.HasPrefix(url, "tyk://") { - url = strings.ReplaceAll(url, "tyk://", "http://") + parsedUrl, err := url.Parse(url) + if err != nil { + return nil, err + } + parsedUrl.Scheme = "http" + url = parsedUrl.String() dataSourceHeaders[apidef.TykInternalApiHeader] = "true" }
	Handle URL scheme changes dynamically for multiple schemes. Use a loop or a map to handle multiple URL schemes dynamically instead of hardcoding only one scheme change. apidef/adapter/gqlengineadapter/v2/adapter_supergraph.go [78-81] -if strings.HasPrefix(url, "tyk://") { - url = strings.ReplaceAll(url, "tyk://", "http://") - dataSourceHeaders[apidef.TykInternalApiHeader] = "true" +schemes := map[string]string{ + "tyk": "http", +} +for oldScheme, newScheme := range schemes { + if strings.HasPrefix(url, oldScheme+"://") { + url = strings.Replace(url, oldScheme+"://", newScheme+"://", 1) + dataSourceHeaders[apidef.TykInternalApiHeader] = "true" + break + } }
	Add logging for errors in setting the merged schema. Add error handling for the SetMergedSchemaFromString method to ensure that the configuration process can gracefully handle failures. apidef/adapter/gqlengineadapter/v2/adapter_supergraph.go [30-32] err := federationConfigV2Factory.SetMergedSchemaFromString(s.ApiDefinition.GraphQL.Supergraph.MergedSDL) if err != nil { + log.Printf("Error setting merged schema: %v", err) return nil, err }
	Add error handling for data source configuration creation. Implement error handling for the graphqlDataSourceConfiguration function to manage potential failures in configuration setup. apidef/adapter/gqlengineadapter/v2/adapter_supergraph.go [56-60] -conf := graphqlDataSourceConfiguration( +conf, err := graphqlDataSourceConfiguration( apiDefSubgraphConf.URL, http.MethodPost, hdr, apiDefSubgraphConf.SubscriptionType) +if err != nil { + return nil, err +}
	Improve URL parsing to handle cases with multiple '?' characters. The function extractURLQueryParamsForEngineV2 should handle cases where the URL contains more than one '?' character. Currently, it only checks for exactly one '?' which might not be sufficient for all URLs. A more robust approach would involve using strings.Index to find the first occurrence of '?' and then splitting the string based on that index. apidef/adapter/gqlengineadapter/v2/utils.go [19-25] -urlParts := strings.Split(url, "?") -urlWithoutParams = urlParts[0] +if idx := strings.Index(url, "?"); idx != -1 { + urlWithoutParams = url[:idx] + queryPart = url[idx+1:] +} else { + urlWithoutParams = url +}
	Add a check to prevent appending empty query values to the query configurations. The function appendURLQueryParamsToEngineV2Queries should check for empty query values before appending them to the engineV2Queries. This will prevent unnecessary query parameters from being sent in requests. apidef/adapter/gqlengineadapter/v2/utils.go [45-47] -*engineV2Queries = append(*engineV2Queries, restDataSource.QueryConfiguration{ - Name: queryKey, - Value: strings.Join(queryValue, ","), -}) +if len(queryValue) > 0 { + *engineV2Queries = append(*engineV2Queries, restDataSource.QueryConfiguration{ + Name: queryKey, + Value: strings.Join(queryValue, ","), + }) +}
	Ensure the HTTP method is normalized to uppercase to avoid method mismatch issues. The function generateRestDataSourceFromGraphql should handle the case where config.Method is not a valid HTTP method by either defaulting to a safe method like GET or by returning an error. apidef/adapter/gqlengineadapter/v2/utils.go [100] -Method: config.Method, +Method: strings.ToUpper(config.Method),
Performance	Add a check for empty headers before conversion to optimize processing. Consider adding a check for empty headers before converting them in ConvertApiDefinitionHeadersToHttpHeaders to avoid unnecessary processing. apidef/adapter/gqlengineadapter/utils_engine_v2.go [39] -Header: ConvertApiDefinitionHeadersToHttpHeaders(dataSourceHeaders), +var header http.Header +if len(dataSourceHeaders) > 0 { + header = ConvertApiDefinitionHeadersToHttpHeaders(dataSourceHeaders) +}
Maintainability	Refactor data source configuration creation into a separate function. Refactor the subgraphDataSourceConfigs function to separate concerns, improving readability and maintainability. apidef/adapter/gqlengineadapter/v2/adapter_supergraph.go [51-66] for _, apiDefSubgraphConf := range s.ApiDefinition.GraphQL.Supergraph.Subgraphs { - if len(apiDefSubgraphConf.SDL) == 0 { - continue - } - hdr := gqlengineadapter.RemoveDuplicateApiDefinitionHeaders(apiDefSubgraphConf.Headers, s.ApiDefinition.GraphQL.Supergraph.GlobalHeaders) - conf := graphqlDataSourceConfiguration( - apiDefSubgraphConf.URL, - http.MethodPost, - hdr, - apiDefSubgraphConf.SubscriptionType) - conf.Federation = graphqlDataSource.FederationConfiguration{ - Enabled: true, - ServiceSDL: apiDefSubgraphConf.SDL, + conf, err := createDataSourceConfig(apiDefSubgraphConf) + if err != nil { + return nil, err } confs = append(confs, conf) }
	Use constants for HTTP status codes. Use a constant for the HTTP status code instead of a hard-coded value to improve code readability and maintainability. gateway/mw_graphql_test.go [501-506] -Code: http.StatusOK +Code: http.StatusOK // Use http constant for status code
	Refactor repeated setup code into a helper function. Refactor repeated code blocks into a helper function to reduce code duplication and improve test maintainability. gateway/mw_graphql_test.go [413-419] -spec.UseKeylessAccess = true -spec.GraphQL.Enabled = true -spec.GraphQL.ExecutionMode = apidef.GraphQLExecutionModeProxyOnly -spec.GraphQL.Version = apidef.GraphQLConfigVersion3Preview -spec.GraphQL.Schema = gqlProxyUpstreamSchema +configureGraphQLProxy(spec)
Bug	Correctly escape regex patterns in BodyMatch. Ensure that the BodyMatch regex patterns are correctly escaped and tested to avoid runtime errors and ensure they match the intended patterns. gateway/mw_graphql_test.go [501-502] -BodyMatch: `"countries":.*{"name":"Turkey"},{"name":"Russia"}.*` +BodyMatch: `"countries":.*\{"name":"Turkey"\},\{"name":"Russia"\}.*`

---

✨ Improve tool usage guide:

---

Overview: The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

[github-actions[bot]]

API Changes

--- prev.txt	2024-05-08 07:21:55.521930831 +0000
+++ current.txt	2024-05-08 07:21:52.269909386 +0000
@@ -1262,9 +1262,10 @@
 type GraphQLConfigVersion string
 
 const (
-	GraphQLConfigVersionNone GraphQLConfigVersion = ""
-	GraphQLConfigVersion1    GraphQLConfigVersion = "1"
-	GraphQLConfigVersion2    GraphQLConfigVersion = "2"
+	GraphQLConfigVersionNone     GraphQLConfigVersion = ""
+	GraphQLConfigVersion1        GraphQLConfigVersion = "1"
+	GraphQLConfigVersion2        GraphQLConfigVersion = "2"
+	GraphQLConfigVersion3Preview GraphQLConfigVersion = "3-preview"
 )
 type GraphQLEngineConfig struct {
 	FieldConfigs  []GraphQLFieldConfig      `bson:"field_configs" json:"field_configs"`
@@ -1291,15 +1292,15 @@
 }
 
 type GraphQLEngineDataSourceConfigKafka struct {
-	BrokerAddresses      []string              `bson:"broker_addresses" json:"broker_addresses"`
-	Topics               []string              `bson:"topics" json:"topics"`
-	GroupID              string                `bson:"group_id" json:"group_id"`
-	ClientID             string                `bson:"client_id" json:"client_id"`
-	KafkaVersion         string                `bson:"kafka_version" json:"kafka_version"`
-	StartConsumingLatest bool                  `json:"start_consuming_latest"`
-	BalanceStrategy      string                `json:"balance_strategy"`
-	IsolationLevel       string                `json:"isolation_level"`
-	SASL                 kafka_datasource.SASL `json:"sasl"`
+	BrokerAddresses      []string               `bson:"broker_addresses" json:"broker_addresses"`
+	Topics               []string               `bson:"topics" json:"topics"`
+	GroupID              string                 `bson:"group_id" json:"group_id"`
+	ClientID             string                 `bson:"client_id" json:"client_id"`
+	KafkaVersion         string                 `bson:"kafka_version" json:"kafka_version"`
+	StartConsumingLatest bool                   `json:"start_consuming_latest"`
+	BalanceStrategy      string                 `json:"balance_strategy"`
+	IsolationLevel       string                 `json:"isolation_level"`
+	SASL                 GraphQLEngineKafkaSASL `json:"sasl"`
 }
 
 type GraphQLEngineDataSourceConfigREST struct {
@@ -1312,6 +1313,12 @@
 
 type GraphQLEngineDataSourceKind string
 
+type GraphQLEngineKafkaSASL struct {
+	Enable   bool   `json:"enable"`
+	User     string `json:"user"`
+	Password string `json:"password"`
+}
+
 type GraphQLExecutionMode string
     GraphQLExecutionMode is the mode in which the GraphQL Middleware should
     operate.
@@ -2009,6 +2016,8 @@
 
 func (g *GraphQLConfigAdapter) EngineConfigV2() (*graphql.EngineV2Configuration, error)
 
+func (g *GraphQLConfigAdapter) EngineConfigV3() (*gqlv2.EngineV2Configuration, error)
+
 type GraphQLConfigAdapterOption func(adapter *GraphQLConfigAdapter)
 
 func WithHttpClient(httpClient *http.Client) GraphQLConfigAdapterOption
@@ -2017,12 +2026,18 @@
 
 func WithStreamingClient(streamingClient *http.Client) GraphQLConfigAdapterOption
 
+func WithV2Schema(schema *gqlv2.Schema) GraphQLConfigAdapterOption
+
 type GraphQLEngineAdapter interface {
 	EngineConfig() (*graphql.EngineV2Configuration, error)
 }
 
 type GraphQLEngineAdapterType int
 
+type GraphQLEngineAdapterV3 interface {
+	EngineConfigV3() (*gqlv2.EngineV2Configuration, error)
+}
+
 type ImportAdapter interface {
 	Import() (*apidef.APIDefinition, error)
 }
@@ -2042,6 +2057,11 @@
 	ErrGraphQLConfigIsMissingOperation = errors.New("graphql data source config is missing an operation")
 )
 
+FUNCTIONS
+
+func ConvertApiDefinitionHeadersToHttpHeaders(apiDefHeaders map[string]string) http.Header
+func RemoveDuplicateApiDefinitionHeaders(headers ...map[string]string) map[string]string
+
 TYPES
 
 type ProxyOnly struct {
@@ -2076,6 +2096,49 @@
 
 func (u *UniversalDataGraph) EngineConfig() (*graphql.EngineV2Configuration, error)
 
+# Package: ./apidef/adapter/gqlengineadapter/enginev3
+
+package enginev3 // import "github.com/TykTechnologies/tyk/apidef/adapter/gqlengineadapter/enginev3"
+
+
+FUNCTIONS
+
+func ConvertApiDefinitionHeadersToHttpHeaders(apiDefHeaders map[string]string) http.Header
+
+TYPES
+
+type ProxyOnly struct {
+	ApiDefinition   *apidef.APIDefinition
+	HttpClient      *http.Client
+	StreamingClient *http.Client
+	Schema          *graphql.Schema
+
+	// Has unexported fields.
+}
+
+func (p *ProxyOnly) EngineConfigV3() (*graphql.EngineV2Configuration, error)
+
+type Supergraph struct {
+	ApiDefinition   *apidef.APIDefinition
+	HttpClient      *http.Client
+	StreamingClient *http.Client
+
+	// Has unexported fields.
+}
+
+func (s *Supergraph) EngineConfigV3() (*graphql.EngineV2Configuration, error)
+
+type UniversalDataGraph struct {
+	ApiDefinition   *apidef.APIDefinition
+	HttpClient      *http.Client
+	StreamingClient *http.Client
+	Schema          *graphql.Schema
+
+	// Has unexported fields.
+}
+
+func (u *UniversalDataGraph) EngineConfigV3() (*graphql.EngineV2Configuration, error)
+
 # Package: ./apidef/importer
 
 package importer // import "github.com/TykTechnologies/tyk/apidef/importer"

[github-actions[bot]]

:boom: CI tests failed :see_no_evil:

git-state

all ok

Please look at the run or in the Checks tab.

[sonarqubecloud[bot]]

Quality Gate failed

Failed conditions
59.0% Coverage on New Code (required ≥ 80%)
9.6% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

[github-actions[bot]]

:boom: CI tests failed :see_no_evil:

git-state

all ok

Please look at the run or in the Checks tab.

[github-actions[bot]]

:boom: CI tests failed :see_no_evil:

git-state

all ok

Please look at the run or in the Checks tab.

[github-actions[bot]]

:boom: CI tests failed :see_no_evil:

git-state

all ok

Please look at the run or in the Checks tab.

[github-actions[bot]]

:boom: CI tests failed :see_no_evil:

git-state

all ok

Please look at the run or in the Checks tab.

[sonarqubecloud[bot]]

Quality Gate failed

Failed conditions
58.6% Coverage on New Code (required ≥ 80%)
9.6% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint