ext_proc: Add support for processing_mode and processing mode override#14822
ext_proc: Add support for processing_mode and processing mode override#14822htuch merged 7 commits intoenvoyproxy:mainfrom
Conversation
This lets the configuration control which calls are made to the external processor, and also lets the processor temporarily change the mode for a single request. Signed-off-by: Gregory Brail <gregbrail@google.com>
|
/lgtm api |
htuch
left a comment
There was a problem hiding this comment.
LGTM. I'm thinking it might be good to get a non-Google reviewer for this series of reviewers for two reasons:
- Increase pool of ext_proc reviewers.
- Follow policy of having cross-company review. Since the original design/protos were already reviewed by Lyft (@mattklein123) I'm not super worried about accidentally over-fitting to Google, but it would be nice to have.
@envoyproxy/maintainers any interested folks?
Signed-off-by: Gregory Brail <gregbrail@google.com>
|
To Harvey's point, next PR will add body handling, so we'll start getting into the nuances of the filter chain and the Buffer classes so experienced reviewers will be essential. |
|
@snowp has kindly volunteered (thanks a bunch). |
|
.. and @lizan. Exciting to have more review coverage on this key piece of the Envoy extensibility story. |
snowp
left a comment
There was a problem hiding this comment.
Thanks this all looks sensible to me, just a few minor comments
| ProcessingResponse resp2; | ||
| auto* headers2 = resp2.mutable_response_headers(); | ||
| auto* response_mutation = headers2->mutable_response()->mutable_header_mutation(); | ||
| auto* add1 = response_mutation->add_set_headers(); |
There was a problem hiding this comment.
I think in general we advocate for a bit more descriptive variable names, but this being test code I'm not too bothered
There was a problem hiding this comment.
I'll keep that in mind as I write more tests.
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
|
It looks like maybe the tests are stuck. Can someone kick them? |
|
@gbrail can you merge master and push? Thanks. |
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Commit Message: ext_proc: Add support for processing_mode and processing mode override. In this PR, only supported for request_headers and response_headers.
Additional Description:
Risk Level: Low
Testing: unit test and integration tests, with new tests added to this PR to support the feature
Docs Changes: Removed "not-implemented-hide" from the "processing_mode" configuration parameter