Added proxy protocol support settings#922
Conversation
|
@AlinsRan could you please review this? I'm using it on the production setup - and it proofs the concept. |
| # - name: prometheus-metrics | ||
| # size: 20m | ||
|
|
||
| proxy_protocol: |
There was a problem hiding this comment.
please use lowerCamelCase
| {{- if .Values.service.http.enabled }} | ||
| - name: apisix-gateway | ||
| port: {{ .Values.service.http.servicePort }} | ||
| {{- if .Values.apisix.proxy_protocol.enabled }} |
There was a problem hiding this comment.
It does not conflict with HTTP or HTTPS, why do we need to share a port?It does not conflict with HTTP or HTTPS, we should add a new service.port.
There was a problem hiding this comment.
Yes, it doesn't conflict. But when you enable proxy protocol on the network load balancer in front of the apache apisix (I'm using Oracle cloud) https://docs.nginx.com/nginx/admin-guide/load-balancer/using-proxy-protocol/
From the documentation of the network load balancer:
"For TCP applications using PROXY protocol v2, NLB adds a PROXY protocol v2 header to each inbound TCP connection."
So if you enable ppv2 on the nlb but your upstream (apisix) cannot accept ppv2 header in the tcp - then it will be rejected. That why we need to change default port on the service that exposes gateway to ports that support ppv2 tcp header.
There was a problem hiding this comment.
I don't know why, but apache apisix won't let you setup proxy protocol port on the same port as node (80/443).
That why I added if statement.
|
There are already two PR open for this same objective: Would be nice to have any of them merged. It is extremely important for cloud deployments (AWS, GCP, Azure...) in which APISIX runs behind a L4 load balancer. Bumping for visibility. |
|
@AlinsRan any plans on having any of those PRs merged any time soon? |
|
@AlinsRan is this something planed to be taken care of? thanks. |
|
Exposing APISIX's proxy_protocol HTTP/HTTPS listeners in the chart is a reasonable direction. The main unresolved issue is the Service exposure model from the previous review thread: with apisix.proxyProtocol.enabled=true, the current diff changes the existing apisix-gateway / apisix-gateway-tls targetPort from the normal HTTP/TLS ports to the proxy protocol ports, instead of adding dedicated service ports. That changes the protocol semantics of the existing Service and does not allow users to expose normal HTTP/HTTPS and PROXY protocol HTTP/HTTPS at the same time. Please first sync with master to resolve the merge conflict and fix the current failed/cancelled CI. Then please adjust the chart to add dedicated proxy protocol service ports/values, or clearly document and test why replacing the existing targetPort is the intended behavior. Please also add Helm rendering coverage for enabled=false, enabled=true, and custom listen_http_port/listen_https_port values. The unrelated formatting changes in values.yaml should be removed to keep the PR focused. |
|
@Baoyuantop Hello! I did it intentionally because:
I verified this behavior while testing within the Oracle Cloud Infrastructure (OCI) environment using Apache APISIX tightly coupled with a cloud load balancer (L4 or L7).
The East-West Traffic Problem |
I was able to overcome that particular problem in OCI by setting Other than that, I can confirm that ApiSix installed from helm chart with changes from this PR works well with OCI Network Load Balancer with PPv2 protocol enabled. It would be nice to have this PR finalized and merged so we can use upstream chart instead of relying on local copy. |
|
Hi @pantherra, could you please resolve the merge conflicts and get the CI passing? Once that’s done, we can continue with the review. |
|
@Baoyuantop fixed. |
Hello again. After syncing my fork with an upcoming changes - I merged my local changes to enable support of the proxy protocol v2 settings on the Apache APISIX.
I tested it on my installation and it works fine. It is listed in the reference: https://docs.api7.ai/apisix/networking/port-reference/