ext_proc: least surprise behavior for DEFAULT in mode_override. #38254
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
The mode_override feature has a very surprising behavior, in which unset fields in ProcessingMode, revert behavior back to the default. For example, it is the default behavior that resposne headers are sent, but if the filter is configured not to send response headers in processing_mode, and a mode_override is sent to explicitly send request body, this then inadvertently triggers the sending of response headers as well, as the mode has been overriden back to the "default". We have seen ext_proc users of mode_override being affected by these confusing semantics. It seems far more sensible that a mode_override only affects the procesisng mode features explicitly set in the mode_override message. Technically this could be a breaking wire change, but it seems unlikely that anyone is relying on the default override, as they would most likely just set response headers to SEND instead of DEFAULT if that was the intended behavior. This is a protocol change proposal, which will be augmented with implementation if there is agreement on this approach. Risk level: Medium Testing: TODO Signed-off-by: Harvey Tuch <htuch@google.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
This makes sense to me. |
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.
The mode_override feature has a very surprising behavior, in which unset fields in ProcessingMode, revert behavior back to the default. For example, it is the default behavior that resposne headers are sent, but if the filter is configured not to send response headers in processing_mode, and a mode_override is sent to explicitly send request body, this then inadvertently triggers the sending of response headers as well, as the mode has been overriden back to the "default". We have seen ext_proc users of mode_override being affected by these confusing semantics.
It seems far more sensible that a mode_override only affects the processing mode features explicitly set in the mode_override message. Technically this could be a breaking wire change, but it seems unlikely that anyone is relying on the default override, as they would most likely just set response headers to SEND instead of DEFAULT if that was the intended behavior.
This is a protocol change proposal, which will be augmented with implementation if there is agreement on this approach.
Risk level: Medium
Testing: TODO