feat(GraphQL): Zero HTTP endpoints are now available at GraphQL admin (GRAPHQL-1118)

[abhimanyusinghgaur]

Fixes DGRAPH-1072

This PR makes Zero HTTP endpoints available in GraphQL admin. It also adds a flag --disable_admin_http in Zero which can be used to disable the administrative HTTP endpoints in Zero. The following are considered as the administrative endpoints for Zero:

• /state
• /removeNode
• /moveTablet
• /assign
• /enterpriseLicense

RFC: https://discuss.dgraph.io/t/moving-zero-http-endpoints-to-alpha-graphql-admin/10725

---

This change is 

Docs Preview:

[codelingo]

LGTM. 1 rules errored during the review.

[codelingo]

LGTM. 1 rules errored during the review.

[pawanrawal]

Looking good till now, will have another look when its done.

Reviewed 5 of 16 files at r1.
Reviewable status: 5 of 16 files reviewed, all discussions resolved

[codelingo]

LGTM. 1 rules errored during the review.

[pawanrawal]

Can we move over all the calls in our existing integration tests to the new GraphQL API instead of the HTTP API?

Reviewed 2 of 16 files at r1, 5 of 9 files at r2.
Reviewable status: 10 of 16 files reviewed, 1 unresolved discussion (waiting on @abhimanyusinghgaur, @manishrjain, @MichaelJCompton, and @vvbalaji-dgraph)

---

graphql/admin/assign.go, line 63 at r2 (raw file):

	}

	var startId, endId, readOnly interface{}

Does this not work if these are defined as int? Do we have any tests for this endpoint through the new GraphQL API or can we move over existing calls for this?

[abhimanyusinghgaur]

Closing. Not required at the moment. Should be reopened later if a need is felt.

[NamanJain8]

