Pass GRPCRouteWeight conformance test by codebot-robot · Pull Request #127 · gke-labs/gateway-api-reference-implementation

codebot-robot · 2026-03-10T06:43:38Z

This PR enhances the Gateway API implementation to support GRPCRoute and weighted backends, and adds GRPCRouteWeight to the conformance test suite.

Key changes:

Support for multiple weighted backends in InternalRule.
Implementation of weighted load balancing in the proxy.
GRPCRoute controller and state management.
Translation of GRPCRoute matches to path matches.
Added GRPCRouteWeight to tests/e2e/conformance_test.go.

This PR was generated by Overseer (powered by the gemini-3-flash-preview model).

Fixes #122

This change adds support for multiple backends with weights in InternalRule and implements weighted selection in the proxy. Issue gke-labs#122

This change adds support for GRPCRoute in the state and controller. Issue gke-labs#122

This change adds GRPCRouteWeight to the list of selected conformance tests. Issue gke-labs#122

justinsb · 2026-03-10T10:14:10Z

So this is not passing e2e @codebot-robot - can you fix?

codebot-robot · 2026-03-10T17:01:11Z

I have created a review task for this PR.

codebot-robot · 2026-03-10T17:01:13Z

I have created an investigate-failures task for this PR.

codebot-robot · 2026-03-10T17:01:15Z

I have created an address-feedback task for this PR.

codebot-robot

Thanks for the PR. While this correctly establishes the relationship between GRPCRoutes, weighted backends, and the data plane, there are critical bugs related to Gateway API spec conformance that must be addressed:

Weight Load Balancing: Returning the first backend when all weights are 0 explicitly violates the API requirement to drop the request with a 503 Service Unavailable.
gRPC Path Matching: Using a path prefix match for an EXACT gRPC method match is functionally incorrect and allows route leaking.
Namespace Validation: Gateway ParentRef checks are missing namespace validation in the status condition logic.
Controller Status: Updating the parent statuses replaces all other controllers' statuses instead of properly merging them.
Backend App Protocol: Backend connection protocols are incorrectly derived from the frontend listener rather than the Service configuration or BackendTLSPolicy.

Please review the inline comments for actionable fixes.

(This review was generated by Overseer)

codebot-robot · 2026-03-10T17:09:20Z

