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(log): do not log errors when parentRef kind is not Gateway #6692

Merged
merged 3 commits into from
Nov 20, 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ Adding a new version? You'll need three changes:
- Fixed Kong client status check causing unnecessary `config update failed` errors
and `KongConfigurationApplyFailed` events being generated.
[#6689](https://github.com/Kong/kubernetes-ingress-controller/pull/6689)
- Do not emit error logs when group and kind of `parentRef` in a route does not
point to a `Gateway` as they can point to other kinds of resources like
`Service` when used in service mesh solutions.
[#6692](https://github.com/Kong/kubernetes-ingress-controller/pull/6692)

### Added

Expand Down
212 changes: 212 additions & 0 deletions internal/controllers/gateway/route_predicate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
package gateway

import (
"testing"

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

"github.com/kong/kubernetes-ingress-controller/v3/internal/controllers"
"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/scheme"
)

func TestIsRouteAttachedToReconciledGateway(t *testing.T) {
type httpRouteTestCase struct {
name string
objects []client.Object
route *gatewayapi.HTTPRoute
gatewayNN controllers.OptionalNamespacedName
expectedResult bool
}

kongGWClass := &gatewayapi.GatewayClass{
ObjectMeta: metav1.ObjectMeta{
Name: "kong",
},
Spec: gatewayapi.GatewayClassSpec{
ControllerName: "konghq.com/kic-gateway-controller",
},
}

anotherGWClass := &gatewayapi.GatewayClass{
ObjectMeta: metav1.ObjectMeta{
Name: "another",
},
Spec: gatewayapi.GatewayClassSpec{
ControllerName: "another",
},
}

kongGW := &gatewayapi.Gateway{
ObjectMeta: metav1.ObjectMeta{
Name: "kong",
Namespace: "default",
},
Spec: gatewayapi.GatewaySpec{
GatewayClassName: "kong",
},
}

anotherGW := &gatewayapi.Gateway{
ObjectMeta: metav1.ObjectMeta{
Name: "another",
Namespace: "default",
},
Spec: gatewayapi.GatewaySpec{
GatewayClassName: "another",
},
}

httpRouteTestCases := []httpRouteTestCase{
{
name: "single parent ref to gateway with expected class",
objects: []client.Object{
kongGW,
kongGWClass,
},
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "kong-httproute",
Namespace: "default",
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: gatewayapi.CommonRouteSpec{
ParentRefs: []gatewayapi.ParentReference{
{
Group: lo.ToPtr(gatewayapi.V1Group),
Kind: lo.ToPtr(gatewayapi.Kind("Gateway")),
Namespace: lo.ToPtr(gatewayapi.Namespace("default")),
Name: gatewayapi.ObjectName("kong"),
},
},
},
},
},
expectedResult: true,
},
{
name: "single parent ref to gateway with another class",
objects: []client.Object{
anotherGW,
anotherGWClass,
},
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "kong-httproute",
Namespace: "default",
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: gatewayapi.CommonRouteSpec{
ParentRefs: []gatewayapi.ParentReference{
{
Group: lo.ToPtr(gatewayapi.V1Group),
Kind: lo.ToPtr(gatewayapi.Kind("Gateway")),
Namespace: lo.ToPtr(gatewayapi.Namespace("default")),
Name: gatewayapi.ObjectName("another"),
},
},
},
},
},
expectedResult: false,
},
{
name: "single parent ref to specified gateway",
objects: []client.Object{
anotherGW,
anotherGWClass,
},
gatewayNN: controllers.NewOptionalNamespacedName(mo.Some(
k8stypes.NamespacedName{
Namespace: "default",
Name: "another",
},
)),
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "kong-httproute",
Namespace: "default",
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: gatewayapi.CommonRouteSpec{
ParentRefs: []gatewayapi.ParentReference{
{
Group: lo.ToPtr(gatewayapi.V1Group),
Kind: lo.ToPtr(gatewayapi.Kind("Gateway")),
Name: gatewayapi.ObjectName("another"),
},
},
},
},
},
expectedResult: true,
},
{
name: "multiple parent refs with one pointing to reconciled gateway",
objects: []client.Object{
kongGW,
kongGWClass,
},
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "kong-httproute",
Namespace: "default",
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: gatewayapi.CommonRouteSpec{
ParentRefs: []gatewayapi.ParentReference{
{
Kind: lo.ToPtr(gatewayapi.Kind("Service")),
Name: gatewayapi.ObjectName("kuma"),
},
{
Group: lo.ToPtr(gatewayapi.V1Group),
Kind: lo.ToPtr(gatewayapi.Kind("Gateway")),
Name: gatewayapi.ObjectName("kong"),
},
},
},
},
},
expectedResult: true,
},
{
name: "parent ref pointing to non-exist gateway",
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "kong-httproute",
Namespace: "default",
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: gatewayapi.CommonRouteSpec{
ParentRefs: []gatewayapi.ParentReference{
{
Group: lo.ToPtr(gatewayapi.V1Group),
Kind: lo.ToPtr(gatewayapi.Kind("Gateway")),
Name: gatewayapi.ObjectName("non-exist"),
},
},
},
},
},
expectedResult: false,
},
}

for _, tc := range httpRouteTestCases {
cl := fakeclient.NewClientBuilder().WithScheme(lo.Must(scheme.Get())).WithObjects(tc.objects...).Build()
t.Run(tc.name, func(t *testing.T) {
logger := logr.Discard()
result := IsRouteAttachedToReconciledGateway[*gatewayapi.HTTPRoute](cl, logger, tc.gatewayNN, tc.route)
require.Equal(t, tc.expectedResult, result)
},
)
}
}
11 changes: 2 additions & 9 deletions internal/controllers/gateway/route_predicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,8 @@ func IsRouteAttachedToReconciledGateway[routeT gatewayapi.RouteT](
if parentRef.Group != nil {
group = string(*parentRef.Group)
}

switch {
case kind == "Gateway" && group == gatewayapi.GroupVersion.Group:
// Check the parent gateway if the parentRef points to a gateway that is possible to be controlled by KIC.
if kind == "Gateway" && group == gatewayapi.GroupVersion.Group {
var gateway gatewayapi.Gateway
err := cl.Get(context.Background(), k8stypes.NamespacedName{Namespace: namespace, Name: string(parentRef.Name)}, &gateway)
if err != nil {
Expand All @@ -87,12 +86,6 @@ func IsRouteAttachedToReconciledGateway[routeT gatewayapi.RouteT](
if isGatewayClassControlled(&gatewayClass) {
return true
}
default:
log.Error(
fmt.Errorf("unsupported parentRef kind %s and group %s", kind, group),
"Got an unexpected kind and group when checking route's parentRefs",
)
return false
}
}

Expand Down
Loading