From 06aa1475898b63e1ecb8ca745308849577dc7830 Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Thu, 13 Feb 2020 16:59:38 +1100 Subject: [PATCH 01/12] Move AttachAccessJwt --- dgraph/cmd/alpha/http.go | 19 +++---------------- dgraph/cmd/alpha/run.go | 4 ++-- graphql/web/http.go | 7 +------ x/x.go | 15 +++++++++++++++ 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/dgraph/cmd/alpha/http.go b/dgraph/cmd/alpha/http.go index 58b9fa51a69..468efee5c30 100644 --- a/dgraph/cmd/alpha/http.go +++ b/dgraph/cmd/alpha/http.go @@ -208,7 +208,7 @@ func queryHandler(w http.ResponseWriter, r *http.Request) { } ctx := context.WithValue(context.Background(), query.DebugKey, isDebugMode) - ctx = attachAccessJwt(ctx, r) + ctx = x.AttachAccessJwt(ctx, r) if queryTimeout != 0 { var cancel context.CancelFunc @@ -397,7 +397,7 @@ func mutationHandler(w http.ResponseWriter, r *http.Request) { req.StartTs = startTs req.CommitNow = commitNow - ctx := attachAccessJwt(context.Background(), r) + ctx := x.AttachAccessJwt(context.Background(), r) resp, err := (&edgraph.Server{}).Query(ctx, req) if err != nil { x.SetStatusWithData(w, x.ErrorInvalidRequest, err.Error()) @@ -548,19 +548,6 @@ func handleCommit(startTs uint64, reqText []byte) (map[string]interface{}, error return response, nil } -func attachAccessJwt(ctx context.Context, r *http.Request) context.Context { - if accessJwt := r.Header.Get("X-Dgraph-AccessToken"); accessJwt != "" { - md, ok := metadata.FromIncomingContext(ctx) - if !ok { - md = metadata.New(nil) - } - - md.Append("accessJwt", accessJwt) - ctx = metadata.NewIncomingContext(ctx, md) - } - return ctx -} - func alterHandler(w http.ResponseWriter, r *http.Request) { if commonHandler(w, r) { return @@ -586,7 +573,7 @@ func alterHandler(w http.ResponseWriter, r *http.Request) { // Pass in an auth token, if present. md.Append("auth-token", r.Header.Get("X-Dgraph-AuthToken")) ctx := metadata.NewIncomingContext(context.Background(), md) - ctx = attachAccessJwt(ctx, r) + ctx = x.AttachAccessJwt(ctx, r) if _, err := (&edgraph.Server{}).Alter(ctx, op); err != nil { x.SetStatus(w, x.Error, err.Error()) return diff --git a/dgraph/cmd/alpha/run.go b/dgraph/cmd/alpha/run.go index c285d525540..bfa405720d3 100644 --- a/dgraph/cmd/alpha/run.go +++ b/dgraph/cmd/alpha/run.go @@ -289,7 +289,7 @@ func healthCheck(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - ctx := attachAccessJwt(context.Background(), r) + ctx := x.AttachAccessJwt(context.Background(), r) var resp *api.Response if resp, err = (&edgraph.Server{}).Health(ctx, true); err != nil { x.SetStatus(w, x.Error, err.Error()) @@ -335,7 +335,7 @@ func stateHandler(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") ctx := context.Background() - ctx = attachAccessJwt(ctx, r) + ctx = x.AttachAccessJwt(ctx, r) var aResp *api.Response if aResp, err = (&edgraph.Server{}).State(ctx); err != nil { diff --git a/graphql/web/http.go b/graphql/web/http.go index 6bb33f3e010..b1531f69ac5 100644 --- a/graphql/web/http.go +++ b/graphql/web/http.go @@ -27,7 +27,6 @@ import ( "github.com/golang/glog" "go.opencensus.io/trace" - "google.golang.org/grpc/metadata" "github.com/dgraph-io/dgraph/graphql/api" "github.com/dgraph-io/dgraph/graphql/resolve" @@ -100,11 +99,7 @@ func (gh *graphqlHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var res *schema.Response gqlReq, err := getRequest(ctx, r) - if accessJwt := r.Header.Get("accessJwt"); accessJwt != "" { - md := metadata.New(nil) - md.Append("accessJwt", accessJwt) - ctx = metadata.NewIncomingContext(ctx, md) - } + ctx = x.AttachAccessJwt(ctx, r) if err != nil { res = schema.ErrorResponse(err) diff --git a/x/x.go b/x/x.go index c806b690cc5..db4b851849c 100644 --- a/x/x.go +++ b/x/x.go @@ -49,6 +49,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/encoding/gzip" + "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" ) @@ -323,6 +324,20 @@ func ParseRequest(w http.ResponseWriter, r *http.Request, data interface{}) bool return true } +// AttachAccessJwt adds any incoming JWT header data into the grpc context metadata +func AttachAccessJwt(ctx context.Context, r *http.Request) context.Context { + if accessJwt := r.Header.Get("X-Dgraph-AccessToken"); accessJwt != "" { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + md = metadata.New(nil) + } + + md.Append("accessJwt", accessJwt) + ctx = metadata.NewIncomingContext(ctx, md) + } + return ctx +} + // Min returns the minimum of the two given numbers. func Min(a, b uint64) uint64 { if a < b { From 91c26f46968c084bb87c0967b7f83eed449f0a32 Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Thu, 13 Feb 2020 17:00:22 +1100 Subject: [PATCH 02/12] Add completion that can add aliases --- graphql/resolve/resolver.go | 99 +++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/graphql/resolve/resolver.go b/graphql/resolve/resolver.go index 4684a885022..8cc4c692bc6 100644 --- a/graphql/resolve/resolver.go +++ b/graphql/resolve/resolver.go @@ -258,6 +258,12 @@ func StdQueryCompletion() CompletionFunc { return removeObjectCompletion(completeDgraphResult) } +// AliasQueryCompletion is the completion steps that get run for admin queries +// those don't have the alias built in like Dgraph queries. +func AliasQueryCompletion() CompletionFunc { + return removeObjectCompletion(injectAliasCompletion(completeResult)) +} + // StdMutationCompletion is the completion steps that get run for add and update mutations func StdMutationCompletion(name string) CompletionFunc { return addPathCompletion(name, addRootFieldCompletion(name, completeDgraphResult)) @@ -487,6 +493,56 @@ func addPathCompletion(name string, cf CompletionFunc) CompletionFunc { }) } +// injectAliasCompletion takes a result with names as per the type names and swaps those for +// any aliases specified in the query before apply cf. +func injectAliasCompletion(cf CompletionFunc) CompletionFunc { + return CompletionFunc(func( + ctx context.Context, field schema.Field, result []byte, err error) ([]byte, error) { + + var val interface{} + if err := json.Unmarshal(result, &val); err != nil { + return nil, schema.GQLWrapLocationf(err, field.Location(), "unable to complete result") + } + + var aliased interface{} + var resErr error + switch val := val.(type) { + case []interface{}: + aliased, resErr = aliasList(field, val) + case map[string]interface{}: + aliased, resErr = aliasObject([]schema.Field{field}, val) + case interface{}: + aliased, resErr = aliasValue(field, val) + } + + res, err := json.Marshal(aliased) + + return cf(ctx, field, res, schema.AppendGQLErrs(err, resErr)) + }) +} + +// completeResult takes a result like {"res":{"a":...,"b":...}} and does the standard +// object completion. This is different to doing completion from Dgraph, because that requires +// handling {"res":[{...}]} even if we expect a single value +func completeResult(ctx context.Context, field schema.Field, result []byte, e error) ( + []byte, error) { + + var val interface{} + if err := json.Unmarshal(result, &val); err != nil { + return nil, schema.GQLWrapLocationf(err, field.Location(), "unable to complete result") + } + + path := make([]interface{}, 0, maxPathLength(field)) + + switch val := val.(type) { + case []interface{}: + return completeList(path, field, val) + case map[string]interface{}: + return completeObject(path, field.Type(), []schema.Field{field}, val) + } + return completeValue(path, field, val) +} + // Once a result has been returned from Dgraph, that result needs to be worked // through for two main reasons: // @@ -969,3 +1025,46 @@ func maxPathLength(f schema.Field) int { return 1 + childMax } + +// TODO: Include this behavior into the standard algorithm above. +// That is, allow the completion algorithms to be like a walk through the +// result structure and then we can apply different behaviors as each point. +// That should eliminate unpacking and packing the result multiple times and +// allow the result processing to be really flexible. +func aliasValue(field schema.Field, val interface{}) (interface{}, error) { + switch val := val.(type) { + case map[string]interface{}: + return aliasObject(field.SelectionSet(), val) + case []interface{}: + return aliasList(field, val) + default: + return val, nil + } +} + +func aliasList(field schema.Field, values []interface{}) ([]interface{}, error) { + var errs error + var result []interface{} + for _, b := range values { + r, err := aliasValue(field, b) + errs = schema.AppendGQLErrs(errs, err) + result = append(result, r) + } + return result, errs +} + +func aliasObject( + fields []schema.Field, + res map[string]interface{}) (interface{}, error) { + + var errs error + result := make(map[string]interface{}) + + for _, f := range fields { + r, err := aliasValue(f, res[f.Name()]) + result[f.ResponseName()] = r + errs = schema.AppendGQLErrs(errs, err) + } + + return result, errs +} From 7e6c7a285911bdfc79c9eced66c1a82c409eceed Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Thu, 13 Feb 2020 17:01:55 +1100 Subject: [PATCH 03/12] admin schema with Dgraph health --- graphql/admin/admin.go | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/graphql/admin/admin.go b/graphql/admin/admin.go index 6e4cf4c5834..903a8088e77 100644 --- a/graphql/admin/admin.go +++ b/graphql/admin/admin.go @@ -47,11 +47,6 @@ const ( "(Please let us know : https://github.com/dgraph-io/dgraph/issues)" // GraphQL schema for /admin endpoint. - // - // Eventually we should generate this from just the types definition. - // But for now, that would add too much into the schema, so this is - // hand crafted to be one of our schemas so we can pass it into the - // pipeline. graphqlAdminSchema = ` type GQLSchema @dgraph(type: "dgraph.graphql") { id: ID! @@ -59,19 +54,18 @@ const ( generatedSchema: String! } - type Health { - message: String! - status: HealthStatus! + """Node state is the state of an individual node int he Dgraph cluster """ + type NodeState { + """node type : either 'alpha' or 'zero'""" + instance: String + address: String + """node health status : either 'healthy' or 'unhealthy'""" + status: String + group: Int + uptime: Int + lastEcho: Int } - enum HealthStatus { - ErrNoConnection - NoGraphQLSchema - Healthy - } - - scalar DateTime - directive @dgraph(type: String, pred: String) on OBJECT | INTERFACE | FIELD_DEFINITION type UpdateGQLSchemaPayload { @@ -134,7 +128,7 @@ const ( type Query { getGQLSchema: GQLSchema - health: Health + health: [NodeState] } type Mutation { From 00ac4a70b6817f1720974b0372a6a3e1e6005c69 Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Thu, 13 Feb 2020 17:04:10 +1100 Subject: [PATCH 04/12] Add admin resolver for Dgraph health --- graphql/admin/admin.go | 79 ++++++++++++++++------------------------- graphql/admin/health.go | 51 +++++++++++--------------- 2 files changed, 52 insertions(+), 78 deletions(-) diff --git a/graphql/admin/admin.go b/graphql/admin/admin.go index 903a8088e77..db51243660b 100644 --- a/graphql/admin/admin.go +++ b/graphql/admin/admin.go @@ -53,7 +53,7 @@ const ( schema: String! @dgraph(type: "dgraph.graphql.schema") generatedSchema: String! } - + """Node state is the state of an individual node int he Dgraph cluster """ type NodeState { """node type : either 'alpha' or 'zero'""" @@ -151,7 +151,6 @@ type gqlSchema struct { type adminServer struct { rf resolve.ResolverFactory resolver *resolve.RequestResolver - status healthStatus // The mutex that locks schema update operations mux sync.Mutex @@ -207,7 +206,6 @@ func newAdminResolver( server := &adminServer{ rf: rf, resolver: resolve.New(adminSchema, rf), - status: errNoConnection, gqlServer: gqlServer, fns: fns, withIntrospection: withIntrospection, @@ -269,7 +267,6 @@ func newAdminResolver( defer server.mux.Unlock() server.schema = newSchema - server.status = healthy server.resetSchema(gqlSchema) }, 1, closer) @@ -282,14 +279,12 @@ func newAdminResolverFactory() resolve.ResolverFactory { rf := resolverFactoryWithErrorMsg(errResolverNotFound). WithQueryResolver("health", func(q schema.Query) resolve.QueryResolver { - health := &healthResolver{ - status: errNoConnection, - } + health := &healthResolver{} return resolve.NewQueryResolver( health, health, - resolve.StdQueryCompletion()) + resolve.AliasQueryCompletion()) }). WithMutationResolver("updateGQLSchema", func(m schema.Mutation) resolve.MutationResolver { return resolve.MutationResolverFunc( @@ -364,26 +359,28 @@ func newAdminResolverFactory() resolve.ResolverFactory { } func (as *adminServer) initServer() { - var waitFor time.Duration + // It takes a few seconds for the Dgraph cluster to be up and running. + // Before that, trying to read the GraphQL schema will result in error: + // "Please retry again, server is not ready to accept requests." + // 5 seconds is a pretty reliable wait for a fresh instance to read the + // schema on a first try. + waitFor := 5 * time.Second + + // Nothing else should be able to lock before here. The admin resolvers aren't yet + // set up (they all just error), so we will obtain the lock here without contention. + // We then setup the admin resolvers and they must wait until we are done before the + // first admin calls will go through. + as.mux.Lock() + defer as.mux.Unlock() + + as.addConnectedAdminResolvers() for { <-time.After(waitFor) - waitFor = 10 * time.Second - - // Nothing else should be able to lock before here. The admin resolvers aren't yet - // set up (they all just error), so we will obtain the lock here without contention. - // We then setup the admin resolvers and they must wait until we are done before the - // first admin calls will go through. - as.mux.Lock() - defer as.mux.Unlock() - - as.addConnectedAdminResolvers() - - as.status = noGraphQLSchema sch, err := getCurrentGraphQLSchema(as.resolver) if err != nil { glog.Infof("Error reading GraphQL schema: %s.", err) - break + continue } else if sch == nil { glog.Infof("No GraphQL schema in Dgraph; serving empty GraphQL API") break @@ -405,7 +402,6 @@ func (as *adminServer) initServer() { glog.Infof("Successfully loaded GraphQL schema. Serving GraphQL API.") as.schema = *sch - as.status = healthy as.resetSchema(generatedSchema) break @@ -424,32 +420,21 @@ func (as *adminServer) addConnectedAdminResolvers() { as.fns.Qe = qryExec as.fns.Me = mutExec - as.rf.WithQueryResolver("health", - func(q schema.Query) resolve.QueryResolver { - health := &healthResolver{ - status: as.status, + as.rf.WithMutationResolver("updateGQLSchema", + func(m schema.Mutation) resolve.MutationResolver { + updResolver := &updateSchemaResolver{ + admin: as, + baseAddRewriter: addRw, + baseMutationRewriter: updRw, + baseMutationExecutor: mutExec, } - return resolve.NewQueryResolver( - health, - health, - resolve.StdQueryCompletion()) + return resolve.NewMutationResolver( + updResolver, + updResolver, + updResolver, + resolve.StdMutationCompletion(m.Name())) }). - WithMutationResolver("updateGQLSchema", - func(m schema.Mutation) resolve.MutationResolver { - updResolver := &updateSchemaResolver{ - admin: as, - baseAddRewriter: addRw, - baseMutationRewriter: updRw, - baseMutationExecutor: mutExec, - } - - return resolve.NewMutationResolver( - updResolver, - updResolver, - updResolver, - resolve.StdMutationCompletion(m.Name())) - }). WithQueryResolver("getGQLSchema", func(q schema.Query) resolve.QueryResolver { getResolver := &getSchemaResolver{ @@ -506,8 +491,6 @@ func (as *adminServer) resetSchema(gqlSchema schema.Schema) { } as.gqlServer.ServeGQL(resolve.New(gqlSchema, resolverFactory)) - - as.status = healthy } func writeResponse(m schema.Mutation, code, message string) []byte { diff --git a/graphql/admin/health.go b/graphql/admin/health.go index 9a38a89fc9a..5335c5f2ba4 100644 --- a/graphql/admin/health.go +++ b/graphql/admin/health.go @@ -17,49 +17,40 @@ package admin import ( + "bytes" "context" - "fmt" + "github.com/dgraph-io/dgo/v2/protos/api" + "github.com/dgraph-io/dgraph/edgraph" "github.com/dgraph-io/dgraph/gql" "github.com/dgraph-io/dgraph/graphql/schema" + "github.com/dgraph-io/dgraph/x" + "github.com/pkg/errors" ) -const ( - errNoConnection healthStatus = "ErrNoConnection" - noGraphQLSchema healthStatus = "NoGraphQLSchema" - healthy healthStatus = "Healthy" -) - -type healthStatus string - type healthResolver struct { - status healthStatus - format string } -var statusMessage = map[healthStatus]string{ - errNoConnection: "Unable to contact Dgraph", - noGraphQLSchema: "Dgraph connection established but there's no GraphQL schema.", - healthy: "Dgraph connection established and serving GraphQL schema.", +func (hr *healthResolver) Rewrite(q schema.Query) (*gql.GraphQuery, error) { + return nil, nil } -func (hr *healthResolver) Rewrite(q schema.Query) (*gql.GraphQuery, error) { - msg := "message" - status := "status" +func (hr *healthResolver) Query(ctx context.Context, query *gql.GraphQuery) ([]byte, error) { + var err error - for _, f := range q.SelectionSet() { - if f.Name() == "message" { - msg = f.ResponseName() - } - if f.Name() == "status" { - status = f.ResponseName() - } + var resp *api.Response + var respErr error + if resp, respErr = (&edgraph.Server{}).Health(ctx, true); respErr != nil { + err = errors.Errorf("%s: %s", x.Error, respErr.Error()) + } + if resp == nil { + err = errors.Errorf("%s: %s", x.ErrorNoData, "No state information available.") } - hr.format = fmt.Sprintf(`{"%s":[{"%s":"%%s","%s":"%%s"}]}`, q.ResponseName(), msg, status) - return nil, nil -} + var buf bytes.Buffer + x.Check2(buf.WriteString(`{ "health":`)) + x.Check2(buf.Write(resp.Json)) + x.Check2(buf.WriteString(`}`)) -func (hr *healthResolver) Query(ctx context.Context, query *gql.GraphQuery) ([]byte, error) { - return []byte(fmt.Sprintf(hr.format, statusMessage[hr.status], string(hr.status))), nil + return buf.Bytes(), err } From 4b08eb45103671d6a06d5c1d2192b395b94a81ba Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Thu, 13 Feb 2020 17:06:09 +1100 Subject: [PATCH 05/12] fix test setup for new health --- graphql/e2e/common/admin.go | 3 +- graphql/e2e/common/common.go | 76 +++++++++++++++--------------------- 2 files changed, 34 insertions(+), 45 deletions(-) diff --git a/graphql/e2e/common/admin.go b/graphql/e2e/common/admin.go index e615fa1b0bd..1c2462fd368 100644 --- a/graphql/e2e/common/admin.go +++ b/graphql/e2e/common/admin.go @@ -183,8 +183,9 @@ func admin(t *testing.T) { client := dgo.NewDgraphClient(api.NewDgraphClient(d)) - err = checkGraphQLHealth(graphqlAdminTestAdminURL, []string{"NoGraphQLSchema"}) + hasSchema, err := hasCurrentGraphQLSchema(graphqlAdminTestAdminURL) require.NoError(t, err) + require.False(t, hasSchema) schemaIsInInitialState(t, client) addGQLSchema(t, client) diff --git a/graphql/e2e/common/common.go b/graphql/e2e/common/common.go index 25b7d21acc9..d3f98b18d8e 100644 --- a/graphql/e2e/common/common.go +++ b/graphql/e2e/common/common.go @@ -148,13 +148,13 @@ type director struct { } func BootstrapServer(schema, data []byte) { - err := checkGraphQLLayerStarted(graphqlAdminURL) + err := checkGraphQLStarted(graphqlAdminURL) if err != nil { panic(fmt.Sprintf("Waited for GraphQL test server to become available, but it never did.\n"+ "Got last error %+v", err.Error())) } - err = checkGraphQLLayerStarted(graphqlAdminTestAdminURL) + err = checkGraphQLStarted(graphqlAdminTestAdminURL) if err != nil { panic(fmt.Sprintf("Waited for GraphQL AdminTest server to become available, "+ "but it never did.\n Got last error: %+v", err.Error())) @@ -178,11 +178,6 @@ func BootstrapServer(schema, data []byte) { panic(err) } - err = checkGraphQLHealth(graphqlAdminURL, []string{"Healthy"}) - if err != nil { - panic(err) - } - if err = d.Close(); err != nil { panic(err) } @@ -578,20 +573,18 @@ func allCountriesAdded() ([]*country, error) { return result.Data.QueryCountry, nil } -func checkGraphQLLayerStarted(url string) error { +func checkGraphQLStarted(url string) error { var err error retries := 6 sleep := 10 * time.Second - // Because of how the test containers are brought up, there's no guarantee - // that the GraphQL layer is running by now. So we + // Because of how GraphQL starts (it needs to read the schema from Dgraph), + // there's no guarantee that GraphQL is available by now. So we // need to try and connect and potentially retry a few times. for retries > 0 { retries-- - // In local dev, we might already have an instance Healthy. In CI, - // we expect the GraphQL layer to be waiting for a first schema. - err = checkGraphQLHealth(url, []string{"NoGraphQLSchema", "Healthy"}) + _, err = hasCurrentGraphQLSchema(url) if err == nil { return nil } @@ -600,52 +593,47 @@ func checkGraphQLLayerStarted(url string) error { return err } -func checkGraphQLHealth(url string, status []string) error { - health := &GraphQLParams{ - Query: `query { - health { - message - status - } - }`, +func hasCurrentGraphQLSchema(url string) (bool, error) { + + schemaQry := &GraphQLParams{ + Query: `query { getGQLSchema { id } }`, } - req, err := health.createGQLPost(url) + req, err := schemaQry.createGQLPost(url) if err != nil { - return errors.Wrap(err, "while creating gql post") + return false, errors.Wrap(err, "while creating gql post") } - resp, err := runGQLRequest(req) + res, err := runGQLRequest(req) if err != nil { - return errors.Wrap(err, "error running GraphQL query") - } - - var healthResult struct { - Data struct { - Health struct { - Message string - Status string - } - } - Errors x.GqlErrorList + return false, errors.Wrap(err, "error running GraphQL query") } - err = json.Unmarshal(resp, &healthResult) + var result *GraphQLResponse + err = json.Unmarshal(res, &result) if err != nil { - return errors.Wrap(err, "error trying to unmarshal GraphQL query result") + return false, errors.Wrap(err, "error unmarshalling result") } - if len(healthResult.Errors) > 0 { - return healthResult.Errors + if len(result.Errors) > 0 { + return false, result.Errors } - for _, s := range status { - if healthResult.Data.Health.Status == s { - return nil + var sch struct { + GetGQLSchema struct { + ID string } } - return errors.Errorf("GraphQL server was not at right health: found %s", - healthResult.Data.Health.Status) + err = json.Unmarshal(result.Data, &sch) + if err != nil { + return false, errors.Wrap(err, "error trying to unmarshal GraphQL query result") + } + + if sch.GetGQLSchema.ID == "" { + return false, nil + } + + return true, nil } func addSchema(url string, schema string) error { From 54b1a4fdc0fe41010090bc8344c44c52b5a09b62 Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Thu, 13 Feb 2020 17:41:17 +1100 Subject: [PATCH 06/12] test /admin health equals /health --- graphql/e2e/common/admin.go | 44 ++++++++++++++++++++++++++++++++++++ graphql/e2e/common/common.go | 2 ++ 2 files changed, 46 insertions(+) diff --git a/graphql/e2e/common/admin.go b/graphql/e2e/common/admin.go index 1c2462fd368..bb69aaba769 100644 --- a/graphql/e2e/common/admin.go +++ b/graphql/e2e/common/admin.go @@ -18,10 +18,16 @@ package common import ( "context" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" "testing" "github.com/dgraph-io/dgo/v2" "github.com/dgraph-io/dgo/v2/protos/api" + "github.com/dgraph-io/dgraph/protos/pb" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" "google.golang.org/grpc" ) @@ -240,3 +246,41 @@ func introspect(t *testing.T, expected string) { require.JSONEq(t, expected, string(gqlResponse.Data)) } + +// The GraphQL /admin health result should be the same as /health +func health(t *testing.T) { + queryParams := &GraphQLParams{ + Query: `query { + health { + instance + address + status + group + uptime + lastEcho + } + }`, + } + gqlResponse := queryParams.ExecuteAsPost(t, graphqlAdminTestAdminURL) + requireNoGQLErrors(t, gqlResponse) + + var result struct { + Health []pb.HealthInfo + } + + err := json.Unmarshal([]byte(gqlResponse.Data), &result) + require.NoError(t, err) + + var health []pb.HealthInfo + url := fmt.Sprintf("%s/health?all", adminDgraphURL) + resp, err := http.Get(url) + require.NoError(t, err) + defer resp.Body.Close() + healthRes, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(healthRes, &health)) + + if diff := cmp.Diff(health, result.Health); diff != "" { + t.Errorf("result mismatch (-want +got):\n%s", diff) + } +} diff --git a/graphql/e2e/common/common.go b/graphql/e2e/common/common.go index d3f98b18d8e..583e8df9ff5 100644 --- a/graphql/e2e/common/common.go +++ b/graphql/e2e/common/common.go @@ -42,6 +42,7 @@ const ( graphqlAdminURL = "http://localhost:8180/admin" alphagRPC = "localhost:9180" + adminDgraphURL = "http://localhost:8280" graphqlAdminTestURL = "http://localhost:8280/graphql" graphqlAdminTestAdminURL = "http://localhost:8280/admin" alphaAdminTestgRPC = "localhost:9280" @@ -187,6 +188,7 @@ func BootstrapServer(schema, data []byte) { func RunAll(t *testing.T) { // admin tests t.Run("admin", admin) + t.Run("health", health) // schema tests t.Run("graphql descriptions", graphQLDescriptions) From 73b0478ec97e7e64ab9ae93382c17a34242b5222 Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Thu, 13 Feb 2020 17:43:26 +1100 Subject: [PATCH 07/12] don't overwrite arg --- graphql/resolve/resolver.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/graphql/resolve/resolver.go b/graphql/resolve/resolver.go index 8cc4c692bc6..08bf54f3c68 100644 --- a/graphql/resolve/resolver.go +++ b/graphql/resolve/resolver.go @@ -500,8 +500,10 @@ func injectAliasCompletion(cf CompletionFunc) CompletionFunc { ctx context.Context, field schema.Field, result []byte, err error) ([]byte, error) { var val interface{} - if err := json.Unmarshal(result, &val); err != nil { - return nil, schema.GQLWrapLocationf(err, field.Location(), "unable to complete result") + if marshErr := json.Unmarshal(result, &val); marshErr != nil { + return nil, + schema.AppendGQLErrs(marshErr, + schema.GQLWrapLocationf(err, field.Location(), "unable to complete result")) } var aliased interface{} @@ -515,7 +517,8 @@ func injectAliasCompletion(cf CompletionFunc) CompletionFunc { aliased, resErr = aliasValue(field, val) } - res, err := json.Marshal(aliased) + res, marshErr := json.Marshal(aliased) + err = schema.AppendGQLErrs(err, marshErr) return cf(ctx, field, res, schema.AppendGQLErrs(err, resErr)) }) From a626c0cb0950df094c72ddf9f4f9b5a3b04bf0c2 Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Thu, 13 Feb 2020 17:52:14 +1100 Subject: [PATCH 08/12] make url a const --- graphql/e2e/common/admin.go | 4 +--- graphql/e2e/common/common.go | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/graphql/e2e/common/admin.go b/graphql/e2e/common/admin.go index bb69aaba769..1325ac52aba 100644 --- a/graphql/e2e/common/admin.go +++ b/graphql/e2e/common/admin.go @@ -19,7 +19,6 @@ package common import ( "context" "encoding/json" - "fmt" "io/ioutil" "net/http" "testing" @@ -272,8 +271,7 @@ func health(t *testing.T) { require.NoError(t, err) var health []pb.HealthInfo - url := fmt.Sprintf("%s/health?all", adminDgraphURL) - resp, err := http.Get(url) + resp, err := http.Get(adminDgraphHealthURL) require.NoError(t, err) defer resp.Body.Close() healthRes, err := ioutil.ReadAll(resp.Body) diff --git a/graphql/e2e/common/common.go b/graphql/e2e/common/common.go index 583e8df9ff5..073d06d277b 100644 --- a/graphql/e2e/common/common.go +++ b/graphql/e2e/common/common.go @@ -42,7 +42,7 @@ const ( graphqlAdminURL = "http://localhost:8180/admin" alphagRPC = "localhost:9180" - adminDgraphURL = "http://localhost:8280" + adminDgraphHealthURL = "http://localhost:8280/health?all" graphqlAdminTestURL = "http://localhost:8280/graphql" graphqlAdminTestAdminURL = "http://localhost:8280/admin" alphaAdminTestgRPC = "localhost:9280" From 53815931f58da86844c8d73da7e1b8e0fb44ac27 Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Thu, 13 Feb 2020 19:01:12 +1100 Subject: [PATCH 09/12] don't let test be flakey --- graphql/e2e/common/admin.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/graphql/e2e/common/admin.go b/graphql/e2e/common/admin.go index 1325ac52aba..49d5c50bcec 100644 --- a/graphql/e2e/common/admin.go +++ b/graphql/e2e/common/admin.go @@ -27,6 +27,7 @@ import ( "github.com/dgraph-io/dgo/v2/protos/api" "github.com/dgraph-io/dgraph/protos/pb" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/require" "google.golang.org/grpc" ) @@ -255,6 +256,7 @@ func health(t *testing.T) { address status group + version uptime lastEcho } @@ -278,7 +280,13 @@ func health(t *testing.T) { require.NoError(t, err) require.NoError(t, json.Unmarshal(healthRes, &health)) - if diff := cmp.Diff(health, result.Health); diff != "" { + // Uptime and LastEcho might have changed between the GraphQL and /health calls. + // If we don't remove them, the test would be flakey. + opts := []cmp.Option{ + cmpopts.IgnoreFields(pb.HealthInfo{}, "Uptime"), + cmpopts.IgnoreFields(pb.HealthInfo{}, "LastEcho"), + } + if diff := cmp.Diff(health, result.Health, opts...); diff != "" { t.Errorf("result mismatch (-want +got):\n%s", diff) } } From cc3398f9e4178ee5e49354ce4b9ac4b7816f6981 Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Thu, 13 Feb 2020 19:20:08 +1100 Subject: [PATCH 10/12] um, er, well --- graphql/admin/admin.go | 1 + 1 file changed, 1 insertion(+) diff --git a/graphql/admin/admin.go b/graphql/admin/admin.go index db51243660b..9542da5b910 100644 --- a/graphql/admin/admin.go +++ b/graphql/admin/admin.go @@ -62,6 +62,7 @@ const ( """node health status : either 'healthy' or 'unhealthy'""" status: String group: Int + version: String uptime: Int lastEcho: Int } From 637cc4aecdaecc9943e4321ce79d8d7bb5515ac7 Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Fri, 14 Feb 2020 09:37:45 +1100 Subject: [PATCH 11/12] review updatae --- graphql/admin/admin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphql/admin/admin.go b/graphql/admin/admin.go index 01a1c624fdd..a7f16d32d61 100644 --- a/graphql/admin/admin.go +++ b/graphql/admin/admin.go @@ -54,7 +54,7 @@ const ( generatedSchema: String! } - """Node state is the state of an individual node int he Dgraph cluster """ + """Node state is the state of an individual node in the Dgraph cluster """ type NodeState { """node type : either 'alpha' or 'zero'""" instance: String From c72e08f31881ffc6f5703f084191dcb1ffab3c04 Mon Sep 17 00:00:00 2001 From: MichaelJCompton Date: Fri, 14 Feb 2020 23:08:43 +1100 Subject: [PATCH 12/12] fix access token --- ee/acl/acl_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/acl/acl_test.go b/ee/acl/acl_test.go index dad62767ecc..c0315d1998b 100644 --- a/ee/acl/acl_test.go +++ b/ee/acl/acl_test.go @@ -443,7 +443,7 @@ func makeRequest(t *testing.T, accessToken string, params testutil.GraphQLParams req, err := http.NewRequest(http.MethodPost, adminUrl, bytes.NewBuffer(b)) require.NoError(t, err) - req.Header.Set("accessJwt", accessToken) + req.Header.Set("X-Dgraph-AccessToken", accessToken) req.Header.Set("Content-Type", "application/json") client := &http.Client{} resp, err := client.Do(req)