From 0acb4dae13ffbbd66ea6380c719f56f59a43f905 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Thu, 11 Jul 2024 09:03:45 -0700 Subject: [PATCH] chore(refs) add grant check logs (#6302) Add trace logs for ReferenceGrant checks. Add comments for ReferenceGrant utilities. Consolidate duplicated utility function. --- internal/dataplane/kongstate/kongstate.go | 59 +++++-- .../dataplane/kongstate/kongstate_test.go | 2 +- internal/dataplane/translator/backendref.go | 2 +- .../dataplane/translator/translate_utils.go | 38 +---- .../translator/translate_utils_test.go | 148 ---------------- .../translator/wrappers_refchecker_test.go | 3 +- internal/gatewayapi/references.go | 107 +++++++++++- internal/gatewayapi/references_test.go | 158 ++++++++++++++++++ 8 files changed, 317 insertions(+), 200 deletions(-) create mode 100644 internal/gatewayapi/references_test.go diff --git a/internal/dataplane/kongstate/kongstate.go b/internal/dataplane/kongstate/kongstate.go index 5cc5b7fddc..6342f738d1 100644 --- a/internal/dataplane/kongstate/kongstate.go +++ b/internal/dataplane/kongstate/kongstate.go @@ -19,6 +19,7 @@ import ( "github.com/kong/kubernetes-ingress-controller/v3/internal/annotations" "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/failures" "github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi" + "github.com/kong/kubernetes-ingress-controller/v3/internal/logging" "github.com/kong/kubernetes-ingress-controller/v3/internal/store" "github.com/kong/kubernetes-ingress-controller/v3/internal/util" kongv1alpha1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1alpha1" @@ -350,18 +351,18 @@ func (ks *KongState) getPluginRelations(cacheStore store.Storer, log logr.Logger RouteRelation entityRelationType = iota ServiceRelation entityRelationType = iota ) - addRelation := func(referer client.Object, plugin annotations.NamespacedKongPlugin, identifier string, t entityRelationType) { + addRelation := func(referrer client.Object, plugin annotations.NamespacedKongPlugin, identifier string, t entityRelationType) { // There are 2 types of KongPlugin references: local and remote. - // A local reference is one where the KongPlugin is in the same namespace as the referer. + // A local reference is one where the KongPlugin is in the same namespace as the referrer. // A remote reference is one where the KongPlugin is in a different namespace. // By default a KongPlugin is considered local. // If the plugin has a namespace specified, it is considered remote. // - // The referer is the entity that the KongPlugin is associated with. + // The referrer is the entity that the KongPlugin is associated with. // // Code in buildPlugins() will combine plugin associations into // multi-entity plugins within the local namespace - namespace, err := extractReferredPluginNamespace(cacheStore, referer, plugin) + namespace, err := extractReferredPluginNamespace(log, cacheStore, referrer, plugin) if err != nil { log.Error(err, "could not bind requested plugin", "plugin", plugin.Name, "namespace", plugin.Namespace) return @@ -435,17 +436,32 @@ type pluginReference struct { Name string } -func isRemotePluginReferenceAllowed(s store.Storer, r pluginReference) error { +func isRemotePluginReferenceAllowed(log logr.Logger, s store.Storer, r pluginReference) error { // remote plugin. considered part of this namespace if a suitable ReferenceGrant exists grants, err := s.ListReferenceGrants() if err != nil { return fmt.Errorf("could not retrieve ReferenceGrants from store when building plugin relations map: %w", err) } - allowed := gatewayapi.GetPermittedForReferenceGrantFrom(gatewayapi.ReferenceGrantFrom{ - Group: gatewayapi.Group(r.Referrer.GetObjectKind().GroupVersionKind().Group), - Kind: gatewayapi.Kind(r.Referrer.GetObjectKind().GroupVersionKind().Kind), - Namespace: gatewayapi.Namespace(r.Referrer.GetNamespace()), - }, grants) + allowed := gatewayapi.GetPermittedForReferenceGrantFrom( + log, + gatewayapi.ReferenceGrantFrom{ + Group: gatewayapi.Group(r.Referrer.GetObjectKind().GroupVersionKind().Group), + Kind: gatewayapi.Kind(r.Referrer.GetObjectKind().GroupVersionKind().Kind), + Namespace: gatewayapi.Namespace(r.Referrer.GetNamespace()), + }, + grants, + ) + + // TODO https://github.com/Kong/kubernetes-ingress-controller/issues/6000 + // Our reference checking code wasn't originally designed to handle multiple object types and relationships and + // wasn't designed with much guidance for future usage. It expects something akin to the BackendRef section of an + // HTTPRoute or similar, since that's what it was originally designed for. A future simplified model should probably + // work with source and target client.Object resources derived from the particular resources' reference specs, as + // those reference formats aren't necessarily consistent. We should possibly push for a standard upstream utility as + // part of https://github.com/kubernetes/enhancements/issues/3766 if it proceeds further, as this is can be difficult + // to wrap your head around when deep in the code that's checking an individual use case. A standard model for "these + // are the inputs and outputs for a reference grant check, derive them from your spec" should help avoid inconsistency + // in check implementation. // we don't have a full plugin resource here for the grant checker, so we build a fake one with the correct // name and namespace. @@ -453,9 +469,19 @@ func isRemotePluginReferenceAllowed(s store.Storer, r pluginReference) error { Namespace: &r.Namespace, Name: r.Name, } + // Because we should check whether it is allowed to refer FROM the referrer TO the plugin here, // we should put the referrer on the "target" and the plugin on the "backendRef". - if !gatewayapi.NewRefCheckerForKongPlugin(r.Referrer, pluginReference).IsRefAllowedByGrant(allowed) { + + log.V(logging.TraceLevel).Info("requested grant to plugins", + "from-namespace", r.Referrer.GetNamespace(), + "from-group", r.Referrer.GetObjectKind().GroupVersionKind().Group, + "from-kind", r.Referrer.GetObjectKind().GroupVersionKind().Kind, + "to-namespace", r.Namespace, + "to-name", r.Name, + ) + + if !gatewayapi.NewRefCheckerForKongPlugin(log, r.Referrer, pluginReference).IsRefAllowedByGrant(allowed) { return fmt.Errorf("no grant found for %s in %s to plugin %s in %s requested remote KongPlugin bind", r.Referrer.GetObjectKind().GroupVersionKind().Kind, r.Referrer.GetNamespace(), r.Name, r.Namespace) } @@ -721,8 +747,8 @@ func (ks *KongState) getPluginRelatedEntitiesRef(cacheStore store.Storer, log lo RelatedEntities: map[string]RelatedEntitiesRef{}, RouteAttachedService: map[string]*Service{}, } - addRelation := func(referer client.Object, plugin annotations.NamespacedKongPlugin, entity any) { - namespace, err := extractReferredPluginNamespace(cacheStore, referer, plugin) + addRelation := func(referrer client.Object, plugin annotations.NamespacedKongPlugin, entity any) { + namespace, err := extractReferredPluginNamespace(log, cacheStore, referrer, plugin) if err != nil { log.Error(err, "could not bind requested plugin", "plugin", plugin.Name, "namespace", plugin.Namespace) return @@ -783,21 +809,22 @@ func (ks *KongState) getPluginRelatedEntitiesRef(cacheStore store.Storer, log lo } func extractReferredPluginNamespace( - cacheStore store.Storer, referrer client.Object, plugin annotations.NamespacedKongPlugin, + log logr.Logger, cacheStore store.Storer, referrer client.Object, plugin annotations.NamespacedKongPlugin, ) (string, error) { // There are 2 types of KongPlugin references: local and remote. - // A local reference is one where the KongPlugin is in the same namespace as the referer. + // A local reference is one where the KongPlugin is in the same namespace as the referrer. // A remote reference is one where the KongPlugin is in a different namespace. // By default a KongPlugin is considered local. // If the plugin has a namespace specified, it is considered remote. // - // The referer is the entity that the KongPlugin is associated with. + // The referrer is the entity that the KongPlugin is associated with. if plugin.Namespace == "" { return referrer.GetNamespace(), nil } // remote KongPlugin, permitted if ReferenceGrant allows. err := isRemotePluginReferenceAllowed( + log, cacheStore, pluginReference{ Referrer: referrer, diff --git a/internal/dataplane/kongstate/kongstate_test.go b/internal/dataplane/kongstate/kongstate_test.go index 6d351ca9bd..fd243daf21 100644 --- a/internal/dataplane/kongstate/kongstate_test.go +++ b/internal/dataplane/kongstate/kongstate_test.go @@ -1632,7 +1632,7 @@ func TestIsRemotePluginReferenceAllowed(t *testing.T) { ReferenceGrants: tc.referenceGrants, }) require.NoError(t, err) - err = isRemotePluginReferenceAllowed(s, pluginReference{ + err = isRemotePluginReferenceAllowed(logr.Discard(), s, pluginReference{ Referrer: tc.referrer, Namespace: tc.pluginNamespace, Name: tc.pluginName, diff --git a/internal/dataplane/translator/backendref.go b/internal/dataplane/translator/backendref.go index 781fb9c2ab..770b14c52e 100644 --- a/internal/dataplane/translator/backendref.go +++ b/internal/dataplane/translator/backendref.go @@ -65,7 +65,7 @@ func backendRefsToKongStateBackends( } if !util.IsBackendRefGroupKindSupported(backendRef.Group, backendRef.Kind) || - !gatewayapi.NewRefCheckerForRoute(route, backendRef).IsRefAllowedByGrant(allowed) { + !gatewayapi.NewRefCheckerForRoute(logger, 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 diff --git a/internal/dataplane/translator/translate_utils.go b/internal/dataplane/translator/translate_utils.go index 6a54498850..c9e7c87939 100644 --- a/internal/dataplane/translator/translate_utils.go +++ b/internal/dataplane/translator/translate_utils.go @@ -2,7 +2,6 @@ package translator import ( "fmt" - "reflect" "github.com/go-logr/logr" "github.com/kong/go-kong/kong" @@ -42,29 +41,6 @@ func convertGatewayMatchHeadersToKongRouteMatchHeaders(headers []gatewayapi.HTTP return convertedHeaders, nil } -// GetPermittedForReferenceGrantFrom takes a ReferenceGrant From (a namespace, group, and kind) and returns a map -// from a namespace to a slice of ReferenceGrant Tos. When a To is included in the slice, the key namespace has a -// ReferenceGrant with those Tos and the input From. -func GetPermittedForReferenceGrantFrom( - from gatewayapi.ReferenceGrantFrom, - grants []*gatewayapi.ReferenceGrant, -) map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo { - allowed := make(map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo) - // loop over all From values in all grants. if we find a match, add all Tos to the list of Tos allowed for the - // grant namespace. this technically could add duplicate copies of the Tos if there are duplicate Froms (it makes - // no sense to add them, but it's allowed), but duplicate Tos are harmless (we only care about having at least one - // matching To when checking if a ReferenceGrant allows a reference) - for _, grant := range grants { - for _, otherFrom := range grant.Spec.From { - if reflect.DeepEqual(from, otherFrom) { - allowed[gatewayapi.Namespace(grant.ObjectMeta.Namespace)] = append(allowed[gatewayapi.Namespace(grant.ObjectMeta.Namespace)], grant.Spec.To...) - } - } - } - - return allowed -} - // generateKongServiceFromBackendRefWithName translates backendRefs into a Kong service for use with the // rules generated from a Gateway APIs route. The service name is provided by the caller. func generateKongServiceFromBackendRefWithName( @@ -82,11 +58,15 @@ func generateKongServiceFromBackendRefWithName( if err != nil { return kongstate.Service{}, fmt.Errorf("could not retrieve ReferenceGrants for %s: %w", objName, err) } - allowed := GetPermittedForReferenceGrantFrom(gatewayapi.ReferenceGrantFrom{ - Group: gatewayapi.Group(route.GetObjectKind().GroupVersionKind().Group), - Kind: gatewayapi.Kind(route.GetObjectKind().GroupVersionKind().Kind), - Namespace: gatewayapi.Namespace(route.GetNamespace()), - }, grants) + allowed := gatewayapi.GetPermittedForReferenceGrantFrom( + logger, + gatewayapi.ReferenceGrantFrom{ + Group: gatewayapi.Group(route.GetObjectKind().GroupVersionKind().Group), + Kind: gatewayapi.Kind(route.GetObjectKind().GroupVersionKind().Kind), + Namespace: gatewayapi.Namespace(route.GetNamespace()), + }, + grants, + ) backends := backendRefsToKongStateBackends(logger, storer, route, backendRefs, allowed) diff --git a/internal/dataplane/translator/translate_utils_test.go b/internal/dataplane/translator/translate_utils_test.go index 9bfb624259..97c6d8cf3a 100644 --- a/internal/dataplane/translator/translate_utils_test.go +++ b/internal/dataplane/translator/translate_utils_test.go @@ -108,154 +108,6 @@ func TestConvertGatewayMatchHeadersToKongRouteMatchHeaders(t *testing.T) { } } -func TestGetPermittedForReferenceGrantFrom(t *testing.T) { - grants := []*gatewayapi.ReferenceGrant{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: uuid.NewString(), - Namespace: "fitrat", - }, - Spec: gatewayapi.ReferenceGrantSpec{ - From: []gatewayapi.ReferenceGrantFrom{ - { - Group: gatewayapi.Group("gateway.networking.k8s.io"), - Kind: gatewayapi.Kind("TCPRoute"), - Namespace: gatewayapi.Namespace("garbage"), - }, - { - Group: gatewayapi.Group("gateway.networking.k8s.io"), - Kind: gatewayapi.Kind("TCPRoute"), - Namespace: gatewayapi.Namespace("behbudiy"), - }, - { - Group: gatewayapi.Group("gateway.networking.k8s.io"), - Kind: gatewayapi.Kind("TCPRoute"), - Namespace: gatewayapi.Namespace("qodiriy"), - }, - }, - To: []gatewayapi.ReferenceGrantTo{ - { - Group: gatewayapi.Group(""), - Kind: gatewayapi.Kind("GrantOne"), - }, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: uuid.NewString(), - Namespace: "cholpon", - }, - Spec: gatewayapi.ReferenceGrantSpec{ - From: []gatewayapi.ReferenceGrantFrom{ - { - Group: gatewayapi.Group("gateway.networking.k8s.io"), - Kind: gatewayapi.Kind("UDPRoute"), - Namespace: gatewayapi.Namespace("behbudiy"), - }, - { - Group: gatewayapi.Group("gateway.networking.k8s.io"), - Kind: gatewayapi.Kind("TCPRoute"), - Namespace: gatewayapi.Namespace("qodiriy"), - }, - }, - To: []gatewayapi.ReferenceGrantTo{ - { - Group: gatewayapi.Group(""), - Kind: gatewayapi.Kind("GrantTwo"), - }, - }, - }, - }, - } - tests := []struct { - msg string - from gatewayapi.ReferenceGrantFrom - result map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo - }{ - { - msg: "no matches whatsoever", - from: gatewayapi.ReferenceGrantFrom{ - Group: gatewayapi.Group("invalid.example"), - Kind: gatewayapi.Kind("invalid"), - Namespace: gatewayapi.Namespace("invalid"), - }, - result: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{}, - }, - { - msg: "non-matching namespace", - from: gatewayapi.ReferenceGrantFrom{ - Group: gatewayapi.Group("gateway.networking.k8s.io"), - Kind: gatewayapi.Kind("UDPRoute"), - Namespace: gatewayapi.Namespace("niyazi"), - }, - result: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{}, - }, - { - msg: "non-matching kind", - from: gatewayapi.ReferenceGrantFrom{ - Group: gatewayapi.Group("gateway.networking.k8s.io"), - Kind: gatewayapi.Kind("TLSRoute"), - Namespace: gatewayapi.Namespace("behbudiy"), - }, - result: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{}, - }, - { - msg: "non-matching group", - from: gatewayapi.ReferenceGrantFrom{ - Group: gatewayapi.Group("invalid.example"), - Kind: gatewayapi.Kind("UDPRoute"), - Namespace: gatewayapi.Namespace("behbudiy"), - }, - result: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{}, - }, - { - msg: "single match", - from: gatewayapi.ReferenceGrantFrom{ - Group: gatewayapi.Group("gateway.networking.k8s.io"), - Kind: gatewayapi.Kind("UDPRoute"), - Namespace: gatewayapi.Namespace("behbudiy"), - }, - result: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{ - "cholpon": { - { - Group: gatewayapi.Group(""), - Kind: gatewayapi.Kind("GrantTwo"), - }, - }, - }, - }, - { - msg: "multiple matches", - from: gatewayapi.ReferenceGrantFrom{ - Group: gatewayapi.Group("gateway.networking.k8s.io"), - Kind: gatewayapi.Kind("TCPRoute"), - Namespace: gatewayapi.Namespace("qodiriy"), - }, - result: map[gatewayapi.Namespace][]gatewayapi.ReferenceGrantTo{ - "cholpon": { - { - Group: gatewayapi.Group(""), - Kind: gatewayapi.Kind("GrantTwo"), - }, - }, - "fitrat": { - { - Group: gatewayapi.Group(""), - Kind: gatewayapi.Kind("GrantOne"), - }, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.msg, func(t *testing.T) { - result := GetPermittedForReferenceGrantFrom(tt.from, grants) - assert.Equal(t, tt.result, result) - }) - } -} - func TestGenerateKongServiceFromBackendRef(t *testing.T) { grants := []*gatewayapi.ReferenceGrant{ { diff --git a/internal/dataplane/translator/wrappers_refchecker_test.go b/internal/dataplane/translator/wrappers_refchecker_test.go index 9a18525c5a..f92300066a 100644 --- a/internal/dataplane/translator/wrappers_refchecker_test.go +++ b/internal/dataplane/translator/wrappers_refchecker_test.go @@ -3,6 +3,7 @@ package translator import ( "testing" + "github.com/go-logr/logr" "github.com/samber/lo" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -111,7 +112,7 @@ func TestRefChecker_IsRefAllowedByGrant(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - rc := gatewayapi.NewRefCheckerForRoute(tc.route, tc.backendRef) + rc := gatewayapi.NewRefCheckerForRoute(logr.Discard(), tc.route, tc.backendRef) result := rc.IsRefAllowedByGrant(tc.allowedRefs) require.Equal(t, tc.expected, result) }) diff --git a/internal/gatewayapi/references.go b/internal/gatewayapi/references.go index c971127398..ae331f7712 100644 --- a/internal/gatewayapi/references.go +++ b/internal/gatewayapi/references.go @@ -1,9 +1,13 @@ package gatewayapi import ( + "fmt" "reflect" + "github.com/go-logr/logr" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kong/kubernetes-ingress-controller/v3/internal/logging" ) // RefChecker is a wrapper type that facilitates checking whether a backendRef is allowed @@ -11,20 +15,23 @@ import ( type RefChecker[T BackendRefT] struct { target client.Object backendRef T + log logr.Logger } // NewRefCheckerForRoute returns a RefChecker for the provided route and backendRef. -func NewRefCheckerForRoute[T BackendRefT](route client.Object, ref T) RefChecker[T] { +func NewRefCheckerForRoute[T BackendRefT](log logr.Logger, route client.Object, ref T) RefChecker[T] { return RefChecker[T]{ target: route, backendRef: ref, + log: log.WithName("refchecker"), } } -func NewRefCheckerForKongPlugin[T BackendRefT](target client.Object, requester T) RefChecker[T] { +func NewRefCheckerForKongPlugin[T BackendRefT](log logr.Logger, target client.Object, requester T) RefChecker[T] { return RefChecker[T]{ target: target, backendRef: requester, + log: log.WithName("refchecker"), } } @@ -55,7 +62,9 @@ func (rc RefChecker[T]) IsRefAllowedByGrant( return true } + rc.log.V(logging.TraceLevel).Info("checking reference for BackendRef") return isRefAllowedByGrant( + rc.log, (*string)(br.Namespace), (string)(br.Name), (string)(*br.Group), @@ -68,7 +77,9 @@ func (rc RefChecker[T]) IsRefAllowedByGrant( return true } + rc.log.V(logging.TraceLevel).Info("checking reference to Secret") return isRefAllowedByGrant( + rc.log, (*string)(br.Namespace), (string)(br.Name), (string)(*br.Group), @@ -77,26 +88,58 @@ func (rc RefChecker[T]) IsRefAllowedByGrant( ) case PluginLabelReference: + rc.log.V(logging.TraceLevel).Info("checking reference to KongPlugin") if br.Namespace == nil { return true } return isRefAllowedByGrant( + rc.log, (br.Namespace), (br.Name), "configuration.konghq.com", // TODO https://github.com/Kong/kubernetes-ingress-controller/issues/6000 "KongPlugin", // TODO These magic strings should become unnecessary once we work with client.Object allowedRefs, ) - } + // TODO this is somewhat like the desired end state of issue #6000, but isn't viable at the moment because we + // don't actually have the From object here, we have the reference that describes it. the assertion here always + // fails because we've already extracted a type string into "br" from the reference. were we constructing an + // object from the reference that'd work, but refactoring that without a bunch of cases to build the object + // isn't obvious. + + // default: + // if obj, ok := br.(client.Object); ok { + // if obj.GetNamespace() == "" { + // return true + // } + // rc.log.V(logging.TraceLevel).Info(fmt.Sprintf("checking reference from client.Object (actual type %t", br)) + + // return isRefAllowedByGrant( + // rc.log, + // lo.ToPtr(obj.GetNamespace()), + // obj.GetName(), + // obj.GetObjectKind().GroupVersionKind().Group, + // obj.GetObjectKind().GroupVersionKind().Kind, + // allowedRefs, + // ) + // } else { + // rc.log.V(logging.TraceLevel).Info(fmt.Sprintf("could not check reference for non-client.Object (actual type %t)", br)) + // } + + } return false } +// TODO https://github.com/Kong/kubernetes-ingress-controller/issues/6000 +// this does not indicate the relationship between the NN+GK args and the allowed arg, which makes it rather +// difficult to understand + // isRefAllowedByGrant checks if backendRef is permitted by the provided namespace-indexed ReferenceGrantTo set: allowed. // allowed is assumed to contain Tos that only match the backendRef's parent's From, as returned by // GetPermittedForReferenceGrantFrom. func isRefAllowedByGrant( + log logr.Logger, namespace *string, name string, group string, @@ -107,19 +150,40 @@ func isRefAllowedByGrant( // local references are always fine return true } - for _, to := range allowed[Namespace(*namespace)] { + scoped := log.WithValues( + "namespace", *namespace, + "requested-group", group, + "requested-kind", kind, + "requested-name", name, + ) + scoped.V(logging.TraceLevel).Info(fmt.Sprintf("checking %d entries for namespace", len(allowed[Namespace(*namespace)]))) + for i, to := range allowed[Namespace(*namespace)] { + toName := "" + if to.Name != nil { + toName = string(*to.Name) + } + scoped = scoped.WithValues( + "to-group", to.Group, + "to-kind", to.Kind, + "to-name", toName, + "to-index", i, + ) if string(to.Group) == group && string(to.Kind) == kind { if to.Name != nil { if string(*to.Name) == name { + scoped.V(logging.TraceLevel).Info("requested ref allowed by grant") return true } } else { // if no referent name specified, matching group/kind is sufficient + scoped.V(logging.TraceLevel).Info("requested ref allowed by grant To") return true } } + scoped.V(logging.TraceLevel).Info("grant To did not match requested ref target") } + scoped.V(logging.TraceLevel).Info("no grants matching requested ref target") return false } @@ -127,6 +191,7 @@ func isRefAllowedByGrant( // from a namespace to a slice of ReferenceGrant Tos. When a To is included in the slice, the key namespace has a // ReferenceGrant with those Tos and the input From. func GetPermittedForReferenceGrantFrom( + log logr.Logger, from ReferenceGrantFrom, grants []*ReferenceGrant, ) map[Namespace][]ReferenceGrantTo { @@ -135,10 +200,44 @@ func GetPermittedForReferenceGrantFrom( // grant namespace. this technically could add duplicate copies of the Tos if there are duplicate Froms (it makes // no sense to add them, but it's allowed), but duplicate Tos are harmless (we only care about having at least one // matching To when checking if a ReferenceGrant allows a reference) + scoped := log.WithName("refchecker") for _, grant := range grants { for _, otherFrom := range grant.Spec.From { if reflect.DeepEqual(from, otherFrom) { + scoped.V(logging.TraceLevel).Info("grant from equal, adding to allowed", + "grant-namespace", grant.Namespace, + "grant-name", grant.Name, + "grant-from-namespace", otherFrom.Namespace, + "grant-from-group", otherFrom.Group, + "grant-from-kind", otherFrom.Kind, + "requested-from-namespace", from.Namespace, + "requested-from-group", from.Group, + "requested-from-kind", from.Kind, + ) allowed[Namespace(grant.ObjectMeta.Namespace)] = append(allowed[Namespace(grant.ObjectMeta.Namespace)], grant.Spec.To...) + for _, to := range grant.Spec.To { + name := "" + if to.Name != nil { + name = string(*to.Name) + } + scoped.V(logging.TraceLevel).Info("added ReferenceGrantTo to namespace allowed list", + "namespace", grant.ObjectMeta.Namespace, + "to-group", to.Group, + "to-kind", to.Kind, + "to-name", name, + ) + } + } else { + scoped.V(logging.TraceLevel).Info("grant from not equal, excluding from allowed", + "grant-namespace", grant.Namespace, + "grant-name", grant.Name, + "grant-from-namespace", otherFrom.Namespace, + "grant-from-group", otherFrom.Group, + "grant-from-kind", otherFrom.Kind, + "requested-from-namespace", from.Namespace, + "requested-from-group", from.Group, + "requested-from-kind", from.Kind, + ) } } } diff --git a/internal/gatewayapi/references_test.go b/internal/gatewayapi/references_test.go new file mode 100644 index 0000000000..1f9b32fb39 --- /dev/null +++ b/internal/gatewayapi/references_test.go @@ -0,0 +1,158 @@ +package gatewayapi + +import ( + "testing" + + "github.com/go-logr/logr" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestGetPermittedForReferenceGrantFrom(t *testing.T) { + grants := []*ReferenceGrant{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: "fitrat", + }, + Spec: ReferenceGrantSpec{ + From: []ReferenceGrantFrom{ + { + Group: Group("gateway.networking.k8s.io"), + Kind: Kind("TCPRoute"), + Namespace: Namespace("garbage"), + }, + { + Group: Group("gateway.networking.k8s.io"), + Kind: Kind("TCPRoute"), + Namespace: Namespace("behbudiy"), + }, + { + Group: Group("gateway.networking.k8s.io"), + Kind: Kind("TCPRoute"), + Namespace: Namespace("qodiriy"), + }, + }, + To: []ReferenceGrantTo{ + { + Group: Group(""), + Kind: Kind("GrantOne"), + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: "cholpon", + }, + Spec: ReferenceGrantSpec{ + From: []ReferenceGrantFrom{ + { + Group: Group("gateway.networking.k8s.io"), + Kind: Kind("UDPRoute"), + Namespace: Namespace("behbudiy"), + }, + { + Group: Group("gateway.networking.k8s.io"), + Kind: Kind("TCPRoute"), + Namespace: Namespace("qodiriy"), + }, + }, + To: []ReferenceGrantTo{ + { + Group: Group(""), + Kind: Kind("GrantTwo"), + }, + }, + }, + }, + } + tests := []struct { + msg string + from ReferenceGrantFrom + result map[Namespace][]ReferenceGrantTo + }{ + { + msg: "no matches whatsoever", + from: ReferenceGrantFrom{ + Group: Group("invalid.example"), + Kind: Kind("invalid"), + Namespace: Namespace("invalid"), + }, + result: map[Namespace][]ReferenceGrantTo{}, + }, + { + msg: "non-matching namespace", + from: ReferenceGrantFrom{ + Group: Group("gateway.networking.k8s.io"), + Kind: Kind("UDPRoute"), + Namespace: Namespace("niyazi"), + }, + result: map[Namespace][]ReferenceGrantTo{}, + }, + { + msg: "non-matching kind", + from: ReferenceGrantFrom{ + Group: Group("gateway.networking.k8s.io"), + Kind: Kind("TLSRoute"), + Namespace: Namespace("behbudiy"), + }, + result: map[Namespace][]ReferenceGrantTo{}, + }, + { + msg: "non-matching group", + from: ReferenceGrantFrom{ + Group: Group("invalid.example"), + Kind: Kind("UDPRoute"), + Namespace: Namespace("behbudiy"), + }, + result: map[Namespace][]ReferenceGrantTo{}, + }, + { + msg: "single match", + from: ReferenceGrantFrom{ + Group: Group("gateway.networking.k8s.io"), + Kind: Kind("UDPRoute"), + Namespace: Namespace("behbudiy"), + }, + result: map[Namespace][]ReferenceGrantTo{ + "cholpon": { + { + Group: Group(""), + Kind: Kind("GrantTwo"), + }, + }, + }, + }, + { + msg: "multiple matches", + from: ReferenceGrantFrom{ + Group: Group("gateway.networking.k8s.io"), + Kind: Kind("TCPRoute"), + Namespace: Namespace("qodiriy"), + }, + result: map[Namespace][]ReferenceGrantTo{ + "cholpon": { + { + Group: Group(""), + Kind: Kind("GrantTwo"), + }, + }, + "fitrat": { + { + Group: Group(""), + Kind: Kind("GrantOne"), + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.msg, func(t *testing.T) { + result := GetPermittedForReferenceGrantFrom(logr.Discard(), tt.from, grants) + assert.Equal(t, tt.result, result) + }) + } +}