ext_proc: Support trailer callbacks #16102
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Commit Message: Support adding and modifying request and response trailers in the ext_proc filter. Additional Description: Existing trailers will be sent to the processing server if the processing mode is set to enable them. If the processing mode is set to sent trailers, but there are no trailers present, then empty trailers will be sent to the server for modification. However, trailers may only be added in the end of the data callback in Envoy, which may come in before a previous gRPC reply returns. Filters that need to be able to consistently add trailers where none existed should enable trailer processing in the Envoy filter configuration instead of relying on being able to turn it on dynamically. Risk Level: Low. Trailers only enabled if a service called by the filter is configured to ask for them. Testing: New integration and unit tests added. Docs Changes: API docs updated in .proto files. Release Notes: When the processing mode is changed to SEND for request or response trailers, a corresponding message will be sent to the server, which can respond with trailer mutations as desired. In addition, if trailer processing is enabled in the filter configuration, then trailer messages will be sent to the server even if trailers are not present. This makes it possible for the server to add trailers where none exist. Finally, at the moment Envoy only implements trailers for the HTTP/2 protocol. Nothing will happen if trailer processing is enabled and Envoy is using HTTP/1 until Envoy implements trailers for HTTP/1. Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
htuch
suggested changes
Apr 22, 2021
Simplify state handling with just a few booleans. Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
htuch
reviewed
Apr 26, 2021
| Http::FilterDataStatus Filter::onData(ProcessorState& state, Buffer::Instance& data, | ||
| bool end_stream) { | ||
| if (end_stream) { | ||
| state.setCompleteBodyDelivered(true); |
There was a problem hiding this comment.
Non-actionable comment but I'm interested in your take; how adversarial are we treating remote ext_proc servers as being? One use case that might be interesting for ext_proc is allowing service chaining where trust relationships between infrastructure providers and tenants imply a threat model that doesn't fully trust the ext_proc server. Any thoughts on what would be required to make this fully robust?
There was a problem hiding this comment.
Right now the relationship is pretty trusting -- ext_proc servers can set most headers, and they can manipulate the body and trailers too. There is an "ignore errors" setting which makes Envoy more resilient to a server failure, but without it a flaky server can definitely cause errors in the proxy as well.
I think that if we don't trust servers, then the user should certainly set up ext_proc to ignore errors from servers, and we might want to consider a model in which a server is limited by the proxy as to what it is allowed to change in the headers, trailers, and body.
Signed-off-by: Gregory Brail <gregbrail@google.com>
Based on code review. Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
gokulnair
pushed a commit
to gokulnair/envoy
that referenced
this pull request
May 6, 2021
Existing trailers will be sent to the processing server if the processing mode is set to enable them. If the processing mode is set to sent trailers, but there are no trailers present, then empty trailers will be sent to the server for modification. However, trailers may only be added in the end of the data callback in Envoy, which may come in before a previous gRPC reply returns. Filters that need to be able to consistently add trailers where none existed should enable trailer processing in the Envoy filter configuration instead of relying on being able to turn it on dynamically. Risk Level: Low. Trailers only enabled if a service called by the filter is configured to ask for them. Testing: New integration and unit tests added. Docs Changes: API docs updated in .proto files. Release Notes: When the processing mode is changed to SEND for request or response trailers, a corresponding message will be sent to the server, which can respond with trailer mutations as desired. In addition, if trailer processing is enabled in the filter configuration, then trailer messages will be sent to the server even if trailers are not present. This makes it possible for the server to add trailers where none exist. Finally, at the moment Envoy only implements trailers for the HTTP/2 protocol. Nothing will happen if trailer processing is enabled and Envoy is using HTTP/1 until Envoy implements trailers for HTTP/1. Signed-off-by: Gregory Brail <gregbrail@google.com> Signed-off-by: Gokul Nair <gnair@twitter.com>
gokulnair
pushed a commit
to gokulnair/envoy
that referenced
this pull request
May 6, 2021
Existing trailers will be sent to the processing server if the processing mode is set to enable them. If the processing mode is set to sent trailers, but there are no trailers present, then empty trailers will be sent to the server for modification. However, trailers may only be added in the end of the data callback in Envoy, which may come in before a previous gRPC reply returns. Filters that need to be able to consistently add trailers where none existed should enable trailer processing in the Envoy filter configuration instead of relying on being able to turn it on dynamically. Risk Level: Low. Trailers only enabled if a service called by the filter is configured to ask for them. Testing: New integration and unit tests added. Docs Changes: API docs updated in .proto files. Release Notes: When the processing mode is changed to SEND for request or response trailers, a corresponding message will be sent to the server, which can respond with trailer mutations as desired. In addition, if trailer processing is enabled in the filter configuration, then trailer messages will be sent to the server even if trailers are not present. This makes it possible for the server to add trailers where none exist. Finally, at the moment Envoy only implements trailers for the HTTP/2 protocol. Nothing will happen if trailer processing is enabled and Envoy is using HTTP/1 until Envoy implements trailers for HTTP/1. Signed-off-by: Gregory Brail <gregbrail@google.com> Signed-off-by: Gokul Nair <gnair@twitter.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit Message: Support adding and modifying request and response trailers
in the ext_proc filter.
Additional Description:
Existing trailers will be sent to the processing
server if the processing mode is set to enable them. If the
processing mode is set to sent trailers, but there are no trailers
present, then empty trailers will be sent to the server for
modification.
However, trailers may only be added in the end of the data callback
in Envoy, which may come in before a previous gRPC reply returns.
Filters that need to be able to consistently add trailers where none
existed should enable trailer processing in the Envoy filter
configuration instead of relying on being able to turn it on
dynamically.
Risk Level: Low. Trailers only enabled if a service called by the filter
is configured to ask for them.
Testing: New integration and unit tests added.
Docs Changes: API docs updated in .proto files.
Release Notes:
When the processing mode is changed to SEND
for request or response trailers, a corresponding message will be
sent to the server, which can respond with trailer mutations as desired.
In addition, if trailer processing is enabled in the filter
configuration, then trailer messages will be sent to the server
even if trailers are not present. This makes it possible for the server
to add trailers where none exist.
Finally, at the moment Envoy only implements trailers for the HTTP/2
protocol. Nothing will happen if trailer processing is enabled and
Envoy is using HTTP/1 until Envoy implements trailers for HTTP/1.
Signed-off-by: Gregory Brail gregbrail@google.com