Skip to content

Commit

Permalink
bug: Route extension filters with different types but the same name a…
Browse files Browse the repository at this point in the history 
…nd namespace aren't correctly cached (#3388)

* Route extension filters are unstructured.Unstructured instances, so
caching them should be done with both the name and type as a key.

Signed-off-by: Lior Okman <lior.okman@sap.com>

* Moved NamespacedNameAndType to the Kubernetes helpers, and renamed it to
be clearer about what it has.

Signed-off-by: Lior Okman <lior.okman@sap.com>

* Also renamed the helper function.

Signed-off-by: Lior Okman <lior.okman@sap.com>

* Moved to the 'utils' package to be beside NamespacedName.

Signed-off-by: Lior Okman <lior.okman@sap.com>

* Renamed structure according to review, and updated the comments

Signed-off-by: Lior Okman <lior.okman@sap.com>

---------

Signed-off-by: Lior Okman <lior.okman@sap.com>
  • Loading branch information
@liorokman
liorokman authored May 20, 2024
1 parent d49f984 commit 95e2e35
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 14 deletions.
6 changes: 3 additions & 3 deletions internal/provider/kubernetes/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,9 @@ type resourceMappings struct {
// Map for storing backendRefs' NamespaceNames referred by various Route objects.
allAssociatedBackendRefs map[gwapiv1.BackendObjectReference]struct{}
// extensionRefFilters is a map of filters managed by an extension.
// The key is the namespaced name of the filter and the value is the
// The key is the namespaced name, group and kind of the filter and the value is the
// unstructured form of the resource.
extensionRefFilters map[types.NamespacedName]unstructured.Unstructured
extensionRefFilters map[utils.NamespacedNameWithGroupKind]unstructured.Unstructured
}

func newResourceMapping() *resourceMappings {
Expand All @@ -143,7 +143,7 @@ func newResourceMapping() *resourceMappings {
allAssociatedTCPRoutes: map[string]struct{}{},
allAssociatedUDPRoutes: map[string]struct{}{},
allAssociatedBackendRefs: map[gwapiv1.BackendObjectReference]struct{}{},
extensionRefFilters: map[types.NamespacedName]unstructured.Unstructured{},
extensionRefFilters: map[utils.NamespacedNameWithGroupKind]unstructured.Unstructured{},
}
}

