feat: enable support for proxy protocol on status port by yann-soubeyrand · Pull Request #55986 · istio/istio

yann-soubeyrand · 2025-04-17T16:16:57Z

Please provide a description of this PR:

On AWS network load balancers, when enabling proxy protocol and configuring health check to query Istio status port on the path /healthz/ready, the health check fails, because Istio status port does not support proxy protocol. However, on the load balancer, proxy protocol cannot be disabled for the health check only.

Envoy supports the proxy protocol with an option to allow requests without proxy protocol. This option breaks conformance to the proxy protocol specification, and should only be enabled when the traffic to the listener comes from a trusted source, which should be the case for the status port. This seems to be a good compromise to allow both requests with and without proxy protocol on the status port.

Fixes #39868

This PR is meant to open a discussion. I’d need help to write some tests if it has a chance of being accepted.

istio-policy-bot · 2025-04-17T16:17:02Z

😊 Welcome @yann-soubeyrand! This is either your first contribution to the Istio istio repo, or it's been
a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

istio-testing · 2025-04-17T16:17:10Z

Hi @yann-soubeyrand. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

keithmattix · 2025-04-17T16:38:16Z

The downside of always enabling is that we now run this listener filter for every single request that comes through, regardless of if it's proxy protocol or not...I would feel more comfortable if there was an option in proxyconfig that allowed configuring this just for proxies that needed it

yann-soubeyrand · 2025-04-17T17:00:53Z

Are there a lot of requests on the status port? Is it possible to modify the bootstrap configuration using ProxyConfig?

zirain · 2025-04-18T07:15:58Z

if this belong to AWS only, this should be guarded by a feature flag(or detect by platform) to make less noise to other users.

yann-soubeyrand · 2025-04-18T07:58:45Z

I’m not sure this concerns AWS only, this is just the example that I’m aware of, but I guess there are other use cases.

zirain · 2025-04-18T08:42:30Z

I’m not sure this concerns AWS only, this is just the example that I’m aware of, but I guess there are other use cases.

I never hear this before, so prefer this behind a flag.

istio-policy-bot · 2025-04-18T09:37:29Z

🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?

Courtesy of your friendly test nag.

yann-soubeyrand · 2025-04-18T09:38:49Z

@zirain I modified the PR, is it the modification that you had in mind?

ramaraochavali · 2025-04-18T12:48:44Z

/ok-to-test

ramaraochavali · 2025-04-18T12:50:54Z

tools/packaging/common/envoy_bootstrap.json

