Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Golang Http Tcp Bridge #36667

Open
wants to merge 73 commits into
base: main
Choose a base branch
from
Open

Feature: Golang Http Tcp Bridge #36667

wants to merge 73 commits into from

Conversation

duxin40
Copy link
Contributor

@duxin40 duxin40 commented Oct 17, 2024

As mentioned in the proposal: #35749 , this PR is to support using Golang to extend TCP upstream proxy, to make changes to connections and data messages in the http2tcp situation of envoy.
(this PR does)

Here is my thought about Golang extension function points:

  1. Support encoding message processing for upstream TCP requests (route and cluster have been determined, and targeted message processing can be performed for route and cluster)
  2. Support the handling of conn connection status during the encoding stage of upstream TCP requests (for example, by setting end_stream=false to avoid envoy semi connected status)
  3. Support decoding message processing and aggregation for upstream TCP response in onUpstreamData.(for example, by setting end_stream=true to indicate that the message is encapsulated and can be passed to downstream)
  4. Aggregate the tcp messages received multiple times by onUpstreamData of tcp response.
  5. Support obtaining route and cluster information, which can be referenced for targeted processing in the above stages.

What we can do with this Golang extension:

With this golang extension, developers can quickly get started with envoy and use golang to implement http2tcp such as http2dubbo、http2rpc.

Commit Message: support Golang TCP Upstream Extension for http2tcp on Envoy
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

duxin40 and others added 4 commits October 17, 2024 11:24
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #36667 was opened by duxin40.

see: more, trace.

@duxin40 duxin40 marked this pull request as ready for review October 17, 2024 08:12
duxin40 added 2 commits October 17, 2024 16:20
Signed-off-by: duxin40 <[email protected]>
Signed-off-by: duxin40 <[email protected]>
@duxin40
Copy link
Contributor Author

duxin40 commented Oct 17, 2024

I will add unit tests for these codes and fix errors related to unit tests.

@duxin40
Copy link
Contributor Author

duxin40 commented Oct 17, 2024

@doujiang24 could you take a look and check if the direction looks good? thanks!

@tyxia
Copy link
Member

tyxia commented Oct 18, 2024

/assign @doujiang24

Per discussion in #35749

Copy link

@duxin40 cannot be assigned to this issue.

🐱

Caused by: a #36667 (comment) was created by @tyxia.

see: more, trace.

Copy link
Member

@doujiang24 doujiang24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better add a simple example that use Golang to implement http => TCP protocol, to show how it could be for users for easier understanding.
It do not need to be runable, just a demo.

source/common/router/router.cc Outdated Show resolved Hide resolved
@duxin40
Copy link
Contributor Author

duxin40 commented Oct 21, 2024

Better add a simple example that use Golang to implement http => TCP protocol, to show how it could be for users for easier understanding. It do not need to be runable, just a demo.

Done. in the dir: contrib/upstreams/http/tcp/test_data/http2dubbo-examples
image

duxin40 added 3 commits October 21, 2024 21:05
Signed-off-by: duxin40 <[email protected]>
Signed-off-by: duxin40 <[email protected]>
@mattklein123 mattklein123 self-assigned this Oct 21, 2024
}

void TcpUpstream::encodeTrailers(const Envoy::Http::RequestTrailerMap&) {
Buffer::OwnedImpl data;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto, we'd better golang convert trailers to data buffer, but we can just add a TODO for it now, it's not a common use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, will finish it in the near future.

…om response header in envoyGoOnUpstreamData

Signed-off-by: duxin40 <[email protected]>
duxin40 added 3 commits January 13, 2025 14:00
Signed-off-by: duxin40 <[email protected]>
Signed-off-by: duxin40 <[email protected]>
Signed-off-by: duxin40 <[email protected]>
@doujiang24
Copy link
Member

/docs

Copy link

Docs for this Pull Request will be rendered here:

https://storage.googleapis.com/envoy-pr/36667/docs/index.html

The docs are (re-)rendered each time the CI envoy-presubmit (precheck docs) job completes.

🐱

Caused by: a #36667 (comment) was created by @doujiang24.

see: more, trace.

Copy link
Member

@doujiang24 doujiang24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@duxin40 Thank you so much for your hard work and patience.

Please run the tests in the asan mode, in your local dev, here is the command from my side:

ENVOY_DOCKER_BUILD_DIR=/path/to/envoy-bazel-cache \
GOPROXY='https://goproxy.cn' \
    bash -x ci/run_envoy_docker.sh 'bash -x ci/do_ci.sh asan'

It's so good enough to merge from my side.

@mattklein123 @wbpcode could you please take another look at this PR.
It is a bit large, but it's also a very important feature to extend HTTP <-> RPC.

docs/root/configuration/http/tcp_bridge/golang.rst Outdated Show resolved Hide resolved
Signed-off-by: duxin40 <[email protected]>
Copy link

Docs for this Pull Request will be rendered here:

https://storage.googleapis.com/envoy-pr/36667/docs/index.html

The docs are (re-)rendered each time the CI envoy-presubmit (precheck docs) job completes.

🐱

Caused by: a #36667 (comment) was created by @duxin40.

see: more, trace.

duxin40 added 2 commits January 19, 2025 21:21
Signed-off-by: duxin40 <[email protected]>
Signed-off-by: duxin40 <[email protected]>
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @mattklein123
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #36667 was synchronize by duxin40.

see: more, trace.

Signed-off-by: duxin40 <[email protected]>
Signed-off-by: duxin40 <[email protected]>
Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for working on this @duxin40

@@ -179,3 +179,73 @@ updates:
schedule:
interval: daily
time: "06:00"

- package-ecosystem: "gomod"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it this still required? afaict this uses the same deps that have been merged, surely we can use an existing go.mod?

docs/root/api-v3/config/contrib/http_tcp_bridge/golang.rst Outdated Show resolved Hide resolved
docs/root/configuration/http/tcp_bridge/golang.rst Outdated Show resolved Hide resolved
docs/root/configuration/http/tcp_bridge/golang.rst Outdated Show resolved Hide resolved
docs/root/configuration/http/tcp_bridge/golang.rst Outdated Show resolved Hide resolved
docs/root/configuration/http/tcp_bridge/golang.rst Outdated Show resolved Hide resolved
docs/root/configuration/http/tcp_bridge/golang.rst Outdated Show resolved Hide resolved
duxin40 and others added 3 commits January 21, 2025 22:25
Signed-off-by: duxin40 <[email protected]>
Signed-off-by: duxin40 <[email protected]>
@duxin40
Copy link
Contributor Author

duxin40 commented Jan 22, 2025

/retest

@mattklein123
Copy link
Member

I don't have time to review this giant PR. Will merge once @doujiang24 and @phlax give the ok.

/wait-any

Signed-off-by: duxin40 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants