Skip to content

Commit

Permalink
chore(refs) add grant check logs (#6302)
Browse files Browse the repository at this point in the history 
Add trace logs for ReferenceGrant checks.

Add comments for ReferenceGrant utilities.

Consolidate duplicated utility function.
  • Loading branch information
@rainest
rainest committed Jul 24, 2024
1 parent cea1315 commit c0f4ff1
Show file tree
Hide file tree
Showing 8 changed files with 317 additions and 200 deletions.
59 changes: 43 additions & 16 deletions internal/dataplane/kongstate/kongstate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,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"
Expand Down Expand Up @@ -352,18 +353,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
Expand Down Expand Up @@ -437,27 +438,52 @@ 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.
pluginReference := gatewayapi.PluginLabelReference{
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)
}
Expand Down Expand Up @@ -894,8 +920,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
Expand Down Expand Up @@ -956,21 +982,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,
Expand Down
2 changes: 1 addition & 1 deletion internal/dataplane/kongstate/kongstate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion internal/dataplane/translator/backendref.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 9 additions & 29 deletions internal/dataplane/translator/translate_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package translator

import (
"fmt"
"reflect"

"github.com/go-logr/logr"
"github.com/kong/go-kong/kong"
Expand Down Expand Up @@ -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(
Expand All @@ -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)

Expand Down
148 changes: 0 additions & 148 deletions internal/dataplane/translator/translate_utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down
3 changes: 2 additions & 1 deletion internal/dataplane/translator/wrappers_refchecker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
})
Expand Down
Loading

0 comments on commit c0f4ff1

Please sign in to comment.