+	// Check if Gateway exists and has matching listeners
+	var gw *GatewayState
+	for _, g := range gateways {
+		if g.Name == string(parentRef.Name) {


Missing namespace validation for the Gateway parent reference. If parentRef.Namespace is provided, it must be verified against g.Namespace. Without this, the route may falsely report an Accepted condition for a Gateway in a different namespace that happens to share the same name.

codebot-robot · 2026-03-10T17:09:20Z

+	} else {
+		matched := false
+		for _, listener := range gw.Spec.Listeners {
+			if listener.Protocol != gatewayv1.HTTPProtocolType && listener.Protocol != gatewayv1.HTTPSProtocolType && listener.Protocol != "TLS" {


The string literal "TLS" is used instead of the correct constant gatewayv1.TLSProtocolType. Furthermore, per the Gateway API specification, GRPCRoute is only intended to bind to HTTP or HTTPS listeners natively.

codebot-robot · 2026-03-10T17:09:20Z

+	*gatewayv1.GRPCRoute
+}
+
+func (s *GRPCRouteState) Validate() error {


The Validate() method simply returns nil without performing any structural validation of the GRPCRoute. It should validate missing required fields, negative backend weights, and unsupported filter types to prevent downstream panics and ensure early rejection.

codebot-robot · 2026-03-10T17:09:20Z

+									path += method
+								}
+								iMatch.Path = &InternalPathMatch{
+									Type:  gatewayv1.PathMatchPathPrefix,


Using PathMatchPathPrefix for a gRPC method match is incorrect when the gRPC method match type is Exact (which is the default). A prefix match on /Service/Method would inadvertently match /Service/MethodX. This should use PathMatchExact.

codebot-robot · 2026-03-10T17:09:20Z

+							service := ValueOf(match.Method.Service)
+							method := ValueOf(match.Method.Method)
+							if service != "" || method != "" {
+								path := "/"


If the gRPC service is not specified, this string concatenation creates an invalid path like "//method". This will not properly match standard gRPC requests (which look like /<Service>/<Method>). If a service is omitted, the translation needs to use a regular expression or a specialized wildcard path matcher.

codebot-robot · 2026-03-10T17:09:21Z

+}
+
+func (r *GRPCRouteReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewControllerManagedBy(mgr).


The controller is only watching GRPCRoute objects. The Accepted status of a Route heavily depends on the presence and validity of the Gateway it references. The controller must also set up watches on Gateway objects so that GRPCRoutes are re-reconciled when a Gateway's listeners change.

codebot-robot · 2026-03-10T17:09:21Z

+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s.grpcRoutes[types.NamespacedName{Namespace: route.Namespace, Name: route.Name}] = rs


Storing the route pointer directly into the map from the reconciler's cache without performing a .DeepCopy() can lead to data races. If the controller cache updates the object or another routine mutates it concurrently, this shared memory will be modified unsafely.

codebot-robot · 2026-03-10T17:09:21Z

+		}
+	}
+
+	r.State.UpsertGRPCRoute(route)


r.State.UpsertGRPCRoute(route) is called here again, but it was already called on line 52. While harmless if intended to update the cached status, it might be cleaner to only upsert it once after the final status modifications are made.

codebot-robot · 2026-03-10T17:09:21Z

 	var proxyRoutes []state.InternalRoute
 	for _, gw := range gateways {
-		proxyRoutes = append(proxyRoutes, gw.BuildInternalRoutes(routes, services, backendTLSPolicies, configMaps, ControllerName)...)
+		proxyRoutes = append(proxyRoutes, gw.BuildInternalRoutes(httpRoutes, grpcRoutes, services, backendTLSPolicies, configMaps, ControllerName)...)


The BuildInternalRoutes parameter list is growing excessively long. Consider encapsulating httpRoutes, grpcRoutes, services, backendTLSPolicies, and configMaps into a state context or options struct for better maintainability.

codebot-robot · 2026-03-10T17:09:21Z

+								Name:      string(backendRef.Name),
+							}
+							var appProtocol *string
+							if svc, ok := services[backendSvcName]; ok {


If the backend Service does not exist in the services map, this logic silently proceeds and forwards traffic via DNS. Per Gateway API, if a referent Service is missing, it should properly reflect as an invalid backend, ideally contributing to a false ResolvedRefs status or yielding a 500 status in the data plane.

codebot-robot · 2026-03-10T17:10:58Z

I have submitted the review generated by Overseer. I've also created an address-feedback task for this PR to handle the new review comments.

codebot-robot · 2026-03-10T18:36:57Z

Overseer deferred investigate-failures task due to limit reached. Will retry later.

codebot-robot · 2026-03-10T19:21:01Z

Overseer has dispatched an investigate-failures task for this PR.

- Adds grpcroutes and grpcroutes/status to gari-controller ClusterRole - Runs ap generate to fix formatting in pkg/proxy/proxy.go

codebot-robot · 2026-03-10T19:28:16Z

--- INVESTIGATION REPORT ---
Run ID: 22890526631
Name: CI Presubmits
Cause: Code Error
Details: There were two failures related to the PR changes:

ap-verify-generate failed because pkg/proxy/proxy.go had a trailing space that needed formatting.
ap-e2e failed because the gari-controller lacked RBAC permissions (grpcroutes and grpcroutes/status) in k8s/controller.yaml. This prevented the controller from listing/watching GRPCRoute resources, failing the e2e test routes setup.
Action Taken: Fix applied. Formatted pkg/proxy/proxy.go using ap generate and added the necessary grpcroutes RBAC rules to k8s/controller.yaml. The fixes have been committed and pushed to the PR branch.

(This report was generated by Overseer)

codebot-robot · 2026-03-10T20:55:01Z

Thanks for the report! Acknowledged.\n\n*(This comment was generated by Overseer)*

codebot-robot added 3 commits March 10, 2026 06:43

Support weighted backends in proxy and state

77070bb

This change adds support for multiple backends with weights in InternalRule and implements weighted selection in the proxy. Issue gke-labs#122

Implement GRPCRoute support

af0728b

This change adds support for GRPCRoute in the state and controller. Issue gke-labs#122

Add GRPCRouteWeight to conformance tests

d4db8dd

This change adds GRPCRouteWeight to the list of selected conformance tests. Issue gke-labs#122

justinsb assigned codebot-robot Mar 10, 2026

codebot-robot removed their assignment Mar 10, 2026

codebot-robot commented Mar 10, 2026

View reviewed changes

Fix RBAC and formatting for GRPCRoute

34c8981

- Adds grpcroutes and grpcroutes/status to gari-controller ClusterRole - Runs ap generate to fix formatting in pkg/proxy/proxy.go

Navigation Menu

Pass GRPCRouteWeight conformance test - #127

Pass GRPCRouteWeight conformance test#127
codebot-robot wants to merge 4 commits into
gke-labs:mainfrom
codebot-robot:issue-122-2gty

codebot-robot commented Mar 10, 2026

justinsb commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot left a comment

codebot-robot Mar 10, 2026

codebot-robot Mar 10, 2026

codebot-robot Mar 10, 2026

codebot-robot Mar 10, 2026

codebot-robot Mar 10, 2026

codebot-robot Mar 10, 2026

codebot-robot Mar 10, 2026

codebot-robot Mar 10, 2026

codebot-robot Mar 10, 2026

codebot-robot Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

Labels

2 participants

Conversation

codebot-robot commented Mar 10, 2026

justinsb commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

codebot-robot commented Mar 10, 2026

Labels

2 participants