[release-1.4] ext_authz: Fix missing body in CheckRequest for HTTP/2 (#8877) (cherry-pick from upstream) #127
Conversation
This patch fixes the missing body in `CheckRequest` when the `with_request_body` is enabled and the downstream protocol is HTTP/2. Fixes envoyproxy#8666 Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
Is the CLA needed for a cherry-pick where no original authorship work has been done? |
|
@googlebot I signed it! |
|
Hi @mayflower-markus, sorry for late reply! This needs:
|
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
@googlebot I consent |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
| void sendExtAuthzResponse() { | ||
| ext_authz_request_->startGrpcStream(); | ||
| envoy::service::auth::v2::CheckResponse check_response; | ||
| check_response.mutable_status()->set_code(Grpc::Status::WellKnownGrpcStatus::Ok); |
There was a problem hiding this comment.
Actually, this doesn't compile, but it should be an easy fix.
There was a problem hiding this comment.
Just replace WellKnownGrpcStatus with GrpcStatus.
There was a problem hiding this comment.
Hum, so do you mean every set_code should be set by Grpc::Status:: instead of value from Grpc::Status::WellKnownGrpcStatus:: the enum? Makes sense.
There was a problem hiding this comment.
Grpc::Status:: was renamed to Grpc::Status::WellKnownGrpcStatus:: recently, so we need to undo that change when backporting.
… avoid fragmentation (#117) (#127) Change OwnedImpl::move to force a copy instead of taking ownership of slices in cases where the offered slices are below kCopyThreshold Risk Level: medium, changes to buffer behavior Testing: Unit Tests Docs Changes: N/A Release Notes: N/A Signed-off-by: Antonio Vicente <avd@google.com> Signed-off-by: Yangmin Zhu <ymzhu@google.com>
|
Release 1.4 has reached the end of support: https://istio.io/latest/news/support/announcing-1.4-eol-final/. No more builds are planned, so I am closing this PR. |
… avoid fragmentation (#117) (#127) Change OwnedImpl::move to force a copy instead of taking ownership of slices in cases where the offered slices are below kCopyThreshold Risk Level: medium, changes to buffer behavior Testing: Unit Tests Docs Changes: N/A Release Notes: N/A Signed-off-by: Antonio Vicente <avd@google.com> Signed-off-by: Yangmin Zhu <ymzhu@google.com>
zh-translation: docs/root/intro/arch_overview/operations/runtime.rst
Hello,
we're having an issue using the ext_authz filter where no body is being forwarded if the connection uses http/2.
It has been fixed upstream envoyproxy#8877 and this PR cherry-picked that commit for the istio/envoy fork.
When testing locally, using the container based build env, I do get a lot of test skips with the cherry-pick applied, but am not able to pin point the source. The ext_authz test results look fine and that's the only code this PR should change (+the integration test).
From the original PR: