Add http2 to HTTP ports

[cvanderschuere]

What this PR does / why we need it:

Allow HTTP2 on non-ssl ports. This is required to support gRPC (or any http2 application) in it's insecure mode.

Example:

conn, err := grpc.Dial("grpc.example.com", grpc.WithInsecure())

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cvanderschuere
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: caseydavenport

Assign the PR to them by writing /assign @caseydavenport in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[cvanderschuere]

I signed it

[aledbf]

Closing. Please check #2313 (comment)

We can add this feature only if:

• there is only one path exposed in the hostname
• all the paths use gRPC

If we do not have this restriction any other path cannot be exposed as HTTPS

[aledbf]

@cvanderschuere off course you can use a custom template in your deployment and just make this change locally

[mumoshu]

@aledbf Hi. Thanks as always for maintaining the project!

I am still sticking to support this in ingress-nginx, after #2444 (comment), and seeing that the author of nginx grpc module is pretty negative to fix this in nginx.

You said:

We can add this feature only if:

• there is only one path exposed in the hostname
• all the paths use gRPC

Then, would it be acceptable if this http2 option is appended either:

• there are two or more ingress.Location, but all the ingress.Location.GRPC are true for the host

Or:

• there is just one ingress.Location for the host

?

[aledbf]

there are two or more ingress.Location, but all the ingress.Location.GRPC are true for the host

right

there is just one ingress.Location for the host

and ingress.Location.GRPC=true

[mumoshu]

@aledbf Thanks. Ah, so the number of ingress.Locations doesn't matter at all?
We should just decide whether http2 should be enabled or not here, by checking every ingress.Location.GRPC is true, right?

[aledbf]

right

[mumoshu]

Update; I have tested this by running grpcurl against nginx ingress whose backend is grpc server built with the go-grpc from grpc/grpc-go#2045

Surprisingly, it turned out that I don't need listen http2 at all to make it actually work.

My working nginx.conf looks like this:

# cat nginx.conf | grep listen
		listen 80 default_server  backlog=511 http2;
		listen [::]:80 default_server  backlog=511 http2;
		listen 443  default_server  backlog=511 ssl http2;
		listen [::]:443  default_server  backlog=511 ssl http2;
		listen 80;
		listen [::]:80;
		listen 80;
		listen [::]:80;
		listen 18080 default_server  backlog=511;
		listen [::]:18080 default_server  backlog=511;

Whereas grpcurl command is like:

# grpcurl  -plaintext mygrpcserver.example.com:54321 mygrpcserver.MyGRPCServer/Ping
{
  "value": "pong!"
}

And my ingress is basically:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/grpc-backend: "true"
    nginx.ingress.kubernetes.io/server-snippet: |
      underscores_in_headers on;
  labels:
    app: mygrpcserver
  name: mygrpcserver
spec:
  rules:
  - host: mygrpcserver.example.com
    http:
      paths:
      - backend:
          serviceName: mygrpcserver
          servicePort: 54321

[mumoshu]

@cvanderschuere Hi. Thanks for the fix! But for me this change wasn't necessarily to make my plaintext grpc(a.k.a grpc over h2c) work.

Did it actually work for you? How did you test it?

[mumoshu]

@aledbf Sorry for bothering but would you mind confirming this? I'm confused

[mumoshu]

Sorry, it turned out I was misreading my nginx.conf. This change is definitely needed in order to make h2c work.
I'll go ahead with implementing https://github.com/kubernetes/ingress-nginx/pull/2465/files#r187064310

[aledbf]

Closing. I prefer to wait for a solution from NGINX and not rely on having just one path in the Ingress
https://forum.nginx.org/read.php?29,278877