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.

Sign up for GitHub

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

Jump to bottom

feat: support expression route for TCPRoute. #4385

Merged
merged 9 commits into from
Aug 21, 2023
Merged

feat: support expression route for TCPRoute. #4385

merged 9 commits into from
Aug 21, 2023

Conversation

rodman10
Copy link
Contributor

@rodman10 rodman10 commented Jul 21, 2023 •
edited

What this PR does / why we need it:

Which issue this PR fixes:

Fixes #4312

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@rodman10 rodman10 changed the title feat： support expression route for 拉 feat: support expression route for l4 protocol. Jul 21, 2023
@randmonkey
Copy link
Contributor

@rodman10 Thanks for contributing. Currently Kong gateway does not support expression router in L4 proxy, and the interface is not totally decided yet. So please do not take too much time on this issue for now. Also, supporting ALL resources in one PR would make the PR too large, so please do not make too large PRs.

@rodman10
Copy link
Contributor Author

@randmonkey Thanks, I just tried to support expression tcproute basically according to Kong/kong#11071 for learning, the remain things haven't begun yet.

@randmonkey
Copy link
Contributor

randmonkey commented Jul 26, 2023 •
edited

@rodman10 now Kong/kong#11071 has been merged. You can continue your development based on current master branch of Kong/kong if you are willing to continue this PR. The feature will be included in Kong 3.4, which is expected to release recently.

@rodman10
Copy link
Contributor Author

Which KIC version is expected to contain this feature, and what is the corresponding milestone timepoint? @randmonkey

@randmonkey
Copy link
Contributor

Which KIC version is expected to contain this feature, and what is the corresponding milestone timepoint? @randmonkey

I think it would not be included in 2.11 (the next release, will be released recently). Likely to be in 2.12, which is expected to release about several weeks later than 2.11. We do not have a very precise time for releasing 2.12 yet.

@rodman10
Copy link
Contributor Author

Time seems ok, I'd like to work on it. Thanks!

@rodman10 rodman10 force-pushed the l4_expression branch 2 times, most recently from 1dcebd1 to 6af4d88 Compare July 31, 2023 04:28
@rodman10 rodman10 force-pushed the l4_expression branch 2 times, most recently from b0a0164 to 8d15c3f Compare August 14, 2023 03:28
randmonkey
randmonkey reviewed Aug 16, 2023
View reviewed changes
Copy link
Contributor

@randmonkey randmonkey left a comment

Choose a reason for hiding this comment

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

Looks good, only some minor things and CHANGELOG entry remaining,

internal/dataplane/parser/translators/l4route_atc.go Outdated Show resolved Hide resolved
internal/dataplane/parser/translators/l4route_atc.go Outdated Show resolved Hide resolved
internal/dataplane/parser/translate_failures_test.go Outdated Show resolved Hide resolved
@rodman10 rodman10 force-pushed the l4_expression branch 2 times, most recently from 40f2a90 to 7a4299e Compare August 20, 2023 13:55
@rodman10 rodman10 marked this pull request as ready for review August 20, 2023 13:55
@rodman10 rodman10 requested a review from a team as a code owner August 20, 2023 13:55
@rodman10 rodman10 changed the title feat: support expression route for l4 protocol. feat: support expression route for TCPRoute. Aug 20, 2023
randmonkey
randmonkey approved these changes Aug 21, 2023
View reviewed changes
Copy link
Contributor

@randmonkey randmonkey left a comment

Choose a reason for hiding this comment

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

LGTM

@randmonkey
Copy link
Contributor

@rodman10 If you want to contribute to support other objects (UDPRoute,TLSRoute,TCPIngress,UDPIngress), please assign one or more of the issues to yourself:
#4539 for UDPRoute
#4540 for TCPRoute
#4541 for TCPIngress
#4542 for UDPIngress
Please do not take more than 2 weeks on them. Thank you!

@randmonkey randmonkey merged commit af6c2fd into Kong:main Aug 21, 2023
17 checks passed
@rodman10
Copy link
Contributor Author

@rodman10 If you want to contribute to support other objects (UDPRoute,TLSRoute,TCPIngress,UDPIngress), please assign one or more of the issues to yourself: #4539 for UDPRoute #4540 for TCPRoute #4541 for TCPIngress #4542 for UDPIngress Please do not take more than 2 weeks on them. Thank you!

Ok, I will try to finish #4540 for TLSRoute and #4542 for UDPIngress first.

@randmonkey
Copy link
Contributor

OK. I assigned #4540 to you, when you finish the PR for supporting TLSRoute, we can continue on next steps.

@rainest rainest mentioned this pull request Sep 5, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@randmonkey randmonkey randmonkey approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support L4 proxy CRDs in ExpressionRoutes mode
2 participants
@rodman10 @randmonkey