Skip to content

Commit

Permalink
fix: handing GRPCRoute hostname from Gateway (#6166)
Browse files Browse the repository at this point in the history 
fix: handing GRPCRoute hostname from Gateway


---------

Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
  • Loading branch information
@tao12345666333
tao12345666333 committed Jun 8, 2024
1 parent d62262e commit c8bbd08
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 13 deletions.
65 changes: 60 additions & 5 deletions internal/dataplane/translator/subtranslator/grpcroute.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/kongstate"
"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
)

Expand Down Expand Up @@ -36,6 +37,7 @@ func getGRPCMatchDefaults() (
func GenerateKongRoutesFromGRPCRouteRule(
grpcroute *gatewayapi.GRPCRoute,
ruleNumber int,
storer store.Storer,
) []kongstate.Route {
if ruleNumber >= len(grpcroute.Spec.Rules) {
return nil
Expand Down Expand Up @@ -67,7 +69,7 @@ func GenerateKongRoutesFromGRPCRouteRule(
Tags: tags,
},
}
if configuredHostnames := getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute); len(configuredHostnames) > 0 {
if configuredHostnames := getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute, storer); len(configuredHostnames) > 0 {
// Match based on hostnames.
r.Hosts = configuredHostnames
} else {
Expand All @@ -87,7 +89,7 @@ func GenerateKongRoutesFromGRPCRouteRule(
Name: routeName(grpcroute.Namespace, grpcroute.Name, ruleNumber, matchNumber),
Protocols: grpcProtocols,
Tags: tags,
Hosts: getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute),
Hosts: getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute, storer),
},
}

