feat: support listen http2 with plaintext #3547

sysulq · 2021-02-07T07:19:37Z

What this PR does / why we need it:

Details can be found in #3496.

Pre-submission checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

tokers · 2021-02-07T11:55:41Z

apisix/cli/ops.lua

        yaml_conf.apisix.node_listen = node_listen
+    elseif type(yaml_conf.apisix.node_listen) == "table" then
+        if type(yaml_conf.apisix.node_listen[1]) == "number" then


Cannot capture your mean for this if.

I think we should check every item, what if the user mixes the integer and the table?

Only support port with number parameter?

spacewander · 2021-02-07T12:02:57Z

t/core/config-default.t

+
+
+
+=== TEST 5: support listen multiple ports with array


Need to add tests in t/grpc-proxy-test.sh.
And need to update the doc/grpc-proxy.md

spacewander · 2021-02-08T03:08:10Z

conf/config.yaml

@@ -31,6 +31,12 @@
 #
 # If the configured environment variable can't be found, an error will be thrown.
 apisix:
+  node_listen:
+    - port: 9080                     # APISIX listening port


We should provide the configuration separately in:

apisix/.travis/linux_openresty_common_runner.sh

Line 125 in 26a4ed4

res=`./bin/apisix start`

spacewander · 2021-02-08T03:09:15Z

apisix/cli/ops.lua

        yaml_conf.apisix.node_listen = node_listen
+    elseif type(yaml_conf.apisix.node_listen) == "table" then
+        if type(yaml_conf.apisix.node_listen[1]) == "number" then


I think we should check every item, what if the user mixes the integer and the table?

spacewander · 2021-02-08T03:26:58Z

doc/grpc-proxy.md

@@ -36,7 +36,8 @@ gRPC client -> APISIX -> gRPC/gRPCS server
 Here's an example, to proxying gRPC service by specified route:

 * attention: the `scheme` of the route's upstream must be `grpc` or `grpcs`.
-* attention: APISIX use TLS‑encrypted HTTP/2 to expose gRPC service, so need to [config SSL certificate](https.md)
+* attention: APISIX use TLS‑encrypted HTTP/2 to expose gRPC service (default port 9443), so need to [config SSL certificate](https.md)
+* attention: APISIX also support to expose gRPC service with plaintext HTTP/2 (default port 9081), which do not need to config SSL certificate, usually used to proxy gRPC service in intranet environment


Need a configuration to show how we can do it.

spacewander · 2021-02-08T03:28:08Z

conf/config.yaml

+    - port: 9080                     # APISIX listening port
+      enable_http2: false            # Disable to support HTTP proxy, nginx currently do not support HTTP and HTTP/2 with plaintext on the same port
+                                     # Details can be found in https://trac.nginx.org/nginx/ticket/816
+    - port: 9081                     # APISIX listening port for HTTP/2 with plaintext


Don't open a port for HTTP/2 by default.

spacewander · 2021-02-08T06:45:00Z

Need to resolve the conflict, so that CI can start.

tokers · 2021-02-08T07:13:37Z

doc/grpc-proxy.md

@@ -37,6 +37,7 @@ Here's an example, to proxying gRPC service by specified route:

 * attention: the `scheme` of the route's upstream must be `grpc` or `grpcs`.
 * attention: APISIX use TLS‑encrypted HTTP/2 to expose gRPC service, so need to [config SSL certificate](https.md)
+* attention: APISIX also support to expose gRPC service with plaintext HTTP/2, which do not need to config SSL certificate, usually used to proxy gRPC service in intranet environment


which do not need to config SSL certificate => which doesn't rely on ALPN/NPN extensions in TLS/SSL protocol.

Maybe this is a little hard to understand for user...? Maybe it's enough to mention in doc that this feature is supported, and show how to configure it?

What about "which doesn't rely on TLS/SSL"?

Alright, done :-)

tokers · 2021-02-08T07:16:21Z

apisix/cli/ops.lua

+    elseif type(yaml_conf.apisix.node_listen) == "table" then
+        local node_listen = {}
+        for index, value in ipairs(yaml_conf.apisix.node_listen) do
+            if type(value) == "number" then


Is the number type really needed? I think the following way is enough.

node_listen: - port: 9080 - port: 9180 enable_http2: true

We need to be backward compatibility unless it is a bugfix.

I mean the element type inside node_listen, not the node_listen itself.

We can also be compatible with this format:

node_listen: - 9080 - 9081

sysulq changed the title ~~feat: support grpc proxy with insecure mode~~ feat: support listen http2 with plaintext Feb 7, 2021

tokers reviewed Feb 7, 2021

View reviewed changes

spacewander reviewed Feb 7, 2021

View reviewed changes

spacewander reviewed Feb 8, 2021

View reviewed changes

sysulq added 11 commits February 8, 2021 15:02

add new listen config for supporting plaintext grpc

9d4a82d

add test cases

1e25f2a

format

e082cb0

improve

1684eb5

fix

ae60601

improve doc, add test

760666b

improve doc

95b413a

doc: improve

91980e9

update

c2090f9

remove

a183628

update

6d6cbf7

sysulq force-pushed the feature/grpc-proxy-plaintext branch from e3f90e5 to 6d6cbf7 Compare February 8, 2021 07:04

remove dir

5be4e93

tokers reviewed Feb 8, 2021

View reviewed changes

spacewander approved these changes Feb 8, 2021

View reviewed changes

improve doc

854df6f

sysulq requested a review from tokers February 8, 2021 09:54

tokers approved these changes Feb 19, 2021

View reviewed changes

spacewander merged commit 5f4e8df into apache:master Feb 19, 2021

spacewander mentioned this pull request Mar 28, 2021

request help: nginx: [emerg] invalid port in "table:" of the "listen" directive in ... #3920

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support listen http2 with plaintext #3547

feat: support listen http2 with plaintext #3547

sysulq commented Feb 7, 2021

tokers Feb 7, 2021

spacewander Feb 8, 2021

sysulq Feb 8, 2021

spacewander Feb 7, 2021

sysulq Feb 8, 2021

spacewander Feb 8, 2021

sysulq Feb 8, 2021

spacewander Feb 8, 2021

spacewander Feb 8, 2021

sysulq Feb 8, 2021

spacewander Feb 8, 2021

sysulq Feb 8, 2021

spacewander commented Feb 8, 2021

tokers Feb 8, 2021

sysulq Feb 8, 2021

tokers Feb 8, 2021

sysulq Feb 8, 2021 •

edited

Loading

tokers Feb 8, 2021

spacewander Feb 8, 2021 •

edited

Loading

tokers Feb 8, 2021

sysulq Feb 8, 2021

feat: support listen http2 with plaintext #3547

feat: support listen http2 with plaintext #3547

Conversation

sysulq commented Feb 7, 2021

What this PR does / why we need it:

Pre-submission checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spacewander commented Feb 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sysulq Feb 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spacewander Feb 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sysulq Feb 8, 2021 •

edited

Loading

spacewander Feb 8, 2021 •

edited

Loading