Skip to content

Commit b937894

Browse files
fix route reports for custom backends (#10361)
Co-authored-by: Omar Hammami <[email protected]>
1 parent 2b20acf commit b937894

File tree

7 files changed

+166
-28
lines changed

7 files changed

+166
-28
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
changelog:
2+
- type: NON_USER_FACING
3+
description: >-
4+
Fix route report issues when we write multiple values
5+
for a status, or when we're reporting for a custom backend
6+
type.

projects/gateway2/translator/gateway_translator_test.go

+24-8
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
5151
gwNN: types.NamespacedName{
5252
Namespace: "default",
5353
Name: "example-gateway",
54-
}}),
54+
},
55+
}),
5556
Entry(
5657
"https gateway with basic routing",
5758
translatorTestCase{
@@ -60,7 +61,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
6061
gwNN: types.NamespacedName{
6162
Namespace: "default",
6263
Name: "example-gateway",
63-
}}),
64+
},
65+
}),
6466
Entry(
6567
"http gateway with multiple listeners on the same port",
6668
translatorTestCase{
@@ -69,7 +71,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
6971
gwNN: types.NamespacedName{
7072
Namespace: "default",
7173
Name: "http",
72-
}}),
74+
},
75+
}),
7376
Entry(
7477
"https gateway with multiple listeners on the same port",
7578
translatorTestCase{
@@ -78,7 +81,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
7881
gwNN: types.NamespacedName{
7982
Namespace: "default",
8083
Name: "http",
81-
}}),
84+
},
85+
}),
8286
Entry(
8387
"http gateway with multiple routing rules and HeaderModifier filter",
8488
translatorTestCase{
@@ -87,7 +91,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
8791
gwNN: types.NamespacedName{
8892
Namespace: "default",
8993
Name: "gw",
90-
}}),
94+
},
95+
}),
9196
Entry(
9297
"http gateway with lambda destination",
9398
translatorTestCase{
@@ -96,7 +101,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
96101
gwNN: types.NamespacedName{
97102
Namespace: "default",
98103
Name: "gw",
99-
}}),
104+
},
105+
}),
100106
Entry(
101107
"http gateway with azure destination",
102108
translatorTestCase{
@@ -105,7 +111,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
105111
gwNN: types.NamespacedName{
106112
Namespace: "default",
107113
Name: "gw",
108-
}}),
114+
},
115+
}),
109116
Entry(
110117
"gateway with correctly sorted routes",
111118
translatorTestCase{
@@ -114,7 +121,8 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
114121
gwNN: types.NamespacedName{
115122
Namespace: "infra",
116123
Name: "example-gateway",
117-
}}),
124+
},
125+
}),
118126
Entry(
119127
"httproute with missing backend reports correctly",
120128
translatorTestCase{
@@ -258,6 +266,14 @@ var _ = DescribeTable("Basic GatewayTranslator Tests",
258266
Name: "example-tcp-gateway",
259267
},
260268
}),
269+
Entry("Plugin Backend", translatorTestCase{
270+
inputFile: "backend-plugin/gateway.yaml",
271+
outputFile: "backend-plugin-proxy.yaml",
272+
gwNN: types.NamespacedName{
273+
Namespace: "default",
274+
Name: "example-gateway",
275+
},
276+
}),
261277
)
262278

