Skip to content

http: configurable ignore of HTTP/1.1 upgrades (#37642)#40407

Closed
dcillera wants to merge 1 commit intoenvoyproxy:release/v1.32from
dcillera:http_config_ignore
Closed

http: configurable ignore of HTTP/1.1 upgrades (#37642)#40407
dcillera wants to merge 1 commit intoenvoyproxy:release/v1.32from
dcillera:http_config_ignore

Conversation

@dcillera
Copy link

Backport from 1.34

Fixes
#36305
and I think this is also related to:
istio/istio#53239

Add configuration to ignore HTTP/1.1 Upgrade headers . See https://datatracker.ietf.org/doc/html/rfc7230#section-6.7:

@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @wbpcode
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #40407 was opened by dcillera.

see: more, trace.

@dcillera dcillera force-pushed the http_config_ignore branch 3 times, most recently from fe4e9ca to b34125d Compare July 25, 2025 07:13
Fixes envoyproxy#36305

Add configuration to ignore HTTP/1.1 Upgrade headers . See
https://datatracker.ietf.org/doc/html/rfc7230#section-6.7:

Signed-off-by: Tom Edge <tom.edge@autotrader.co.uk>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Co-authored-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Dario Cillerai <dcillera@redhat.com>
@dcillera dcillera force-pushed the http_config_ignore branch from b34125d to 84bfbaf Compare July 25, 2025 09:27
agrawroh pushed a commit that referenced this pull request Jul 30, 2025
Created by Envoy dependency bot for @phlax

Fix #40407



Signed-off-by: dependency-envoy[bot]
<148525496+dependency-envoy[bot]@users.noreply.github.com>

Signed-off-by: dependency-envoy[bot] <148525496+dependency-envoy[bot]@users.noreply.github.com>
Co-authored-by: dependency-envoy[bot] <148525496+dependency-envoy[bot]@users.noreply.github.com>
@dcillera
Copy link
Author

Hey @agrawroh I understood #4090 has fixed the problem. Hasn't it?
However, I would need a solution/workaround in release/v1.32 and in this release the rules_cc library is not present (I guess more commits would be needed to install and use it). Do you think we could re-open and merge this PR oly for 1.32? or your fix in #4090 is recommended?

@agrawroh agrawroh reopened this Aug 4, 2025
@agrawroh
Copy link
Member

agrawroh commented Aug 4, 2025

Hey @agrawroh I understood #4090 has fixed the problem. Hasn't it? However, I would need a solution/workaround in release/v1.32 and in this release the rules_cc library is not present (I guess more commits would be needed to install and use it). Do you think we could re-open and merge this PR oly for 1.32? or your fix in #4090 is recommended?

@dcillera This was unintentional. The dependency PR had an incorrect link and when it got merged, GitHub closed this PR instead of the actual issue we wanted it to close.

@dcillera
Copy link
Author

dcillera commented Aug 4, 2025

Thanks for re-opening it, @agrawroh :-)

@phlax
Copy link
Member

phlax commented Aug 8, 2025

/retest

@phlax
Copy link
Member

phlax commented Aug 8, 2025

@dcillera if we are backporting this to 1.32 - can we also add any other branches that require it please - at least 1.33

also a little apprehensive in that we dont generally backport features - is this arguably a bug fix ?

@dcillera
Copy link
Author

dcillera commented Aug 8, 2025

@phlax This PR was intended to fix #36305 in 1.32.
But I have the following doubt . As the modification is concerned with API, I suppose that the corresponding modification in Istio is necessary and in fact there is:
istio/istio#53239
that is still open, even in newer releases.
What I would conclude is that the fix is not really available in any releases.

@phlax
Copy link
Member

phlax commented Aug 8, 2025

i see so iiuc arguably a fix but an imperfect one?

@dcillera
Copy link
Author

dcillera commented Aug 8, 2025

I'll ask for more info to fully understand, because actually it was merged in: #37642

@wbpcode
Copy link
Member

wbpcode commented Aug 9, 2025

Hmmm, I basically OK to this backport. But for this a complex backport, I will inclinded let the maintainer to do that if it strongly is required for end users. Or we need take long time to review the code again and again. ESP this is merged for a while and I almost forget all previous impression.

@ggreenway
Copy link
Member

Yes in general we don't backport features, but this one is to maintain compatibility with the ecosystem, so I'm ok with backporting it. But it must also go to 1.33; we can't support this in 1.32 and 1.34 but not 1.33.

@dcillera
Copy link
Author

Hello @ggreenway, I also wonder if other modifications are necessary in Istio for the related configuration to be sent to Envoy through the API.

@phlax
Copy link
Member

phlax commented Aug 18, 2025

i will be cycling a set of patch release at latest tomorrow morning (european time) so if this is to be included it will need to land asap.

@dcillera can you raise for all other relevant branches please

@ggreenway gtm ?

@ggreenway
Copy link
Member

@phlax that sounds fine as long as it's included in 1.33

@ggreenway
Copy link
Member

I'll open backports PRs for both versions, as requested by @wbpcode

@ggreenway ggreenway closed this Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants