Skip to content

Commit 5673671

Browse files
committed
[feat gw-api]implement hostname uniqueness for httproute and grpcroute cross serving
1 parent aefed36 commit 5673671

File tree

9 files changed

+262
-20
lines changed

9 files changed

+262
-20
lines changed

go.mod

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ require (
8080
github.com/distribution/reference v0.6.0 // indirect
8181
github.com/docker/cli v25.0.1+incompatible // indirect
8282
github.com/docker/distribution v2.8.3+incompatible // indirect
83-
github.com/docker/docker v25.0.6+incompatible // indirect
83+
github.com/docker/docker v27.1.1+incompatible // indirect
8484
github.com/docker/docker-credential-helpers v0.7.0 // indirect
8585
github.com/docker/go-connections v0.5.0 // indirect
8686
github.com/docker/go-metrics v0.0.1 // indirect
@@ -173,7 +173,6 @@ require (
173173
go.opentelemetry.io/otel/trace v1.33.0 // indirect
174174
go.uber.org/multierr v1.11.0 // indirect
175175
golang.org/x/crypto v0.39.0 // indirect
176-
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
177176
golang.org/x/oauth2 v0.27.0 // indirect
178177
golang.org/x/sync v0.15.0 // indirect
179178
golang.org/x/sys v0.33.0 // indirect

go.sum

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ github.com/docker/distribution v2.8.3+incompatible h1:AtKxIZ36LoNK51+Z6RpzLpddBi
133133
github.com/docker/distribution v2.8.3+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
134134
github.com/docker/docker v25.0.6+incompatible h1:5cPwbwriIcsua2REJe8HqQV+6WlWc1byg2QSXzBxBGg=
135135
github.com/docker/docker v25.0.6+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
136+
github.com/docker/docker v27.1.1+incompatible h1:hO/M4MtV36kzKldqnA37IWhebRA+LnqqcqDja6kVaKY=
137+
github.com/docker/docker v27.1.1+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
136138
github.com/docker/docker-credential-helpers v0.7.0 h1:xtCHsjxogADNZcdv1pKUHXryefjlVRqWqIhk/uXJp0A=
137139
github.com/docker/docker-credential-helpers v0.7.0/go.mod h1:rETQfLdHNT3foU5kuNkFR1R1V12OJRRO5lzt2D1b5X0=
138140
github.com/docker/go-connections v0.5.0 h1:USnMq7hx7gwdVZq1L49hLXaFtUdTADjXGp+uj1Br63c=
@@ -147,8 +149,6 @@ github.com/emicklei/go-restful/v3 v3.12.0 h1:y2DdzBAURM29NFF94q6RaY4vjIH1rtwDapw
147149
github.com/emicklei/go-restful/v3 v3.12.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
148150
github.com/evanphx/json-patch v5.9.0+incompatible h1:fBXyNpNMuTTDdquAq/uisOr2lShz4oaXpDTX2bLe7ls=
149151
github.com/evanphx/json-patch v5.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
150-
github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg=
151-
github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ=
152152
github.com/evanphx/json-patch/v5 v5.9.11 h1:/8HVnzMq13/3x9TPvjG08wUGqBTmZBsCWzjTM0wiaDU=
153153
github.com/evanphx/json-patch/v5 v5.9.11/go.mod h1:3j+LviiESTElxA4p3EMKAB9HXj3/XEtnUf6OZxqIQTM=
154154
github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f h1:Wl78ApPPB2Wvf/TIe2xdyJxTlb6obmF18d8QdkxNDu4=
@@ -504,8 +504,6 @@ golang.org/x/crypto v0.0.0-20210513164829-c07d793c2f9a/go.mod h1:P+XmwS30IXTQdn5
504504
golang.org/x/crypto v0.0.0-20220214200702-86341886e292/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
505505
golang.org/x/crypto v0.39.0 h1:SHs+kF4LP+f+p14esP5jAoDpHU8Gu/v9lFRK6IT5imM=
506506
golang.org/x/crypto v0.39.0/go.mod h1:L+Xg3Wf6HoL4Bn4238Z6ft6KfEpN0tJGo53AAPC632U=
507-
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8=
508-
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY=
509507
golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
510508
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
511509
golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
@@ -655,8 +653,6 @@ moul.io/http2curl/v2 v2.3.0 h1:9r3JfDzWPcbIklMOs2TnIFzDYvfAZvjeavG6EzP7jYs=
655653
moul.io/http2curl/v2 v2.3.0/go.mod h1:RW4hyBjTWSYDOxapodpNEtX0g5Eb16sxklBqmd2RHcE=
656654
oras.land/oras-go v1.2.5 h1:XpYuAwAb0DfQsunIyMfeET92emK8km3W4yEzZvUbsTo=
657655
oras.land/oras-go v1.2.5/go.mod h1:PuAwRShRZCsZb7g8Ar3jKKQR/2A/qN+pkYxIOd/FAoo=
658-
sigs.k8s.io/controller-runtime v0.19.3 h1:XO2GvC9OPftRst6xWCpTgBZO04S2cbp0Qqkj8bX1sPw=
659-
sigs.k8s.io/controller-runtime v0.19.3/go.mod h1:j4j87DqtsThvwTv5/Tc5NFRyyF/RF0ip4+62tbTSIUM=
660656
sigs.k8s.io/controller-runtime v0.21.0 h1:CYfjpEuicjUecRk+KAeyYh+ouUBn4llGyDYytIGcJS8=
661657
sigs.k8s.io/controller-runtime v0.21.0/go.mod h1:OSg14+F65eWqIu4DceX7k/+QRAbTTvxeQSNSOQpukWM=
662658
sigs.k8s.io/gateway-api v1.2.0 h1:LrToiFwtqKTKZcZtoQPTuo3FxhrrhTgzQG0Te+YGSo8=

pkg/gateway/routeutils/listener_attachment_helper.go

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"github.com/go-logr/logr"
77
"github.com/pkg/errors"
8+
"k8s.io/apimachinery/pkg/types"
89
"k8s.io/apimachinery/pkg/util/sets"
910
"sigs.k8s.io/controller-runtime/pkg/client"
1011
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
@@ -13,7 +14,7 @@ import (
1314
// listenerAttachmentHelper is an internal utility interface that can be used to determine if a listener will allow
1415
// a route to attach to it.
1516
type listenerAttachmentHelper interface {
16-
listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, deferredRouteReconciler RouteReconciler) (bool, error)
17+
listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, deferredRouteReconciler RouteReconciler, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) (bool, error)
1718
}
1819

1920
var _ listenerAttachmentHelper = &listenerAttachmentHelperImpl{}
@@ -33,7 +34,7 @@ func newListenerAttachmentHelper(k8sClient client.Client, logger logr.Logger) li
3334

3435
// listenerAllowsAttachment utility method to determine if a listener will allow a route to connect using
3536
// Gateway API rules to determine compatibility between lister and route.
36-
func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, deferredRouteReconciler RouteReconciler) (bool, error) {
37+
func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, deferredRouteReconciler RouteReconciler, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) (bool, error) {
3738
// check namespace
3839
namespaceOK, err := attachmentHelper.namespaceCheck(ctx, gw, listener, route)
3940
if err != nil {
@@ -63,14 +64,25 @@ func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(c
6364
return false, err
6465
}
6566
if !hostnameOK {
66-
// hostname is not ok, print out gwName and gwNamespace test-gw-alb gateway-alb
6767
deferredRouteReconciler.Enqueue(
6868
GenerateRouteData(false, true, string(gwv1.RouteReasonNoMatchingListenerHostname), RouteStatusInfoRejectedMessageNoMatchingHostname, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw),
6969
)
7070
return false, nil
7171
}
7272
}
7373

74+
// check cross serving hostname uniqueness
75+
if route.GetRouteKind() == HTTPRouteKind || route.GetRouteKind() == GRPCRouteKind {
76+
hostnameUniquenessOK, conflictRoute := attachmentHelper.crossServingHostnameUniquenessCheck(route, hostnamesFromHttpRoutes, hostnamesFromGrpcRoutes)
77+
if !hostnameUniquenessOK {
78+
message := fmt.Sprintf("HTTPRoute and GRPCRoute have overlap hostname, attachment is rejected. Conflict route: %s", conflictRoute)
79+
deferredRouteReconciler.Enqueue(
80+
GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), message, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw),
81+
)
82+
return false, nil
83+
}
84+
}
85+
7486
return true, nil
7587
}
7688

