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

Add Support for TCPRoute API #643

Closed
arkodg opened this issue Oct 21, 2022 · 10 comments · Fixed by #737
Closed

Add Support for TCPRoute API #643

arkodg opened this issue Oct 21, 2022 · 10 comments · Fixed by #737
Assignees
Labels
area/api API-related issues kind/enhancement New feature or request priority/medium Label used to express the "medium" priority level
Milestone

Comments

@arkodg
Copy link
Contributor

arkodg commented Oct 21, 2022

Description:
Add support for TCPRoute API defined here

Related #47

@arkodg arkodg added the kind/enhancement New feature or request label Oct 21, 2022
@arkodg
Copy link
Contributor Author

arkodg commented Oct 21, 2022

we should wait on #413 which will refactor the provider before starting this issue

@arkodg arkodg added this to the 0.3.0-rc.1 milestone Oct 21, 2022
@danehans
Copy link
Contributor

@arkodg this seems like a dupe of #47.

@arkodg
Copy link
Contributor Author

arkodg commented Oct 21, 2022

@danehans I broke up #47 into individual issues and linked it back to #47 (one for TCP and other for UDP)

@Xunzhuo
Copy link
Member

Xunzhuo commented Oct 23, 2022

I am assigning myself, but if this is required #413, I think the finished time cannot be measured correctly. Not sure if this can be done in v0.3.0-rc1, but let us see the progress.

@Xunzhuo Xunzhuo self-assigned this Oct 23, 2022
@arkodg
Copy link
Contributor Author

arkodg commented Oct 25, 2022

@Xunzhuo suggest following the order mentioned here #641 (comment)
The work in IR and Xds Translator is already done thanks to @chauhanshubham , and probably just needs a test case :)

@youngnick
Copy link
Contributor

Similarly to UDPRoute(#641), I think we need some design docs around the tradeoffs we'll need to make between transparent mode and proxy mode for TCP streams. The choice is basically between:

  • transparent mode: client address available, routing changes required somehow to avoid return path check failure in transit.
  • proxy mode: client address will appear to be the Envoy IP.

@arkodg
Copy link
Contributor Author

arkodg commented Oct 28, 2022

@youngnick we are already using the the tcp_proxy filter for TLS Passthrough which works well, and envoy only supports proxy mode for TCP (docs are here) .
Imho this should unblock the implementation for TCPRoute in the Provider and Gateway API Translator.
To highlight this design decision of only supporting proxy mode for TCP, I can update the decision here. Are there any other considerations/decisions we should make before continuing with this implementation ?

@danehans danehans added help wanted Extra attention is needed and removed help wanted Extra attention is needed labels Oct 31, 2022
@youngnick
Copy link
Contributor

I think it's fine as long as we call out the limitation. I wouldn't be surprised if people end up wanting us to put in something about PROXY mode eventually, but that seems fine to me.

arkodg added a commit to arkodg/gateway that referenced this issue Nov 1, 2022
@arkodg arkodg added the priority/medium Label used to express the "medium" priority level label Nov 3, 2022
@Xunzhuo Xunzhuo added the area/api API-related issues label Nov 5, 2022
@Xunzhuo
Copy link
Member

Xunzhuo commented Nov 17, 2022

I will start to work on this, other filters implementations are blocked for the release fo v0.6.0 GWAPI.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Dec 17, 2022
@Xunzhuo Xunzhuo removed the stale label Dec 17, 2022
arkodg pushed a commit that referenced this issue Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api API-related issues kind/enhancement New feature or request priority/medium Label used to express the "medium" priority level
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants