From b937894fcdc15f2af39c20dd0caa358f98a591a1 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Mon, 25 Nov 2024 15:10:25 -0800 Subject: [PATCH] fix route reports for custom backends (#10361) Co-authored-by: Omar Hammami <58956785+puertomontt@users.noreply.github.com> --- .../v1.18.0-rc3/custom-be-route-report.yaml | 6 +++ .../translator/gateway_translator_test.go | 32 ++++++++++---- .../gateway_http_route_translator.go | 44 ++++++++++++------- .../inputs/backend-plugin/gateway.yaml | 39 ++++++++++++++++ .../outputs/backend-plugin-proxy.yaml | 31 +++++++++++++ .../translator/testutils/test_queries.go | 4 +- .../translator/translator_case_test.go | 38 +++++++++++++++- 7 files changed, 166 insertions(+), 28 deletions(-) create mode 100644 changelog/v1.18.0-rc3/custom-be-route-report.yaml create mode 100644 projects/gateway2/translator/testutils/inputs/backend-plugin/gateway.yaml create mode 100644 projects/gateway2/translator/testutils/outputs/backend-plugin-proxy.yaml diff --git a/changelog/v1.18.0-rc3/custom-be-route-report.yaml b/changelog/v1.18.0-rc3/custom-be-route-report.yaml new file mode 100644 index 00000000000..1b2caea6b63 --- /dev/null +++ b/changelog/v1.18.0-rc3/custom-be-route-report.yaml @@ -0,0 +1,6 @@ +changelog: + - type: NON_USER_FACING + description: >- + Fix route report issues when we write multiple values + for a status, or when we're reporting for a custom backend + type. diff --git a/projects/gateway2/translator/gateway_translator_test.go b/projects/gateway2/translator/gateway_translator_test.go index e54b53c643a..c6cacc14b09 100644 --- a/projects/gateway2/translator/gateway_translator_test.go +++ b/projects/gateway2/translator/gateway_translator_test.go @@ -51,7 +51,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests", gwNN: types.NamespacedName{ Namespace: "default", Name: "example-gateway", - }}), + }, + }), Entry( "https gateway with basic routing", translatorTestCase{ @@ -60,7 +61,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests", gwNN: types.NamespacedName{ Namespace: "default", Name: "example-gateway", - }}), + }, + }), Entry( "http gateway with multiple listeners on the same port", translatorTestCase{ @@ -69,7 +71,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests", gwNN: types.NamespacedName{ Namespace: "default", Name: "http", - }}), + }, + }), Entry( "https gateway with multiple listeners on the same port", translatorTestCase{ @@ -78,7 +81,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests", gwNN: types.NamespacedName{ Namespace: "default", Name: "http", - }}), + }, + }), Entry( "http gateway with multiple routing rules and HeaderModifier filter", translatorTestCase{ @@ -87,7 +91,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests", gwNN: types.NamespacedName{ Namespace: "default", Name: "gw", - }}), + }, + }), Entry( "http gateway with lambda destination", translatorTestCase{ @@ -96,7 +101,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests", gwNN: types.NamespacedName{ Namespace: "default", Name: "gw", - }}), + }, + }), Entry( "http gateway with azure destination", translatorTestCase{ @@ -105,7 +111,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests", gwNN: types.NamespacedName{ Namespace: "default", Name: "gw", - }}), + }, + }), Entry( "gateway with correctly sorted routes", translatorTestCase{ @@ -114,7 +121,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests", gwNN: types.NamespacedName{ Namespace: "infra", Name: "example-gateway", - }}), + }, + }), Entry( "httproute with missing backend reports correctly", translatorTestCase{ @@ -258,6 +266,14 @@ var _ = DescribeTable("Basic GatewayTranslator Tests", Name: "example-tcp-gateway", }, }), + Entry("Plugin Backend", translatorTestCase{ + inputFile: "backend-plugin/gateway.yaml", + outputFile: "backend-plugin-proxy.yaml", + gwNN: types.NamespacedName{ + Namespace: "default", + Name: "example-gateway", + }, + }), ) var _ = DescribeTable("Route Delegation translator", diff --git a/projects/gateway2/translator/httproute/gateway_http_route_translator.go b/projects/gateway2/translator/httproute/gateway_http_route_translator.go index a2323dafd45..cb38e2a4e68 100644 --- a/projects/gateway2/translator/httproute/gateway_http_route_translator.go +++ b/projects/gateway2/translator/httproute/gateway_http_route_translator.go @@ -16,6 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" + "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/solo-io/gloo/projects/gateway2/query" @@ -353,13 +354,6 @@ func setRouteAction( clusterName := "blackhole_cluster" ns := "blackhole_ns" - obj, err := gwroute.GetBackendForRef(backendRef.BackendObjectReference) - ptrClusterName := query.ProcessBackendRef(obj, err, reporter, backendRef.BackendObjectReference) - if ptrClusterName != nil { - clusterName = *ptrClusterName - ns = obj.GetNamespace() - } - var weight *wrappers.UInt32Value if backendRef.Weight != nil { weight = &wrappers.UInt32Value{ @@ -372,20 +366,25 @@ func setRouteAction( } } - fromPlugin := false - for _, bp := range pluginRegistry.GetBackendPlugins() { - if dest, ok := bp.ApplyBackendPlugin(obj, backendRef.BackendObjectReference); ok { - fromPlugin = true + obj, err := gwroute.GetBackendForRef(backendRef.BackendObjectReference) + if err == nil { + // Only apply backend plugin when the backend is resolved. + // If any backend plugin matches this ref, we don't need the standard + // reports or validation path. + if dest, ok := applyBackendPlugins(obj, backendRef.BackendObjectReference, pluginRegistry); ok { weightedDestinations = append(weightedDestinations, &v1.WeightedDestination{ Destination: dest, Weight: weight, }) - break + continue } } - // TODO break out a buildDestination func to avoid this awkwardness - if fromPlugin { - continue + + // only call ProcessBackendRef when the plugin didn't handle it + ptrClusterName := query.ProcessBackendRef(obj, err, reporter, backendRef.BackendObjectReference) + if ptrClusterName != nil { + clusterName = *ptrClusterName + ns = obj.GetNamespace() } var port uint32 @@ -445,7 +444,7 @@ func setRouteAction( }) default: - contextutils.LoggerFrom(ctx).Errorf("unsupported backend type for kind: %v and type: %v", *backendRef.BackendObjectReference.Kind, *backendRef.BackendObjectReference.Group) + contextutils.LoggerFrom(ctx).Errorf("unsupported backend type for kind: %v and type: %v", backendRef.BackendObjectReference.Kind, backendRef.BackendObjectReference.Group) } } @@ -520,3 +519,16 @@ func makeDestinationSpec(upstream *gloov1.Upstream, filters []gwv1.HTTPRouteFilt } return nil, nil } + +func applyBackendPlugins( + obj client.Object, + backendRef gwv1.BackendObjectReference, + plugins registry.PluginRegistry, +) (*v1.Destination, bool) { + for _, bp := range plugins.GetBackendPlugins() { + if dest, ok := bp.ApplyBackendPlugin(obj, backendRef); ok { + return dest, true + } + } + return nil, false +} diff --git a/projects/gateway2/translator/testutils/inputs/backend-plugin/gateway.yaml b/projects/gateway2/translator/testutils/inputs/backend-plugin/gateway.yaml new file mode 100644 index 00000000000..08806aeda37 --- /dev/null +++ b/projects/gateway2/translator/testutils/inputs/backend-plugin/gateway.yaml @@ -0,0 +1,39 @@ +#$ Used in: +#$ - site-src/guides/http-routing.md +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: example-gateway +spec: + gatewayClassName: example-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: example-route +spec: + parentRefs: + - name: example-gateway + hostnames: + - "example.com" + rules: + - backendRefs: + - name: example-svc + kind: test-backend-plugin + port: 80 +--- +apiVersion: v1 +kind: Service +metadata: + name: example-svc +spec: + selector: + test: test + ports: + - protocol: TCP + port: 80 + targetPort: test diff --git a/projects/gateway2/translator/testutils/outputs/backend-plugin-proxy.yaml b/projects/gateway2/translator/testutils/outputs/backend-plugin-proxy.yaml new file mode 100644 index 00000000000..b367e7c1bcf --- /dev/null +++ b/projects/gateway2/translator/testutils/outputs/backend-plugin-proxy.yaml @@ -0,0 +1,31 @@ +--- +listeners: +- aggregateListener: + httpFilterChains: + - matcher: {} + virtualHostRefs: + - http~example_com + httpResources: + virtualHosts: + http~example_com: + domains: + - example.com + name: http~example_com + routes: + - matchers: + - prefix: / + name: httproute-example-route-default-0-0 + options: {} + routeAction: + single: + upstream: + name: test-backend-plugin-us + bindAddress: '::' + bindPort: 8080 + name: http +metadata: + labels: + created_by: gloo-kube-gateway-api + gateway_namespace: default + name: default-example-gateway + namespace: gloo-system diff --git a/projects/gateway2/translator/testutils/test_queries.go b/projects/gateway2/translator/testutils/test_queries.go index 1e495fc1989..de94e7d7cbe 100644 --- a/projects/gateway2/translator/testutils/test_queries.go +++ b/projects/gateway2/translator/testutils/test_queries.go @@ -21,8 +21,8 @@ func BuildIndexedFakeClient(objs []client.Object, funcs ...IndexIteratorFunc) cl return builder.WithObjects(objs...).Build() } -func BuildGatewayQueriesWithClient(fakeClient client.Client) query.GatewayQueries { - return query.NewData(fakeClient, schemes.TestingScheme()) +func BuildGatewayQueriesWithClient(fakeClient client.Client, opts ...query.Option) query.GatewayQueries { + return query.NewData(fakeClient, schemes.TestingScheme(), opts...) } func BuildGatewayQueries( diff --git a/projects/gateway2/translator/translator_case_test.go b/projects/gateway2/translator/translator_case_test.go index 61533f4d01e..db9839f6cc2 100644 --- a/projects/gateway2/translator/translator_case_test.go +++ b/projects/gateway2/translator/translator_case_test.go @@ -22,9 +22,11 @@ import ( "github.com/solo-io/gloo/pkg/utils/statusutils" sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" solokubev1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1/kube/apis/gateway.solo.io/v1" + "github.com/solo-io/gloo/projects/gateway2/query" gwquery "github.com/solo-io/gloo/projects/gateway2/query" "github.com/solo-io/gloo/projects/gateway2/reports" . "github.com/solo-io/gloo/projects/gateway2/translator" + "github.com/solo-io/gloo/projects/gateway2/translator/plugins" httplisquery "github.com/solo-io/gloo/projects/gateway2/translator/plugins/httplisteneroptions/query" lisquery "github.com/solo-io/gloo/projects/gateway2/translator/plugins/listeneroptions/query" "github.com/solo-io/gloo/projects/gateway2/translator/plugins/registry" @@ -52,6 +54,36 @@ func CompareProxy(expectedFile string, actualProxy *v1.Proxy) (string, error) { return cmp.Diff(expectedProxy, actualProxy, protocmp.Transform(), cmpopts.EquateNaNs()), nil } +var ( + _ plugins.BackendPlugin = &testBackendPlugin{} + _ query.BackendRefResolver = &testBackendPlugin{} +) + +type testBackendPlugin struct{} + +// GetBackendForRef implements query.BackendRefResolver. +func (tp *testBackendPlugin) GetBackendForRef(ctx context.Context, obj gwquery.From, ref *gwv1.BackendObjectReference) (client.Object, error, bool) { + if ref.Kind == nil || *ref.Kind != "test-backend-plugin" { + return nil, nil, false + } + // doesn't matter as long as its not nil + return &gwv1.HTTPRoute{}, nil, true +} + +func (tp *testBackendPlugin) ApplyBackendPlugin( + resolvedBackend client.Object, + ref gwv1.BackendObjectReference, +) (*v1.Destination, bool) { + if ref.Kind == nil || *ref.Kind != "test-backend-plugin" { + return nil, false + } + return &v1.Destination{ + DestinationType: &v1.Destination_Upstream{ + Upstream: &core.ResourceRef{Name: "test-backend-plugin-us"}, + }, + }, true +} + func (tc TestCase) Run(ctx context.Context) (map[types.NamespacedName]ActualTestResult, error) { var ( gateways []*gwv1.Gateway @@ -94,7 +126,7 @@ func (tc TestCase) Run(ctx context.Context) (map[types.NamespacedName]ActualTest lisquery.IterateIndices, httplisquery.IterateIndices, ) - queries := testutils.BuildGatewayQueriesWithClient(fakeClient) + queries := testutils.BuildGatewayQueriesWithClient(fakeClient, query.WithBackendRefResolvers(&testBackendPlugin{})) resourceClientFactory := &factory.MemoryResourceClientFactory{ Cache: memory.NewInMemoryResourceCache(), @@ -109,7 +141,9 @@ func (tc TestCase) Run(ctx context.Context) (map[types.NamespacedName]ActualTest routeOptionCollection := krt.NewStaticCollection(routeOptions) vhOptionCollection := krt.NewStatic[*solokubev1.VirtualHostOption](nil, true).AsCollection() - pluginRegistry := registry.NewPluginRegistry(registry.BuildPlugins(queries, fakeClient, routeOptionCollection, vhOptionCollection, statusReporter)) + allPlugins := registry.BuildPlugins(queries, fakeClient, routeOptionCollection, vhOptionCollection, statusReporter) + allPlugins = append(allPlugins, &testBackendPlugin{}) + pluginRegistry := registry.NewPluginRegistry(allPlugins) results := make(map[types.NamespacedName]ActualTestResult)