feat(conf) add port_maps configuration option #5861

bungle · 2020-05-06T17:39:52Z

Summary

Layer 4 port mapping (or port forwarding) is used a lot when running Kong inside a container. Common example is exposing ports 80 and 443 externally while running Kong inside a container with internal ports 8000 and 8443. It also allows Kong to started without elevated privileges as it is quite common that you need root-access to bind processes to TCP port < 1024.

As this translation is done in many cases without using layer 4 proxy_protocol or layer 7 X-Forwarded-Port HTTP header, the Kong has no knowledge about the target port that client originally connected to.

This PR adds port_maps configuration parameter:

#port_maps =                     # With this configuration parameter you can
                                 # let the Kong to know about the port from
                                 # which the packets are forwarded to it. This
                                 # is fairly common when running Kong in a
                                 # containerized or virtualized environment.
                                 # For example `port_maps=80:8000, 443:8443`
                                 # instructs Kong that the port 80 is mapped
                                 # to 8000 (and the port 443 to 8443), where
                                 # 8000 and 8443 are the ports the Kong is
                                 # listening to.
                                 #
                                 # This parameter helps Kong to set proper
                                 # forwarded upstream HTTP request header or to
                                 # get proper forwarded port with a Kong PDK
                                 # (in case other means determining it have
                                 # failed). It changes routing by a destination
                                 # port to route by a port from which packets
                                 # are forwarded to Kong, and similarly it
                                 # changes the default plugin log serializer to
                                 # use the port according to this mapping
                                 # instead of reporting the port Kong is
                                 # listening to.

This gets parsed into kong.configuration.host_ports table.

E.g. KONG_PORT_MAP="80:8000,443:8443" where the first number (80/443) before : is the host published port and the second one (8000/8443) is the port that Kong is listening translates to following kong.configuration.host_ports:

{
  [8000]   = 80,
  ["8000"] = 80,
  [8443]   = 443,
  ["8443"] = 443,
}

This PR also adds a new nginx context variable ngx.ctx.host_port which contains this translated port. So instead of using ngx.var.server_port the more correct one in many cases is ngx.ctx.host_port.

Kong PDK is also changed to take this in account when using:

local port = kong.request.get_forwarded_port()

This commit also changes the X-Forwarded-Port to use this. Additionally stream routing by destination port will use this too. Basic log serializer was changed to use ngx.var.host_port as well.

Currently it is only usable in proxy/stream, so things like admin api is not affected by this.

Tieske · 2020-05-06T21:06:56Z

I think I mentioned this elsewhere, but can we add a warning in the logs when Kong is started in a container, and this option is not set?

hbagdi · 2020-05-08T23:06:33Z

Additionally stream routing by destination port will use this too. Basic log serializer was changed to use ngx.var.host_port as well.

I would recommend that we start small rather than going big from the beginning. The primary motivation for this feature is to correct set x-forwarded-port problem when upstream services depend on it. I have not heard any problems with other cases that this PR targets. Do we have feature requests or complaints to change the log serializer and stream routing itself?

I've consulted with users on stream proxy and I do know of users who map different port on different LBs to the same port on Kong and then they route based on the port on Kong. This would be a break change for such setups. These should be one-offs and rare but my point stands.

bungle · 2020-05-09T15:21:59Z

Additionally stream routing by destination port will use this too. Basic log serializer was changed to use ngx.var.host_port as well.

I would recommend that we start small rather than going big from the beginning. The primary motivation for this feature is to correct set x-forwarded-port problem when upstream services depend on it. I have not heard any problems with other cases that this PR targets. Do we have feature requests or complaints to change the log serializer and stream routing itself?

I've consulted with users on stream proxy and I do know of users who map different port on different LBs to the same port on Kong and then they route based on the port on Kong. This would be a break change for such setups. These should be one-offs and rare but my point stands.

I don't see this being starting big. It is rather small. Nothing will be broken for anyone as nobody configures this as it is a new feature. You can also configure it for some ports and not all. In general you could also have environment where pods bind to random port. In such case you cannot practically do any routing by destination port as it is not kown before pod is launched (but the published port certainly will). This is far more common scenario.

