fix: resoultion of core resources and rest-mapper (#12)

* fix(handler): enhance logging for resource attributes and GVK retrieval

- Improved logging structure by adding child loggers for resource attributes and cluster name.
- Updated error logging to include GVR details when fetching GVK.
- Changed log message for ruleless mode to clarify its purpose.
- Ensured proper handling of empty resource group by defaulting to "core".

* docs(contributing): add debugging instructions for telepresence with kind cluster

- Included steps for installing Telepresence

* feat: change mapperProvider behaviour to allow for all resource identification

* fix(handler): correct group handling for resource attributes

- Ensure default group is set to "core" when not specified in resource attributes.
- Update object type formatting to use the correct group value.
- Refactor code for clarity and maintainability.

---------

Co-authored-by: aaronschweig <[email protected]>

Author: nexus49

CONTRIBUTING.md

@@ -29,6 +29,15 @@ task mockery
 ``` 
 P.S. If you have golang installed, it automatically installs the mockery binary in `golang-commons/bin` directory.
 
+
+## Debugging using telepresence against local kind cluster
+
+- Install Telepresence as outlined in their documentation [link](https://telepresence.io/docs/quick-start)
+- Point your kubeconfig against the local kind cluster
+- Start the webhook locally using the kcp kubeconfig from the kind cluster. you may need to adjust the domain to (kcp.api.portal.dev.local:8443)
+- Connect telepresense to the openmfp-system namespace: `telepresence connect -n openmfp-system`
+- Start intercepting traffic to the webhook using: `telepresence intercept rebac-authz-webhook --port 9443:9443`
+
 ## Issues
 We use GitHub issues to track bugs. Please ensure your description is
 clear and includes sufficient instructions to reproduce the issue.

cmd/serve.go

@@ -15,6 +15,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -27,6 +28,7 @@ import (
 	"github.com/platform-mesh/rebac-authz-webhook/pkg/client"
 	"github.com/platform-mesh/rebac-authz-webhook/pkg/config"
 	"github.com/platform-mesh/rebac-authz-webhook/pkg/handler"
+	"github.com/platform-mesh/rebac-authz-webhook/pkg/mapperprovider"
 )
 
 var (
@@ -56,7 +58,9 @@ func serve() { // coverage-ignore
 		return otelhttp.NewTransport(rt)
 	})
 
-	mgr, provider := initializeMultiClusterManager(ctx, restCfg, log, serverCfg, defaultCfg)
+	mps := mapperprovider.New()
+
+	mgr, provider := initializeMultiClusterManager(ctx, restCfg, log, serverCfg, defaultCfg, mps)
 	fga := client.MustCreateInClusterClient(serverCfg.OpenFGA.Addr)
 
 	kcpScheme := runtime.NewScheme()
@@ -66,7 +70,7 @@ func serve() { // coverage-ignore
 	srv := mgr.GetWebhookServer()
 	cmw := &ContextMiddleware{Logger: log}
 
-	authHandler, err := handler.NewAuthorizationHandler(fga, mgr, serverCfg.Kcp.AccountInfoName)
+	authHandler, err := handler.NewAuthorizationHandler(fga, mgr, serverCfg.Kcp.AccountInfoName, mps)
 	if err != nil {
 		log.Fatal().Err(err).Msg("could not create authorization handler")
 	}
@@ -93,7 +97,7 @@ func serve() { // coverage-ignore
 
 }
 
-func initializeMultiClusterManager(ctx context.Context, restCfg *rest.Config, log *logger.Logger, serviceCfg config.Config, defaultConfig *commonconfig.CommonServiceConfig) (mcmanager.Manager, *apiexport.Provider) {
+func initializeMultiClusterManager(ctx context.Context, restCfg *rest.Config, log *logger.Logger, serviceCfg config.Config, defaultConfig *commonconfig.CommonServiceConfig, mps *mapperprovider.MapperProviders) (mcmanager.Manager, *apiexport.Provider) {
 	log.Info().Msg("Initializing multicluster manager")
 	kubeconfigPath := serviceCfg.Kcp.KubeconfigPath
 	kcpCfg, err := clientcmd.BuildConfigFromFlags("", kubeconfigPath)
@@ -108,9 +112,22 @@ func initializeMultiClusterManager(ctx context.Context, restCfg *rest.Config, lo
 		kcpCfg.Host = serverCfg.Kcp.ClusterURL
 	}
 
-	provider, err := apiexport.New(kcpCfg, apiexport.Options{
+	wildcardCache, err := apiexport.NewWildcardCache(kcpCfg, cache.Options{
 		Scheme: scheme,
 	})
+	if err != nil {
+		log.Fatal().Err(err).Msg("unable to create wildcard cache")
+	}
+
+	err = mapperprovider.Run(ctx, kcpCfg, mps, wildcardCache, log)
+	if err != nil {
+		log.Fatal().Err(err).Msg("unable to run mapper provider")
+	}
+
+	provider, err := apiexport.New(kcpCfg, apiexport.Options{
+		WildcardCache: wildcardCache,
+		Scheme:        scheme,
+	})
 	if err != nil {
 		log.Fatal().Err(err).Msg("unable to construct cluster provider")
 	}

pkg/handler/handler.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/kcp-dev/logicalcluster/v3"
 	openfgav1 "github.com/openfga/api/proto/openfga/v1"
 	corev1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1"
 	"github.com/prometheus/client_golang/prometheus"
@@ -20,13 +21,15 @@ import (
 
 	"github.com/platform-mesh/golang-commons/logger"
 
+	"github.com/platform-mesh/rebac-authz-webhook/pkg/mapperprovider"
 	"github.com/platform-mesh/rebac-authz-webhook/pkg/util"
 )
 
 type AuthorizationHandler struct {
 	fga             openfgav1.OpenFGAServiceClient
 	accountInfoName string
 	mgr             mcmanager.Manager
+	mps             *mapperprovider.MapperProviders
 }
 
 var (
@@ -37,12 +40,13 @@ var (
 	})
 )
 
-func NewAuthorizationHandler(fga openfgav1.OpenFGAServiceClient, mgr mcmanager.Manager, accountInfoName string) (*AuthorizationHandler, error) {
+func NewAuthorizationHandler(fga openfgav1.OpenFGAServiceClient, mgr mcmanager.Manager, accountInfoName string, mps *mapperprovider.MapperProviders) (*AuthorizationHandler, error) {
 
 	return &AuthorizationHandler{
 		fga:             fga,
 		accountInfoName: accountInfoName,
 		mgr:             mgr,
+		mps:             mps,
 	}, nil
 }
 
@@ -116,6 +120,12 @@ func (a *AuthorizationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
+	log = log.ChildLogger("resourceAttributes", sar.Spec.ResourceAttributes.String()).
+		ChildLogger("group", sar.Spec.ResourceAttributes.Group).
+		ChildLogger("resource", sar.Spec.ResourceAttributes.Resource).
+		ChildLogger("subresource", sar.Spec.ResourceAttributes.Subresource).
+		ChildLogger("verb", sar.Spec.ResourceAttributes.Verb)
+
 	log.Debug().Str("sar", fmt.Sprintf("%+v", sar)).Msg("Received SubjectAccessReview")
 
 	// For resource attributes, we need to get the store ID
@@ -126,12 +136,6 @@ func (a *AuthorizationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	log = log.ChildLogger("resourceAttributes", sar.Spec.ResourceAttributes.String()).
-		ChildLogger("group", sar.Spec.ResourceAttributes.Group).
-		ChildLogger("resource", sar.Spec.ResourceAttributes.Resource).
-		ChildLogger("subresource", sar.Spec.ResourceAttributes.Subresource).
-		ChildLogger("verb", sar.Spec.ResourceAttributes.Verb)
-
 	group := util.CapGroupToRelationLength(sar, 50)
 	group = strings.ReplaceAll(group, ".", "_")
 	relation := sar.Spec.ResourceAttributes.Verb
@@ -142,34 +146,30 @@ func (a *AuthorizationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 			clusterName = clusterNames[0]
 		}
 	}
+	log = log.ChildLogger("clusterName", clusterName)
 
 	var namespaced bool
 	var gvk schema.GroupVersionKind
 
-	cluster, err := a.mgr.GetCluster(r.Context(), clusterName)
-	if err != nil {
-		log.Error().Err(err).Str("cluster", clusterName).Msg("error getting cluster")
-		noOpinion(w, sar)
-		return
-	}
-
-	restMapper := cluster.GetRESTMapper()
-	if err != nil {
+	restMapper, ok := a.mps.GetMapper(logicalcluster.Name(clusterName))
+	if !ok {
 		log.Error().Err(err).Msg("error getting provider")
 		noOpinion(w, sar)
 		return
 	}
 
-	gvk, err = restMapper.KindFor(schema.GroupVersionResource{
+	gvr := schema.GroupVersionResource{
 		Group:    sar.Spec.ResourceAttributes.Group,
 		Resource: sar.Spec.ResourceAttributes.Resource,
 		Version:  sar.Spec.ResourceAttributes.Version,
-	})
+	}
+	gvk, err = restMapper.KindFor(gvr)
 	if err != nil {
-		log.Error().Err(err).Msg("error getting GVK")
+		log.Error().Err(err).Str("gvr", fmt.Sprintf("%+v", gvr)).Msg("error getting GVK")
 		noOpinion(w, sar)
 		return
 	}
+	log.Debug().Str("gvr", fmt.Sprintf("%+v", gvr)).Str("gvk", fmt.Sprintf("%+v", gvk)).Msg("Got GVK")
 
 	namespaced, err = apiutil.IsGVKNamespaced(gvk, restMapper)
 	if err != nil {
@@ -178,15 +178,14 @@ func (a *AuthorizationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	groupForType := strings.ReplaceAll(sar.Spec.ResourceAttributes.Group, ".", "_")
 	resourceType := sar.Spec.ResourceAttributes.Resource
 
 	if singularResource, err := restMapper.ResourceSingularizer(sar.Spec.ResourceAttributes.Resource); err == nil {
 		resourceType = singularResource
 		log.Debug().Str("resource", sar.Spec.ResourceAttributes.Resource).Str("singular", resourceType).Msg("Converted resource to singular form")
 	}
 
-	objectType := fmt.Sprintf("%s_%s", groupForType, resourceType)
+	objectType := fmt.Sprintf("%s_%s", group, resourceType)
 
 	longestObjectType := fmt.Sprintf("create_%ss", objectType)
 	if len(longestObjectType) > 50 {
@@ -233,7 +232,7 @@ func (a *AuthorizationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		}
 	}
 
-	log.Debug().Str("object", object).Str("relation", relation).Any("contextualTuples", contextualTuples).Msg("ruleless mode, using contextual tuples")
+	log.Debug().Str("object", object).Str("relation", relation).Any("contextualTuples", contextualTuples).Msg("check call elements")
 
 	if a.fga == nil {
 		log.Warn().Msg("FGA client is nil, returning no opinion")
@@ -260,7 +259,7 @@ func (a *AuthorizationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		noOpinion(w, sar)
 		return
 	}
-	log.Info().Bool("allowed", res.Allowed).Str("user", sar.Spec.User).Str("object", object).Str("relation", relation).Msg("sar response")
+	log.Info().Str("allowed", fmt.Sprintf("%t", res.Allowed)).Str("user", sar.Spec.User).Str("object", object).Str("relation", relation).Msg("sar response")
 	if !res.Allowed {
 		noOpinion(w, sar)
 		return

pkg/handler/handler_test.go

@@ -142,7 +142,7 @@ func TestInvalidStoreOperations(t *testing.T) {
 			assert.NoError(t, err)
 
 			mockManager := createSimpleMockManager()
-			handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account")
+			handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account", nil)
 			assert.NoError(t, err)
 
 			recorder := httptest.NewRecorder()
@@ -208,7 +208,7 @@ func TestAuthorizationHandlerWithFGA(t *testing.T) {
 			}
 
 			mockManager := createSimpleMockManager()
-			handler, err := handler.NewAuthorizationHandler(mockFGAClient, mockManager, "account")
+			handler, err := handler.NewAuthorizationHandler(mockFGAClient, mockManager, "account", nil)
 			assert.NoError(t, err)
 
 			recorder := httptest.NewRecorder()
@@ -232,7 +232,7 @@ func TestAuthorizationHandler(t *testing.T) {
 		assert.NoError(t, err)
 
 		mockManager := createSimpleMockManager()
-		handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account")
+		handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account", nil)
 		assert.NoError(t, err)
 
 		recorder := httptest.NewRecorder()
@@ -245,15 +245,15 @@ func TestAuthorizationHandler(t *testing.T) {
 func TestNewAuthorizationHandler(t *testing.T) {
 	t.Run("should succeed with valid parameters", func(t *testing.T) {
 		mockManager := createSimpleMockManager()
-		handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account")
+		handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account", nil)
 		assert.NoError(t, err)
 		assert.NotNil(t, handler)
 	})
 
 	t.Run("should succeed with FGA client", func(t *testing.T) {
 		mockFGAClient := mocks.NewOpenFGAServiceClient(t)
 		mockManager := createSimpleMockManager()
-		handler, err := handler.NewAuthorizationHandler(mockFGAClient, mockManager, "account")
+		handler, err := handler.NewAuthorizationHandler(mockFGAClient, mockManager, "account", nil)
 		assert.NoError(t, err)
 		assert.NotNil(t, handler)
 	})
@@ -282,7 +282,7 @@ func TestNonResourceAttributes(t *testing.T) {
 		assert.NoError(t, err)
 
 		mockManager := createSimpleMockManager()
-		handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account")
+		handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account", nil)
 		assert.NoError(t, err)
 
 		recorder := httptest.NewRecorder()
@@ -319,7 +319,7 @@ func TestNonResourceAttributes(t *testing.T) {
 		assert.NoError(t, err)
 
 		mockManager := createSimpleMockManager()
-		handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account")
+		handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account", nil)
 		assert.NoError(t, err)
 
 		recorder := httptest.NewRecorder()
@@ -363,7 +363,7 @@ func TestMultiClusterFunctionality(t *testing.T) {
 		assert.NoError(t, err)
 
 		mockManager := createSimpleMockManager()
-		handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account")
+		handler, err := handler.NewAuthorizationHandler(nil, mockManager, "account", nil)
 		assert.NoError(t, err)
 
 		recorder := httptest.NewRecorder()
@@ -406,7 +406,7 @@ func TestMultiClusterFunctionality(t *testing.T) {
 		// Don't set up any expectations - this will test error handling
 
 		mockManager := createSimpleMockManager()
-		handler, err := handler.NewAuthorizationHandler(mockFGAClient, mockManager, "account")
+		handler, err := handler.NewAuthorizationHandler(mockFGAClient, mockManager, "account", nil)
 		assert.NoError(t, err)
 
 		recorder := httptest.NewRecorder()