Expand Down
27 changes: 20 additions & 7 deletions internal/provider/kubernetes/routes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,17 @@ func (r *gatewayAPIReconciler) processGRPCRoutes(ctx context.Context, gatewayNam
if filter.Type == gwapiv1.GRPCRouteFilterExtensionRef {
// NOTE: filters must be in the same namespace as the GRPCRoute
// Check if it's a Kind managed by an extension and add to resourceTree
key := types.NamespacedName{
Namespace: grpcRoute.Namespace,
Name: string(filter.ExtensionRef.Name),
key := utils.NamespacedNameWithGroupKind{
NamespacedName: types.NamespacedName{
Namespace: grpcRoute.Namespace,
Name: string(filter.ExtensionRef.Name),
},
GroupKind: schema.GroupKind{
Group: string(filter.ExtensionRef.Group),
Kind: string(filter.ExtensionRef.Kind),
},
}

extRefFilter, ok := resourceMap.extensionRefFilters[key]
if !ok {
r.log.Error(
Expand Down Expand Up @@ -229,7 +236,7 @@ func (r *gatewayAPIReconciler) processHTTPRoutes(ctx context.Context, gatewayNam
}
for i := range extensionRefFilters {
filter := extensionRefFilters[i]
resourceMap.extensionRefFilters[utils.NamespacedName(&filter)] = filter
resourceMap.extensionRefFilters[utils.GetNamespacedNameWithGroupKind(&filter)] = filter
}

if err := r.client.List(ctx, httpRouteList, &client.ListOptions{
Expand Down Expand Up @@ -367,9 +374,15 @@ func (r *gatewayAPIReconciler) processHTTPRoutes(ctx context.Context, gatewayNam
} else if filter.Type == gwapiv1.HTTPRouteFilterExtensionRef {
// NOTE: filters must be in the same namespace as the HTTPRoute
// Check if it's a Kind managed by an extension and add to resourceTree
key := types.NamespacedName{
Namespace: httpRoute.Namespace,
Name: string(filter.ExtensionRef.Name),
key := utils.NamespacedNameWithGroupKind{
NamespacedName: types.NamespacedName{
Namespace: httpRoute.Namespace,
Name: string(filter.ExtensionRef.Name),
},
GroupKind: schema.GroupKind{
Group: string(filter.ExtensionRef.Group),
Kind: string(filter.ExtensionRef.Kind),
},
}
extRefFilter, ok := resourceMap.extensionRefFilters[key]
if !ok {
Expand Down
110 changes: 106 additions & 4 deletions internal/provider/kubernetes/routes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,102 @@ func TestProcessHTTPRoutes(t *testing.T) {
},
expected: true,
},
{
name: "httproute with extension filter multiple types same name",
routes: []*gwapiv1.HTTPRoute{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: httpRouteNS,
Name: "test",
},
Spec: gwapiv1.HTTPRouteSpec{
CommonRouteSpec: gwapiv1.CommonRouteSpec{
ParentRefs: []gwapiv1.ParentReference{
{
Name: "test",
},
},
},
Rules: []gwapiv1.HTTPRouteRule{
{
Matches: []gwapiv1.HTTPRouteMatch{
{
Path: &gwapiv1.HTTPPathMatch{
Type: ptr.To(gwapiv1.PathMatchPathPrefix),
Value: ptr.To("/"),
},
},
},
Filters: []gwapiv1.HTTPRouteFilter{
{
Type: gwapiv1.HTTPRouteFilterExtensionRef,
ExtensionRef: &gwapiv1.LocalObjectReference{
Group: gwapiv1.Group("gateway.example.io"),
Kind: gwapiv1.Kind("Bar"),
Name: gwapiv1.ObjectName("test"),
},
},
{
Type: gwapiv1.HTTPRouteFilterExtensionRef,
ExtensionRef: &gwapiv1.LocalObjectReference{
Group: gwapiv1.Group("gateway.example.io"),
Kind: gwapiv1.Kind("Foo"),
Name: gwapiv1.ObjectName("test"),
},
},
},
BackendRefs: []gwapiv1.HTTPBackendRef{
{
BackendRef: gwapiv1.BackendRef{
BackendObjectReference: gwapiv1.BackendObjectReference{
Group: gatewayapi.GroupPtr(corev1.GroupName),
Kind: gatewayapi.KindPtr(gatewayapi.KindService),
Name: "test",
},
},
},
},
},
},
},
},
},
extensionFilters: []*unstructured.Unstructured{
{
Object: map[string]interface{}{
"apiVersion": "gateway.example.io/v1alpha1",
"kind": "Bar",
"metadata": map[string]interface{}{
"name": "test",
"namespace": httpRouteNS,
},
},
},
{
Object: map[string]interface{}{
"apiVersion": "gateway.example.io/v1alpha1",
"kind": "Foo",
"metadata": map[string]interface{}{
"name": "test",
"namespace": httpRouteNS,
},
},
},
},
extensionAPIGroups: []schema.GroupVersionKind{
{
Group: "gateway.example.io",
Version: "v1alpha1",
Kind: "Bar",
},
{
Group: "gateway.example.io",
Version: "v1alpha1",
Kind: "Foo",
},
},
expected: true,
},
{
name: "httproute with one filter_from_extension",
routes: []*gwapiv1.HTTPRoute{
Expand Down Expand Up @@ -294,10 +390,16 @@ func TestProcessHTTPRoutes(t *testing.T) {
// Ensure the resource tree and map are as expected.
require.Equal(t, tc.routes, resourceTree.HTTPRoutes)
if tc.extensionFilters != nil {
for i, filter := range tc.extensionFilters {
key := types.NamespacedName{
Namespace: tc.routes[i].Namespace,
Name: filter.GetName(),
for _, filter := range tc.extensionFilters {
key := utils.NamespacedNameWithGroupKind{
NamespacedName: types.NamespacedName{
Namespace: tc.routes[0].Namespace,
Name: filter.GetName(),
},
GroupKind: schema.GroupKind{
Group: filter.GroupVersionKind().Group,
Kind: filter.GroupVersionKind().Kind,
},
}
require.Equal(t, *filter, resourceMap.extensionRefFilters[key])
}
Expand Down
20 changes: 20 additions & 0 deletions internal/utils/misc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,30 @@ import (
"hash/fnv"
"strings"

"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
)

type NamespacedNameWithGroupKind struct {
types.NamespacedName
schema.GroupKind
}

// GetNamespacedNameWithGroupKind creates and returns object's NamespacedNameWithGroupKind.
func GetNamespacedNameWithGroupKind(obj client.Object) NamespacedNameWithGroupKind {
return NamespacedNameWithGroupKind{
NamespacedName: types.NamespacedName{
Namespace: obj.GetNamespace(),
Name: obj.GetName(),
},
GroupKind: schema.GroupKind{
Group: obj.GetObjectKind().GroupVersionKind().GroupKind().Group,
Kind: obj.GetObjectKind().GroupVersionKind().GroupKind().Kind,
},
}
}

// NamespacedName creates and returns object's NamespacedName.
func NamespacedName(obj client.Object) types.NamespacedName {
return types.NamespacedName{
Expand Down

0 comments on commit 95e2e35

Please sign in to comment.