Also if you are running LB we still respect forwarded headers before reverting to this.

hbagdi · 2020-05-11T15:43:56Z

Nothing will be broken for anyone as nobody configures this as it is a new feature. You can also configure it for sone ports and not all.

Two fair points.

In general you could also have environment where pods bind to random port. In such case you cannot practically do any routing by destination port as it is not kown before pod is launched (but the published port certainly will). This is far more common scenario.

I'm not sure if this is a common scenario. It hasn't been so in my experience.

With the points made, I think it is okay to proceed with this feature.

bungle · 2020-05-13T08:05:50Z

I think I mentioned this elsewhere, but can we add a warning in the logs when Kong is started in a container, and this option is not set?

This is a bit problematic.

Kong needs to know if it is a container
Kong needs to know if published port is different than listen port

Perhaps later we can add support for container APIs where we can get this auto-configured.

hbagdi · 2020-05-13T18:14:15Z

kong.conf.default

@@ -29,6 +29,17 @@
                                 # Each Kong process must have a separate
                                 # working directory.

+#port_map =                      # When running Kong behind layer 4 port mapping


s/port mapping/proxy

You mean layer 4 proxy? I don't think port-forwarding/-mapping can be described as proxy. Or what do you mean by this?

I rewrote the whole thing. Not sure if it is any better.

hbagdi · 2020-05-13T18:14:43Z

kong.conf.default

@@ -29,6 +29,17 @@
                                 # Each Kong process must have a separate
                                 # working directory.

+#port_map =                      # When running Kong behind layer 4 port mapping
+                                 # (or port-forwarding), e.g. with
+                                 # `docker run -p 80:8000`, you can use this


rather than providing commands here, it would help to provide examples like docker, L4 LBs in the cloud

hbagdi · 2020-05-13T18:16:36Z

kong.conf.default

+                                 # (or port-forwarding), e.g. with
+                                 # `docker run -p 80:8000`, you can use this
+                                 # parameter to let Kong know about published
+                                 # port when it differs from the one Kong is


published port is ambiguous because every port at kong or lb is published in some sense.

Published port is a term that docker uses for -p. The port that Kong listens (in that case) is not published e.g. you can only access it from container. For public it is published by docker at port x. What do you suggest instead?

I rewrote the whole thing. Not sure if it is any better.

kong.conf.default

kong/conf_loader.lua

hbagdi · 2020-05-13T18:19:06Z

kong.conf.default

@@ -29,6 +29,17 @@
                                 # Each Kong process must have a separate
                                 # working directory.

+#port_map =                      # When running Kong behind layer 4 port mapping


Another point, it would be better to call this setting port_maps because it can contain an array of values (csv based).

Ok, I pluralized it. We btw. have proxy_listen not proxy_listeners. And word mapping to which map refers to is ok to use in singular even in plural contexts.

Good point. My bad.

hbagdi · 2020-05-13T18:21:52Z

kong/conf_loader.lua

+
+        if  (host_port_num and host_port_num >= MIN_PORT and host_port_num <= MAX_PORT)
+        and (kong_port_num and kong_port_num >= MIN_PORT and kong_port_num <= MAX_PORT)
+        then


Can we add a check to ensure that kong is configured to listen on kong_port_num?
I'm not sure how much scope creep happens because of this. I'll leave that up to you, Aapo.

@hbagdi I looked at it, and it is a bit complicated (listeners are parsed later). The good side is that if the port is not the one that Kong is not listening, it will not do anything. It is just excess configuration. I will create a card about this.

spec/02-integration/05-proxy/01-proxy_spec.lua

hbagdi · 2020-05-21T23:11:26Z

@hishamhm I added this to 2.1.0 milestone. Please change if needed.

kikito

LGTM