263279
var _ = DescribeTable("Route Delegation translator",

projects/gateway2/translator/httproute/gateway_http_route_translator.go

+28-16
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1717
"k8s.io/apimachinery/pkg/types"
1818
"k8s.io/apimachinery/pkg/util/sets"
19+
"sigs.k8s.io/controller-runtime/pkg/client"
1920
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
2021

2122
"github.com/solo-io/gloo/projects/gateway2/query"
@@ -353,13 +354,6 @@ func setRouteAction(
353354
clusterName := "blackhole_cluster"
354355
ns := "blackhole_ns"
355356

356-
obj, err := gwroute.GetBackendForRef(backendRef.BackendObjectReference)
357-
ptrClusterName := query.ProcessBackendRef(obj, err, reporter, backendRef.BackendObjectReference)
358-
if ptrClusterName != nil {
359-
clusterName = *ptrClusterName
360-
ns = obj.GetNamespace()
361-
}
362-
363357
var weight *wrappers.UInt32Value
364358
if backendRef.Weight != nil {
365359
weight = &wrappers.UInt32Value{
@@ -372,20 +366,25 @@ func setRouteAction(
372366
}
373367
}
374368

375-
fromPlugin := false
376-
for _, bp := range pluginRegistry.GetBackendPlugins() {
377-
if dest, ok := bp.ApplyBackendPlugin(obj, backendRef.BackendObjectReference); ok {
378-
fromPlugin = true
369+
obj, err := gwroute.GetBackendForRef(backendRef.BackendObjectReference)
370+
if err == nil {
371+
// Only apply backend plugin when the backend is resolved.
372+
// If any backend plugin matches this ref, we don't need the standard
373+
// reports or validation path.
374+
if dest, ok := applyBackendPlugins(obj, backendRef.BackendObjectReference, pluginRegistry); ok {
379375
weightedDestinations = append(weightedDestinations, &v1.WeightedDestination{
380376
Destination: dest,
381377
Weight: weight,
382378
})
383-
break
379+
continue
384380
}
385381
}
386-
// TODO break out a buildDestination func to avoid this awkwardness
387-
if fromPlugin {
388-
continue
382+
383+
// only call ProcessBackendRef when the plugin didn't handle it
384+
ptrClusterName := query.ProcessBackendRef(obj, err, reporter, backendRef.BackendObjectReference)
385+
if ptrClusterName != nil {
386+
clusterName = *ptrClusterName
387+
ns = obj.GetNamespace()
389388
}
390389

391390
var port uint32
@@ -445,7 +444,7 @@ func setRouteAction(
445444
})
446445

447446
default:
448-
contextutils.LoggerFrom(ctx).Errorf("unsupported backend type for kind: %v and type: %v", *backendRef.BackendObjectReference.Kind, *backendRef.BackendObjectReference.Group)
447+
contextutils.LoggerFrom(ctx).Errorf("unsupported backend type for kind: %v and type: %v", backendRef.BackendObjectReference.Kind, backendRef.BackendObjectReference.Group)
449448
}
450449
}
451450

@@ -520,3 +519,16 @@ func makeDestinationSpec(upstream *gloov1.Upstream, filters []gwv1.HTTPRouteFilt
520519
}
521520
return nil, nil
522521
}
522+
523+
func applyBackendPlugins(
524+
obj client.Object,
525+
backendRef gwv1.BackendObjectReference,
526+
plugins registry.PluginRegistry,
527+
) (*v1.Destination, bool) {
528+
for _, bp := range plugins.GetBackendPlugins() {
529+
if dest, ok := bp.ApplyBackendPlugin(obj, backendRef); ok {
530+
return dest, true
531+
}
532+
}
533+
return nil, false
534+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#$ Used in:
2+
#$ - site-src/guides/http-routing.md
3+
apiVersion: gateway.networking.k8s.io/v1
4+
kind: Gateway
5+
metadata:
6+
name: example-gateway
7+
spec:
8+
gatewayClassName: example-gateway-class
9+
listeners:
10+
- name: http
11+
protocol: HTTP
12+
port: 80
13+
---
14+
apiVersion: gateway.networking.k8s.io/v1
15+
kind: HTTPRoute
16+
metadata:
17+
name: example-route
18+
spec:
19+
parentRefs:
20+
- name: example-gateway
21+
hostnames:
22+
- "example.com"
23+
rules:
24+
- backendRefs:
25+
- name: example-svc
26+
kind: test-backend-plugin
27+
port: 80
28+
---
29+
apiVersion: v1
30+
kind: Service
31+
metadata:
32+
name: example-svc
33+
spec:
34+
selector:
35+
test: test
36+
ports:
37+
- protocol: TCP
38+
port: 80
39+
targetPort: test
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
listeners:
3+
- aggregateListener:
4+
httpFilterChains:
5+
- matcher: {}
6+
virtualHostRefs:
7+
- http~example_com
8+
httpResources:
9+
virtualHosts:
10+
http~example_com:
11+
domains:
12+
- example.com
13+
name: http~example_com
14+
routes:
15+
- matchers:
16+
- prefix: /
17+
name: httproute-example-route-default-0-0
18+
options: {}
19+
routeAction:
20+
single:
21+
upstream:
22+
name: test-backend-plugin-us
23+
bindAddress: '::'
24+
bindPort: 8080
25+
name: http
26+
metadata:
27+
labels:
28+
created_by: gloo-kube-gateway-api
29+
gateway_namespace: default
30+
name: default-example-gateway
31+
namespace: gloo-system

projects/gateway2/translator/testutils/test_queries.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ func BuildIndexedFakeClient(objs []client.Object, funcs ...IndexIteratorFunc) cl
2121
return builder.WithObjects(objs...).Build()
2222
}
2323

24-
func BuildGatewayQueriesWithClient(fakeClient client.Client) query.GatewayQueries {
25-
return query.NewData(fakeClient, schemes.TestingScheme())
24+
func BuildGatewayQueriesWithClient(fakeClient client.Client, opts ...query.Option) query.GatewayQueries {
25+
return query.NewData(fakeClient, schemes.TestingScheme(), opts...)
2626
}
2727

2828
func BuildGatewayQueries(

projects/gateway2/translator/translator_case_test.go

+36-2
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@ import (
2222
"github.com/solo-io/gloo/pkg/utils/statusutils"
2323
sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1"
2424
solokubev1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1/kube/apis/gateway.solo.io/v1"
25+
"github.com/solo-io/gloo/projects/gateway2/query"
2526
gwquery "github.com/solo-io/gloo/projects/gateway2/query"
2627
"github.com/solo-io/gloo/projects/gateway2/reports"
2728
. "github.com/solo-io/gloo/projects/gateway2/translator"
29+
"github.com/solo-io/gloo/projects/gateway2/translator/plugins"
2830
httplisquery "github.com/solo-io/gloo/projects/gateway2/translator/plugins/httplisteneroptions/query"
2931
lisquery "github.com/solo-io/gloo/projects/gateway2/translator/plugins/listeneroptions/query"
3032
"github.com/solo-io/gloo/projects/gateway2/translator/plugins/registry"
@@ -52,6 +54,36 @@ func CompareProxy(expectedFile string, actualProxy *v1.Proxy) (string, error) {
5254
return cmp.Diff(expectedProxy, actualProxy, protocmp.Transform(), cmpopts.EquateNaNs()), nil
5355
}
5456

57+
var (
58+
_ plugins.BackendPlugin = &testBackendPlugin{}
59+
_ query.BackendRefResolver = &testBackendPlugin{}
60+
)
61+
62+
type testBackendPlugin struct{}
63+
64+
// GetBackendForRef implements query.BackendRefResolver.
65+
func (tp *testBackendPlugin) GetBackendForRef(ctx context.Context, obj gwquery.From, ref *gwv1.BackendObjectReference) (client.Object, error, bool) {
66+
if ref.Kind == nil || *ref.Kind != "test-backend-plugin" {
67+
return nil, nil, false
68+
}
69+
// doesn't matter as long as its not nil
70+
return &gwv1.HTTPRoute{}, nil, true
71+
}
72+
73+
func (tp *testBackendPlugin) ApplyBackendPlugin(
74+
resolvedBackend client.Object,
75+
ref gwv1.BackendObjectReference,
76+
) (*v1.Destination, bool) {
77+
if ref.Kind == nil || *ref.Kind != "test-backend-plugin" {
78+
return nil, false
79+
}
80+
return &v1.Destination{
81+
DestinationType: &v1.Destination_Upstream{
82+
Upstream: &core.ResourceRef{Name: "test-backend-plugin-us"},
83+
},
84+
}, true
85+
}
86+
5587
func (tc TestCase) Run(ctx context.Context) (map[types.NamespacedName]ActualTestResult, error) {
5688
var (
5789
gateways []*gwv1.Gateway
@@ -94,7 +126,7 @@ func (tc TestCase) Run(ctx context.Context) (map[types.NamespacedName]ActualTest
94126
lisquery.IterateIndices,
95127
httplisquery.IterateIndices,
96128
)
97-
queries := testutils.BuildGatewayQueriesWithClient(fakeClient)
129+
queries := testutils.BuildGatewayQueriesWithClient(fakeClient, query.WithBackendRefResolvers(&testBackendPlugin{}))
98130

99131
resourceClientFactory := &factory.MemoryResourceClientFactory{
100132
Cache: memory.NewInMemoryResourceCache(),
@@ -109,7 +141,9 @@ func (tc TestCase) Run(ctx context.Context) (map[types.NamespacedName]ActualTest
109141
routeOptionCollection := krt.NewStaticCollection(routeOptions)
110142
vhOptionCollection := krt.NewStatic[*solokubev1.VirtualHostOption](nil, true).AsCollection()
111143

112-
pluginRegistry := registry.NewPluginRegistry(registry.BuildPlugins(queries, fakeClient, routeOptionCollection, vhOptionCollection, statusReporter))
144+
allPlugins := registry.BuildPlugins(queries, fakeClient, routeOptionCollection, vhOptionCollection, statusReporter)
145+
allPlugins = append(allPlugins, &testBackendPlugin{})
146+
pluginRegistry := registry.NewPluginRegistry(allPlugins)
113147

114148
results := make(map[types.NamespacedName]ActualTestResult)
115149

0 commit comments

Comments
 (0)