Allocator gRPC doesn't work without TLS

[k-kai]

What you expected to happen:
Probably grpc/grpc-go#555

How to reproduce it (as minimally and precisely as possible):
Use gRPC Client with grpc.WithInsecure() DialOption.

Environment:

• Agones version: 1.11.0

[markmandel]

As per our bug template, please provide the rest of the details of your installation:

Environment:

• Kubernetes version (use kubectl version):
• Cloud provider or hardware configuration:
• Install method (yaml/helm):
• Troubleshooting guide log(s):
• Others:

I'm specifically interested in the install method, and what parameters you used on installation.

What settings did you provide for agones.allocator.disableMTLS and/or agones.allocator.disableTLS ?

[k-kai]

Sorry. I filled out environment template.

Environment:

Kubernetes version (use kubectl version): 1.17
Cloud provider or hardware configuration: GKE
Install method (yaml/helm): yaml by output of helm template command
Troubleshooting guide log(s):
Others:
disableMTLS and disableTLS were setting true.

[markmandel]

@pooneh-m can you look at this? I feel like this came up a while ago - I thought you could setup the allocator without TLS, but I couldn't find the documentation. Can you confirm this is possible? (we should probably add a section to https://agones.dev/site/docs/advanced/allocator-service/)

@k-kai can you also provide the error message you are getting when you try and connect, so we can double check this issue, and also confirm on our end?

[k-kai]

can you also provide the error message you are getting when you try and connect

Sorry. I can't find the error message now, because I have already fixed my client to use a http client without TLS, but you can see the error message if you use a gRPC client with grpc.WithInsecure() DialOption.

I thought you could setup the allocator without TLS

I think there aren't examples and tests to use gRPC without TLS.
also, the allocator e2e test seem it only tests gRPC with TLS.

e2e/allocator_test.go

[pooneh-m]

@k-kai thanks for reporting the issue. I tried the gRPC service with mTLS disabled and it works, but with mTLS and TLS both disabled, it does not work anymore. I tried the Agones v1.10 installation and disabling TLS works; so something is broken between the versions 1.10 and version 1.11., though looking at the code, I haven't found the issue yet.

As you pointed out because Agones does not have a functionality to test different helm configurations, the issue was not caught before the release.

[k-kai]

@pooneh-m Thanks for confirming.
What is the cause,,, I think the gRPCServer.ServeHttp function could cause this issue.

agones/cmd/allocator/main.go

Lines 69 to 74 in 93d8c10

	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	// This is a partial recreation of gRPC's internal checks https://github.com/grpc/grpc-go/pull/514/files#diff-95e9a25b738459a2d3030e1e6fa2a718R61
	// We switch on HTTP/1.1 or HTTP/2 by checking the ProtoMajor
	if r.ProtoMajor == 2 && strings.Contains(r.Header.Get("Content-Type"), "application/grpc") {
	grpcServer.ServeHTTP(w, r)
	} else {

It's the experimental function.

When the gRPCServer(before version 1.11.0,) without TLS worked, it had used a gRPCServer.Serve function.

agones/cmd/allocator/main.go

Lines 183 to 187 in 13a0e14

	go func() {
	err := grpcServer.Serve(listener)
	logger.WithError(err).Fatal("allocation service crashed")
	os.Exit(1)
	}()

The same issue as below.
grpc/grpc-go#555

[pooneh-m]

Thanks @k-kai. Reading about it more it seems that's the issue.
Here is one simple solution that may work for us: grpc/grpc-go#555 (comment)

@kdima do you have a suggestion?

[markmandel]

This library just got shared with me for something else - https://github.com/soheilhy/cmux

Does this help at all?

[pooneh-m]

This grpc/grpc-go#555 (comment) also suggested the library you shared. It may work. I am not sure about the quality.

I suggest for the first step, documenting that disabling the TLS is only for the REST API. WDYT? If someone has the cycle to work on this, then we can introduce the functionality back for gRPC.

@k-kai what is the use case for you to disable the TLS for gRPC? Client can always ignore the cert provided by the server.

[markmandel]

I am not sure about the quality.

Crux comes from a Googler I believe works on gRPC, so I think we can count on it. Also comes quite well recommended.

But documentation seems like a good first step at least.

[k-kai]

what is the use case for you to disable the TLS for gRPC?

In my case, I’d like to deploy the Agones and other services on the same network.(VPC)
So, I don’t intend to encrypt communications between them.
Of course, The client to communicate with the Agones allocator will be deployed there( ´•౪•`)

[roberthbailey]

We just merged #2272 which should make it possible to run gRPC without TLS again. You will need to separate the gRPC server from the REST server (either by disabling the REST server or by running them on separate ports) and then you can disable TLS.

[zmerlynn]

@roberthbailey Saw this as a good first issue, is this stale given your previous comment?

[roberthbailey]

Yes, I think we can close this.

@2272 makes it so that you can run gRPC without TLS as long as you run it on a separate port from the rest endpoint (but it's a good idea to separate them in any case for performance reasons).

[k-kai]

Sorry, I didn't get back to comments.
Thank you for dealing with this issue!