[artifactory] allow to use proxy protocol for nginx#2156
[artifactory] allow to use proxy protocol for nginx#2156rufdoSICKAG wants to merge 1 commit intojfrog:masterfrom
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
rufdoSICKAG
force-pushed
the
proxy-protocol
branch
from
f918859 to
c4a9bcb
Compare
stable/artifactory/values.yaml
Outdated
| - sh | ||
| - -c | ||
| - curl -s -k --fail --max-time {{ .Values.probes.timeoutSeconds }} {{ include "nginx.scheme" . }}://localhost:{{ include "nginx.port" . }}/ | ||
| - curl {{ if .Values.nginx.httpUseProxyProtocol }}--haproxy-protocol{{ end }} -s -k --fail --max-time {{ .Values.probes.timeoutSeconds }} {{ include "nginx.scheme" . }}://localhost:{{ include "nginx.port" . }}/ |
There was a problem hiding this comment.
The --haproxy-protocol flag is gated on httpUseProxyProtocol only, but the probe target is determined by nginx.scheme/nginx.port which may resolve to the HTTPS listener. If a user enables httpsUseProxyProtocol: true with HTTP disabled, probes hit the HTTPS port without --haproxy-protocol → nginx rejects every probe → CrashLoopBackOff
The condition should account for both.
| ssh: | ||
| internalPort: 1339 | ||
| externalPort: 1339 | ||
| httpUseProxyProtocol: false |
There was a problem hiding this comment.
Can you add a comment that once proxy_protocol is enabled on a listener, all connections to that port must send a PROXY protocol header — including LB health checks and kubectl port-forward.
Users may need to reconfigure their LB health checks accordingly.
|
This PR only covers stable/artifactory. The stable/artifactory-ha/ chart has identical nginx config and probes. If possible, please include those changes as well. |
When using a load balancer in front of nginx it is often required to enable proxy protocol. This commit adds the required configuration options to enable proxy protocol support in the nginx configuration. Signed-off-by: Dominik Ruf <dominik.ruf@sick.de>
rufdoSICKAG
force-pushed
the
proxy-protocol
branch
from
c4a9bcb to
08a6bde
Compare
|
@shahiinn thanks for your review. |
|
Thanks for the contribution and for addressing the review feedback, @rufdoSICKAG! The changes look good overall. One remaining item -- could you please update the CHANGELOG.md as well? On our end, we will be picking this up internally and it will be part of one of the upcoming Artifactory chart releases. Once that release is out, this PR will be merged here. |
|
@shahiinn thanks for your feedback. |
2 participants
When using a load balancer in front of nginx it is often required to enable proxy protocol.
This commit adds the required configuration options to enable proxy protocol support in the nginx configuration.
PR Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]
[artifactory])What this PR does / why we need it:
When using a load balancer in front of nginx it is often required to
enable proxy protocol.
This commit adds the required configuration options to enable proxy
protocol support in the nginx configuration.