ext_proc: Support CONTINUE_AND_REPLACE from header callbacks#16437
ext_proc: Support CONTINUE_AND_REPLACE from header callbacks#16437snowp merged 3 commits intoenvoyproxy:mainfrom
Conversation
Commit Message: Support the CONTINUE_AND_REPLACE status for reponses from header callbacks Additional Description: This makes it possible for a processor to add a body to a request or response that does not have one, or replace the entire body in the response from a header callback without otherwise touching it. Risk Level: Low. Not enabled by default. Testing: New unit and integration tests. Release Notes: The CONTINUE_AND_REPLACE processing status can be used to replace the body or create a new body from inside the response to a header callback. Signed-off-by: Gregory Brail <gregbrail@google.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
/assign-from @envoyproxy/first-pass-reviewers |
|
@envoyproxy/first-pass-reviewers assignee is @antoniovicente |
|
@antoniovicente I think it makes sense for @snowp to take this one e2e. |
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
Thanks for working on this.
A minor API comment.
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
snowp
left a comment
snowp
left a comment
There was a problem hiding this comment.
Thanks this looks good to me, just a few comments that apply to old code so not really blockung less you want to fix them with this PR, lmk.
| @@ -14,8 +14,8 @@ namespace ExternalProcessing { | |||
| class MutationUtils : public Logger::Loggable<Logger::Id::filter> { | |||
| public: | |||
| // Convert a header map until a protobuf | |||
There was a problem hiding this comment.
If we change the other thing here, then I'll certainly do it -- if not, then I'd like to do it later.
There was a problem hiding this comment.
i believe it was a suggestion on fixing a typo
| // Convert a header map until a protobuf | |
| // Convert a header map into a protobuf |
| static void headersToProto(const Http::HeaderMap& headers_in, | ||
| envoy::config::core::v3::HeaderMap& proto_out); |
There was a problem hiding this comment.
Old code but I think unless there is a good reason this should be returning the HeaderMap instead of taking an out param?
There was a problem hiding this comment.
Interesting question, actually. The only places we call this method we're using it to set the fields of an embedded message field in protobuf. So I think that if we change it, then the code that calls this function has to first get a pointer to an embedded message by calling "mutable_whatever" on the field, and then needs to copy the result of this function into the result. Doing it this way saves the copy which seems like a good thing.
There was a problem hiding this comment.
Alright that sounds like a good enough reason for me :)
|
Needs @envoyproxy/api-shepherds review then this should be good to go |
|
/lgtm api |
…oxy#16437) This makes it possible for a processor to add a body to a request or response that does not have one, or replace the entire body in the response from a header callback without otherwise touching it. Signed-off-by: Gregory Brail <gregbrail@google.com>
6 participants
Commit Message: Support the CONTINUE_AND_REPLACE status
for reponses from header callbacks
Additional Description: This makes it possible for a processor
to add a body to a request or response that does not have one,
or replace the entire body in the response from a header callback
without otherwise touching it.
Risk Level: Low. Not enabled by default.
Testing: New unit and integration tests.
Release Notes: The CONTINUE_AND_REPLACE processing status can be
used to replace the body or create a new body from inside the response
to a header callback.
Signed-off-by: Gregory Brail gregbrail@google.com