### Summary Layer 4 port mapping (or port forwarding) is used a lot when running Kong inside a container. Common example is exposing ports 80 and 443 externally while running Kong inside a container with internal ports 8000 and 8443. It also allows Kong to started without elevated privileges as it is quite common that you need root-access to bind processes to TCP port < 1024. As this translation is done in many cases without using layer 4 proxy_protocol or layer 7 X-Forwarded-Port HTTP header, the Kong has no knowledge about the target port that client originally connected to. This PR adds `port_maps` configuration parameter: ``` #port_maps = # With this configuration parameter you can # let the Kong to know about the port from # which the packets are forwarded to it. This # is fairly common when running Kong in a # containerized or virtualized environment. # For example `port_maps=80:8000, 443:8443` # instructs Kong that the port 80 is mapped # to 8000 (and the port 443 to 8443), where # 8000 and 8443 are the ports the Kong is # listening to. # # This parameter helps Kong to set proper # forwarded upstream HTTP request header or to # get proper forwarded port with a Kong PDK # (in case other means determining it have # failed). It changes routing by a destination # port to route by a port from which packets # are forwarded to Kong, and similarly it # changes the default plugin log serializer to # use the port according to this mapping # instead of reporting the port Kong is # listening to. ``` This gets parsed into `kong.configuration.host_ports` table. E.g. `KONG_PORT_MAP="80:8000,443:8443"` where the first number (`80`/`443`) before `:` is the host published port and the second one (`8000`/`8443`) is the port that Kong is listening translates to following `kong.configuration.host_ports`: ```lua { [8000] = 80, ["8000"] = 80, [8443] = 443, ["8443"] = 443, } ``` This PR also adds a new nginx context variable `ngx.ctx.host_port` which contains this translated port. So instead of using `ngx.var.server_port` the more correct one in many cases is `ngx.ctx.host_port`. Kong PDK is also changed to take this in account when using: ``` local port = kong.request.get_forwarded_port() ``` This commit also changes the `X-Forwarded-Port` to use this. Additionally `stream routing` by `destination port` will use this too. Basic log serializer was changed to use `ngx.var.host_port` as well. Currently it is only usable in proxy/stream, so things like `admin api` is not affected by this.

seh · 2020-06-18T18:44:24Z

kong.conf.default

@@ -125,6 +125,29 @@
                                 # This value can be set to `off`, thus disabling
                                 # the plugin server and Go plugin loading.

+#port_maps =                     # With this configuration parameter you can
+                                 # let the Kong to know about the port from


s/the Kong to/Kong/

seh · 2020-06-18T18:44:52Z

kong.conf.default

+                                 # is fairly common when running Kong in a
+                                 # containerized or virtualized environment.
+                                 # For example `port_maps=80:8000, 443:8443`
+                                 # instructs Kong that the port 80 is mapped


s/the port/port/

seh · 2020-06-18T18:45:02Z

kong.conf.default

+                                 # containerized or virtualized environment.
+                                 # For example `port_maps=80:8000, 443:8443`
+                                 # instructs Kong that the port 80 is mapped
+                                 # to 8000 (and the port 443 to 8443), where


s/the port/port/

seh · 2020-06-18T18:45:20Z

kong.conf.default

+                                 # For example `port_maps=80:8000, 443:8443`
+                                 # instructs Kong that the port 80 is mapped
+                                 # to 8000 (and the port 443 to 8443), where
+                                 # 8000 and 8443 are the ports the Kong is


"the ports on which Kong is listening"

seh · 2020-06-18T18:45:40Z

kong.conf.default

+                                 # 8000 and 8443 are the ports the Kong is
+                                 # listening to.
+                                 #
+                                 # This parameter helps Kong to set proper


s/to set/set the/

bungle added the pr/wip A work in progress PR opened to receive feedback label May 6, 2020

bungle force-pushed the feat/conf-portmap branch from b74e9ea to c847cd6 Compare May 6, 2020 17:58

bungle force-pushed the feat/conf-portmap branch 3 times, most recently from 0879676 to 81c3555 Compare May 7, 2020 18:25

gszr force-pushed the next branch from acbf968 to b359471 Compare May 7, 2020 18:27