Reviewable status: 2 of 18 files reviewed, 8 unresolved discussions (waiting on @abhimanyusinghgaur, @manishrjain, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

---

graphql/admin/admin.go, line 314 at r3 (raw file):

		ID of the destination group where the predicate is to be moved.
		"""
		groupId: UInt64!

The namespace should also be part of the input as moveTablet should be the guardian of galaxy operation.

---

graphql/admin/admin.go, line 498 at r3 (raw file):

		"restore":           guardianOfTheGalaxyMutationMWs,
		"shutdown":          guardianOfTheGalaxyMutationMWs,
		"removeNode":        commonAdminMutationMWs,

guardianOfTheGalaxyMutationMWs

---

graphql/admin/assign.go, line 95 at r3 (raw file):

	inputRef := &assignInput{}
	inputRef.What, ok = inputArg["what"].(string)

Maybe capitalize the What field, so that uid Uid UID all are accepted?

---

graphql/admin/moveTablet.go, line 43 at r3 (raw file):

	}

	ns, _ := x.ExtractNamespace(ctx)

namespace should be part of input

---

graphql/admin/removeNode.go, line 44 at r3 (raw file):

	}

	if _, err = worker.RemoveNodeOverNetwork(ctx, &pb.RemoveNodeRequest{NodeId: input.NodeId,

What if the alpha that received the request is itself being removed? In that case, the user will not get a failure message, I think.

---

graphql/admin/removeNode.go, line 78 at r3 (raw file):

}

func parseAsUint64(val interface{}) (uint64, error) {

These functions also lie around in alpha/http.go, can these be moved to a common place like x package or they use something different. Please check.

---

worker/zero.go, line 54 at r3 (raw file):

func ApplyLicenseOverNetwork(ctx context.Context, request *pb.ApplyLicenseRequest) (*pb.Status,
	error) {
	pl := groups().AnyServer(0)

Why not directly connect to zero leader?

[abhimanyusinghgaur]

Reviewable status: 1 of 22 files reviewed, 8 unresolved discussions (waiting on @manishrjain, @MichaelJCompton, @NamanJain8, @pawanrawal, and @vvbalaji-dgraph)

---

graphql/admin/admin.go, line 314 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

The namespace should also be part of the input as moveTablet should be the guardian of galaxy operation.

Done. Thanks!

---

graphql/admin/admin.go, line 498 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

guardianOfTheGalaxyMutationMWs

Done. Thanks!

---

graphql/admin/assign.go, line 63 at r2 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Does this not work if these are defined as int? Do we have any tests for this endpoint through the new GraphQL API or can we move over existing calls for this?

we sometimes need to send null for them in response, so need them as interface{}.
Added tests.

---

graphql/admin/assign.go, line 95 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

Maybe capitalize the What field, so that uid Uid UID all are accepted?

no, that is how it is defined in the GraphQL schema, so the user has to send exactly that string, otherwise, it would fail schema validation.

---

graphql/admin/moveTablet.go, line 43 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

namespace should be part of input

Done. Thanks!

---

graphql/admin/removeNode.go, line 44 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

What if the alpha that received the request is itself being removed? In that case, the user will not get a failure message, I think.

Nice catch!
I tried this, but in my case, the alpha returned the response back and then logged fatal.
But, I agree that there can be a race condition between the fatal log and returning the response, and any one of them may happen first.
So, in tests, I have specifically set the group ids for each alpha and only removed alpha 2 & 3 using alpha1.

From the user's point of view, I think we can document this.

---

graphql/admin/removeNode.go, line 78 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

These functions also lie around in alpha/http.go, can these be moved to a common place like x package or they use something different. Please check.

Yeah! alpha/http.go has a parseUint64 func, but that parses a query variable in an HTTP request.
This one here is very specific to the constraints of the GraphQL admin server, and should only be used by the admin server.
So, can't merge the two funcs or put them in x.

---

worker/zero.go, line 54 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

Why not directly connect to zero leader?

EnterpriseLicense and RemoveNode don't necessarily require a zero leader. Even in their HTTP API versions, they can process the request on any server of group zero. So, not enforcing that constraint here.

[manishrjain]

Reviewed 1 of 16 files at r1, 14 of 17 files at r3, 7 of 7 files at r4.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @abhimanyusinghgaur, @MichaelJCompton, @NamanJain8, @pawanrawal, and @vvbalaji-dgraph)

---

dgraph/cmd/zero/http.go, line 127 at r4 (raw file):

	}

	if _, err := st.zero.RemoveNode(context.Background(), &pb.RemoveNodeRequest{NodeId: nodeId,

move to next line.

---

dgraph/cmd/zero/http.go, line 188 at r4 (raw file):

	var resp *pb.Status
	var err error
	if resp, err = st.zero.MoveTablet(context.Background(), &pb.MoveTabletRequest{Namespace: ns,

move to next line.

---

graphql/admin/admin.go, line 288 at r4 (raw file):

	input RemoveNodeInput {

Try to reduce the vertical spaces.

---

graphql/admin/enterpriseLicense.go, line 51 at r4 (raw file):

func getEnterpriseLicenseInput(m schema.Mutation) (*enterpriseLicenseInput,
	error) {

merge to previous line.

---

graphql/admin/moveTablet.go, line 73 at r4 (raw file):

		ns, err := parseAsUint64(inputArg["namespace"])
		if err != nil {
			return nil, inputArgError(schema.GQLWrapf(err, "can't convert input.namespace to uint64"))

100 chars.

---

worker/zero.go, line 54 at r4 (raw file):

// ApplyLicenseOverNetwork sends a request to apply the given enterprise license to a zero server.
// This operation doesn't necessarily require a zero leader.
func ApplyLicenseOverNetwork(ctx context.Context, request *pb.ApplyLicenseRequest) (*pb.Status,

move from , request.. to next line. s/request/req.

[abhimanyusinghgaur]

Reviewable status: 16 of 23 files reviewed, 14 unresolved discussions (waiting on @manishrjain, @MichaelJCompton, @NamanJain8, @pawanrawal, and @vvbalaji-dgraph)

---

dgraph/cmd/zero/http.go, line 127 at r4 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

move to next line.

Done.

---

dgraph/cmd/zero/http.go, line 188 at r4 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

move to next line.

Done.

---

graphql/admin/admin.go, line 288 at r4 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Try to reduce the vertical spaces.

Done.

---

graphql/admin/enterpriseLicense.go, line 51 at r4 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

merge to previous line.

Done.

---

graphql/admin/moveTablet.go, line 73 at r4 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

100 chars.

Done.

---

worker/zero.go, line 54 at r4 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

move from , request.. to next line. s/request/req.

Done.