@@ -199,3 +211,34 @@ func (attachmentHelper *listenerAttachmentHelperImpl) hostnameCheck(listener gwv
199211
}
200212
return false, nil
201213
}
214+
215+
func (attachmentHelper *listenerAttachmentHelperImpl) crossServingHostnameUniquenessCheck(route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) (bool, string) {
216+
namespacedName := route.GetRouteNamespacedName()
217+
hostnames := route.GetHostnames()
218+
routeKind := route.GetRouteKind()
219+
var conflictMap map[types.NamespacedName][]gwv1.Hostname
220+
221+
switch routeKind {
222+
case GRPCRouteKind:
223+
hostnamesFromGrpcRoutes[namespacedName] = hostnames
224+
if len(hostnamesFromHttpRoutes) > 0 {
225+
conflictMap = hostnamesFromHttpRoutes
226+
}
227+
case HTTPRouteKind:
228+
hostnamesFromHttpRoutes[namespacedName] = hostnames
229+
if len(hostnamesFromGrpcRoutes) > 0 {
230+
conflictMap = hostnamesFromGrpcRoutes
231+
}
232+
}
233+
234+
for _, hostname := range hostnames {
235+
for conflictNamespacedName, conflictHostnames := range conflictMap {
236+
for _, conflictHostname := range conflictHostnames {
237+
if isHostnameCompatible(string(hostname), string(conflictHostname)) {
238+
return false, conflictNamespacedName.String()
239+
}
240+
}
241+
}
242+
}
243+
return true, ""
244+
}