+            "name": "envoy.listener.proxy_protocol",
+            "typed_config": {
+              "@type": "type.googleapis.com/envoy.extensions.filters.listener.proxy_protocol.v3.ProxyProtocol",
+              "allow_requests_without_proxy_protocol": true,


is this still needed now?

The "allow_requests_without_proxy_protocol": true part you mean? I think so, because the Kubernetes probes won’t use proxy protocol I guess.

I was wondering if we can have multiple filter chain matches that distinguishes health check call from AWS NLB vs K8s and this filter accoridingly

I don't think it's possible, since the proxy protocol filter is a listener filter, or is it? Do you prefer that we add another dedicated status listener with proxy protocol support, for example on port 15022?

I think how its written is good we can just make this opt-in

Oh which you already did. LGTM then personally

Oh. Sorry. This is listener filter. You can not do what I suggested and do not think adding another port is needed. I am good with this

pilot/pkg/features/experimental.go

On AWS network load balancers, when enabling proxy protocol and configuring health check to query Istio status port on the path /healthz/ready, the health check fails, because Istio status port does not support proxy protocol. However, on the load balancer, proxy protocol cannot be disabled for the health check only. Envoy supports the proxy protocol with an option to allow requests without proxy protocol. This option breaks conformance to the proxy protocol specification, and should only be enabled when the traffic to the listener comes from a trusted source, which should be the case for the status port. This seems to be a good compromise to allow both requests with and without proxy protocol on the status port. Fixes istio#39868

istio-testing · 2025-06-04T11:32:03Z

@yann-soubeyrand: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
integ-ambient-mc_istio	`e4a8673`	link	false	`/test integ-ambient-mc`

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

yann-soubeyrand · 2025-06-04T12:27:08Z

/test integ-pilot-multicluster

zirain

LGTM, thanks!

yann-soubeyrand · 2025-06-06T11:41:07Z

Thanks everyone for your feedback helping this PR go its way to the merge!

* upstream/master: (28 commits) Automator: update common-files@master in istio/istio@master (istio#56545) Automator: update proxy@master in istio/istio@master (istio#56544) Automator: update go-control-plane in istio/istio@master (istio#56543) Automator: update proxy@master in istio/istio@master (istio#56540) Automator: update ztunnel@master in istio/istio@master (istio#56532) Ambient: In ambient index, filter configs by revision (istio#56477) Automator: update istio/client-go@master dependency in istio/istio@master (istio#56539) Automator: update proxy@master in istio/istio@master (istio#56538) Automator: update common-files@master in istio/istio@master (istio#56537) optimization: allow for lazy sidecar initialization (istio#47221) static collection eager indexes (istio#56530) fix typo in flag (istio#56534) feat: enable support for proxy protocol on status port (istio#55986) remove finding of pods by IP (istio#56502) Automator: update proxy@master in istio/istio@master (istio#56528) migrate file monitor to krt (istio#55970) Automator: update istio/client-go@master dependency in istio/istio@master (istio#56525) Automator: update ztunnel@master in istio/istio@master (istio#56518) Fix crash in merging http routes (istio#56499) krt: add assertions (istio#56510) ...

jeffmccune · 2025-10-07T23:48:24Z

@yann-soubeyrand You're my hero today, thank you.

yann-soubeyrand requested a review from a team as a code owner April 17, 2025 16:16

istio-policy-bot added the area/networking label Apr 17, 2025

istio-testing added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-ok-to-test labels Apr 17, 2025

yann-soubeyrand force-pushed the status-port-proxy-protocol branch from 63e9cf6 to 4345183 Compare April 18, 2025 09:37

istio-testing added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 18, 2025

istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Apr 18, 2025

ramaraochavali reviewed Apr 18, 2025

View reviewed changes

zirain reviewed Apr 19, 2025

View reviewed changes

pilot/pkg/features/experimental.go Show resolved Hide resolved

istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label May 9, 2025

istio-policy-bot added the lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while label May 20, 2025

yann-soubeyrand force-pushed the status-port-proxy-protocol branch from 4345183 to 73669c4 Compare May 20, 2025 11:44

istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label May 20, 2025

istio-policy-bot closed this Jun 4, 2025

istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label Jun 4, 2025

zirain reopened this Jun 4, 2025

zirain removed lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. labels Jun 4, 2025

yann-soubeyrand force-pushed the status-port-proxy-protocol branch from 73669c4 to e879337 Compare June 4, 2025 11:01

istio-testing added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 4, 2025

yann-soubeyrand force-pushed the status-port-proxy-protocol branch from e879337 to 38b8e74 Compare June 4, 2025 11:05

yann-soubeyrand force-pushed the status-port-proxy-protocol branch from 38b8e74 to e4a8673 Compare June 4, 2025 11:07

zirain approved these changes Jun 5, 2025

View reviewed changes

yann-soubeyrand requested review from howardjohn and ramaraochavali June 6, 2025 06:50

ramaraochavali approved these changes Jun 6, 2025

View reviewed changes

istio-testing merged commit 57ef90d into istio:master Jun 6, 2025
29 of 30 checks passed

yann-soubeyrand deleted the status-port-proxy-protocol branch June 6, 2025 11:41

Conversation

yann-soubeyrand commented Apr 17, 2025 • edited by istio-policy-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

istio-policy-bot commented Apr 17, 2025

Uh oh!

istio-testing commented Apr 17, 2025

Uh oh!

keithmattix commented Apr 17, 2025

Uh oh!

yann-soubeyrand commented Apr 17, 2025

Uh oh!

zirain commented Apr 18, 2025

Uh oh!

yann-soubeyrand commented Apr 18, 2025

Uh oh!

zirain commented Apr 18, 2025

Uh oh!

istio-policy-bot commented Apr 18, 2025

Uh oh!

yann-soubeyrand commented Apr 18, 2025

Uh oh!

ramaraochavali commented Apr 18, 2025

Uh oh!

ramaraochavali Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

yann-soubeyrand Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

ramaraochavali Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

yann-soubeyrand Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

howardjohn Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

howardjohn Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

ramaraochavali Apr 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

istio-testing commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yann-soubeyrand commented Jun 4, 2025

Uh oh!

zirain left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yann-soubeyrand commented Jun 6, 2025

Uh oh!

jeffmccune commented Oct 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

yann-soubeyrand commented Apr 17, 2025 •

edited by istio-policy-bot

Loading

istio-testing commented Jun 4, 2025 •

edited

Loading