From c4e11dea511b5980697d3a0cf2cd2b5533af7bf7 Mon Sep 17 00:00:00 2001 From: Vishwas R <30438425+vrajashkr@users.noreply.github.com> Date: Sat, 28 Sep 2024 15:51:04 +0000 Subject: [PATCH] feat(scale-out): use gqlparser for parsing incoming query Signed-off-by: Vishwas Rajashekar --- .../config-cluster-member0.json | 2 +- .../config-cluster-member1.json | 2 +- go.mod | 2 +- go.sum | 2 + pkg/extensions/extension_search.go | 5 +- .../search/gql_proxy/global_search_handler.go | 4 +- pkg/extensions/search/gql_proxy/gql_proxy.go | 73 ++++++++++--------- pkg/extensions/search/search_test.go | 1 - 8 files changed, 50 insertions(+), 41 deletions(-) diff --git a/examples/scale-out-cluster-local/config-cluster-member0.json b/examples/scale-out-cluster-local/config-cluster-member0.json index edabd8df35..99f5fc9982 100644 --- a/examples/scale-out-cluster-local/config-cluster-member0.json +++ b/examples/scale-out-cluster-local/config-cluster-member0.json @@ -1,7 +1,7 @@ { "distSpecVersion": "1.1.0", "storage": { - "rootDirectory": "/tmp/zot0", + "rootDirectory": "/workspace/zot/data/mem1", "dedupe": false }, "http": { diff --git a/examples/scale-out-cluster-local/config-cluster-member1.json b/examples/scale-out-cluster-local/config-cluster-member1.json index ac78d72b98..ba6b9f7982 100644 --- a/examples/scale-out-cluster-local/config-cluster-member1.json +++ b/examples/scale-out-cluster-local/config-cluster-member1.json @@ -1,7 +1,7 @@ { "distSpecVersion": "1.1.0", "storage": { - "rootDirectory": "/tmp/zot1", + "rootDirectory": "/workspace/zot/data/mem2", "dedupe": false }, "http": { diff --git a/go.mod b/go.mod index 9fd1c09aab..2a19a07dec 100644 --- a/go.mod +++ b/go.mod @@ -57,7 +57,7 @@ require ( github.com/stretchr/testify v1.9.0 github.com/swaggo/http-swagger v1.3.4 github.com/swaggo/swag v1.16.3 - github.com/vektah/gqlparser/v2 v2.5.16 + github.com/vektah/gqlparser/v2 v2.5.17 github.com/zitadel/oidc/v3 v3.30.0 go.etcd.io/bbolt v1.3.11 golang.org/x/crypto v0.27.0 diff --git a/go.sum b/go.sum index 9f7c6fc0ba..1c4e7fd811 100644 --- a/go.sum +++ b/go.sum @@ -1503,6 +1503,8 @@ github.com/vbauerster/mpb/v8 v8.7.5 h1:hUF3zaNsuaBBwzEFoCvfuX3cpesQXZC0Phm/JcHZQ github.com/vbauerster/mpb/v8 v8.7.5/go.mod h1:bRCnR7K+mj5WXKsy0NWB6Or+wctYGvVwKn6huwvxKa0= github.com/vektah/gqlparser/v2 v2.5.16 h1:1gcmLTvs3JLKXckwCwlUagVn/IlV2bwqle0vJ0vy5p8= github.com/vektah/gqlparser/v2 v2.5.16/go.mod h1:1lz1OeCqgQbQepsGxPVywrjdBHW2T08PUS3pJqepRww= +github.com/vektah/gqlparser/v2 v2.5.17 h1:9At7WblLV7/36nulgekUgIaqHZWn5hxqluxrxGUhOmI= +github.com/vektah/gqlparser/v2 v2.5.17/go.mod h1:1lz1OeCqgQbQepsGxPVywrjdBHW2T08PUS3pJqepRww= github.com/veraison/go-cose v1.2.1 h1:Gj4x20D0YP79J2+cK3anjGEMwIkg2xX+TKVVGUXwNAc= github.com/veraison/go-cose v1.2.1/go.mod h1:t6V8WJzHm1PD5HNsuDjW3KLv577uWb6UTzbZGvdQHD8= github.com/vmihailenco/msgpack/v4 v4.3.12/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+NXzzngzBKDPIqw4= diff --git a/pkg/extensions/extension_search.go b/pkg/extensions/extension_search.go index f114a758e6..25ae02c0bd 100644 --- a/pkg/extensions/extension_search.go +++ b/pkg/extensions/extension_search.go @@ -92,17 +92,18 @@ func SetupSearchRoutes(conf *config.Config, router *mux.Router, storeController } resConfig := search.GetResolverConfig(log, storeController, metaDB, cveInfo) + executableSchema := gql_generated.NewExecutableSchema(resConfig) allowedMethods := zcommon.AllowedMethods(http.MethodGet, http.MethodPost) - gqlProxy := gqlproxy.GqlProxyRequestHandler(conf, log) + gqlProxy := gqlproxy.GqlProxyRequestHandler(conf, log, executableSchema.Schema()) extRouter := router.PathPrefix(constants.ExtSearchPrefix).Subrouter() extRouter.Use(zcommon.CORSHeadersMiddleware(conf.HTTP.AllowOrigin)) extRouter.Use(zcommon.ACHeadersMiddleware(conf, allowedMethods...)) extRouter.Use(zcommon.AddExtensionSecurityHeaders()) extRouter.Methods(allowedMethods...). - Handler(gqlProxy(gqlHandler.NewDefaultServer(gql_generated.NewExecutableSchema(resConfig)))) + Handler(gqlProxy(gqlHandler.NewDefaultServer(executableSchema))) log.Info().Msg("finished setting up search routes") } diff --git a/pkg/extensions/search/gql_proxy/global_search_handler.go b/pkg/extensions/search/gql_proxy/global_search_handler.go index 475e4b220b..57e83923a2 100644 --- a/pkg/extensions/search/gql_proxy/global_search_handler.go +++ b/pkg/extensions/search/gql_proxy/global_search_handler.go @@ -96,7 +96,7 @@ func HandleGlobalSearchResult( ) } - responseBody, err := json.Marshal(collatedResult) + responseBody, err := json.MarshalIndent(collatedResult, "", " ") if err != nil { log.Error(). Str(LoggerFieldOperation, HandlerOperation). @@ -106,6 +106,8 @@ func HandleGlobalSearchResult( return } + response.Header().Set("Content-Type", "application/json") + _, err = response.Write(responseBody) if err != nil { log.Error(). diff --git a/pkg/extensions/search/gql_proxy/gql_proxy.go b/pkg/extensions/search/gql_proxy/gql_proxy.go index d12778df2c..14f601434c 100644 --- a/pkg/extensions/search/gql_proxy/gql_proxy.go +++ b/pkg/extensions/search/gql_proxy/gql_proxy.go @@ -2,7 +2,9 @@ package gqlproxy import ( "net/http" - "strings" + + "github.com/vektah/gqlparser/v2" + "github.com/vektah/gqlparser/v2/ast" "zotregistry.dev/zot/pkg/api/config" "zotregistry.dev/zot/pkg/api/constants" @@ -14,7 +16,11 @@ import ( // Requests are only proxied in local cluster mode as in this mode, each instance holds only the // metadata for the images that it serves, however, in shared storage mode, // all the instances have access to all the metadata so any can respond. -func GqlProxyRequestHandler(config *config.Config, log log.Logger) func(handler http.Handler) http.Handler { +func GqlProxyRequestHandler( + config *config.Config, + log log.Logger, + gqlSchema *ast.Schema, +) func(handler http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { // If not running in cluster mode, no op. @@ -37,28 +43,40 @@ func GqlProxyRequestHandler(config *config.Config, log log.Logger) func(handler return } - // General Structure for GQL Requests - // Query String contains the full GraphQL Request (it's NOT JSON) - // e.g. {(query:"", requestedPage: {limit:3 offset:0 sortBy: DOWNLOADS} ) - // {Page {TotalCount ItemCount} Repos {Name LastUpdated Size Platforms { Os Arch } - // IsStarred IsBookmarked NewestImage { Tag Vulnerabilities {MaxSeverity Count} - // Description IsSigned SignGlobalSearchatureInfo { Tool IsTrusted Author } Licenses Vendor Labels } - // StarCount DownloadCount}}} - - // General Payload Structure for GQL Response - /* - { - "errors": [CUSTOM_ERRORS_HERE], - "data": { - "NameOfQuery": {CUSTOM_SCHEMA_HERE} - } + query := request.URL.Query().Get("query") + + // Load the query using gqlparser. + // This helps to read the Operation correctly which is in turn used to + // dynamically hand-off the processing to the appropriate handler. + processedGql, errList := gqlparser.LoadQuery(gqlSchema, query) + + if len(errList) != 0 { + for _, err := range errList { + log.Error().Str("query", query).Err(err).Msg(err.Message) } - */ + http.Error(response, "Failed to process GQL request", http.StatusInternalServerError) - query := request.URL.Query().Get("query") + return + } - operation, ok := computeGqlOperation(query) - if !ok { + // Look at the first operation in the query. + // TODO: for completeness, this should support multiple + // operations at once. + operation := "" + for _, op := range processedGql.Operations { + for _, ss := range op.SelectionSet { + switch ss := ss.(type) { + case *ast.Field: + operation = ss.Name + default: + log.Error().Str("query", query).Msg("Unsupported type") + } + + break + } + } + + if operation == "" { log.Error().Str("query", query).Msg("Failed to compute operation from query") http.Error(response, "Failed to process GQL request", http.StatusInternalServerError) @@ -80,16 +98,3 @@ func GqlProxyRequestHandler(config *config.Config, log log.Logger) func(handler }) } } - -// Naively compute which operation is requested for GQL. -// TODO: Need to replace this with better custom GQL parsing -// or a parsing library that can conver the GQL query to -// a struct where operations data and schema are available for reading. -func computeGqlOperation(request string) (string, bool) { - openParenthesisIndex := strings.Index(request, "(") - if openParenthesisIndex == -1 { - return "", false - } - - return request[1:openParenthesisIndex], true -} diff --git a/pkg/extensions/search/search_test.go b/pkg/extensions/search/search_test.go index ad52afe9f8..d47912edc0 100644 --- a/pkg/extensions/search/search_test.go +++ b/pkg/extensions/search/search_test.go @@ -3916,7 +3916,6 @@ func TestGlobalSearch(t *testing.T) { } func TestGlobalSearchWithScaleOutProxyLocalStorage(t *testing.T) { - // When there are 2 zot instances, the same GlobalSearch query should // return aggregated data from both instances when both instances are queried. Convey("In a local scale-out cluster with 2 members, should return correct data for GlobalSearch", t, func() {