pkg/gateway/routeutils/listener_attachment_helper_test.go

Lines changed: 200 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/pkg/errors"
77
"github.com/stretchr/testify/assert"
88
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"k8s.io/apimachinery/pkg/types"
910
"k8s.io/apimachinery/pkg/util/sets"
1011
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
1112
"testing"
@@ -69,10 +70,12 @@ func Test_listenerAllowsAttachment(t *testing.T) {
6970
attachmentHelper := listenerAttachmentHelperImpl{
7071
logger: logr.Discard(),
7172
}
73+
hostnameFromHttpRoute := map[types.NamespacedName][]gwv1.Hostname{}
74+
hostnameFromGrpcRoute := map[types.NamespacedName][]gwv1.Hostname{}
7275
mockReconciler := NewMockRouteReconciler()
7376
result, err := attachmentHelper.listenerAllowsAttachment(context.Background(), gw, gwv1.Listener{
7477
Protocol: tc.listenerProtocol,
75-
}, route, mockReconciler)
78+
}, route, mockReconciler, hostnameFromHttpRoute, hostnameFromGrpcRoute)
7679
assert.NoError(t, err)
7780
assert.Equal(t, tc.expected, result)
7881
})
@@ -409,3 +412,199 @@ func Test_kindCheck(t *testing.T) {
409412
})
410413
}
411414
}
415+
416+
func Test_hostnameCheck(t *testing.T) {
417+
validHostname := gwv1.Hostname("example.com")
418+
invalidHostname := gwv1.Hostname("invalid..hostname")
419+
invalidHostnameTwo := gwv1.Hostname("another..invalid")
420+
421+
tests := []struct {
422+
name string
423+
listener gwv1.Listener
424+
route preLoadRouteDescriptor
425+
expectedResult bool
426+
expectedError bool
427+
}{
428+
{
429+
name: "listener has no hostname - should pass",
430+
listener: gwv1.Listener{
431+
Hostname: nil,
432+
},
433+
route: &mockRoute{
434+
hostnames: []gwv1.Hostname{"example.com"},
435+
},
436+
expectedResult: true,
437+
expectedError: false,
438+
},
439+
{
440+
name: "route has no hostnames - should pass",
441+
listener: gwv1.Listener{
442+
Hostname: &validHostname,
443+
},
444+
route: &mockRoute{
445+
hostnames: []gwv1.Hostname{},
446+
},
447+
expectedResult: true,
448+
expectedError: false,
449+
},
450+
{
451+
name: "listener hostname invalid - should fail",
452+
listener: gwv1.Listener{
453+
Hostname: &invalidHostname,
454+
},
455+
route: &mockRoute{
456+
hostnames: []gwv1.Hostname{"example.com"},
457+
},
458+
expectedResult: false,
459+
expectedError: true,
460+
},
461+
{
462+
name: "compatible hostnames - should pass",
463+
listener: gwv1.Listener{
464+
Hostname: &validHostname,
465+
},
466+
route: &mockRoute{
467+
hostnames: []gwv1.Hostname{"example.com"},
468+
},
469+
expectedResult: true,
470+
expectedError: false,
471+
},
472+
{
473+
name: "incompatible hostnames - should fail",
474+
listener: gwv1.Listener{
475+
Hostname: &validHostname,
476+
},
477+
route: &mockRoute{
478+
hostnames: []gwv1.Hostname{"example.test.com"},
479+
},
480+
expectedResult: false,
481+
expectedError: false,
482+
},
483+
{
484+
name: "route has invalid hostname but valid one matches - should pass",
485+
listener: gwv1.Listener{
486+
Hostname: &validHostname,
487+
},
488+
route: &mockRoute{
489+
hostnames: []gwv1.Hostname{invalidHostname, "example.com"},
490+
},
491+
expectedResult: true,
492+
expectedError: false,
493+
},
494+
{
495+
name: "route has only invalid hostnames - should fail",
496+
listener: gwv1.Listener{
497+
Hostname: &validHostname,
498+
},
499+
route: &mockRoute{
500+
hostnames: []gwv1.Hostname{invalidHostname, invalidHostnameTwo},
501+
},
502+
expectedResult: false,
503+
expectedError: false,
504+
},
505+
}
506+
507+
for _, tt := range tests {
508+
t.Run(tt.name, func(t *testing.T) {
509+
helper := &listenerAttachmentHelperImpl{
510+
logger: logr.Discard(),
511+
}
512+
513+
result, err := helper.hostnameCheck(tt.listener, tt.route)
514+
515+
assert.Equal(t, tt.expectedResult, result)
516+
if tt.expectedError {
517+
assert.Error(t, err)
518+
} else {
519+
assert.NoError(t, err)
520+
}
521+
})
522+
}
523+
}
524+
525+
func Test_crossServingHostnameUniquenessCheck(t *testing.T) {
526+
hostnames := []gwv1.Hostname{"example.com"}
527+
namespace := "test-namespace"
528+
httpRouteName := "http-route-name"
529+
grpcRouteName := "grpc-route-name"
530+
tests := []struct {
531+
name string
532+
route preLoadRouteDescriptor
533+
hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname
534+
hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname
535+
expected bool
536+
}{
537+
{
538+
name: "GRPC route only - should pass",
539+
route: &mockRoute{
540+
routeKind: GRPCRouteKind,
541+
hostnames: hostnames,
542+
namespacedName: types.NamespacedName{Name: grpcRouteName, Namespace: namespace},
543+
},
544+
hostnamesFromHttpRoutes: map[types.NamespacedName][]gwv1.Hostname{},
545+
hostnamesFromGrpcRoutes: map[types.NamespacedName][]gwv1.Hostname{},
546+
expected: true,
547+
},
548+
{
549+
name: "HTTP route only - should pass",
550+
route: &mockRoute{
551+
routeKind: HTTPRouteKind,
552+
hostnames: hostnames,
553+
namespacedName: types.NamespacedName{Name: httpRouteName, Namespace: namespace},
554+
},
555+
hostnamesFromHttpRoutes: map[types.NamespacedName][]gwv1.Hostname{},
556+
hostnamesFromGrpcRoutes: map[types.NamespacedName][]gwv1.Hostname{},
557+
expected: true,
558+
},
559+
{
560+
name: "GRPC route with overlapping HTTP route hostname - should fail",
561+
route: &mockRoute{
562+
routeKind: GRPCRouteKind,
563+
hostnames: hostnames,
564+
namespacedName: types.NamespacedName{Name: grpcRouteName, Namespace: namespace},
565+
},
566+
hostnamesFromHttpRoutes: map[types.NamespacedName][]gwv1.Hostname{
567+
{Name: httpRouteName, Namespace: namespace}: hostnames,
568+
},
569+
hostnamesFromGrpcRoutes: map[types.NamespacedName][]gwv1.Hostname{},
570+
expected: false,
571+
},
572+
{
573+
name: "HTTP route with overlapping GRPC route hostname - should fail",
574+
route: &mockRoute{
575+
routeKind: HTTPRouteKind,
576+
hostnames: hostnames,
577+
namespacedName: types.NamespacedName{Name: httpRouteName, Namespace: namespace},
578+
},
579+
hostnamesFromHttpRoutes: map[types.NamespacedName][]gwv1.Hostname{},
580+
hostnamesFromGrpcRoutes: map[types.NamespacedName][]gwv1.Hostname{
581+
{Name: grpcRouteName, Namespace: namespace}: hostnames,
582+
},
583+
expected: false,
584+
},
585+
{
586+
name: "GRPC route with non-overlapping HTTP route hostname - should pass",
587+
route: &mockRoute{
588+
routeKind: GRPCRouteKind,
589+
hostnames: []gwv1.Hostname{"grpc.example.com"},
590+
namespacedName: types.NamespacedName{Name: grpcRouteName, Namespace: namespace},
591+
},
592+
hostnamesFromHttpRoutes: map[types.NamespacedName][]gwv1.Hostname{
593+
{Name: httpRouteName, Namespace: namespace}: {"http.example.com"},
594+
},
595+
hostnamesFromGrpcRoutes: map[types.NamespacedName][]gwv1.Hostname{},
596+
expected: true,
597+
},
598+
}
599+
600+
for _, tt := range tests {
601+
t.Run(tt.name, func(t *testing.T) {
602+
helper := &listenerAttachmentHelperImpl{
603+
logger: logr.Discard(),
604+
}
605+
606+
result, _ := helper.crossServingHostnameUniquenessCheck(tt.route, tt.hostnamesFromHttpRoutes, tt.hostnamesFromGrpcRoutes)
607+
assert.Equal(t, tt.expected, result)
608+
})
609+
}
610+
}

pkg/gateway/routeutils/loader_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type mockRoute struct {
2929
namespacedName types.NamespacedName
3030
routeKind RouteKind
3131
generation int64
32+
hostnames []gwv1.Hostname
3233
}
3334

3435
func (m *mockRoute) loadAttachedRules(context context.Context, k8sClient client.Client) (RouteDescriptor, []routeLoadError) {
@@ -44,8 +45,7 @@ func (m *mockRoute) GetRouteKind() RouteKind {
4445
}
4546

4647
func (m *mockRoute) GetHostnames() []gwv1.Hostname {
47-
//TODO implement me
48-
panic("implement me")
48+
return m.hostnames
4949
}
5050

5151
func (m *mockRoute) GetParentRefs() []gwv1.ParentReference {

0 commit comments

Comments
 (0)