From 1e54a8fa99c41a9a98a328680edd4b32de23f510 Mon Sep 17 00:00:00 2001 From: OlegErshov Date: Mon, 13 Apr 2026 14:38:02 +0200 Subject: [PATCH 1/6] feat: move store resource into system command On-behalf-of: SAP aleh.yarshou@sap.com --- cmd/operator.go | 5 - cmd/system.go | 29 +++++- internal/config/config.go | 6 ++ .../controller/apiexportpolicy_controller.go | 11 ++- .../authorization_model_controller.go | 4 +- internal/controller/idp_controller.go | 6 +- internal/controller/store_controller.go | 20 +++- internal/subroutine/authorization_model.go | 8 +- internal/subroutine/tuples.go | 27 +++--- internal/subroutine/tuples_test.go | 95 +++++++++---------- 10 files changed, 126 insertions(+), 85 deletions(-) diff --git a/cmd/operator.go b/cmd/operator.go index 3c0fb8e2..edc1a4d5 100644 --- a/cmd/operator.go +++ b/cmd/operator.go @@ -163,11 +163,6 @@ var operatorCmd = &cobra.Command{ return err } - if err = controller.NewStoreReconciler(ctx, log, fga, mgr, &operatorCfg). - SetupWithManager(mgr, defaultCfg); err != nil { - log.Error().Err(err).Str("controller", "store").Msg("unable to create controller") - return err - } if err = controller. NewAuthorizationModelReconciler(log, fga, mgr). SetupWithManager(mgr, defaultCfg); err != nil { diff --git a/cmd/system.go b/cmd/system.go index 36bf871e..33258dcf 100644 --- a/cmd/system.go +++ b/cmd/system.go @@ -7,6 +7,7 @@ import ( openfgav1 "github.com/openfga/api/proto/openfga/v1" platformeshcontext "github.com/platform-mesh/golang-commons/context" iclient "github.com/platform-mesh/security-operator/internal/client" + "github.com/platform-mesh/security-operator/internal/config" "github.com/platform-mesh/security-operator/internal/controller" "github.com/platform-mesh/security-operator/internal/fga" "github.com/spf13/cobra" @@ -16,6 +17,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" + multiprovider "sigs.k8s.io/multicluster-runtime/providers/multi" "k8s.io/client-go/rest" @@ -63,15 +65,30 @@ var systemCmd = &cobra.Command{ opts.LeaderElectionConfig = inClusterCfg } - provider, err := apiexport.New(restCfg, systemCfg.APIExportEndpointSlices.SystemPlatformMeshIO, apiexport.Options{ + systemProvider, err := apiexport.New(restCfg, systemCfg.APIExportEndpointSlices.SystemPlatformMeshIO, apiexport.Options{ Scheme: scheme, }) if err != nil { - setupLog.Error(err, "unable to create apiexport provider") + setupLog.Error(err, "unable to create system apiexport provider") return err } - mgr, err := mcmanager.New(restCfg, provider, opts) + coreProvider, err := apiexport.New(restCfg, systemCfg.APIExportEndpointSlices.CorePlatformMeshIO, apiexport.Options{ + Scheme: scheme, + }) + if err != nil { + setupLog.Error(err, "unable to create core apiexport provider") + return err + } + multiProv := multiprovider.New(multiprovider.Options{}) + if err := multiProv.AddProvider(config.SystemProviderName, systemProvider); err != nil { + return err + } + if err := multiProv.AddProvider(config.CoreProviderName, coreProvider); err != nil { + return err + } + + mgr, err := mcmanager.New(restCfg, multiProv, opts) if err != nil { setupLog.Error(err, "unable to create manager") return err @@ -113,6 +130,12 @@ var systemCmd = &cobra.Command{ return err } + if err = controller.NewStoreReconciler(ctx, log, fgaClient, mgr, &operatorCfg). + SetupWithManager(mgr, defaultCfg); err != nil { + log.Error().Err(err).Str("controller", "store").Msg("unable to create controller") + return err + } + if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { log.Error().Err(err).Msg("unable to set up health check") return err diff --git a/internal/config/config.go b/internal/config/config.go index 44e2f17e..430ce219 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -7,6 +7,12 @@ import ( "github.com/spf13/pflag" ) +const ( + CoreProviderName = "core" + SystemProviderName = "system" + ProviderSeparator = "#" +) + type KeycloakConfig struct { BaseURL string ClientID string diff --git a/internal/controller/apiexportpolicy_controller.go b/internal/controller/apiexportpolicy_controller.go index 9f224910..5f0f8689 100644 --- a/internal/controller/apiexportpolicy_controller.go +++ b/internal/controller/apiexportpolicy_controller.go @@ -68,7 +68,11 @@ func (r *APIExportPolicyReconciler) SetupWithManager(mgr mcmanager.Manager, cfg return mcbuilder.ControllerManagedBy(mgr). Named("apiexportpolicy"). - For(&corev1alpha1.APIExportPolicy{}). + For(&corev1alpha1.APIExportPolicy{}, + mcbuilder.WithClusterFilter(func(clusterName string, _ cluster.Cluster) bool { + return strings.HasPrefix(clusterName, config.SystemProviderName) + }), + ). WithOptions(opts). WithEventFilter(predicate.And(predicates...)). Watches( @@ -112,14 +116,15 @@ func (r *APIExportPolicyReconciler) enqueueAllAPIExportPolicies(ctx context.Cont trimmedExpr := strings.TrimPrefix(expr, ":") if trimmedExpr == "root:orgs:*" { - clusterName := logicalcluster.From(&policy) + clusterName := multiProviderName(config.SystemProviderName, logicalcluster.From(&policy).String()) + requests = append(requests, mcreconcile.Request{ Request: reconcile.Request{ NamespacedName: types.NamespacedName{ Name: policy.Name, }, }, - ClusterName: clusterName.String(), + ClusterName: clusterName, }) break } diff --git a/internal/controller/authorization_model_controller.go b/internal/controller/authorization_model_controller.go index 84a7233c..51c0ee70 100644 --- a/internal/controller/authorization_model_controller.go +++ b/internal/controller/authorization_model_controller.go @@ -8,6 +8,7 @@ import ( "github.com/platform-mesh/golang-commons/controller/filter" "github.com/platform-mesh/golang-commons/logger" corev1alpha1 "github.com/platform-mesh/security-operator/api/v1alpha1" + iclient "github.com/platform-mesh/security-operator/internal/client" "github.com/platform-mesh/security-operator/internal/subroutine" "github.com/platform-mesh/subroutines/lifecycle" ctrl "sigs.k8s.io/controller-runtime" @@ -25,9 +26,10 @@ type AuthorizationModelReconciler struct { } func NewAuthorizationModelReconciler(log *logger.Logger, fga openfgav1.OpenFGAServiceClient, mcMgr mcmanager.Manager) *AuthorizationModelReconciler { + kcpClientHelper := iclient.NewKcpHelper(mcMgr.GetLocalManager().GetConfig(), mcMgr.GetLocalManager().GetScheme()) lc := lifecycle.New(mcMgr, "AuthorizationModelReconciler", func() client.Object { return &corev1alpha1.AuthorizationModel{} - }, subroutine.NewTupleSubroutine(fga, mcMgr)) + }, subroutine.NewTupleSubroutine(fga, mcMgr, kcpClientHelper)) return &AuthorizationModelReconciler{ log: log, diff --git a/internal/controller/idp_controller.go b/internal/controller/idp_controller.go index 65de344c..7266c487 100644 --- a/internal/controller/idp_controller.go +++ b/internal/controller/idp_controller.go @@ -3,6 +3,7 @@ package controller import ( "context" "fmt" + "strings" platformeshconfig "github.com/platform-mesh/golang-commons/config" "github.com/platform-mesh/golang-commons/controller/filter" @@ -15,6 +16,7 @@ import ( "github.com/platform-mesh/subroutines/lifecycle" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/cluster" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/predicate" mcbuilder "sigs.k8s.io/multicluster-runtime/pkg/builder" @@ -65,7 +67,9 @@ func (r *IdentityProviderConfigurationReconciler) SetupWithManager(mgr mcmanager predicates := append([]predicate.Predicate{filter.DebugResourcesBehaviourPredicate(cfg.DebugLabelValue)}, evp...) return mcbuilder.ControllerManagedBy(mgr). Named("identityprovider"). - For(&corev1alpha1.IdentityProviderConfiguration{}). + For(&corev1alpha1.IdentityProviderConfiguration{}, mcbuilder.WithClusterFilter(func(clusterName string, _ cluster.Cluster) bool { + return strings.HasPrefix(clusterName, "system") + })). WithOptions(opts). WithEventFilter(predicate.And(predicates...)). Complete(r) diff --git a/internal/controller/store_controller.go b/internal/controller/store_controller.go index 2f645eeb..bea7b2b4 100644 --- a/internal/controller/store_controller.go +++ b/internal/controller/store_controller.go @@ -2,6 +2,7 @@ package controller import ( "context" + "strings" openfgav1 "github.com/openfga/api/proto/openfga/v1" platformeshconfig "github.com/platform-mesh/golang-commons/config" @@ -42,6 +43,7 @@ func NewStoreReconciler(ctx context.Context, log *logger.Logger, fga openfgav1.O if err != nil { log.Fatal().Err(err).Msg("unable to create new client") } + kcpClientHelper := iclient.NewKcpHelper(mcMgr.GetLocalManager().GetConfig(), mcMgr.GetLocalManager().GetScheme()) lc := lifecycle.New(mcMgr, "StoreReconciler", func() client.Object { return &corev1alpha1.Store{} @@ -50,7 +52,7 @@ func NewStoreReconciler(ctx context.Context, log *logger.Logger, fga openfgav1.O subroutine.NewAuthorizationModelSubroutine(fga, mcMgr, allClient, func(cfg *rest.Config) discovery.DiscoveryInterface { return discovery.NewDiscoveryClientForConfigOrDie(cfg) }, log), - subroutine.NewTupleSubroutine(fga, mcMgr), + subroutine.NewTupleSubroutine(fga, mcMgr, kcpClientHelper), ).WithConditions(conditions.NewManager()) return &StoreReconciler{ @@ -69,7 +71,11 @@ func (r *StoreReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platforme predicates := append([]predicate.Predicate{filter.DebugResourcesBehaviourPredicate(cfg.DebugLabelValue)}, evp...) b := mcbuilder.ControllerManagedBy(mgr). Named("store"). - For(&corev1alpha1.Store{}). + For(&corev1alpha1.Store{}, + mcbuilder.WithClusterFilter(func(clusterName string, _ cluster.Cluster) bool { + return strings.HasPrefix(clusterName, config.SystemProviderName) + }), + ). WithOptions(controller.TypedOptions[mcreconcile.Request]{MaxConcurrentReconciles: cfg.MaxConcurrentReconciles}). WithEventFilter(predicate.And(predicates...)) @@ -82,6 +88,7 @@ func (r *StoreReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platforme if !ok { return nil } + storeClusterName := multiProviderName(config.SystemProviderName, model.Spec.StoreRef.Cluster) return []mcreconcile.Request{ { @@ -90,11 +97,18 @@ func (r *StoreReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platforme Name: model.Spec.StoreRef.Name, }, }, - ClusterName: model.Spec.StoreRef.Cluster, + ClusterName: storeClusterName, }, } }) }, mcbuilder.WithPredicates(predicate.GenerationChangedPredicate{}), + mcbuilder.WithClusterFilter(func(clusterName string, _ cluster.Cluster) bool { + return strings.HasPrefix(clusterName, config.CoreProviderName) + }), ).Complete(r) } + +func multiProviderName(providerName, clusterID string) string { + return providerName + config.ProviderSeparator + clusterID +} diff --git a/internal/subroutine/authorization_model.go b/internal/subroutine/authorization_model.go index ca24aadf..275fa2b5 100644 --- a/internal/subroutine/authorization_model.go +++ b/internal/subroutine/authorization_model.go @@ -95,12 +95,6 @@ var _ subroutines.Processor = &authorizationModelSubroutine{} func (a *authorizationModelSubroutine) GetName() string { return "AuthorizationModel" } func getRelatedAuthorizationModels(ctx context.Context, k8s client.Client, store *securityv1alpha1.Store) (securityv1alpha1.AuthorizationModelList, error) { - - storeClusterKey, ok := mccontext.ClusterFrom(ctx) - if !ok { - return securityv1alpha1.AuthorizationModelList{}, fmt.Errorf("unable to get cluster key from context") - } - allCtx := mccontext.WithCluster(ctx, "") allAuthorizationModels := securityv1alpha1.AuthorizationModelList{} @@ -110,7 +104,7 @@ func getRelatedAuthorizationModels(ctx context.Context, k8s client.Client, store var extendingModules securityv1alpha1.AuthorizationModelList for _, model := range allAuthorizationModels.Items { - if model.Spec.StoreRef.Name != store.Name || model.Spec.StoreRef.Cluster != storeClusterKey { + if model.Spec.StoreRef.Name != store.Name { continue } diff --git a/internal/subroutine/tuples.go b/internal/subroutine/tuples.go index a92a8826..daa0ed8a 100644 --- a/internal/subroutine/tuples.go +++ b/internal/subroutine/tuples.go @@ -8,17 +8,21 @@ import ( openfgav1 "github.com/openfga/api/proto/openfga/v1" "github.com/platform-mesh/golang-commons/logger" securityv1alpha1 "github.com/platform-mesh/security-operator/api/v1alpha1" + iclient "github.com/platform-mesh/security-operator/internal/client" "github.com/platform-mesh/security-operator/internal/fga" "github.com/platform-mesh/subroutines" "sigs.k8s.io/controller-runtime/pkg/client" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" "k8s.io/apimachinery/pkg/types" + + "github.com/kcp-dev/logicalcluster/v3" ) type tupleSubroutine struct { - fga openfgav1.OpenFGAServiceClient - mgr mcmanager.Manager + fga openfgav1.OpenFGAServiceClient + mgr mcmanager.Manager + kcpHelper iclient.KcpClientHelper } // Finalize implements subroutines.Finalizer. @@ -37,13 +41,13 @@ func (t *tupleSubroutine) Finalize(ctx context.Context, obj client.Object) (subr case *securityv1alpha1.AuthorizationModel: managedTuples = o.Status.ManagedTuples - storeCluster, err := t.mgr.GetCluster(ctx, o.Spec.StoreRef.Cluster) + cl, err := t.kcpHelper.NewClientForLogicalCluster(logicalcluster.Name(o.Spec.StoreRef.Cluster)) if err != nil { - return subroutines.OK(), fmt.Errorf("unable to get store cluster: %w", err) + return subroutines.OK(), fmt.Errorf("unable to create client to store cluster: %w", err) } var store securityv1alpha1.Store - err = storeCluster.GetClient().Get(ctx, types.NamespacedName{ + err = cl.Get(ctx, types.NamespacedName{ Name: o.Spec.StoreRef.Name, }, &store) if err != nil { @@ -97,13 +101,13 @@ func (t *tupleSubroutine) Process(ctx context.Context, obj client.Object) (subro specTuples = o.Spec.Tuples managedTuples = o.Status.ManagedTuples - storeCluster, err := t.mgr.GetCluster(ctx, o.Spec.StoreRef.Cluster) + cl, err := t.kcpHelper.NewClientForLogicalCluster(logicalcluster.Name(o.Spec.StoreRef.Cluster)) if err != nil { - return subroutines.OK(), fmt.Errorf("unable to get store cluster: %w", err) + return subroutines.OK(), fmt.Errorf("unable to create client to store cluster: %w", err) } var store securityv1alpha1.Store - err = storeCluster.GetClient().Get(ctx, types.NamespacedName{ + err = cl.Get(ctx, types.NamespacedName{ Name: o.Spec.StoreRef.Name, }, &store) if err != nil { @@ -141,10 +145,11 @@ func (t *tupleSubroutine) Process(ctx context.Context, obj client.Object) (subro return subroutines.OK(), nil } -func NewTupleSubroutine(fga openfgav1.OpenFGAServiceClient, mgr mcmanager.Manager) *tupleSubroutine { +func NewTupleSubroutine(fga openfgav1.OpenFGAServiceClient, mgr mcmanager.Manager, kcpHelper iclient.KcpClientHelper) *tupleSubroutine { return &tupleSubroutine{ - fga: fga, - mgr: mgr, + fga: fga, + mgr: mgr, + kcpHelper: kcpHelper, } } diff --git a/internal/subroutine/tuples_test.go b/internal/subroutine/tuples_test.go index 681767af..b28806ec 100644 --- a/internal/subroutine/tuples_test.go +++ b/internal/subroutine/tuples_test.go @@ -13,16 +13,20 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/rest" + + "github.com/kcp-dev/logicalcluster/v3" ) func TestTupleGetName(t *testing.T) { - subroutine := subroutine.NewTupleSubroutine(nil, nil) + subroutine := subroutine.NewTupleSubroutine(nil, nil, nil) assert.Equal(t, "TupleSubroutine", subroutine.GetName()) } func TestTupleFinalizers(t *testing.T) { - subroutine := subroutine.NewTupleSubroutine(nil, nil) + subroutine := subroutine.NewTupleSubroutine(nil, nil, nil) assert.Equal(t, []string{"core.platform-mesh.io/fga-tuples"}, subroutine.Finalizers(nil)) } @@ -164,7 +168,13 @@ func TestTupleProcessWithStore(t *testing.T) { test.mgrMocks(manager) } - subroutine := subroutine.NewTupleSubroutine(fga, manager) + // Mock GetLocalManager for Store tests + localMgr := mocks.NewCTRLManager(t) + manager.EXPECT().GetLocalManager().Return(localMgr).Maybe() + localMgr.EXPECT().GetConfig().Return(&rest.Config{}).Maybe() + localMgr.EXPECT().GetScheme().Return(runtime.NewScheme()).Maybe() + + subroutine := subroutine.NewTupleSubroutine(fga, manager, nil) _, err := subroutine.Process(context.Background(), test.store) if test.expectError { @@ -180,12 +190,11 @@ func TestTupleProcessWithStore(t *testing.T) { func TestTupleProcessWithAuthorizationModel(t *testing.T) { tests := []struct { - name string - store *securityv1alpha1.AuthorizationModel - fgaMocks func(*mocks.MockOpenFGAServiceClient) - k8sMocks func(*mocks.MockClient) - mgrMocks func(*mocks.MockManager) - expectError bool + name string + store *securityv1alpha1.AuthorizationModel + fgaMocks func(*mocks.MockOpenFGAServiceClient) + kcpHelperMocks func(*mocks.MockKcpHelper) + expectError bool }{ { name: "should process and add tuples to the authorization model", @@ -222,14 +231,9 @@ func TestTupleProcessWithAuthorizationModel(t *testing.T) { fgaMocks: func(fga *mocks.MockOpenFGAServiceClient) { fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, nil) }, - k8sMocks: func(k8s *mocks.MockClient) { - // Not used for AuthorizationModel - }, - mgrMocks: func(mgr *mocks.MockManager) { - storeCluster := mocks.NewMockCluster(t) + kcpHelperMocks: func(kcpHelper *mocks.MockKcpHelper) { storeClient := mocks.NewMockClient(t) - mgr.EXPECT().GetCluster(mock.Anything, "store-cluster").Return(storeCluster, nil) - storeCluster.EXPECT().GetClient().Return(storeClient) + kcpHelper.EXPECT().NewClientForLogicalCluster(logicalcluster.Name("store-cluster")).Return(storeClient, nil) storeClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { store := o.(*securityv1alpha1.Store) *store = securityv1alpha1.Store{ @@ -287,14 +291,9 @@ func TestTupleProcessWithAuthorizationModel(t *testing.T) { // Apply (batch write) + Delete (batch delete) fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, nil).Twice() }, - k8sMocks: func(k8s *mocks.MockClient) { - // Not used for AuthorizationModel - }, - mgrMocks: func(mgr *mocks.MockManager) { - storeCluster := mocks.NewMockCluster(t) + kcpHelperMocks: func(kcpHelper *mocks.MockKcpHelper) { storeClient := mocks.NewMockClient(t) - mgr.EXPECT().GetCluster(mock.Anything, "store-cluster").Return(storeCluster, nil) - storeCluster.EXPECT().GetClient().Return(storeClient) + kcpHelper.EXPECT().NewClientForLogicalCluster(logicalcluster.Name("store-cluster")).Return(storeClient, nil) storeClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { store := o.(*securityv1alpha1.Store) *store = securityv1alpha1.Store{ @@ -315,15 +314,12 @@ func TestTupleProcessWithAuthorizationModel(t *testing.T) { test.fgaMocks(fga) } - manager := mocks.NewMockManager(t) - if test.mgrMocks != nil { - test.mgrMocks(manager) - } - if test.k8sMocks != nil { - test.k8sMocks(mocks.NewMockClient(t)) + kcpHelper := mocks.NewMockKcpHelper(t) + if test.kcpHelperMocks != nil { + test.kcpHelperMocks(kcpHelper) } - subroutine := subroutine.NewTupleSubroutine(fga, manager) + subroutine := subroutine.NewTupleSubroutine(fga, nil, kcpHelper) ctx := context.Background() @@ -341,12 +337,11 @@ func TestTupleProcessWithAuthorizationModel(t *testing.T) { func TestTupleFinalizationWithAuthorizationModel(t *testing.T) { tests := []struct { - name string - store *securityv1alpha1.AuthorizationModel - fgaMocks func(*mocks.MockOpenFGAServiceClient) - k8sMocks func(*mocks.MockClient) - mgrMocks func(*mocks.MockManager) - expectError bool + name string + store *securityv1alpha1.AuthorizationModel + fgaMocks func(*mocks.MockOpenFGAServiceClient) + kcpHelperMocks func(*mocks.MockKcpHelper) + expectError bool }{ { name: "should finalize the authorization model", @@ -376,14 +371,9 @@ func TestTupleFinalizationWithAuthorizationModel(t *testing.T) { // delete call fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, nil) }, - k8sMocks: func(k8s *mocks.MockClient) { - // Not used for AuthorizationModel - }, - mgrMocks: func(mgr *mocks.MockManager) { - storeCluster := mocks.NewMockCluster(t) + kcpHelperMocks: func(kcpHelper *mocks.MockKcpHelper) { storeClient := mocks.NewMockClient(t) - mgr.EXPECT().GetCluster(mock.Anything, "store-cluster").Return(storeCluster, nil) - storeCluster.EXPECT().GetClient().Return(storeClient) + kcpHelper.EXPECT().NewClientForLogicalCluster(logicalcluster.Name("store-cluster")).Return(storeClient, nil) storeClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { store := o.(*securityv1alpha1.Store) *store = securityv1alpha1.Store{ @@ -404,15 +394,12 @@ func TestTupleFinalizationWithAuthorizationModel(t *testing.T) { test.fgaMocks(fga) } - manager := mocks.NewMockManager(t) - if test.mgrMocks != nil { - test.mgrMocks(manager) - } - if test.k8sMocks != nil { - test.k8sMocks(mocks.NewMockClient(t)) + kcpHelper := mocks.NewMockKcpHelper(t) + if test.kcpHelperMocks != nil { + test.kcpHelperMocks(kcpHelper) } - subroutine := subroutine.NewTupleSubroutine(fga, manager) + subroutine := subroutine.NewTupleSubroutine(fga, nil, kcpHelper) ctx := context.Background() @@ -487,7 +474,13 @@ func TestTupleFinalizationWithStore(t *testing.T) { test.mgrMocks(manager) } - subroutine := subroutine.NewTupleSubroutine(fga, manager) + // Mock GetLocalManager for Store tests + localMgr := mocks.NewCTRLManager(t) + manager.EXPECT().GetLocalManager().Return(localMgr).Maybe() + localMgr.EXPECT().GetConfig().Return(&rest.Config{}).Maybe() + localMgr.EXPECT().GetScheme().Return(runtime.NewScheme()).Maybe() + + subroutine := subroutine.NewTupleSubroutine(fga, manager, nil) _, err := subroutine.Finalize(context.Background(), test.store) if test.expectError { From 1b0c7f7218d192d821931401cecbfc4b14bb6545 Mon Sep 17 00:00:00 2001 From: OlegErshov Date: Mon, 13 Apr 2026 14:43:23 +0200 Subject: [PATCH 2/6] refactor: use const insted of string in provider name in idp controller On-behalf-of: SAP aleh.yarshou@sap.com --- internal/controller/idp_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/idp_controller.go b/internal/controller/idp_controller.go index 7266c487..7952f68f 100644 --- a/internal/controller/idp_controller.go +++ b/internal/controller/idp_controller.go @@ -68,7 +68,7 @@ func (r *IdentityProviderConfigurationReconciler) SetupWithManager(mgr mcmanager return mcbuilder.ControllerManagedBy(mgr). Named("identityprovider"). For(&corev1alpha1.IdentityProviderConfiguration{}, mcbuilder.WithClusterFilter(func(clusterName string, _ cluster.Cluster) bool { - return strings.HasPrefix(clusterName, "system") + return strings.HasPrefix(clusterName, config.SystemProviderName) })). WithOptions(opts). WithEventFilter(predicate.And(predicates...)). From 0bbbfa5b1d5b3bc0e8b5d4504b81a5d7df375fe4 Mon Sep 17 00:00:00 2001 From: OlegErshov Date: Mon, 13 Apr 2026 16:22:47 +0200 Subject: [PATCH 3/6] chore: add function comments On-behalf-of: SAP aleh.yarshou@sap.com --- internal/controller/apiexportpolicy_controller.go | 1 + internal/controller/store_controller.go | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/controller/apiexportpolicy_controller.go b/internal/controller/apiexportpolicy_controller.go index 5f0f8689..f87df634 100644 --- a/internal/controller/apiexportpolicy_controller.go +++ b/internal/controller/apiexportpolicy_controller.go @@ -116,6 +116,7 @@ func (r *APIExportPolicyReconciler) enqueueAllAPIExportPolicies(ctx context.Cont trimmedExpr := strings.TrimPrefix(expr, ":") if trimmedExpr == "root:orgs:*" { + // apiExportPolicies are engaged by system provider clusterName := multiProviderName(config.SystemProviderName, logicalcluster.From(&policy).String()) requests = append(requests, mcreconcile.Request{ diff --git a/internal/controller/store_controller.go b/internal/controller/store_controller.go index bea7b2b4..62e7c769 100644 --- a/internal/controller/store_controller.go +++ b/internal/controller/store_controller.go @@ -88,6 +88,8 @@ func (r *StoreReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platforme if !ok { return nil } + // stores are engaged by system provider, to trigger a reconciliation with multi provider + // it's required to use provider's prefix for request storeClusterName := multiProviderName(config.SystemProviderName, model.Spec.StoreRef.Cluster) return []mcreconcile.Request{ @@ -109,6 +111,8 @@ func (r *StoreReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platforme ).Complete(r) } -func multiProviderName(providerName, clusterID string) string { - return providerName + config.ProviderSeparator + clusterID +// multiProviderName returns a cluster name with provider prefix and separator for multi provider. +// The multi.Provider prefixes cluster names as "providerName#clusterName" +func multiProviderName(providerName, clusterName string) string { + return providerName + config.ProviderSeparator + clusterName } From cdf5c75166ed0fa725a859c8be2b9e96ac977395 Mon Sep 17 00:00:00 2001 From: OlegErshov Date: Fri, 15 May 2026 13:26:43 +0200 Subject: [PATCH 4/6] fix: resolve lineter and tests complains On-behalf-of: SAP aleh.yarshou@sap.com --- cmd/operator.go | 8 -- cmd/system.go | 15 +++- internal/config/config.go | 6 -- .../authorization_model_controller.go | 14 +++- internal/controller/store_controller.go | 23 +++--- internal/subroutine/tuples.go | 24 +++--- internal/subroutine/tuples_test.go | 75 +++++++------------ 7 files changed, 68 insertions(+), 97 deletions(-) diff --git a/cmd/operator.go b/cmd/operator.go index 75772cb9..3377b7c4 100644 --- a/cmd/operator.go +++ b/cmd/operator.go @@ -149,14 +149,6 @@ var operatorCmd = &cobra.Command{ log.Error().Err(err).Msg("Failed to create in cluster client") return err } - providerLister := iclient.NewProviderLister(provider.Provider.Provider) - - if err = controller. - NewAuthorizationModelReconciler(log, fga, mgr). - SetupWithManager(mgr, defaultCfg); err != nil { - log.Error().Err(err).Str("controller", "authorizationmodel").Msg("unable to create controller") - return err - } kcpClientGetter := iclient.NewManagerKCPClientGetter(mgr, provider.Provider.Provider) kcpClientGetterWithConfig := iclient.NewConfigSchemeKCPClientGetter(restCfg, scheme) diff --git a/cmd/system.go b/cmd/system.go index 8c0dcb4c..108ebbe7 100644 --- a/cmd/system.go +++ b/cmd/system.go @@ -65,7 +65,7 @@ var systemCmd = &cobra.Command{ opts.LeaderElectionConfig = inClusterCfg } - systemProvider, err := apiexport.New(restCfg, systemCfg.APIExportEndpointSlices.SystemPlatformMeshIO, apiexport.Options{ + systemProvider, err := pathaware.New(restCfg, systemCfg.APIExportEndpointSlices.SystemPlatformMeshIO, apiexport.Options{ Scheme: scheme, }) if err != nil { @@ -73,14 +73,14 @@ var systemCmd = &cobra.Command{ return err } - coreProvider, err := apiexport.New(restCfg, systemCfg.APIExportEndpointSlices.CorePlatformMeshIO, apiexport.Options{ + coreProvider, err := pathaware.New(restCfg, systemCfg.APIExportEndpointSlices.CorePlatformMeshIO, apiexport.Options{ Scheme: scheme, }) if err != nil { setupLog.Error(err, "unable to create core apiexport provider") return err } - + multiProv := multiprovider.New(multiprovider.Options{}) if err := multiProv.AddProvider(config.SystemProviderName, systemProvider); err != nil { return err @@ -131,12 +131,19 @@ var systemCmd = &cobra.Command{ return err } - if err = controller.NewStoreReconciler(ctx, log, fgaClient, mgr, &operatorCfg). + if err = controller.NewStoreReconciler(ctx, log, fgaClient, mgr, &operatorCfg, providerLister, kcpClientGetter). SetupWithManager(mgr, defaultCfg); err != nil { log.Error().Err(err).Str("controller", "store").Msg("unable to create controller") return err } + if err = controller. + NewAuthorizationModelReconciler(log, fgaClient, mgr, kcpClientGetter). + SetupWithManager(mgr, defaultCfg); err != nil { + log.Error().Err(err).Str("controller", "authorizationmodel").Msg("unable to create controller") + return err + } + if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { log.Error().Err(err).Msg("unable to set up health check") return err diff --git a/internal/config/config.go b/internal/config/config.go index dbd788f7..2f28730d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -15,12 +15,6 @@ const ( OrgsClusterPath = "root:orgs" ) -const ( - CoreProviderName = "core" - SystemProviderName = "system" - ProviderSeparator = "#" -) - type KeycloakConfig struct { BaseURL string ClientID string diff --git a/internal/controller/authorization_model_controller.go b/internal/controller/authorization_model_controller.go index e12c52c4..f2fbfbcd 100644 --- a/internal/controller/authorization_model_controller.go +++ b/internal/controller/authorization_model_controller.go @@ -2,6 +2,7 @@ package controller import ( "context" + "strings" "time" openfgav1 "github.com/openfga/api/proto/openfga/v1" @@ -9,15 +10,19 @@ import ( "github.com/platform-mesh/golang-commons/controller/filter" "github.com/platform-mesh/golang-commons/logger" corev1alpha1 "github.com/platform-mesh/security-operator/api/v1alpha1" + iclient "github.com/platform-mesh/security-operator/internal/client" + "github.com/platform-mesh/security-operator/internal/config" "github.com/platform-mesh/security-operator/internal/metrics" "github.com/platform-mesh/security-operator/internal/subroutine" "github.com/platform-mesh/subroutines/lifecycle" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/cluster" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/predicate" mcbuilder "sigs.k8s.io/multicluster-runtime/pkg/builder" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" + "sigs.k8s.io/multicluster-runtime/pkg/multicluster" mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" ) @@ -26,11 +31,10 @@ type AuthorizationModelReconciler struct { lifecycle *lifecycle.Lifecycle } -func NewAuthorizationModelReconciler(log *logger.Logger, fga openfgav1.OpenFGAServiceClient, mcMgr mcmanager.Manager) *AuthorizationModelReconciler { - kcpClientHelper := iclient.NewKcpHelper(mcMgr.GetLocalManager().GetConfig(), mcMgr.GetLocalManager().GetScheme()) +func NewAuthorizationModelReconciler(log *logger.Logger, fga openfgav1.OpenFGAServiceClient, mcMgr mcmanager.Manager, kcpClientGetter iclient.KCPClientGetter) *AuthorizationModelReconciler { lc := lifecycle.New(mcMgr, "AuthorizationModelReconciler", func() client.Object { return &corev1alpha1.AuthorizationModel{} - }, subroutine.NewTupleSubroutine(fga, mcMgr, kcpClientHelper)) + }, subroutine.NewTupleSubroutine(fga, kcpClientGetter)) return &AuthorizationModelReconciler{ log: log, @@ -57,7 +61,9 @@ func (r *AuthorizationModelReconciler) SetupWithManager(mgr mcmanager.Manager, c predicates := append([]predicate.Predicate{filter.DebugResourcesBehaviourPredicate(cfg.DebugLabelValue)}, evp...) return mcbuilder.ControllerManagedBy(mgr). Named("authorizationmodel"). - For(&corev1alpha1.AuthorizationModel{}). + For(&corev1alpha1.AuthorizationModel{}, mcbuilder.WithClusterFilter(func(clusterName multicluster.ClusterName, _ cluster.Cluster) bool { + return strings.HasPrefix(string(clusterName), config.CoreProviderName) + })). WithOptions(opts). WithEventFilter(predicate.And(predicates...)). Complete(r) diff --git a/internal/controller/store_controller.go b/internal/controller/store_controller.go index 72c0ce8c..b15876cd 100644 --- a/internal/controller/store_controller.go +++ b/internal/controller/store_controller.go @@ -2,6 +2,7 @@ package controller import ( "context" + "strings" "time" openfgav1 "github.com/openfga/api/proto/openfga/v1" @@ -40,7 +41,7 @@ type StoreReconciler struct { lifecycle *lifecycle.Lifecycle } -func NewStoreReconciler(ctx context.Context, log *logger.Logger, fga openfgav1.OpenFGAServiceClient, mcMgr mcmanager.Manager, cfg *config.Config, lister iclient.Lister) *StoreReconciler { +func NewStoreReconciler(ctx context.Context, log *logger.Logger, fga openfgav1.OpenFGAServiceClient, mcMgr mcmanager.Manager, cfg *config.Config, lister iclient.Lister, kcpClientGetter iclient.KCPClientGetter) *StoreReconciler { lc := lifecycle.New(mcMgr, "StoreReconciler", func() client.Object { return &corev1alpha1.Store{} }, @@ -48,7 +49,7 @@ func NewStoreReconciler(ctx context.Context, log *logger.Logger, fga openfgav1.O subroutine.NewAuthorizationModelSubroutine(fga, mcMgr, lister, func(cfg *rest.Config) discovery.DiscoveryInterface { return discovery.NewDiscoveryClientForConfigOrDie(cfg) }, log), - subroutine.NewTupleSubroutine(fga, mcMgr, kcpClientHelper), + subroutine.NewTupleSubroutine(fga, kcpClientGetter), ).WithConditions(conditions.NewManager()) return &StoreReconciler{ @@ -76,8 +77,8 @@ func (r *StoreReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platforme b := mcbuilder.ControllerManagedBy(mgr). Named("store"). For(&corev1alpha1.Store{}, - mcbuilder.WithClusterFilter(func(clusterName string, _ cluster.Cluster) bool { - return strings.HasPrefix(clusterName, config.SystemProviderName) + mcbuilder.WithClusterFilter(func(clusterName multicluster.ClusterName, _ cluster.Cluster) bool { + return strings.HasPrefix(string(clusterName), config.SystemProviderName) }), ). WithOptions(controller.TypedOptions[mcreconcile.Request]{MaxConcurrentReconciles: cfg.MaxConcurrentReconciles}). @@ -94,7 +95,7 @@ func (r *StoreReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platforme } // stores are engaged by system provider, to trigger a reconciliation with multi provider // it's required to use provider's prefix for request - storeClusterName := multiProviderName(config.SystemProviderName, model.Spec.StoreRef.Cluster) + storeClusterName := config.MultiProviderName(config.SystemProviderName, model.Spec.StoreRef.Cluster) return []mcreconcile.Request{ { @@ -103,20 +104,14 @@ func (r *StoreReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platforme Name: model.Spec.StoreRef.Name, }, }, - ClusterName: multicluster.ClusterName(model.Spec.StoreRef.Cluster), + ClusterName: storeClusterName, }, } }) }, mcbuilder.WithPredicates(predicate.GenerationChangedPredicate{}), - mcbuilder.WithClusterFilter(func(clusterName string, _ cluster.Cluster) bool { - return strings.HasPrefix(clusterName, config.CoreProviderName) + mcbuilder.WithClusterFilter(func(clusterName multicluster.ClusterName, _ cluster.Cluster) bool { + return strings.HasPrefix(string(clusterName), config.CoreProviderName) }), ).Complete(r) } - -// multiProviderName returns a cluster name with provider prefix and separator for multi provider. -// The multi.Provider prefixes cluster names as "providerName#clusterName" -func multiProviderName(providerName, clusterName string) string { - return providerName + config.ProviderSeparator + clusterName -} diff --git a/internal/subroutine/tuples.go b/internal/subroutine/tuples.go index 24f7edfe..9b80283f 100644 --- a/internal/subroutine/tuples.go +++ b/internal/subroutine/tuples.go @@ -12,18 +12,13 @@ import ( "github.com/platform-mesh/security-operator/internal/fga" "github.com/platform-mesh/subroutines" "sigs.k8s.io/controller-runtime/pkg/client" - mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" - "sigs.k8s.io/multicluster-runtime/pkg/multicluster" "k8s.io/apimachinery/pkg/types" - - "github.com/kcp-dev/logicalcluster/v3" ) type tupleSubroutine struct { - fga openfgav1.OpenFGAServiceClient - mgr mcmanager.Manager - kcpHelper iclient.KcpClientHelper + fga openfgav1.OpenFGAServiceClient + kcpClientGetter iclient.KCPClientGetter } // Finalize implements subroutines.Finalizer. @@ -42,13 +37,13 @@ func (t *tupleSubroutine) Finalize(ctx context.Context, obj client.Object) (subr case *securityv1alpha1.AuthorizationModel: managedTuples = o.Status.ManagedTuples - storeCluster, err := t.mgr.GetCluster(ctx, multicluster.ClusterName(o.Spec.StoreRef.Cluster)) + storeClient, err := t.kcpClientGetter.NewClientForLogicalCluster(ctx, o.Spec.StoreRef.Cluster) if err != nil { return subroutines.OK(), fmt.Errorf("unable to create client to store cluster: %w", err) } var store securityv1alpha1.Store - err = cl.Get(ctx, types.NamespacedName{ + err = storeClient.Get(ctx, types.NamespacedName{ Name: o.Spec.StoreRef.Name, }, &store) if err != nil { @@ -102,13 +97,13 @@ func (t *tupleSubroutine) Process(ctx context.Context, obj client.Object) (subro specTuples = o.Spec.Tuples managedTuples = o.Status.ManagedTuples - storeCluster, err := t.mgr.GetCluster(ctx, multicluster.ClusterName(o.Spec.StoreRef.Cluster)) + storeClient, err := t.kcpClientGetter.NewClientForLogicalCluster(ctx, o.Spec.StoreRef.Cluster) if err != nil { return subroutines.OK(), fmt.Errorf("unable to create client to store cluster: %w", err) } var store securityv1alpha1.Store - err = cl.Get(ctx, types.NamespacedName{ + err = storeClient.Get(ctx, types.NamespacedName{ Name: o.Spec.StoreRef.Name, }, &store) if err != nil { @@ -146,11 +141,10 @@ func (t *tupleSubroutine) Process(ctx context.Context, obj client.Object) (subro return subroutines.OK(), nil } -func NewTupleSubroutine(fga openfgav1.OpenFGAServiceClient, mgr mcmanager.Manager, kcpHelper iclient.KcpClientHelper) *tupleSubroutine { +func NewTupleSubroutine(fga openfgav1.OpenFGAServiceClient, kcpClientGetter iclient.KCPClientGetter) *tupleSubroutine { return &tupleSubroutine{ - fga: fga, - mgr: mgr, - kcpHelper: kcpHelper, + fga: fga, + kcpClientGetter: kcpClientGetter, } } diff --git a/internal/subroutine/tuples_test.go b/internal/subroutine/tuples_test.go index 5274e8ae..1d45cec4 100644 --- a/internal/subroutine/tuples_test.go +++ b/internal/subroutine/tuples_test.go @@ -11,23 +11,20 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/multicluster-runtime/pkg/multicluster" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" - - "github.com/kcp-dev/logicalcluster/v3" ) func TestTupleGetName(t *testing.T) { - subroutine := subroutine.NewTupleSubroutine(nil, nil, nil) + subroutine := subroutine.NewTupleSubroutine(nil, nil) assert.Equal(t, "TupleSubroutine", subroutine.GetName()) } func TestTupleFinalizers(t *testing.T) { - subroutine := subroutine.NewTupleSubroutine(nil, nil, nil) + subroutine := subroutine.NewTupleSubroutine(nil, nil) assert.Equal(t, []string{"core.platform-mesh.io/fga-tuples"}, subroutine.Finalizers(nil)) } @@ -170,12 +167,12 @@ func TestTupleProcessWithStore(t *testing.T) { } // Mock GetLocalManager for Store tests - localMgr := mocks.NewCTRLManager(t) + localMgr := mocks.NewMockCTRLManager(t) manager.EXPECT().GetLocalManager().Return(localMgr).Maybe() localMgr.EXPECT().GetConfig().Return(&rest.Config{}).Maybe() localMgr.EXPECT().GetScheme().Return(runtime.NewScheme()).Maybe() - subroutine := subroutine.NewTupleSubroutine(fga, manager, nil) + subroutine := subroutine.NewTupleSubroutine(fga, nil) _, err := subroutine.Process(context.Background(), test.store) if test.expectError { @@ -191,11 +188,11 @@ func TestTupleProcessWithStore(t *testing.T) { func TestTupleProcessWithAuthorizationModel(t *testing.T) { tests := []struct { - name string - store *securityv1alpha1.AuthorizationModel - fgaMocks func(*mocks.MockOpenFGAServiceClient) - kcpHelperMocks func(*mocks.MockKcpHelper) - expectError bool + name string + store *securityv1alpha1.AuthorizationModel + fgaMocks func(*mocks.MockOpenFGAServiceClient) + kcpClientGetterMocks func(*mocks.MockKCPClientGetter) + expectError bool }{ { name: "should process and add tuples to the authorization model", @@ -232,10 +229,9 @@ func TestTupleProcessWithAuthorizationModel(t *testing.T) { fgaMocks: func(fga *mocks.MockOpenFGAServiceClient) { fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, nil) }, - kcpHelperMocks: func(kcpHelper *mocks.MockKcpHelper) { + kcpClientGetterMocks: func(kcpClientGetter *mocks.MockKCPClientGetter) { storeClient := mocks.NewMockClient(t) - mgr.EXPECT().GetCluster(mock.Anything, multicluster.ClusterName("store-cluster")).Return(storeCluster, nil) - storeCluster.EXPECT().GetClient().Return(storeClient) + kcpClientGetter.EXPECT().NewClientForLogicalCluster(mock.Anything, "store-cluster").Return(storeClient, nil) storeClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { store := o.(*securityv1alpha1.Store) *store = securityv1alpha1.Store{ @@ -293,10 +289,9 @@ func TestTupleProcessWithAuthorizationModel(t *testing.T) { // Apply (batch write) + Delete (batch delete) fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, nil).Twice() }, - kcpHelperMocks: func(kcpHelper *mocks.MockKcpHelper) { + kcpClientGetterMocks: func(kcpClientGetter *mocks.MockKCPClientGetter) { storeClient := mocks.NewMockClient(t) - mgr.EXPECT().GetCluster(mock.Anything, multicluster.ClusterName("store-cluster")).Return(storeCluster, nil) - storeCluster.EXPECT().GetClient().Return(storeClient) + kcpClientGetter.EXPECT().NewClientForLogicalCluster(mock.Anything, "store-cluster").Return(storeClient, nil) storeClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { store := o.(*securityv1alpha1.Store) *store = securityv1alpha1.Store{ @@ -317,12 +312,12 @@ func TestTupleProcessWithAuthorizationModel(t *testing.T) { test.fgaMocks(fga) } - kcpHelper := mocks.NewMockKcpHelper(t) - if test.kcpHelperMocks != nil { - test.kcpHelperMocks(kcpHelper) + kcpClientGetter := mocks.NewMockKCPClientGetter(t) + if test.kcpClientGetterMocks != nil { + test.kcpClientGetterMocks(kcpClientGetter) } - subroutine := subroutine.NewTupleSubroutine(fga, nil, kcpHelper) + subroutine := subroutine.NewTupleSubroutine(fga, kcpClientGetter) ctx := context.Background() @@ -340,11 +335,11 @@ func TestTupleProcessWithAuthorizationModel(t *testing.T) { func TestTupleFinalizationWithAuthorizationModel(t *testing.T) { tests := []struct { - name string - store *securityv1alpha1.AuthorizationModel - fgaMocks func(*mocks.MockOpenFGAServiceClient) - kcpHelperMocks func(*mocks.MockKcpHelper) - expectError bool + name string + store *securityv1alpha1.AuthorizationModel + fgaMocks func(*mocks.MockOpenFGAServiceClient) + kcpClientGetterMocks func(*mocks.MockKCPClientGetter) + expectError bool }{ { name: "should finalize the authorization model", @@ -374,10 +369,9 @@ func TestTupleFinalizationWithAuthorizationModel(t *testing.T) { // delete call fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, nil) }, - kcpHelperMocks: func(kcpHelper *mocks.MockKcpHelper) { + kcpClientGetterMocks: func(kcpClientGetter *mocks.MockKCPClientGetter) { storeClient := mocks.NewMockClient(t) - mgr.EXPECT().GetCluster(mock.Anything, multicluster.ClusterName("store-cluster")).Return(storeCluster, nil) - storeCluster.EXPECT().GetClient().Return(storeClient) + kcpClientGetter.EXPECT().NewClientForLogicalCluster(mock.Anything, "store-cluster").Return(storeClient, nil) storeClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { store := o.(*securityv1alpha1.Store) *store = securityv1alpha1.Store{ @@ -398,12 +392,12 @@ func TestTupleFinalizationWithAuthorizationModel(t *testing.T) { test.fgaMocks(fga) } - kcpHelper := mocks.NewMockKcpHelper(t) - if test.kcpHelperMocks != nil { - test.kcpHelperMocks(kcpHelper) + kcpClientGetter := mocks.NewMockKCPClientGetter(t) + if test.kcpClientGetterMocks != nil { + test.kcpClientGetterMocks(kcpClientGetter) } - subroutine := subroutine.NewTupleSubroutine(fga, nil, kcpHelper) + subroutine := subroutine.NewTupleSubroutine(fga, kcpClientGetter) ctx := context.Background() @@ -473,18 +467,7 @@ func TestTupleFinalizationWithStore(t *testing.T) { test.fgaMocks(fga) } - manager := mocks.NewMockManager(t) - if test.mgrMocks != nil { - test.mgrMocks(manager) - } - - // Mock GetLocalManager for Store tests - localMgr := mocks.NewCTRLManager(t) - manager.EXPECT().GetLocalManager().Return(localMgr).Maybe() - localMgr.EXPECT().GetConfig().Return(&rest.Config{}).Maybe() - localMgr.EXPECT().GetScheme().Return(runtime.NewScheme()).Maybe() - - subroutine := subroutine.NewTupleSubroutine(fga, manager, nil) + subroutine := subroutine.NewTupleSubroutine(fga, nil) _, err := subroutine.Finalize(context.Background(), test.store) if test.expectError { From f17b016bc2f3e3859b554ece9cacdebab0b04f7c Mon Sep 17 00:00:00 2001 From: OlegErshov Date: Tue, 19 May 2026 14:12:23 +0200 Subject: [PATCH 5/6] fix: use proper cluster naming On-behalf-of: SAP aleh.yarshou@sap.com --- internal/config/config.go | 10 ++++++++++ internal/subroutine/authorization_model.go | 4 ++-- internal/subroutine/tuples.go | 5 +++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 2f28730d..927e56f7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,6 +2,7 @@ package config import ( "os" + "strings" "time" "github.com/spf13/pflag" @@ -195,3 +196,12 @@ func (config Config) TerminatorName() string { func MultiProviderName(providerName, clusterName string) multicluster.ClusterName { return multicluster.ClusterName(providerName + providerSeparator + clusterName) } + +// Strip provider prefix from cluster name ("core#1kar1u6c65ykt4ea" -> "1kar1u6c65ykt4ea") +func StripProviderPrefix(clusterName multicluster.ClusterName) string { + prefixedClusterName := string(clusterName) + if _, ClusteName, found := strings.Cut(prefixedClusterName, providerSeparator); found { + return ClusteName + } + return prefixedClusterName +} diff --git a/internal/subroutine/authorization_model.go b/internal/subroutine/authorization_model.go index fa41f31d..df8b90d8 100644 --- a/internal/subroutine/authorization_model.go +++ b/internal/subroutine/authorization_model.go @@ -13,6 +13,7 @@ import ( "github.com/platform-mesh/golang-commons/logger" securityv1alpha1 "github.com/platform-mesh/security-operator/api/v1alpha1" iclient "github.com/platform-mesh/security-operator/internal/client" + "github.com/platform-mesh/security-operator/internal/config" "github.com/platform-mesh/security-operator/internal/util" "github.com/platform-mesh/subroutines" "google.golang.org/protobuf/encoding/protojson" @@ -108,7 +109,7 @@ func getRelatedAuthorizationModels(ctx context.Context, lister iclient.Lister, s var extendingModules securityv1alpha1.AuthorizationModelList for _, model := range allAuthorizationModels.Items { - if model.Spec.StoreRef.Name != store.Name || model.Spec.StoreRef.Cluster != string(storeClusterKey) { + if model.Spec.StoreRef.Name != store.Name || model.Spec.StoreRef.Cluster != config.StripProviderPrefix(storeClusterKey) { continue } @@ -203,7 +204,6 @@ func (a *authorizationModelSubroutine) Process(ctx context.Context, obj client.O if string(currentRaw) == string(desiredRaw) { return subroutines.OK(), nil } - } res, err := a.fga.WriteAuthorizationModel(ctx, &openfgav1.WriteAuthorizationModelRequest{ diff --git a/internal/subroutine/tuples.go b/internal/subroutine/tuples.go index 9b80283f..224ef5f1 100644 --- a/internal/subroutine/tuples.go +++ b/internal/subroutine/tuples.go @@ -9,6 +9,7 @@ import ( "github.com/platform-mesh/golang-commons/logger" securityv1alpha1 "github.com/platform-mesh/security-operator/api/v1alpha1" iclient "github.com/platform-mesh/security-operator/internal/client" + "github.com/platform-mesh/security-operator/internal/config" "github.com/platform-mesh/security-operator/internal/fga" "github.com/platform-mesh/subroutines" "sigs.k8s.io/controller-runtime/pkg/client" @@ -37,7 +38,7 @@ func (t *tupleSubroutine) Finalize(ctx context.Context, obj client.Object) (subr case *securityv1alpha1.AuthorizationModel: managedTuples = o.Status.ManagedTuples - storeClient, err := t.kcpClientGetter.NewClientForLogicalCluster(ctx, o.Spec.StoreRef.Cluster) + storeClient, err := t.kcpClientGetter.NewClientForLogicalCluster(ctx, string(config.MultiProviderName(config.SystemProviderName, o.Spec.StoreRef.Cluster))) if err != nil { return subroutines.OK(), fmt.Errorf("unable to create client to store cluster: %w", err) } @@ -97,7 +98,7 @@ func (t *tupleSubroutine) Process(ctx context.Context, obj client.Object) (subro specTuples = o.Spec.Tuples managedTuples = o.Status.ManagedTuples - storeClient, err := t.kcpClientGetter.NewClientForLogicalCluster(ctx, o.Spec.StoreRef.Cluster) + storeClient, err := t.kcpClientGetter.NewClientForLogicalCluster(ctx, string(config.MultiProviderName(config.SystemProviderName, o.Spec.StoreRef.Cluster))) if err != nil { return subroutines.OK(), fmt.Errorf("unable to create client to store cluster: %w", err) } From b7388e59c5f44caa42dd4d08efa38ca48fe9ada8 Mon Sep 17 00:00:00 2001 From: OlegErshov Date: Tue, 19 May 2026 14:28:33 +0200 Subject: [PATCH 6/6] chore: address failing tests On-behalf-of: SAP aleh.yarshou@sap.com --- internal/subroutine/tuples_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/subroutine/tuples_test.go b/internal/subroutine/tuples_test.go index 1d45cec4..cb25c4b3 100644 --- a/internal/subroutine/tuples_test.go +++ b/internal/subroutine/tuples_test.go @@ -231,7 +231,7 @@ func TestTupleProcessWithAuthorizationModel(t *testing.T) { }, kcpClientGetterMocks: func(kcpClientGetter *mocks.MockKCPClientGetter) { storeClient := mocks.NewMockClient(t) - kcpClientGetter.EXPECT().NewClientForLogicalCluster(mock.Anything, "store-cluster").Return(storeClient, nil) + kcpClientGetter.EXPECT().NewClientForLogicalCluster(mock.Anything, "system#store-cluster").Return(storeClient, nil) storeClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { store := o.(*securityv1alpha1.Store) *store = securityv1alpha1.Store{ @@ -291,7 +291,7 @@ func TestTupleProcessWithAuthorizationModel(t *testing.T) { }, kcpClientGetterMocks: func(kcpClientGetter *mocks.MockKCPClientGetter) { storeClient := mocks.NewMockClient(t) - kcpClientGetter.EXPECT().NewClientForLogicalCluster(mock.Anything, "store-cluster").Return(storeClient, nil) + kcpClientGetter.EXPECT().NewClientForLogicalCluster(mock.Anything, "system#store-cluster").Return(storeClient, nil) storeClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { store := o.(*securityv1alpha1.Store) *store = securityv1alpha1.Store{ @@ -371,7 +371,7 @@ func TestTupleFinalizationWithAuthorizationModel(t *testing.T) { }, kcpClientGetterMocks: func(kcpClientGetter *mocks.MockKCPClientGetter) { storeClient := mocks.NewMockClient(t) - kcpClientGetter.EXPECT().NewClientForLogicalCluster(mock.Anything, "store-cluster").Return(storeClient, nil) + kcpClientGetter.EXPECT().NewClientForLogicalCluster(mock.Anything, "system#store-cluster").Return(storeClient, nil) storeClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { store := o.(*securityv1alpha1.Store) *store = securityv1alpha1.Store{