Add filter state receive_before_connect to tcp_proxy #38189
Conversation
|
Hi @akshita31, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
|
Can we get this added please, believe this immensely important. |
|
@cccsss01 I am working on it and will try to get it merged as soon as possible, it is a really helpful feature for us as well. |
|
@akshita31 still waiting on comment #38189 (comment) /wait |
Add filter state object bool proxy_receive_before_connect that tcp_proxy filter checks for and when set to 'true' will not set readDisable on the downstream connection before upstream connection is established. This change allows advanced cases where a network read filter needs to receive (and possibly send) downstream data that may change the destination of the upstream TCP connection, e.g. via metadata set based on the received data. If this is the case, then the read filter preceding the tcp_proxy filter must return StopIteration from its onNewConnection() call, causing tcp_proxy filter to postpone its upstream connection establishment until onData() returns Continue. Any data reaching the tcp_proxy filter before the upstream connection is established is buffered so that the downstream filters do not see the same data again which would be the case if it would remaining in the buffer and more data is received. This also allows downstream filters inject data before the upstream connection is established; such injected data would be lost if tcp_proxy would not buffer it while connection establishment is still ongoing. An existing dynamic metadata integration test is modified to use proxy_receive_before_connect=true. This makes the use case less reliant on the tcp_proxy internal detail, such as balancing and timing of the readDisable() calls on the downstream connection. Signed-off-by: Jarno Rajahalme <[email protected]>
Signed-off-by: Jarno Rajahalme <[email protected]>
Signed-off-by: Jarno Rajahalme <[email protected]>
akshita31
force-pushed
the
akshita-tcp-proxy
branch
from
79041ff to
db0544b
Compare
Signed-off-by: Akshita Agarwal <[email protected]>
akshita31
force-pushed
the
akshita-tcp-proxy
branch
from
db0544b to
113d16c
Compare
Signed-off-by: Akshita Agarwal <[email protected]>
akshita31
force-pushed
the
akshita-tcp-proxy
branch
from
03ddb54 to
426f2d2
Compare
|
Please do not rebase or force-push your PR: Line 136 in 4a113b5 |
Signed-off-by: Akshita Agarwal <[email protected]>
Signed-off-by: Akshita Agarwal <[email protected]>
akshita31
force-pushed
the
akshita-tcp-proxy
branch
from
5c8339f to
dab0487
Compare
Please stop force-pushing to the PR! It makes it very difficult to review. /wait-any |
|
@ggreenway apology for the issue, I didn't remember the contributing guide well previously. But after your comment, the last 2 commits I pushed preserve the commit history. I hadn't signed the last 2 commit so I had to sign and force push as per the DCO instructions. Apology for the inconvenience, I will make sure to sign and not force push now. I just have last few feedback comments to resolve. Thanks a lot for your patience and understanding. |
Signed-off-by: Akshita Agarwal <[email protected]>
Signed-off-by: Akshita Agarwal <[email protected]>
Signed-off-by: Akshita Agarwal <[email protected]>
|
/wait |
…s received Signed-off-by: Akshita Agarwal <[email protected]>
Signed-off-by: Akshita Agarwal <[email protected]>
Signed-off-by: Akshita Agarwal <[email protected]>
Signed-off-by: Akshita Agarwal <[email protected]>
|
/wait |
Signed-off-by: Akshita Agarwal <[email protected]>
There was a problem hiding this comment.
This looks good overall. If you're getting useful information from the new metric, I'm fine leaving it in. It's also useful for tests, although if we wanted to remove it you could add a trace log in the code and have the test check for the log message.
You need to document the new metric at https://github.com/envoyproxy/envoy/blob/main/docs/root/configuration/listeners/network_filters/tcp_proxy_filter.rst?plain=1#L75 if you want to keep it.
/wait
Signed-off-by: Akshita Agarwal <[email protected]>
Signed-off-by: Akshita Agarwal <[email protected]>
…yproxy#38189) Signed-off-by: Akshita Agarwal <[email protected]> Co-authored-by: Jarno Rajahalme <[email protected]>
Commit Message:
This PR picks up the work done in #25804. It can help to fix #9023 by using the
receive_before_connectfilter state.receive_before_connectfilter state which can be read by TCP_PROXY and if set TCP_PROXY will not disable downstream reads until the upstream connection is established. This can hence be used by filters before TCP_PROXY to set the filter state ininitializeReadFilterCallbacks, and then StopIteration in onNewConnection and onData, until they have read the required amount of data before upstream connection establishment.Additional Description:
Risk Level: Low
Testing: Added integration and unit tests
Docs Changes: Done
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]