Skip to content

Commit

Permalink
feat: pass store down to backendRefsToKongStateBackends to allow Serv…
Browse files Browse the repository at this point in the history
…ice lookup
  • Loading branch information
pmalek committed Jan 15, 2024
1 parent 2259919 commit c386d83
Show file tree
Hide file tree
Showing 10 changed files with 782 additions and 63 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ Adding a new version? You'll need three changes:
[#5354](https://github.com/Kong/kubernetes-ingress-controller/pull/5354)
[#5384](https://github.com/Kong/kubernetes-ingress-controller/pull/5384)


### Fixed

- Validators of `KongPlugin` and `KongClusterPlugin` will not return `500` on
Expand All @@ -161,6 +160,9 @@ Adding a new version? You'll need three changes:
[#5401](https://github.com/Kong/kubernetes-ingress-controller/pull/5401)
- Support properly ConsumerGroups when fallback to the last valid configuration.
[#5438](https://github.com/Kong/kubernetes-ingress-controller/pull/5438)
- When specifying Gateway API Routes' `backendRef`s with namespace specified, those
refs are checked for existence and allowed if they exist.
[#5392](https://github.com/Kong/kubernetes-ingress-controller/pull/5392)

### Changed

Expand Down
94 changes: 64 additions & 30 deletions internal/dataplane/translator/backendref.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,30 @@
package translator

import (
"errors"
"fmt"

"github.com/go-logr/logr"
k8stypes "k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

"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"
)

// backendRefsToKongStateBackends takes a list of BackendRefs and returns a list of ServiceBackends.
// The backendRefs are checked for the following conditions. If any of these conditions are met, the BackendRef is
// not included in the returned list:
// - If a BackendRef is not permitted by the provided ReferenceGrantTo set,
// - If a BackendRef is not found,
// - If a BackendRef Group & Kind pair is not supported (currently only Service is supported),
// - If a BackendRef is missing a port.
// The provided client is used to retrieve the Backend referenced by the BackendRef
// to check if it exists.
func backendRefsToKongStateBackends(
logger logr.Logger,
storer store.Storer,
route client.Object,
backendRefs []gatewayapi.BackendRef,
allowed map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo,
Expand All @@ -23,42 +34,65 @@ func backendRefsToKongStateBackends(
for _, backendRef := range backendRefs {
logger := loggerForBackendRef(logger, route, backendRef)

if util.IsBackendRefGroupKindSupported(
backendRef.Group,
backendRef.Kind,
) && newRefCheckerForRoute(route, backendRef).IsRefAllowedByGrant(allowed) {
port := int32(-1)
if backendRef.Port != nil {
port = int32(*backendRef.Port)
}
namespace := route.GetNamespace()
if backendRef.Namespace != nil {
namespace = string(*backendRef.Namespace)
}
backend, err := kongstate.NewServiceBackendForService(
k8stypes.NamespacedName{
Namespace: namespace,
Name: string(backendRef.Name),
},
kongstate.PortDef{
Mode: kongstate.PortModeByNumber,
Number: port,
},
)
if err != nil {
logger.Error(err, "failed to create ServiceBackend for backendRef")
}
if backendRef.Weight != nil {
backend.SetWeight(*backendRef.Weight)
nn := client.ObjectKey{
Name: string(backendRef.Name),
Namespace: route.GetNamespace(),
}
if backendRef.Namespace != nil {
nn.Namespace = string(*backendRef.Namespace)
}

if backendRef.Kind == nil {
logger.Error(nil, "Object requested backendRef to target, but no Kind was specified, skipping...")
continue
}

var err error
switch *backendRef.Kind {
case "Service":
_, err = storer.GetService(nn.Namespace, nn.Name)
default:
err = fmt.Errorf("unsupported kind %q, only 'Service' is Supported", *backendRef.Kind)
}
if err != nil {
switch errors.As(err, &store.NotFoundError{}) {
case true:
logger.Error(nil, "Object requested backendRef to target, but it does not exist, skipping...")
case false:
logger.Error(nil, "Object requested backendRef to target, but no ReferenceGrant permits it, skipping...")
}
backends = append(backends, backend)
} else {
continue
}

if !util.IsBackendRefGroupKindSupported(backendRef.Group, backendRef.Kind) ||
!newRefCheckerForRoute(route, backendRef).IsRefAllowedByGrant(allowed) {
// we log impermissible refs rather than failing the entire rule. while we cannot actually route to
// these, we do not want a single impermissible ref to take the entire rule offline. in the case of edits,
// failing the entire rule could potentially delete routes that were previously online and in use, and
// that remain viable (because they still have some permissible backendRefs)
logger.Error(nil, "Object requested backendRef to target, but no ReferenceGrant permits it, skipping...")
continue
}

port := int32(-1)
if backendRef.Port != nil {
port = int32(*backendRef.Port)
}
backend, err := kongstate.NewServiceBackendForService(
nn,
kongstate.PortDef{
Mode: kongstate.PortModeByNumber,
Number: port,
},
)
if err != nil {
logger.Error(err, "failed to create ServiceBackend for backendRef")
continue
}
if backendRef.Weight != nil {
backend.SetWeight(*backendRef.Weight)
}
backends = append(backends, backend)
}

return backends
Expand Down
167 changes: 167 additions & 0 deletions internal/dataplane/translator/backendref_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
package translator

import (
"testing"

"github.com/go-logr/logr"
"github.com/samber/lo"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8stypes "k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

"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/builder"
)

func TestBackendRefsToKongStateBackends(t *testing.T) {
testcases := []struct {
name string
route client.Object
backendRefs []gatewayapi.BackendRef
allowed map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo
objects store.FakeObjects
expected kongstate.ServiceBackends
}{
{
name: "correct ReferenceGrant and an existing Service as backendRef returns a KongStateBackend with a Service",
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "basic-httproute",
Namespace: corev1.NamespaceDefault,
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: commonRouteSpecMock("fake-gateway-1"),
Hostnames: []gatewayapi.Hostname{
"konghq.com",
"www.konghq.com",
},
Rules: []gatewayapi.HTTPRouteRule{{
// BackendRefs are taken from the backendRefs argument.
// This is done this way because the backendRefs are
// extracted from the route's spec before.
// Potentially this could be refactored to be extracted
// in backendRefsToKongStateBackends using a type switch.
}},
},
},
backendRefs: []gatewayapi.BackendRef{
builder.NewBackendRef("fake-service").WithPort(80).Build(),
},
allowed: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{
gatewayapi.Namespace(corev1.NamespaceDefault): {
{
Group: "",
Kind: "Service",
Name: lo.ToPtr(gatewayapi.ObjectName("fake-service")),
},
},
},
objects: store.FakeObjects{
Services: []*corev1.Service{
{
ObjectMeta: metav1.ObjectMeta{
Name: "fake-service",
Namespace: corev1.NamespaceDefault,
},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
builder.NewServicePort().WithPort(80).Build(),
},
},
},
},
},
expected: func() kongstate.ServiceBackends {
svcBackend, err := kongstate.NewServiceBackend(
kongstate.ServiceBackendTypeKubernetesService,
k8stypes.NamespacedName{Namespace: corev1.NamespaceDefault, Name: "fake-service"},
kongstate.PortDef{
Mode: kongstate.PortModeByNumber,
Number: 80,
},
)
require.NoError(t, err)
return kongstate.ServiceBackends{svcBackend}
}(),
},
{
name: "no ReferenceGrant and an existing Service as backendRef doesn't return a KongStateBackend with the Service",
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "basic-httproute",
Namespace: corev1.NamespaceDefault,
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: commonRouteSpecMock("fake-gateway-1"),
Hostnames: []gatewayapi.Hostname{
"konghq.com",
"www.konghq.com",
},
},
},
allowed: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{
gatewayapi.Namespace(corev1.NamespaceDefault): {
{
Group: "",
Kind: "ImaginaryKind",
Name: lo.ToPtr(gatewayapi.ObjectName("fake-service")),
},
},
},
objects: store.FakeObjects{
Services: []*corev1.Service{
{
ObjectMeta: metav1.ObjectMeta{
Name: "fake-service",
Namespace: corev1.NamespaceDefault,
},
},
},
},
expected: kongstate.ServiceBackends{},
},
{
name: "ReferenceGrant and a non existing Service as backendRef doesn't return a KongStateBackend with the Service",
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "basic-httproute",
Namespace: corev1.NamespaceDefault,
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: commonRouteSpecMock("fake-gateway-1"),
Hostnames: []gatewayapi.Hostname{
"konghq.com",
"www.konghq.com",
},
},
},
backendRefs: []gatewayapi.BackendRef{
builder.NewBackendRef("fake-service").WithPort(80).Build(),
},
allowed: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{
gatewayapi.Namespace(corev1.NamespaceDefault): {
{
Group: "",
Kind: "Service",
Name: lo.ToPtr(gatewayapi.ObjectName("fake-service")),
},
},
},
expected: kongstate.ServiceBackends{},
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
fakestore, err := store.NewFakeStore(tc.objects)
require.NoError(t, err)
logger := logr.Discard()
ret := backendRefsToKongStateBackends(logger, fakestore, tc.route, tc.backendRefs, tc.allowed)
require.Equal(t, tc.expected, ret)
})
}
}
Loading

0 comments on commit c386d83

Please sign in to comment.