feat(multi-tenancy): Make GraphQL work with multiple namespaces #7400
Conversation
… ahsan/subscription
…o abhimanyu/graphql-multitenancy
There was a problem hiding this comment.
Looks good to me. Got some minor comments, make sure everything works.
Reviewable status: 0 of 31 files reviewed, 5 unresolved discussions (waiting on @danielmai, @manishrjain, @pawanrawal, and @vvbalaji-dgraph)
dgraph/cmd/alpha/http.go, line 647 at r1 (raw file):
ctx = x.AttachJWTNamespace(ctx) return adminServer.ResolveWithNs(ctx, 0, gqlReq)
Use x.GalaxyNamespace instead of 0.
edgraph/server.go, line 458 at r1 (raw file):
// TODO - Get GQLSchema for the correct namespace. // query the GraphQL schema and keep it in memory, so it can be inserted again _, graphQLSchema, err := GetGQLSchema(x.ExtractNamespace(ctx))
can use namespace variable, instead of extracting namespace again here. Is the TODO sitll applicable?
graphql/admin/admin.go, line 375 at r1 (raw file):
mainHealthStore = &GraphQLHealthStore{} // adminServerVar *adminServer
Probably this needs description
graphql/e2e/common/common.go, line 1075 at r1 (raw file):
} // if resp.Header.Get("Access-Control-Allow-Origin") != "*" {
A TODO here? or remove these?
graphql/e2e/normal/normal_test.go, line 33 at r1 (raw file):
func TestRunAll_Normal(t *testing.T) { common.RunAll(t) // common.RunCorsTest(t)
Need to fix these.
There was a problem hiding this comment.
Reviewable status: 0 of 43 files reviewed, 5 unresolved discussions (waiting on @ahsanbarkati, @danielmai, @jatindevdg, @manishrjain, @martinmr, and @vvbalaji-dgraph)
dgraph/cmd/alpha/http.go, line 647 at r1 (raw file):
Previously, ahsanbarkati (Ahsan Barkati) wrote…
Use
x.GalaxyNamespaceinstead of 0.
Done
edgraph/server.go, line 458 at r1 (raw file):
Previously, ahsanbarkati (Ahsan Barkati) wrote…
can use
namespacevariable, instead of extracting namespace again here. Is the TODO sitll applicable?
Done
graphql/admin/admin.go, line 375 at r1 (raw file):
Previously, ahsanbarkati (Ahsan Barkati) wrote…
Probably this needs description
Done
graphql/e2e/common/common.go, line 1075 at r1 (raw file):
Previously, ahsanbarkati (Ahsan Barkati) wrote…
A TODO here? or remove these?
added a TODO for @jatindevdg
graphql/e2e/normal/normal_test.go, line 33 at r1 (raw file):
Previously, ahsanbarkati (Ahsan Barkati) wrote…
Need to fix these.
added a todo for @jatindevdg
… ahsan/subscription
… ahsan/subscription
…en for ACL enabled
…hema (GRAPHQL-1016) (#7431) It is a continuation of #7400. This PR does a bunch of things: * Makes `@auth` work with multi-tenancy * Removes `dgraph.graphql.schema_history` leftovers * Removes `dgraph.cors` as an internal predicate and any associated admin mutations * Accepts CORS allowed origins through `# Dgraph.Allow-Origin` in GraphQL schema * Makes subscriptions work with GraphQL schema lazy loading * Adds tests for all the above scenarios
…UpdateCounter. (#7489) Fixes GRAPHQL-1050 There were a couple of tests that were behaving flakily because for a single schema update we were incrementing schemaUpdateCounter multiple times, in case the schema update was for a non-Galaxy namespace. This behavior was caused by a bug in schema lazy loading which got added recently with multi-tenancy in PR #7400. Consider a scenario when we don't have any schema stored in the admin GraphQL server for a non-Galaxy namespace. Sending a schema POST request at /admin/schema gives a badger subscription update for this schema. Since we don't have any schema in the admin GraphQL server, we update the schemaUpdateCounter but don't update the schema in both the admin and the main GraphQL server, because this schema should be loaded lazily when the main GraphQL server receives a GraphQL request. Now, what happens is, badger gives another update from the subscription, which contains the same schema with a higher version. Since the admin server didn't store any information about this schema previously, so it is not able to decide whether to reject this update. Resulting in the schemaUpdateCounter being incremented again. Yet, nothing stored in the admin server about the schema. This results in the schemaUpdateCounter being incremented multiple times for a single schema update request. This behavior persists until the main GraphQL server doesn't get a request for that namespace resulting in the lazy loading of the schema, which both stores the schema in the admin server and loads it into the main server. Once that happens, updating schema once would only update the schemaUpdateCounter once as well. So, to fix this bug we introduced a new variable loaded in gqlSchema struct. This flag is set to true whenever a schema is loaded in the main GraphQL server. If there is no schema stored in the admin server or if this flag is false and we get an update from badger subscription, then we just store the gqlSchema on the admin server with this flag set to false, but don't load it into the main GraphQL server. Only if this flag were true, then the schema would be loaded in the main GraphQL server. This mechanism lets the admin server know correctly when to increment the schemaUpdateCounter.
This PR also removes the schema history feature. If needed it can be reimplemented in Slash GraphQL.
This change is