Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix route reports for custom backends #10361

Merged
merged 15 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/v1.18.0-rc3/custom-be-route-report.yaml
Original file line number Diff line number Diff line change
@@ -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.
32 changes: 24 additions & 8 deletions projects/gateway2/translator/gateway_translator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
gwNN: types.NamespacedName{
Namespace: "default",
Name: "example-gateway",
}}),
},
}),
Entry(
"https gateway with basic routing",
translatorTestCase{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -87,7 +91,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
gwNN: types.NamespacedName{
Namespace: "default",
Name: "gw",
}}),
},
}),
Entry(
"http gateway with lambda destination",
translatorTestCase{
Expand All @@ -96,7 +101,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
gwNN: types.NamespacedName{
Namespace: "default",
Name: "gw",
}}),
},
}),
Entry(
"http gateway with azure destination",
translatorTestCase{
Expand All @@ -105,7 +111,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
gwNN: types.NamespacedName{
Namespace: "default",
Name: "gw",
}}),
},
}),
Entry(
"gateway with correctly sorted routes",
translatorTestCase{
Expand All @@ -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{
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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 {
danehans marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions projects/gateway2/translator/testutils/test_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
38 changes: 36 additions & 2 deletions projects/gateway2/translator/translator_case_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand All @@ -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)

Expand Down
Loading