You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "heresie (via GitHub)" <gi...@apache.org> on 2023/04/03 16:09:01 UTC
[GitHub] [apisix-helm-chart] heresie opened a new pull request, #528: feat: allow proxy protocol configuration for APISIX
heresie opened a new pull request, #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528
Hello everybody 👋
I'm currently trying to install APISIX on Kubernetes clusters that need to use Proxy Protocol in order to be usable. Unfortunately, the current Helm Chart does not allow to configure APISIX with proxy protocol by using `values.yaml` configuration.
The previous PR #353 about this subject is getting old, and @hgranillo has moved from APISIX, so here is a new one, updated and ready to merge.
I tested this Chart update on our K8S clusters, without any problem.
As stated in the original PR :
> This PR adds the option to enable Proxy Protocol in the APISIX configuration file and allows to add the proxy protocol listeners to the gateway Kubernetes service.
>
> Made all ports configurable in a fashion similar to `apisix-gateway` and `apisix-gateway-tls`.
>
> I left `enable_tcp_pp_to_upstream` out because at this moment I have no way to test it. I can add the toggle if needed to merge this PR.
>
> I tested these changes with the following configuration in one of my EKS Clusters.
> I'm using the [aws-load-balancer-controller](https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.4/) to provision a NBL with SSL/TLS Offloading and Proxy Protocol enabled (see loadbalancer annotations below)
>
> This allows me to send HTTP(80) -> ProxyProtocol HTTP(9181) and HTTPS 443 -> ProxyProtocol HTTP (9181)
Here is the `values.yaml` I used for tests :
```yaml
global:
enableIPv6: false
storageClass: <my-storage-class>
apisix:
kind: DaemonSet
proxyProtocol:
enabled: true
listenHttpPort: 9181
listenHttpsPort: 9182
gateway:
type: NodePort
http:
enabled: true
tls:
enabled: true
proxyProtocol:
http:
enabled: true
nodePort: 30080
containerPort: 9181
https:
enabled: true
nodePort: 30443
containerPort: 9182
```
This PR also bumps Chart Version to the next minor : `1.4.0`.
All comments are welcome.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] heresie commented on a diff in pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "heresie (via GitHub)" <gi...@apache.org>.
heresie commented on code in PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#discussion_r1185771077
##########
charts/apisix/templates/configmap.yaml:
##########
@@ -69,14 +69,16 @@ data:
enable_ipv6: {{ .Values.apisix.enableIPv6 }} # Enable nginx IPv6 resolver
enable_server_tokens: {{ .Values.apisix.enableServerTokens }} # Whether the APISIX version number should be shown in Server header
- # proxy_protocol: # Proxy Protocol configuration
- # listen_http_port: 9181 # The port with proxy protocol for http, it differs from node_listen and admin_listen.
- # # This port can only receive http request with proxy protocol, but node_listen & admin_listen
- # # can only receive http request. If you enable proxy protocol, you must use this port to
- # # receive http request with proxy protocol
- # listen_https_port: 9182 # The port with proxy protocol for https
- # enable_tcp_pp: true # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
- # enable_tcp_pp_to_upstream: true # Enables the proxy protocol to the upstream server
+ {{- if .Values.apisix.proxyProtocol.enabled }}
+ proxy_protocol: # Proxy Protocol configuration
+ listen_http_port: {{ .Values.apisix.proxyProtocol.listenHttpPort }} # The port with proxy protocol for http, it differs from node_listen and port_admin.
+ # This port can only receive http request with proxy protocol, but node_listen & port_admin
+ # can only receive http request. If you enable proxy protocol, you must use this port to
+ # receive http request with proxy protocol
+ listen_https_port: {{ .Values.apisix.proxyProtocol.listenHttpsPort }} # The port with proxy protocol for https
+ enable_tcp_pp: {{ .Values.apisix.proxyProtocol.enabled }} # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
+ # enable_tcp_pp_to_upstream: true # Enables the proxy protocol to the upstream server
Review Comment:
Hello @Gallardot thank you for your feedback, I just added the possibility to configure the pp for upstream server.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] Gallardot commented on a diff in pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "Gallardot (via GitHub)" <gi...@apache.org>.
Gallardot commented on code in PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#discussion_r1185732393
##########
charts/apisix/templates/configmap.yaml:
##########
@@ -69,14 +69,16 @@ data:
enable_ipv6: {{ .Values.apisix.enableIPv6 }} # Enable nginx IPv6 resolver
enable_server_tokens: {{ .Values.apisix.enableServerTokens }} # Whether the APISIX version number should be shown in Server header
- # proxy_protocol: # Proxy Protocol configuration
- # listen_http_port: 9181 # The port with proxy protocol for http, it differs from node_listen and admin_listen.
- # # This port can only receive http request with proxy protocol, but node_listen & admin_listen
- # # can only receive http request. If you enable proxy protocol, you must use this port to
- # # receive http request with proxy protocol
- # listen_https_port: 9182 # The port with proxy protocol for https
- # enable_tcp_pp: true # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
- # enable_tcp_pp_to_upstream: true # Enables the proxy protocol to the upstream server
+ {{- if .Values.apisix.proxyProtocol.enabled }}
+ proxy_protocol: # Proxy Protocol configuration
+ listen_http_port: {{ .Values.apisix.proxyProtocol.listenHttpPort }} # The port with proxy protocol for http, it differs from node_listen and port_admin.
+ # This port can only receive http request with proxy protocol, but node_listen & port_admin
+ # can only receive http request. If you enable proxy protocol, you must use this port to
+ # receive http request with proxy protocol
+ listen_https_port: {{ .Values.apisix.proxyProtocol.listenHttpsPort }} # The port with proxy protocol for https
+ enable_tcp_pp: {{ .Values.apisix.proxyProtocol.enabled }} # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
+ # enable_tcp_pp_to_upstream: true # Enables the proxy protocol to the upstream server
Review Comment:
This configuration item can also be configured. Is that OK?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] tao12345666333 commented on a diff in pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on code in PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#discussion_r1240994496
##########
charts/apisix/Chart.yaml:
##########
@@ -31,7 +31,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 1.4.0
+version: 1.4.1
Review Comment:
We can leave this unchanged for now, right? This will trigger the release of a new version.
I want to check the recent modifications and see if they include any changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] tao12345666333 commented on a diff in pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on code in PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#discussion_r1187139221
##########
charts/apisix/templates/configmap.yaml:
##########
@@ -69,14 +69,16 @@ data:
enable_ipv6: {{ .Values.apisix.enableIPv6 }} # Enable nginx IPv6 resolver
enable_server_tokens: {{ .Values.apisix.enableServerTokens }} # Whether the APISIX version number should be shown in Server header
- # proxy_protocol: # Proxy Protocol configuration
- # listen_http_port: 9181 # The port with proxy protocol for http, it differs from node_listen and admin_listen.
- # # This port can only receive http request with proxy protocol, but node_listen & admin_listen
- # # can only receive http request. If you enable proxy protocol, you must use this port to
- # # receive http request with proxy protocol
- # listen_https_port: 9182 # The port with proxy protocol for https
- # enable_tcp_pp: true # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
- # enable_tcp_pp_to_upstream: true # Enables the proxy protocol to the upstream server
+ {{- if .Values.apisix.proxyProtocol.enabled }}
+ proxy_protocol: # Proxy Protocol configuration
+ listen_http_port: {{ .Values.apisix.proxyProtocol.listenHttpPort }} # The port with proxy protocol for http, it differs from node_listen and port_admin.
+ # This port can only receive http request with proxy protocol, but node_listen & port_admin
+ # can only receive http request. If you enable proxy protocol, you must use this port to
+ # receive http request with proxy protocol
+ listen_https_port: {{ .Values.apisix.proxyProtocol.listenHttpsPort }} # The port with proxy protocol for https
+ enable_tcp_pp: {{ .Values.apisix.proxyProtocol.enabled }} # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
Review Comment:
we can use a new configuration item
##########
charts/apisix/Chart.yaml:
##########
@@ -31,7 +31,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 1.3.1
+version: 1.4.0
Review Comment:
we can just bump to 1.3.2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] heresie commented on a diff in pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "heresie (via GitHub)" <gi...@apache.org>.
heresie commented on code in PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#discussion_r1188233615
##########
charts/apisix/Chart.yaml:
##########
@@ -31,7 +31,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 1.3.1
+version: 1.4.0
Review Comment:
✅
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] heresie commented on a diff in pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "heresie (via GitHub)" <gi...@apache.org>.
heresie commented on code in PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#discussion_r1194884997
##########
charts/apisix/templates/configmap.yaml:
##########
@@ -69,14 +69,16 @@ data:
enable_ipv6: {{ .Values.apisix.enableIPv6 }} # Enable nginx IPv6 resolver
enable_server_tokens: {{ .Values.apisix.enableServerTokens }} # Whether the APISIX version number should be shown in Server header
- # proxy_protocol: # Proxy Protocol configuration
- # listen_http_port: 9181 # The port with proxy protocol for http, it differs from node_listen and admin_listen.
- # # This port can only receive http request with proxy protocol, but node_listen & admin_listen
- # # can only receive http request. If you enable proxy protocol, you must use this port to
- # # receive http request with proxy protocol
- # listen_https_port: 9182 # The port with proxy protocol for https
- # enable_tcp_pp: true # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
- # enable_tcp_pp_to_upstream: true # Enables the proxy protocol to the upstream server
+ {{- if .Values.apisix.proxyProtocol.enabled }}
+ proxy_protocol: # Proxy Protocol configuration
+ listen_http_port: {{ .Values.apisix.proxyProtocol.listenHttpPort }} # The port with proxy protocol for http, it differs from node_listen and port_admin.
+ # This port can only receive http request with proxy protocol, but node_listen & port_admin
+ # can only receive http request. If you enable proxy protocol, you must use this port to
+ # receive http request with proxy protocol
+ listen_https_port: {{ .Values.apisix.proxyProtocol.listenHttpsPort }} # The port with proxy protocol for https
+ enable_tcp_pp: {{ .Values.apisix.proxyProtocol.enabled }} # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
Review Comment:
Hello @tao12345666333 , I added a new configuration item for tcp_pp as requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] heresie commented on pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "heresie (via GitHub)" <gi...@apache.org>.
heresie commented on PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#issuecomment-1521282770
Hello!
Thanks for the CI trigger.
I've updated the Apisix Helm Chart Readme accordingly with `helm-docs`, as asked by the GitHub workflow. (Nice tool I didn't know by the way!)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] heresie commented on pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "heresie (via GitHub)" <gi...@apache.org>.
heresie commented on PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#issuecomment-1606862194
@tao12345666333 said :
> And please resolve the conflicts.
The conflicts are resolved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] heresie commented on pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "heresie (via GitHub)" <gi...@apache.org>.
heresie commented on PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#issuecomment-1556716831
Hello @tao12345666333, have you seen latest changes for this topic? Thank you in advance for your help.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] heresie commented on a diff in pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "heresie (via GitHub)" <gi...@apache.org>.
heresie commented on code in PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#discussion_r1188232304
##########
charts/apisix/templates/configmap.yaml:
##########
@@ -69,14 +69,16 @@ data:
enable_ipv6: {{ .Values.apisix.enableIPv6 }} # Enable nginx IPv6 resolver
enable_server_tokens: {{ .Values.apisix.enableServerTokens }} # Whether the APISIX version number should be shown in Server header
- # proxy_protocol: # Proxy Protocol configuration
- # listen_http_port: 9181 # The port with proxy protocol for http, it differs from node_listen and admin_listen.
- # # This port can only receive http request with proxy protocol, but node_listen & admin_listen
- # # can only receive http request. If you enable proxy protocol, you must use this port to
- # # receive http request with proxy protocol
- # listen_https_port: 9182 # The port with proxy protocol for https
- # enable_tcp_pp: true # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
- # enable_tcp_pp_to_upstream: true # Enables the proxy protocol to the upstream server
+ {{- if .Values.apisix.proxyProtocol.enabled }}
+ proxy_protocol: # Proxy Protocol configuration
+ listen_http_port: {{ .Values.apisix.proxyProtocol.listenHttpPort }} # The port with proxy protocol for http, it differs from node_listen and port_admin.
+ # This port can only receive http request with proxy protocol, but node_listen & port_admin
+ # can only receive http request. If you enable proxy protocol, you must use this port to
+ # receive http request with proxy protocol
+ listen_https_port: {{ .Values.apisix.proxyProtocol.listenHttpsPort }} # The port with proxy protocol for https
+ enable_tcp_pp: {{ .Values.apisix.proxyProtocol.enabled }} # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
Review Comment:
Hello @tao12345666333, thank you for the feedback.
I'm afraid I don't understand what you mean by "new configuration item".
I already match the reference configuration file (https://github.com/apache/apisix/blob/master/conf/config-default.yaml#L37-L44).
Can you please be more specific?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] heresie commented on pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "heresie (via GitHub)" <gi...@apache.org>.
heresie commented on PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#issuecomment-1520286067
Hello!
Some checks [have failed](https://github.com/apache/apisix-helm-chart/actions/runs/4598885299/jobs/8436635257?pr=528) due to a missing space in chart comments.
It has been fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] franek commented on pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "franek (via GitHub)" <gi...@apache.org>.
franek commented on PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#issuecomment-1608919483
🎉
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] tao12345666333 commented on pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#issuecomment-1605809697
Other parts LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] heresie commented on a diff in pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "heresie (via GitHub)" <gi...@apache.org>.
heresie commented on code in PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#discussion_r1241739603
##########
charts/apisix/Chart.yaml:
##########
@@ -31,7 +31,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 1.4.0
+version: 1.4.1
Review Comment:
Hello @tao12345666333, I made a rollback on the chart semver as asked.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] tao12345666333 commented on pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#issuecomment-1536315196
I will add this to my list and review it ASAP. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] heresie commented on pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "heresie (via GitHub)" <gi...@apache.org>.
heresie commented on PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#issuecomment-1534284481
Hello!
Is there any maintainer for the PR review? Thanks in advance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] tao12345666333 commented on pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528#issuecomment-1605813582
And please resolve the conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] tao12345666333 merged pull request #528: feat: allow proxy protocol configuration for APISIX
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 merged PR #528:
URL: https://github.com/apache/apisix-helm-chart/pull/528
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org