Skip to content

Commit

Permalink
fix(controllers) skip routes bound to excluded GWs (#5642)
Browse files Browse the repository at this point in the history
In single-Gateway mode, properly skip route parent references for other
Gateways when determining whether to include a route.
  • Loading branch information
rainest committed Feb 22, 2024
1 parent 18c2bdc commit 17c6968
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 3 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ Adding a new version? You'll need three changes:
- When managed Kong gateways are OSS edition, KIC will not apply licenses to
the Kong gateway instances to avoid invalid configurations.
[#5640](https://github.com/Kong/kubernetes-ingress-controller/pull/5640)
- Fixed an issue where single-Gateway mode did not actually filter out routes
associated with other Gateways in the controller class.
[#5642](https://github.com/Kong/kubernetes-ingress-controller/pull/5642)

## [3.1.0]

Expand Down
10 changes: 7 additions & 3 deletions internal/controllers/gateway/route_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,13 @@ func getSupportedGatewayForRoute[T gatewayapi.RouteT](ctx context.Context, logge
// If the flag `--gateway-to-reconcile` is set, KIC will only reconcile the specified gateway.
// https://github.com/Kong/kubernetes-ingress-controller/issues/5322
if gatewayToReconcile, ok := specifiedGW.Get(); ok {
namespace = gatewayToReconcile.Namespace
name = gatewayToReconcile.Name

parentNamespace := route.GetNamespace()
if parentRef.Namespace != nil {
parentNamespace = string(*parentRef.Namespace)
}
if !(parentNamespace == gatewayToReconcile.Namespace && string(parentRef.Name) == gatewayToReconcile.Name) {
continue
}
}

// pull the Gateway object from the cached client
Expand Down
129 changes: 129 additions & 0 deletions internal/controllers/gateway/route_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/uuid"
"github.com/samber/lo"
"github.com/samber/mo"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -1342,6 +1343,134 @@ func TestGetSupportedGatewayForRoute(t *testing.T) {
_, err := getSupportedGatewayForRoute(context.Background(), logr.Discard(), fakeClient, bustedParentHTTPRoute, controllers.OptionalNamespacedName{})
require.Equal(t, fmt.Errorf("unsupported parent kind %s/%s", string(badGroup), string(badKind)), err)
})

t.Run("single Gateway", func(t *testing.T) {
namedGateway := func(name string) *gatewayapi.Gateway {
return &gatewayapi.Gateway{
TypeMeta: gatewayapi.V1GatewayTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: "test-namespace",
UID: "ce7f0678-f59a-483c-80d1-243d3738d22c",
},
Spec: gatewayapi.GatewaySpec{
GatewayClassName: "test-gatewayclass",
Listeners: builder.NewListener("http").WithPort(80).HTTP().IntoSlice(),
},
Status: gatewayapi.GatewayStatus{
Listeners: []gatewayapi.ListenerStatus{
{
Name: "http",
Conditions: []metav1.Condition{
{
Type: string(gatewayapi.ListenerConditionProgrammed),
Status: metav1.ConditionTrue,
},
},
SupportedKinds: supportedRouteGroupKinds,
},
},
},
}
}

basicHTTPRoute := func(gateway string) *gatewayapi.HTTPRoute {
return &gatewayapi.HTTPRoute{
TypeMeta: gatewayapi.V1GatewayTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Name: "basic-httproute",
Namespace: "test-namespace",
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: gatewayapi.CommonRouteSpec{
ParentRefs: []gatewayapi.ParentReference{
{
Group: &goodGroup,
Kind: &goodKind,
Name: gatewayapi.ObjectName(gateway),
},
},
},
Rules: []gatewayapi.HTTPRouteRule{
{
BackendRefs: []gatewayapi.HTTPBackendRef{
builder.NewHTTPBackendRef("fake-service").WithPort(80).Build(),
},
},
},
},
}
}

tests := []struct {
name string
route *gatewayapi.HTTPRoute
expected []expected
expectedErr error
objects []client.Object
}{
{
name: "HTTPRoute with bound Gateway parent is accepted",
route: basicHTTPRoute("good-gateway"),
objects: []client.Object{
namedGateway("good-gateway"),
gatewayClass,
namespace,
},
expected: []expected{
{
condition: routeConditionAccepted(metav1.ConditionTrue, gatewayapi.RouteReasonAccepted),
},
},
},
{
name: "HTTPRoute with other parent Gateway finds no matching Gateways",
route: basicHTTPRoute("bad-gateway"),
objects: []client.Object{
namedGateway("bad-gateway"),
gatewayClass,
namespace,
},
expected: []expected{},
expectedErr: fmt.Errorf("no supported Gateway found for route"),
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
fakeClient := fakeclient.
NewClientBuilder().
WithScheme(scheme.Scheme).
WithObjects(tt.objects...).
Build()

got, err := getSupportedGatewayForRoute(
context.Background(),
logr.Discard(),
fakeClient,
tt.route,
mo.Some(k8stypes.NamespacedName{
Namespace: namespace.Name,
Name: "good-gateway",
}),
)
if tt.expectedErr != nil {
require.Equal(t, tt.expectedErr, err)
} else {
require.NoError(t, err)
}
require.Len(t, got, len(tt.expected))

for i := range got {
assert.Equalf(t, "test-namespace", got[i].gateway.Namespace, "gateway namespace #%d", i)
assert.Equalf(t, "good-gateway", got[i].gateway.Name, "gateway name #%d", i)
assert.Equalf(t, tt.expected[i].listenerName, got[i].listenerName, "listenerName #%d", i)
assert.Equalf(t, tt.expected[i].condition, got[i].condition, "condition #%d", i)
}
})
}
})
}

func TestEnsureParentsProgrammedCondition(t *testing.T) {
Expand Down

0 comments on commit 17c6968

Please sign in to comment.