From f18fcf3688b850f5e89530bb1bccf002fd592473 Mon Sep 17 00:00:00 2001 From: Kevin Pollet Date: Mon, 21 Oct 2024 10:10:04 +0200 Subject: [PATCH] Preserve GRPCRoute filters order --- .../grpcroute/filter_extension_ref.yml | 56 ++++ pkg/provider/kubernetes/gateway/grpcroute.go | 27 +- .../kubernetes/gateway/kubernetes_test.go | 291 ++++++++++++++++++ pkg/provider/kubernetes/k8s/parser.go | 2 +- 4 files changed, 366 insertions(+), 10 deletions(-) create mode 100644 pkg/provider/kubernetes/gateway/fixtures/grpcroute/filter_extension_ref.yml diff --git a/pkg/provider/kubernetes/gateway/fixtures/grpcroute/filter_extension_ref.yml b/pkg/provider/kubernetes/gateway/fixtures/grpcroute/filter_extension_ref.yml new file mode 100644 index 000000000..675374765 --- /dev/null +++ b/pkg/provider/kubernetes/gateway/fixtures/grpcroute/filter_extension_ref.yml @@ -0,0 +1,56 @@ +--- +kind: GatewayClass +apiVersion: gateway.networking.k8s.io/v1 +metadata: + name: my-gateway-class +spec: + controllerName: traefik.io/gateway-controller + +--- +kind: Gateway +apiVersion: gateway.networking.k8s.io/v1 +metadata: + name: my-gateway + namespace: default +spec: + gatewayClassName: my-gateway-class + listeners: # Use GatewayClass defaults for listener definition. + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + kinds: + - kind: GRPCRoute + group: gateway.networking.k8s.io + namespaces: + from: Same + +--- +kind: GRPCRoute +apiVersion: gateway.networking.k8s.io/v1 +metadata: + name: grpc-app-1 + namespace: default +spec: + parentRefs: + - name: my-gateway + kind: Gateway + group: gateway.networking.k8s.io + hostnames: + - foo.com + rules: + - backendRefs: + - name: whoami + port: 80 + weight: 1 + filters: + - type: ExtensionRef + extensionRef: + group: traefik.io + kind: Middleware + name: my-first-middleware + - type: ExtensionRef + extensionRef: + group: traefik.io + kind: Middleware + name: my-second-middleware \ No newline at end of file diff --git a/pkg/provider/kubernetes/gateway/grpcroute.go b/pkg/provider/kubernetes/gateway/grpcroute.go index 021a3909c..cb61428ec 100644 --- a/pkg/provider/kubernetes/gateway/grpcroute.go +++ b/pkg/provider/kubernetes/gateway/grpcroute.go @@ -278,20 +278,30 @@ func (p *Provider) loadGRPCBackendRef(route *gatev1.GRPCRoute, backendRef gatev1 } func (p *Provider) loadGRPCMiddlewares(conf *dynamic.Configuration, namespace, routerName string, filters []gatev1.GRPCRouteFilter) ([]string, error) { - middlewares := make(map[string]*dynamic.Middleware) + type namedMiddleware struct { + Name string + Config *dynamic.Middleware + } + + var middlewares []namedMiddleware for i, filter := range filters { name := fmt.Sprintf("%s-%s-%d", routerName, strings.ToLower(string(filter.Type)), i) switch filter.Type { case gatev1.GRPCRouteFilterRequestHeaderModifier: - middlewares[name] = createRequestHeaderModifier(filter.RequestHeaderModifier) + middlewares = append(middlewares, namedMiddleware{ + name, + createRequestHeaderModifier(filter.RequestHeaderModifier), + }) case gatev1.GRPCRouteFilterExtensionRef: name, middleware, err := p.loadHTTPRouteFilterExtensionRef(namespace, filter.ExtensionRef) if err != nil { return nil, fmt.Errorf("loading ExtensionRef filter %s: %w", filter.Type, err) } - - middlewares[name] = middleware + middlewares = append(middlewares, namedMiddleware{ + name, + middleware, + }) default: // As per the spec: https://gateway-api.sigs.k8s.io/api-types/httproute/#filters-optional @@ -303,12 +313,11 @@ func (p *Provider) loadGRPCMiddlewares(conf *dynamic.Configuration, namespace, r } var middlewareNames []string - for name, middleware := range middlewares { - if middleware != nil { - conf.HTTP.Middlewares[name] = middleware + for _, m := range middlewares { + if m.Config != nil { + conf.HTTP.Middlewares[m.Name] = m.Config } - - middlewareNames = append(middlewareNames, name) + middlewareNames = append(middlewareNames, m.Name) } return middlewareNames, nil diff --git a/pkg/provider/kubernetes/gateway/kubernetes_test.go b/pkg/provider/kubernetes/gateway/kubernetes_test.go index 985096362..e82ad3810 100644 --- a/pkg/provider/kubernetes/gateway/kubernetes_test.go +++ b/pkg/provider/kubernetes/gateway/kubernetes_test.go @@ -18,6 +18,7 @@ import ( "github.com/traefik/traefik/v3/pkg/provider/kubernetes/k8s" "github.com/traefik/traefik/v3/pkg/tls" "github.com/traefik/traefik/v3/pkg/types" + "google.golang.org/grpc/codes" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -3231,6 +3232,296 @@ func TestLoadHTTPRoutes_filterExtensionRef(t *testing.T) { } } +func TestLoadGRPCRoutes_filterExtensionRef(t *testing.T) { + testCases := []struct { + desc string + groupKindFilterFuncs map[string]map[string]BuildFilterFunc + expected *dynamic.Configuration + entryPoints map[string]Entrypoint + }{ + { + desc: "ExtensionRef filter", + groupKindFilterFuncs: map[string]map[string]BuildFilterFunc{ + traefikv1alpha1.GroupName: {"Middleware": func(name, namespace string) (string, *dynamic.Middleware, error) { + return namespace + "-" + name, nil, nil + }}, + }, + entryPoints: map[string]Entrypoint{"web": { + Address: ":80", + }}, + expected: &dynamic.Configuration{ + UDP: &dynamic.UDPConfiguration{ + Routers: map[string]*dynamic.UDPRouter{}, + Services: map[string]*dynamic.UDPService{}, + }, + TCP: &dynamic.TCPConfiguration{ + Routers: map[string]*dynamic.TCPRouter{}, + Middlewares: map[string]*dynamic.TCPMiddleware{}, + Services: map[string]*dynamic.TCPService{}, + ServersTransports: map[string]*dynamic.TCPServersTransport{}, + }, + HTTP: &dynamic.HTTPConfiguration{ + Routers: map[string]*dynamic.Router{ + "default-grpc-app-1-my-gateway-web-0-74471866db6e94e08d00": { + EntryPoints: []string{"web"}, + Service: "default-grpc-app-1-my-gateway-web-0-wrr", + Rule: "Host(`foo.com`) && PathPrefix(`/`)", + Priority: 22, + RuleSyntax: "v3", + Middlewares: []string{ + "default-my-first-middleware", + "default-my-second-middleware", + }, + }, + }, + Middlewares: map[string]*dynamic.Middleware{}, + Services: map[string]*dynamic.Service{ + "default-grpc-app-1-my-gateway-web-0-wrr": { + Weighted: &dynamic.WeightedRoundRobin{ + Services: []dynamic.WRRService{ + { + Name: "default-whoami-80", + Weight: ptr.To(1), + }, + }, + }, + }, + "default-whoami-80": { + LoadBalancer: &dynamic.ServersLoadBalancer{ + Servers: []dynamic.Server{ + { + URL: "h2c://10.10.0.1:80", + }, + { + URL: "h2c://10.10.0.2:80", + }, + }, + PassHostHeader: ptr.To(true), + ResponseForwarding: &dynamic.ResponseForwarding{ + FlushInterval: ptypes.Duration(100 * time.Millisecond), + }, + }, + }, + }, + ServersTransports: map[string]*dynamic.ServersTransport{}, + }, + TLS: &dynamic.TLSConfiguration{}, + }, + }, + { + desc: "ExtensionRef filter with middleware creation", + groupKindFilterFuncs: map[string]map[string]BuildFilterFunc{ + traefikv1alpha1.GroupName: {"Middleware": func(name, namespace string) (string, *dynamic.Middleware, error) { + return namespace + "-" + name, &dynamic.Middleware{Headers: &dynamic.Headers{CustomRequestHeaders: map[string]string{"Test-Header": "Test"}}}, nil + }}, + }, + entryPoints: map[string]Entrypoint{"web": { + Address: ":80", + }}, + expected: &dynamic.Configuration{ + UDP: &dynamic.UDPConfiguration{ + Routers: map[string]*dynamic.UDPRouter{}, + Services: map[string]*dynamic.UDPService{}, + }, + TCP: &dynamic.TCPConfiguration{ + Routers: map[string]*dynamic.TCPRouter{}, + Middlewares: map[string]*dynamic.TCPMiddleware{}, + Services: map[string]*dynamic.TCPService{}, + ServersTransports: map[string]*dynamic.TCPServersTransport{}, + }, + HTTP: &dynamic.HTTPConfiguration{ + Routers: map[string]*dynamic.Router{ + "default-grpc-app-1-my-gateway-web-0-74471866db6e94e08d00": { + EntryPoints: []string{"web"}, + Service: "default-grpc-app-1-my-gateway-web-0-wrr", + Rule: "Host(`foo.com`) && PathPrefix(`/`)", + Priority: 22, + RuleSyntax: "v3", + Middlewares: []string{ + "default-my-first-middleware", + "default-my-second-middleware", + }, + }, + }, + Middlewares: map[string]*dynamic.Middleware{ + "default-my-first-middleware": {Headers: &dynamic.Headers{CustomRequestHeaders: map[string]string{"Test-Header": "Test"}}}, + "default-my-second-middleware": {Headers: &dynamic.Headers{CustomRequestHeaders: map[string]string{"Test-Header": "Test"}}}, + }, + Services: map[string]*dynamic.Service{ + "default-grpc-app-1-my-gateway-web-0-wrr": { + Weighted: &dynamic.WeightedRoundRobin{ + Services: []dynamic.WRRService{ + { + Name: "default-whoami-80", + Weight: ptr.To(1), + }, + }, + }, + }, + "default-whoami-80": { + LoadBalancer: &dynamic.ServersLoadBalancer{ + Servers: []dynamic.Server{ + { + URL: "h2c://10.10.0.1:80", + }, + { + URL: "h2c://10.10.0.2:80", + }, + }, + PassHostHeader: ptr.To(true), + ResponseForwarding: &dynamic.ResponseForwarding{ + FlushInterval: ptypes.Duration(100 * time.Millisecond), + }, + }, + }, + }, + ServersTransports: map[string]*dynamic.ServersTransport{}, + }, + TLS: &dynamic.TLSConfiguration{}, + }, + }, + { + desc: "Unknown ExtensionRef filter", + entryPoints: map[string]Entrypoint{"web": { + Address: ":80", + }}, + expected: &dynamic.Configuration{ + UDP: &dynamic.UDPConfiguration{ + Routers: map[string]*dynamic.UDPRouter{}, + Services: map[string]*dynamic.UDPService{}, + }, + TCP: &dynamic.TCPConfiguration{ + Routers: map[string]*dynamic.TCPRouter{}, + Middlewares: map[string]*dynamic.TCPMiddleware{}, + Services: map[string]*dynamic.TCPService{}, + ServersTransports: map[string]*dynamic.TCPServersTransport{}, + }, + HTTP: &dynamic.HTTPConfiguration{ + Routers: map[string]*dynamic.Router{ + "default-grpc-app-1-my-gateway-web-0-74471866db6e94e08d00": { + EntryPoints: []string{"web"}, + Service: "default-grpc-app-1-my-gateway-web-0-74471866db6e94e08d00-err-wrr", + Rule: "Host(`foo.com`) && PathPrefix(`/`)", + Priority: 22, + RuleSyntax: "v3", + }, + }, + Middlewares: map[string]*dynamic.Middleware{}, + Services: map[string]*dynamic.Service{ + "default-grpc-app-1-my-gateway-web-0-74471866db6e94e08d00-err-wrr": { + Weighted: &dynamic.WeightedRoundRobin{ + Services: []dynamic.WRRService{ + { + Name: "invalid-grpcroute-filter", + Weight: ptr.To(1), + GRPCStatus: &dynamic.GRPCStatus{ + Code: codes.Unavailable, + Msg: "Service Unavailable", + }, + }, + }, + }, + }, + }, + ServersTransports: map[string]*dynamic.ServersTransport{}, + }, + TLS: &dynamic.TLSConfiguration{}, + }, + }, + { + desc: "ExtensionRef filter with filterFunc error", + groupKindFilterFuncs: map[string]map[string]BuildFilterFunc{ + traefikv1alpha1.GroupName: {"Middleware": func(name, namespace string) (string, *dynamic.Middleware, error) { + return "", nil, errors.New("BOOM") + }}, + }, + entryPoints: map[string]Entrypoint{"web": { + Address: ":80", + }}, + expected: &dynamic.Configuration{ + UDP: &dynamic.UDPConfiguration{ + Routers: map[string]*dynamic.UDPRouter{}, + Services: map[string]*dynamic.UDPService{}, + }, + TCP: &dynamic.TCPConfiguration{ + Routers: map[string]*dynamic.TCPRouter{}, + Middlewares: map[string]*dynamic.TCPMiddleware{}, + Services: map[string]*dynamic.TCPService{}, + ServersTransports: map[string]*dynamic.TCPServersTransport{}, + }, + HTTP: &dynamic.HTTPConfiguration{ + Routers: map[string]*dynamic.Router{ + "default-grpc-app-1-my-gateway-web-0-74471866db6e94e08d00": { + EntryPoints: []string{"web"}, + Service: "default-grpc-app-1-my-gateway-web-0-74471866db6e94e08d00-err-wrr", + Rule: "Host(`foo.com`) && PathPrefix(`/`)", + Priority: 22, + RuleSyntax: "v3", + }, + }, + Middlewares: map[string]*dynamic.Middleware{}, + Services: map[string]*dynamic.Service{ + "default-grpc-app-1-my-gateway-web-0-74471866db6e94e08d00-err-wrr": { + Weighted: &dynamic.WeightedRoundRobin{ + Services: []dynamic.WRRService{ + { + Name: "invalid-grpcroute-filter", + Weight: ptr.To(1), + GRPCStatus: &dynamic.GRPCStatus{ + Code: codes.Unavailable, + Msg: "Service Unavailable", + }, + }, + }, + }, + }, + }, + ServersTransports: map[string]*dynamic.ServersTransport{}, + }, + TLS: &dynamic.TLSConfiguration{}, + }, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + if test.expected == nil { + return + } + + k8sObjects, gwObjects := readResources(t, []string{"services.yml", "grpcroute/filter_extension_ref.yml"}) + + kubeClient := kubefake.NewSimpleClientset(k8sObjects...) + gwClient := newGatewaySimpleClientSet(t, gwObjects...) + + client := newClientImpl(kubeClient, gwClient) + + eventCh, err := client.WatchAll(nil, make(chan struct{})) + require.NoError(t, err) + + if len(k8sObjects) > 0 || len(gwObjects) > 0 { + // just wait for the first event + <-eventCh + } + + p := Provider{ + EntryPoints: test.entryPoints, + client: client, + } + + for group, kindFuncs := range test.groupKindFilterFuncs { + for kind, filterFunc := range kindFuncs { + p.RegisterFilterFuncs(group, kind, filterFunc) + } + } + conf := p.loadConfigurationFromGateways(context.Background()) + assert.Equal(t, test.expected, conf) + }) + } +} + func TestLoadTCPRoutes(t *testing.T) { testCases := []struct { desc string diff --git a/pkg/provider/kubernetes/k8s/parser.go b/pkg/provider/kubernetes/k8s/parser.go index 422513ade..053a912a1 100644 --- a/pkg/provider/kubernetes/k8s/parser.go +++ b/pkg/provider/kubernetes/k8s/parser.go @@ -12,7 +12,7 @@ import ( // MustParseYaml parses a YAML to objects. func MustParseYaml(content []byte) []runtime.Object { - acceptedK8sTypes := regexp.MustCompile(`^(Namespace|Deployment|EndpointSlice|Node|Service|ConfigMap|Ingress|IngressRoute|IngressRouteTCP|IngressRouteUDP|Middleware|MiddlewareTCP|Secret|TLSOption|TLSStore|TraefikService|IngressClass|ServersTransport|ServersTransportTCP|GatewayClass|Gateway|HTTPRoute|TCPRoute|TLSRoute|ReferenceGrant|BackendTLSPolicy)$`) + acceptedK8sTypes := regexp.MustCompile(`^(Namespace|Deployment|EndpointSlice|Node|Service|ConfigMap|Ingress|IngressRoute|IngressRouteTCP|IngressRouteUDP|Middleware|MiddlewareTCP|Secret|TLSOption|TLSStore|TraefikService|IngressClass|ServersTransport|ServersTransportTCP|GatewayClass|Gateway|GRPCRoute|HTTPRoute|TCPRoute|TLSRoute|ReferenceGrant|BackendTLSPolicy)$`) files := strings.Split(string(content), "---\n") retVal := make([]runtime.Object, 0, len(files))