Expand Down Expand Up @@ -134,11 +136,64 @@ func GenerateKongRoutesFromGRPCRouteRule(
// getGRPCRouteHostnamesAsSliceOfStringPointers translates the hostnames defined
// in an GRPCRoute specification into a []*string slice, which is the type required
// by kong.Route{}.
func getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute *gatewayapi.GRPCRoute) []*string {
if len(grpcroute.Spec.Hostnames) == 0 {
// The hostname field is optional. If not specified, the configured value will be obtained from parentRefs.
func getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute *gatewayapi.GRPCRoute, storer store.Storer) []*string {
if len(grpcroute.Spec.Hostnames) > 0 {
return lo.Map(grpcroute.Spec.Hostnames, func(h gatewayapi.Hostname, _ int) *string {
return lo.ToPtr(string(h))
})
}

// If no hostnames are specified, we will use the hostname from the Gateway
// that the GRPCRoute is attached to.
namespace := grpcroute.GetNamespace()

if grpcroute.Spec.ParentRefs == nil {
return nil
}
return lo.Map(grpcroute.Spec.Hostnames, func(h gatewayapi.Hostname, _ int) *string {

hostnames := make([]gatewayapi.Hostname, 0)
for _, parentRef := range grpcroute.Spec.ParentRefs {
// we only care about Gateways
if parentRef.Kind != nil && *parentRef.Kind != "Gateway" {
continue
}

if parentRef.Namespace != nil {
namespace = string(*parentRef.Namespace)
}

name := string(parentRef.Name)

gateway, err := storer.GetGateway(namespace, name)
// As parentRef has already been validated before, the error here will not actually occur.
// This is where defensive programming takes place.
if err != nil {
// TODO: Add logging.
// https://github.com/Kong/kubernetes-ingress-controller/pull/6166#discussion_r1631250776
return nil
}

if parentRef.SectionName != nil {
sectionName := string(*parentRef.SectionName)

for _, listener := range gateway.Spec.Listeners {
if string(listener.Name) == sectionName {
if listener.Hostname != nil {
hostnames = append(hostnames, *listener.Hostname)
}
}
}
} else {
for _, listener := range gateway.Spec.Listeners {
if listener.Hostname != nil {
hostnames = append(hostnames, *listener.Hostname)
}
}
}
}

return lo.Map(hostnames, func(h gatewayapi.Hostname, _ int) *string {
return lo.ToPtr(string(h))
})
}
2 changes: 1 addition & 1 deletion internal/dataplane/translator/subtranslator/grpcroute_atc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func TestGenerateKongExpressionRoutesFromGRPCRouteRule(t *testing.T) {
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
grpcroute := makeTestGRPCRoute(tc.objectName, "default", tc.annotations, tc.hostnames, []gatewayapi.GRPCRouteRule{tc.rule})
grpcroute := makeTestGRPCRoute(tc.objectName, "default", tc.annotations, tc.hostnames, []gatewayapi.GRPCRouteRule{tc.rule}, nil)
routes := GenerateKongExpressionRoutesFromGRPCRouteRule(grpcroute, 0)
require.Equal(t, tc.expectedRoutes, routes)
})
Expand Down
71 changes: 68 additions & 3 deletions internal/dataplane/translator/subtranslator/grpcroute_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/kongstate"
"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
)

Expand All @@ -29,6 +30,7 @@ func makeTestGRPCRoute(
name string, namespace string, annotations map[string]string,
hostnames []string,
rules []gatewayapi.GRPCRouteRule,
parentRef []gatewayapi.ParentReference,
) *gatewayapi.GRPCRoute {
return &gatewayapi.GRPCRoute{
TypeMeta: metav1.TypeMeta{
Expand All @@ -45,6 +47,9 @@ func makeTestGRPCRoute(
return gatewayapi.Hostname(h)
}),
Rules: rules,
CommonRouteSpec: gatewayapi.CommonRouteSpec{
ParentRefs: parentRef,
},
},
}
}
Expand All @@ -57,6 +62,8 @@ func TestGenerateKongRoutesFromGRPCRouteRule(t *testing.T) {
hostnames []string
rule gatewayapi.GRPCRouteRule
expectedRoutes []kongstate.Route
parentRef []gatewayapi.ParentReference
storer store.Storer
}{
{
name: "single match without hostname",
Expand Down Expand Up @@ -277,13 +284,70 @@ func TestGenerateKongRoutesFromGRPCRouteRule(t *testing.T) {
},
},
},
{
name: "no match with hostname from gateway",
objectName: "hostname-from-gateway",
annotations: map[string]string{},
rule: gatewayapi.GRPCRouteRule{},
expectedRoutes: []kongstate.Route{
{
Ingress: util.K8sObjectInfo{
Name: "hostname-from-gateway",
Namespace: "default",
Annotations: map[string]string{},
GroupVersionKind: grpcRouteGVK,
},
Route: kong.Route{
Name: kong.String("grpcroute.default.hostname-from-gateway.0.0"),
Hosts: kong.StringSlice("bar.com"),
Protocols: kong.StringSlice("grpc", "grpcs"),
Tags: kong.StringSlice(
"k8s-name:hostname-from-gateway",
"k8s-namespace:default",
"k8s-kind:GRPCRoute",
"k8s-group:gateway.networking.k8s.io",
"k8s-version:v1",
),
},
},
},
parentRef: []gatewayapi.ParentReference{
{
Name: "gateway",
Namespace: lo.ToPtr(gatewayapi.Namespace("default")),
SectionName: lo.ToPtr(gatewayapi.SectionName("listener-1")),
},
},
storer: lo.Must(store.NewFakeStore(store.FakeObjects{
Gateways: []*gatewayapi.Gateway{
{
ObjectMeta: metav1.ObjectMeta{
Name: "gateway",
Namespace: "default",
},
TypeMeta: metav1.TypeMeta{
Kind: "Gateway",
APIVersion: "gateway.networking.k8s.io/v1",
},
Spec: gatewayapi.GatewaySpec{
Listeners: []gatewayapi.Listener{
{
Name: "listener-1",
Hostname: lo.ToPtr(gatewayapi.Hostname("bar.com")),
},
},
},
},
},
})),
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
grpcroute := makeTestGRPCRoute(tc.objectName, "default", tc.annotations, tc.hostnames, []gatewayapi.GRPCRouteRule{tc.rule})
routes := GenerateKongRoutesFromGRPCRouteRule(grpcroute, 0)
grpcroute := makeTestGRPCRoute(tc.objectName, "default", tc.annotations, tc.hostnames, []gatewayapi.GRPCRouteRule{tc.rule}, tc.parentRef)
routes := GenerateKongRoutesFromGRPCRouteRule(grpcroute, 0, tc.storer)
require.Equal(t, tc.expectedRoutes, routes)
})
}
Expand Down Expand Up @@ -325,7 +389,8 @@ func TestGetGRPCRouteHostnamesAsSliceOfStringPointers(t *testing.T) {
},
} {
t.Run(tC.name, func(t *testing.T) {
result := getGRPCRouteHostnamesAsSliceOfStringPointers(tC.grpcroute)
storer := lo.Must(store.NewFakeStore(store.FakeObjects{}))
result := getGRPCRouteHostnamesAsSliceOfStringPointers(tC.grpcroute, storer)
require.Equal(t, tC.expected, result)
})
}
Expand Down
2 changes: 1 addition & 1 deletion internal/dataplane/translator/translate_grpcroute.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (t *Translator) ingressRulesFromGRPCRoute(result *ingressRules, grpcroute *
if err != nil {
return err
}
service.Routes = append(service.Routes, subtranslator.GenerateKongRoutesFromGRPCRouteRule(grpcroute, ruleNumber)...)
service.Routes = append(service.Routes, subtranslator.GenerateKongRoutesFromGRPCRouteRule(grpcroute, ruleNumber, t.storer)...)

// cache the service to avoid duplicates in further loop iterations
result.ServiceNameToServices[*service.Service.Name] = service
Expand Down
3 changes: 0 additions & 3 deletions test/conformance/gateway_conformance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ import (
var skippedTestsForTraditionalRoutes = []string{
// core conformance
tests.HTTPRouteHeaderMatching.ShortName,
// There is an issue with KIC when processing this scenario.
// TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/6136
tests.GRPCRouteListenerHostnameMatching.ShortName,
// tests.GRPCRouteHeaderMatching.ShortName and tests.GRPCExactMethodMatching.ShortName may
// have some conflicts, skipping either one will still pass normally.
// TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/6144
Expand Down

0 comments on commit c8bbd08

Please sign in to comment.