bungle force-pushed the feat/conf-portmap branch 3 times, most recently from 127ee2d to 9eb3c9a Compare May 8, 2020 14:03

bungle force-pushed the feat/conf-portmap branch 2 times, most recently from 565ad73 to a6a3961 Compare May 12, 2020 19:51

bungle added pr/please review and removed pr/wip A work in progress PR opened to receive feedback labels May 12, 2020

bungle force-pushed the feat/conf-portmap branch from a6a3961 to 521e7e2 Compare May 13, 2020 07:30

bungle force-pushed the feat/conf-portmap branch from 521e7e2 to 5b64753 Compare May 13, 2020 13:40

hbagdi reviewed May 13, 2020

View reviewed changes

bungle force-pushed the feat/conf-portmap branch 2 times, most recently from 9d67a28 to dd2aad2 Compare May 19, 2020 16:08

hbagdi added this to the 2.1.0 milestone May 21, 2020

kikito approved these changes May 27, 2020

View reviewed changes

bungle force-pushed the feat/conf-portmap branch from dd2aad2 to f6958e9 Compare May 27, 2020 18:11

bungle changed the title ~~feat(conf) add port_map configuration option~~ feat(conf) add port_maps configuration option May 27, 2020

bungle force-pushed the feat/conf-portmap branch from f6958e9 to e7ba565 Compare May 28, 2020 11:23

chore(deps) bump the kong-build-tools dependency

f13bb5d

bungle force-pushed the feat/conf-portmap branch from 0080b77 to f13bb5d Compare May 28, 2020 16:23

bungle merged commit a17435c into next May 29, 2020

bungle deleted the feat/conf-portmap branch May 29, 2020 08:13

hbagdi mentioned this pull request Jun 18, 2020

why "X-Forwarded-Port“ value is not client request port? #5018

Closed

seh reviewed Jun 18, 2020

View reviewed changes

mflendrich mentioned this pull request Jun 29, 2020

feature: use port mapping feature of Kong Kong/kubernetes-ingress-controller#691

Closed

feifeixiyu mentioned this pull request Jul 4, 2020

[stable/jenkins] It appears that your reverse proxy set up is broken helm/charts#21194

Closed

mflendrich mentioned this pull request Jul 5, 2020

[kong] define KONG_PORT_MAPS for the proxy service Kong/charts#169

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(conf) add port_maps configuration option #5861

feat(conf) add port_maps configuration option #5861

bungle commented May 6, 2020 •

edited

Loading

Tieske commented May 6, 2020

hbagdi commented May 8, 2020

bungle commented May 9, 2020 •

edited

Loading

hbagdi commented May 11, 2020

bungle commented May 13, 2020

hbagdi May 13, 2020

bungle May 19, 2020

bungle May 19, 2020

hbagdi May 13, 2020

hbagdi May 13, 2020

bungle May 19, 2020 •

edited

Loading

bungle May 19, 2020

hbagdi May 13, 2020

bungle May 19, 2020 •

edited

Loading

hbagdi May 19, 2020

hbagdi May 13, 2020

bungle May 27, 2020

hbagdi commented May 21, 2020

kikito left a comment

seh Jun 18, 2020

seh Jun 18, 2020

seh Jun 18, 2020

seh Jun 18, 2020

seh Jun 18, 2020

feat(conf) add port_maps configuration option #5861

feat(conf) add port_maps configuration option #5861

Conversation

bungle commented May 6, 2020 • edited Loading

Summary

Tieske commented May 6, 2020

hbagdi commented May 8, 2020

bungle commented May 9, 2020 • edited Loading

hbagdi commented May 11, 2020

bungle commented May 13, 2020

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

bungle May 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bungle May 19, 2020 • edited Loading

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

hbagdi commented May 21, 2020

kikito left a 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

bungle commented May 6, 2020 •

edited

Loading

bungle commented May 9, 2020 •

edited

Loading

bungle May 19, 2020 •

edited

Loading

bungle May 19, 2020 •

edited

Loading