Add gprcroute support to inbound policy API #12785
Merged
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
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
mateiidavid
approved these changes
Jul 2, 2024
There was a problem hiding this comment.
Looks good and pretty straight to the point. I haven't tested this, fwiw. I like how nicely this slots in with the existing logic we have in the index and it seems like most of the logic is non contentious (wiring up another resource).
I did have a question about gRPC probes but I figured that'd be a non-goal for this change anyway. Besides, gRPC probes are a k8s 1.27 feature 🤷🏻 it'd probably be trivial to add it in later when the time comes.
Comment on lines
+1851
to
+1852
| .filter(|(_, route)| route.selects_server(server_name)) | ||
| .filter(|(_, route)| route.accepted_by_server(server_name)) |
There was a problem hiding this comment.
would it make sense to collapse these two in the same line?
.filter(|(_, route)| route.selects_server(server_name) && route.accepted_by_server(server_name))
olix0r
approved these changes
Jul 3, 2024
Comment on lines
383
to
388
| fn reset_grpc_route( | ||
| &mut self, | ||
| routes: Vec<k8s_gateway_api::GrpcRoute>, | ||
| deleted: HashMap<String, HashSet<String>>, | ||
| ) { | ||
| let _span = info_span!("gprcroute reset").entered(); |
There was a problem hiding this comment.
tioli
Suggested change
| fn reset_grpc_route( | |
| &mut self, | |
| routes: Vec<k8s_gateway_api::GrpcRoute>, | |
| deleted: HashMap<String, HashSet<String>>, | |
| ) { | |
| let _span = info_span!("gprcroute reset").entered(); | |
| #[tracing::instrument(skip_all)] | |
| fn reset_grpc_route( | |
| &mut self, | |
| routes: Vec<k8s_gateway_api::GrpcRoute>, | |
| deleted: HashMap<String, HashSet<String>>, | |
| ) { |
Signed-off-by: Alex Leong <alex@buoyant.io>
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.
We add support for grpcroute in the inbound policy API. When a Server resource has the proxy protocol set to grpc, we will now serve grpc as the protocol in the inbound policy API along with any GrpcRoutes which have been defined and attached to the Server. If grpc is specified as the proxy protocol but no GrpcRoutes are attached, we return a default catch-all grpc route.