Copilot commented on code in PR #2570: URL: https://github.com/apache/apisix-ingress-controller/pull/2570#discussion_r2366583887
########## test/e2e/scaffold/grpc.go: ########## @@ -0,0 +1,154 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package scaffold + +import ( + "context" + _ "embed" + "fmt" + "strings" + "time" + + "github.com/apache/apisix-ingress-controller/test/e2e/framework" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + pb "sigs.k8s.io/gateway-api/conformance/echo-basic/grpcechoserver" +) + +type RequestMetadata struct { + // The :authority pseudoheader to set on the outgoing request. + Authority string + + // Outgoing metadata pairs to add to the request. + Metadata map[string]string +} + +type ExpectedResponse struct { + EchoRequest *pb.EchoRequest + EchoTwoRequest *pb.EchoRequest + EchoThreeRequest *pb.EchoRequest + + RequestMetadata *RequestMetadata + + Headers map[string]string + + EchoResponse EchoResponse +} + +type EchoResponse struct { + Code codes.Code + Headers *metadata.MD + Trailers *metadata.MD + Response *pb.EchoResponse +} + +func (s *Scaffold) DeployGRPCBackend() { + s.Framework.DeployGRPCBackend(framework.GRPCBackendOpts{ + KubectlOptions: s.kubectlOptions, + }) +} + +func (s *Scaffold) RequestEchoBackend(exp ExpectedResponse) error { + endpoint := s.apisixTunnels.HTTP.Endpoint() + + endpoint = strings.Replace(endpoint, "localhost", "127.0.0.1", 1) + + dialOpts := []grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())} + if exp.RequestMetadata != nil && exp.RequestMetadata.Authority != "" { + dialOpts = append(dialOpts, grpc.WithAuthority(exp.RequestMetadata.Authority)) + } + conn, err := grpc.NewClient(endpoint, dialOpts...) + if err != nil { + return err + } + defer func() { _ = conn.Close() }() + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + if exp.RequestMetadata != nil && len(exp.RequestMetadata.Metadata) > 0 { + ctx = metadata.NewOutgoingContext(ctx, metadata.New(exp.RequestMetadata.Metadata)) + } + + var ( + resp = &EchoResponse{ + Headers: &metadata.MD{}, + Trailers: &metadata.MD{}, + } + ) + + client := pb.NewGrpcEchoClient(conn) + switch { + case exp.EchoRequest != nil: + resp.Response, err = client.Echo(ctx, exp.EchoRequest, grpc.Header(resp.Headers), grpc.Trailer(resp.Trailers)) + case exp.EchoTwoRequest != nil: + resp.Response, err = client.EchoTwo(ctx, exp.EchoTwoRequest, grpc.Header(resp.Headers), grpc.Trailer(resp.Trailers)) + case exp.EchoThreeRequest != nil: + resp.Response, err = client.EchoThree(ctx, exp.EchoThreeRequest, grpc.Header(resp.Headers), grpc.Trailer(resp.Trailers)) + } + if err != nil { + resp.Code = status.Code(err) + fmt.Printf("RPC finished with error: %v\n", err) + } else { + resp.Code = codes.OK + } + if err := expectEchoResponses(&exp, resp); err != nil { + return err + } + return nil +} + +func expectEchoResponses(expected *ExpectedResponse, actual *EchoResponse) error { + if expected.EchoResponse.Code != actual.Code { + return fmt.Errorf("expected status code to be %s (%d), but got %s (%d)", + expected.EchoResponse.Code.String(), + expected.EchoResponse.Code, + actual.Code.String(), + actual.Code, + ) + } + if expected.EchoResponse.Headers != nil { + for key, values := range *expected.EchoResponse.Headers { + actualValues := actual.Headers.Get(key) + if len(values) != len(actualValues) { + return fmt.Errorf("expected header %q to have %d values, but got %d", key, len(values), len(actualValues)) + } + for i, v := range values { + if actualValues[i] != v { + return fmt.Errorf("expected header %q to have value %q, but got %q", key, v, actualValues[i]) + } + } + } + } + if len(expected.Headers) > 0 { + msgHeaders := actual.Response.GetAssertions().GetHeaders() + + kv := make(map[string]string) + for _, header := range msgHeaders { + kv[header.GetKey()] = header.GetValue() + } + for key, value := range expected.Headers { + if actualValue, ok := kv[strings.ToLower(key)]; !ok && value != "" { + return fmt.Errorf("expected header %q to be present, but not found", key) + } else if actualValue != value { Review Comment: The condition logic is incorrect. When `!ok && value != \"\"` is true, it means the key is missing but we expect a non-empty value, so we should return an error. However, the code continues to the else-if which will always be false since `ok` is false. ```suggestion actualValue, ok := kv[strings.ToLower(key)] if !ok { if value != "" { return fmt.Errorf("expected header %q to be present, but not found", key) } continue } if actualValue != value { ``` ########## internal/controller/grpcroute_controller.go: ########## @@ -0,0 +1,584 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package controller + +import ( + "cmp" + "context" + "fmt" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + discoveryv1 "k8s.io/api/discovery/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + k8stypes "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/apache/apisix-ingress-controller/api/v1alpha1" + "github.com/apache/apisix-ingress-controller/internal/controller/indexer" + "github.com/apache/apisix-ingress-controller/internal/controller/status" + "github.com/apache/apisix-ingress-controller/internal/manager/readiness" + "github.com/apache/apisix-ingress-controller/internal/provider" + "github.com/apache/apisix-ingress-controller/internal/types" + "github.com/apache/apisix-ingress-controller/internal/utils" +) + +// GRPCRouteReconciler reconciles a GatewayClass object. +type GRPCRouteReconciler struct { //nolint:revive + client.Client + Scheme *runtime.Scheme + + Log logr.Logger + + Provider provider.Provider + + genericEvent chan event.GenericEvent + + Updater status.Updater + Readier readiness.ReadinessManager +} + +// SetupWithManager sets up the controller with the Manager. +func (r *GRPCRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { + r.genericEvent = make(chan event.GenericEvent, 100) + + bdr := ctrl.NewControllerManagedBy(mgr). + For(&gatewayv1.GRPCRoute{}). + WithEventFilter(predicate.GenerationChangedPredicate{}). + Watches(&discoveryv1.EndpointSlice{}, + handler.EnqueueRequestsFromMapFunc(r.listGRPCRoutesByServiceBef), + ). + Watches(&v1alpha1.PluginConfig{}, + handler.EnqueueRequestsFromMapFunc(r.listGRPCRoutesByExtensionRef), + ). + Watches(&gatewayv1.Gateway{}, + handler.EnqueueRequestsFromMapFunc(r.listGRPCRoutesForGateway), + builder.WithPredicates( + predicate.Funcs{ + GenericFunc: func(e event.GenericEvent) bool { + return false + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return false + }, + CreateFunc: func(e event.CreateEvent) bool { + return true + }, + UpdateFunc: func(e event.UpdateEvent) bool { + return true + }, + }, + ), + ). + Watches(&v1alpha1.BackendTrafficPolicy{}, + handler.EnqueueRequestsFromMapFunc(r.listGRPCRoutesForBackendTrafficPolicy), + builder.WithPredicates( + BackendTrafficPolicyPredicateFunc(r.genericEvent), + ), + ). + Watches(&v1alpha1.GatewayProxy{}, + handler.EnqueueRequestsFromMapFunc(r.listGRPCRoutesForGatewayProxy), + ). + WatchesRawSource( + source.Channel( + r.genericEvent, + handler.EnqueueRequestsFromMapFunc(r.listGRPCRouteForGenericEvent), + ), + ) + + if GetEnableReferenceGrant() { + bdr.Watches(&v1beta1.ReferenceGrant{}, + handler.EnqueueRequestsFromMapFunc(r.listGRPCRoutesForReferenceGrant), + builder.WithPredicates(referenceGrantPredicates(KindGRPCRoute)), + ) + } + + return bdr.Complete(r) +} + +func (r *GRPCRouteReconciler) listGRPCRoutesByExtensionRef(ctx context.Context, obj client.Object) []reconcile.Request { + pluginconfig, ok := obj.(*v1alpha1.PluginConfig) + if !ok { + r.Log.Error(fmt.Errorf("unexpected object type"), "failed to convert object to EndpointSlice") Review Comment: The error message is misleading - it says 'failed to convert object to EndpointSlice' but the function is handling a PluginConfig object. ```suggestion r.Log.Error(fmt.Errorf("unexpected object type"), "failed to convert object to PluginConfig") ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
