From b8805682d9c20b1e2b1617a35ca592ce970bb4e7 Mon Sep 17 00:00:00 2001 From: Denis Makarenkov Date: Thu, 28 May 2026 21:50:14 +0300 Subject: [PATCH] feat: scoped kubeconfigs (#524) Signed-off-by: Denis Makarenkov --- internal/subroutine/account_tuples.go | 81 +++--- internal/subroutine/account_tuples_test.go | 234 +++++++++--------- internal/subroutine/workspace_initializer.go | 20 +- ...ma-accountinfos.core.platform-mesh.io.yaml | 12 + 4 files changed, 164 insertions(+), 183 deletions(-) diff --git a/internal/subroutine/account_tuples.go b/internal/subroutine/account_tuples.go index 6a677055..9c4fd408 100644 --- a/internal/subroutine/account_tuples.go +++ b/internal/subroutine/account_tuples.go @@ -16,7 +16,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" - "github.com/kcp-dev/logicalcluster/v3" kcpcorev1alpha1 "github.com/kcp-dev/sdk/apis/core/v1alpha1" ) @@ -45,50 +44,36 @@ func (s *AccountTuplesSubroutine) Initialize(ctx context.Context, obj client.Obj func (s *AccountTuplesSubroutine) reconcile(ctx context.Context, obj client.Object) (subroutines.Result, error) { lc := obj.(*kcpcorev1alpha1.LogicalCluster) - accountPath, err := platformmeshpath.NewAccountPathFromLogicalCluster(lc) - if err != nil { + if _, err := platformmeshpath.NewAccountPathFromLogicalCluster(lc); err != nil { return subroutines.OK(), fmt.Errorf("getting AccountPath from LogicalCluster: %w", err) } - storeID, err := s.storeIDGetter.Get(ctx, accountPath.Org().Base()) + accountInfo, err := s.getLocalAccountInfo(ctx) if err != nil { - return subroutines.OK(), fmt.Errorf("getting store ID: %w", err) + return subroutines.OK(), err } - - // Determine the parent's and grandParent's LogicalCluster ID - parentPath, _ := accountPath.Parent() - parentAccountClusterID, parentAccountLC, err := s.clusterAndIDFromLogicalClusterForPath(ctx, parentPath) - if err != nil { - return subroutines.OK(), fmt.Errorf("getting parent account's LogicalCluster: %w", err) + if accountInfo.Spec.ParentAccount == nil { + return subroutines.OK(), fmt.Errorf("parent account is not set on AccountInfo") + } + if accountInfo.Spec.Account.Creator == nil || *accountInfo.Spec.Account.Creator == "" { + return subroutines.OK(), fmt.Errorf("account creator is nil or empty") } - grandParentAccountClusterID := parentAccountLC.Spec.Owner.Cluster - // Retrieve the Account resource out of the parent workspace to determine - // the creator - parentAccountClient, err := s.kcpClientGetter.NewClientForLogicalCluster(ctx, parentPath.String()) + storeID, err := s.storeIDGetter.Get(ctx, accountInfo.Spec.Organization.Name) if err != nil { - return subroutines.OK(), fmt.Errorf("getting client for parent account cluster: %w", err) - } - var acc accountsv1alpha1.Account - if err := parentAccountClient.Get(ctx, client.ObjectKey{ - Name: accountPath.Base(), - }, &acc); err != nil { - return subroutines.OK(), fmt.Errorf("getting Account in parent account cluster: %w", err) - } - if acc.Spec.Creator == nil || *acc.Spec.Creator == "" { - return subroutines.OK(), fmt.Errorf("account creator is nil or empty") + return subroutines.OK(), fmt.Errorf("getting store ID: %w", err) } tuples, err := fga.InitialTuplesForAccount(fga.InitialTuplesForAccountInput{ BaseTuplesInput: fga.BaseTuplesInput{ - Creator: *acc.Spec.Creator, - AccountOriginClusterID: parentAccountClusterID, - AccountName: accountPath.Base(), + Creator: *accountInfo.Spec.Account.Creator, + AccountOriginClusterID: accountInfo.Spec.ParentAccount.GeneratedClusterId, + AccountName: accountInfo.Spec.Account.Name, CreatorRelation: s.creatorRelation, ObjectType: s.objectType, }, - ParentOriginClusterID: grandParentAccountClusterID, - ParentName: parentPath.Base(), + ParentOriginClusterID: accountInfo.Spec.ParentAccount.OriginClusterId, + ParentName: accountInfo.Spec.ParentAccount.Name, ParentRelation: s.parentRelation, }) if err != nil { @@ -109,15 +94,16 @@ func (s *AccountTuplesSubroutine) Terminate(ctx context.Context, obj client.Obje if err != nil { return subroutines.OK(), fmt.Errorf("getting AccountPath from LogicalCluster: %w", err) } - parentPath, _ := accountPath.Parent() - - // Determine the parent's LogicalClusterID - parentClusterID, _, err := s.clusterAndIDFromLogicalClusterForPath(ctx, parentPath) + accountInfo, err := s.getLocalAccountInfo(ctx) if err != nil { - return subroutines.OK(), fmt.Errorf("getting parent account's LogicalCluster: %w", err) + return subroutines.OK(), err + } + if accountInfo.Spec.ParentAccount == nil { + return subroutines.OK(), fmt.Errorf("parent account is not set on AccountInfo") } - storeID, err := s.storeIDGetter.Get(ctx, accountPath.Org().Base()) + parentClusterID := accountInfo.Spec.ParentAccount.GeneratedClusterId + storeID, err := s.storeIDGetter.Get(ctx, accountInfo.Spec.Organization.Name) if err != nil { return subroutines.OK(), fmt.Errorf("getting store ID: %w", err) } @@ -173,25 +159,16 @@ var ( _ subroutines.Terminator = &AccountTuplesSubroutine{} ) -// clusterAndIDFromLogicalClusterForPath retrieves the LogicalCluster of a given -// path and returns its cluster ID and the LogicalCluster object. -func (s *AccountTuplesSubroutine) clusterAndIDFromLogicalClusterForPath(ctx context.Context, p logicalcluster.Path) (string, kcpcorev1alpha1.LogicalCluster, error) { - var lc kcpcorev1alpha1.LogicalCluster - - clusterClient, err := s.kcpClientGetter.NewClientForLogicalCluster(ctx, p.String()) +func (s *AccountTuplesSubroutine) getLocalAccountInfo(ctx context.Context) (*accountsv1alpha1.AccountInfo, error) { + cluster, err := s.mgr.ClusterFromContext(ctx) if err != nil { - return "", lc, fmt.Errorf("getting account cluster client: %w", err) - } - if err := clusterClient.Get(ctx, client.ObjectKey{ - Name: "cluster", - }, &lc); err != nil { - return "", lc, fmt.Errorf("getting account's LogicalCluster: %w", err) + return nil, fmt.Errorf("failed to get cluster from context: %w", err) } - clusterID, ok := lc.Annotations["kcp.io/cluster"] - if !ok || clusterID == "" { - return "", lc, fmt.Errorf("cluster-annotation kcp.io/cluster on LogicalCluster is not set") + var accountInfo accountsv1alpha1.AccountInfo + if err := cluster.GetClient().Get(ctx, client.ObjectKey{Name: "account"}, &accountInfo); err != nil { + return nil, fmt.Errorf("getting local AccountInfo: %w", err) } - return clusterID, lc, nil + return &accountInfo, nil } diff --git a/internal/subroutine/account_tuples_test.go b/internal/subroutine/account_tuples_test.go index 61aec773..40fc4f63 100644 --- a/internal/subroutine/account_tuples_test.go +++ b/internal/subroutine/account_tuples_test.go @@ -21,8 +21,9 @@ import ( ) const ( - accountLCPath = "root:orgs:myorg:myaccount" - parentClusterID = "parent-cluster-id" + accountLCPath = "root:orgs:myorg:myaccount" + parentClusterID = "parent-cluster-id" + grandParentClusterID = "grand-cluster-id" ) func newAccountLogicalCluster() *kcpcorev1alpha1.LogicalCluster { @@ -35,13 +36,43 @@ func newAccountLogicalCluster() *kcpcorev1alpha1.LogicalCluster { } } -func mockParentLogicalCluster(kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient) { - kcpHelper.EXPECT().NewClientForLogicalCluster(mock.Anything, mock.Anything).Return(parentClient, nil).Once() - parentClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "cluster"}, mock.Anything). +func newLocalAccountInfo(creator *string) accountsv1alpha1.AccountInfo { + return accountsv1alpha1.AccountInfo{ + ObjectMeta: metav1.ObjectMeta{Name: "account"}, + Spec: accountsv1alpha1.AccountInfoSpec{ + Account: accountsv1alpha1.AccountLocation{ + Name: "myaccount", + Creator: creator, + Path: accountLCPath, + }, + ParentAccount: &accountsv1alpha1.AccountLocation{ + Name: "myorg", + GeneratedClusterId: parentClusterID, + OriginClusterId: grandParentClusterID, + }, + Organization: accountsv1alpha1.AccountLocation{ + Name: "myorg", + }, + }, + } +} + +func mockLocalAccountInfo( + mgr *mocks.MockManager, + cluster *mocks.MockCluster, + localClient *mocks.MockClient, + accountInfo accountsv1alpha1.AccountInfo, + getErr error, +) { + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil).Once() + cluster.EXPECT().GetClient().Return(localClient).Once() + localClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "account"}, mock.Anything). RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - if lc, ok := o.(*kcpcorev1alpha1.LogicalCluster); ok { - lc.Annotations = map[string]string{"kcp.io/cluster": parentClusterID} - lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{Cluster: "grand-cluster-id"} + if getErr != nil { + return getErr + } + if ai, ok := o.(*accountsv1alpha1.AccountInfo); ok { + *ai = accountInfo } return nil }).Once() @@ -54,21 +85,14 @@ func TestAccountTuplesSubroutine_GetName(t *testing.T) { func TestAccountTuplesSubroutine_Process(t *testing.T) { storeIDGetter := mocks.NewMockStoreIDGetter(t) - kcpHelper := mocks.NewMockKCPClientGetter(t) - parentClient := mocks.NewMockClient(t) + mgr := mocks.NewMockManager(t) + cluster := mocks.NewMockCluster(t) + localClient := mocks.NewMockClient(t) fgaClient := mocks.NewMockOpenFGAServiceClient(t) - storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) - mockParentLogicalCluster(kcpHelper, parentClient) - kcpHelper.EXPECT().NewClientForLogicalCluster(mock.Anything, mock.Anything).Return(parentClient, nil).Once() creator := "user@example.com" - parentClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "myaccount"}, mock.Anything). - RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - if acc, ok := o.(*accountsv1alpha1.Account); ok { - acc.Spec.Creator = &creator - } - return nil - }).Once() + mockLocalAccountInfo(mgr, cluster, localClient, newLocalAccountInfo(&creator), nil) + storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) expectedTuples, err := fga.InitialTuplesForAccount(fga.InitialTuplesForAccountInput{ BaseTuplesInput: fga.BaseTuplesInput{ @@ -78,7 +102,7 @@ func TestAccountTuplesSubroutine_Process(t *testing.T) { CreatorRelation: "creator", ObjectType: "account", }, - ParentOriginClusterID: "grand-cluster-id", + ParentOriginClusterID: grandParentClusterID, ParentName: "myorg", ParentRelation: "parent", }) @@ -102,7 +126,7 @@ func TestAccountTuplesSubroutine_Process(t *testing.T) { return true })).Return(&openfgav1.WriteResponse{}, nil).Once() - sub := subroutine.NewAccountTuplesSubroutine(nil, fgaClient, storeIDGetter, "creator", "parent", "account", kcpHelper) + sub := subroutine.NewAccountTuplesSubroutine(mgr, fgaClient, storeIDGetter, "creator", "parent", "account", nil) _, err = sub.Process(context.Background(), newAccountLogicalCluster()) assert.NoError(t, err) } @@ -111,100 +135,67 @@ func TestAccountTuplesSubroutine_Initialize(t *testing.T) { tests := []struct { name string obj *kcpcorev1alpha1.LogicalCluster - mockSetup func(*mocks.MockStoreIDGetter, *mocks.MockKCPClientGetter, *mocks.MockClient, *mocks.MockOpenFGAServiceClient) + mockSetup func(*mocks.MockStoreIDGetter, *mocks.MockManager, *mocks.MockCluster, *mocks.MockClient, *mocks.MockOpenFGAServiceClient) expectError bool }{ { name: "error: missing path annotation", obj: &kcpcorev1alpha1.LogicalCluster{}, - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - }, - expectError: true, - }, - { - name: "error: storeIDGetter fails", - obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("", assert.AnError) - }, - expectError: true, - }, - { - name: "error: NewForLogicalCluster fails for parent cluster", - obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) - kcpHelper.EXPECT().NewClientForLogicalCluster(mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() + mockSetup: func(_ *mocks.MockStoreIDGetter, _ *mocks.MockManager, _ *mocks.MockCluster, _ *mocks.MockClient, _ *mocks.MockOpenFGAServiceClient) { }, expectError: true, }, { - name: "error: Get LogicalCluster fails", + name: "error: cluster from context fails", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) - kcpHelper.EXPECT().NewClientForLogicalCluster(mock.Anything, mock.Anything).Return(parentClient, nil).Once() - parentClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "cluster"}, mock.Anything).Return(assert.AnError).Once() + mockSetup: func(_ *mocks.MockStoreIDGetter, mgr *mocks.MockManager, _ *mocks.MockCluster, _ *mocks.MockClient, _ *mocks.MockOpenFGAServiceClient) { + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(nil, assert.AnError).Once() }, expectError: true, }, { - name: "error: kcp.io/cluster annotation missing on parent LogicalCluster", + name: "error: get accountInfo fails", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) - kcpHelper.EXPECT().NewClientForLogicalCluster(mock.Anything, mock.Anything).Return(parentClient, nil).Once() - parentClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "cluster"}, mock.Anything).Return(nil).Once() + mockSetup: func(_ *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, _ *mocks.MockOpenFGAServiceClient) { + mockLocalAccountInfo(mgr, cluster, localClient, accountsv1alpha1.AccountInfo{}, assert.AnError) }, expectError: true, }, { - name: "error: NewForLogicalCluster fails for parent account client", + name: "error: parent account missing", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) - mockParentLogicalCluster(kcpHelper, parentClient) - kcpHelper.EXPECT().NewClientForLogicalCluster(mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() + mockSetup: func(_ *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, _ *mocks.MockOpenFGAServiceClient) { + accountInfo := newLocalAccountInfo(nil) + accountInfo.Spec.ParentAccount = nil + mockLocalAccountInfo(mgr, cluster, localClient, accountInfo, nil) }, expectError: true, }, { - name: "error: Get Account fails", + name: "error: account creator is empty", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) - mockParentLogicalCluster(kcpHelper, parentClient) - kcpHelper.EXPECT().NewClientForLogicalCluster(mock.Anything, mock.Anything).Return(parentClient, nil).Once() - parentClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "myaccount"}, mock.Anything).Return(assert.AnError).Once() + mockSetup: func(_ *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, _ *mocks.MockOpenFGAServiceClient) { + mockLocalAccountInfo(mgr, cluster, localClient, newLocalAccountInfo(nil), nil) }, expectError: true, }, { - name: "error: account creator is empty", + name: "error: storeIDGetter fails", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) - mockParentLogicalCluster(kcpHelper, parentClient) - kcpHelper.EXPECT().NewClientForLogicalCluster(mock.Anything, mock.Anything).Return(parentClient, nil).Once() - parentClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "myaccount"}, mock.Anything).Return(nil).Once() + mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, _ *mocks.MockOpenFGAServiceClient) { + creator := "user@example.com" + mockLocalAccountInfo(mgr, cluster, localClient, newLocalAccountInfo(&creator), nil) + storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("", assert.AnError) }, expectError: true, }, { name: "error: fga.Write fails", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) - mockParentLogicalCluster(kcpHelper, parentClient) - kcpHelper.EXPECT().NewClientForLogicalCluster(mock.Anything, mock.Anything).Return(parentClient, nil).Once() + mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { creator := "user@example.com" - parentClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "myaccount"}, mock.Anything). - RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - if acc, ok := o.(*accountsv1alpha1.Account); ok { - acc.Spec.Creator = &creator - } - return nil - }).Once() + mockLocalAccountInfo(mgr, cluster, localClient, newLocalAccountInfo(&creator), nil) + storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) fgaClient.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, assert.AnError) }, expectError: true, @@ -212,18 +203,10 @@ func TestAccountTuplesSubroutine_Initialize(t *testing.T) { { name: "success", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) - mockParentLogicalCluster(kcpHelper, parentClient) - kcpHelper.EXPECT().NewClientForLogicalCluster(mock.Anything, mock.Anything).Return(parentClient, nil).Once() + mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { creator := "user@example.com" - parentClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "myaccount"}, mock.Anything). - RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { - if acc, ok := o.(*accountsv1alpha1.Account); ok { - acc.Spec.Creator = &creator - } - return nil - }).Once() + mockLocalAccountInfo(mgr, cluster, localClient, newLocalAccountInfo(&creator), nil) + storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) fgaClient.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil) }, expectError: false, @@ -233,13 +216,14 @@ func TestAccountTuplesSubroutine_Initialize(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { storeIDGetter := mocks.NewMockStoreIDGetter(t) - kcpHelper := mocks.NewMockKCPClientGetter(t) - parentClient := mocks.NewMockClient(t) + mgr := mocks.NewMockManager(t) + cluster := mocks.NewMockCluster(t) + localClient := mocks.NewMockClient(t) fgaClient := mocks.NewMockOpenFGAServiceClient(t) - test.mockSetup(storeIDGetter, kcpHelper, parentClient, fgaClient) + test.mockSetup(storeIDGetter, mgr, cluster, localClient, fgaClient) - sub := subroutine.NewAccountTuplesSubroutine(nil, fgaClient, storeIDGetter, "creator", "parent", "account", kcpHelper) + sub := subroutine.NewAccountTuplesSubroutine(mgr, fgaClient, storeIDGetter, "creator", "parent", "account", nil) _, err := sub.Initialize(context.Background(), test.obj) if test.expectError { assert.Error(t, err) @@ -254,29 +238,40 @@ func TestAccountTuplesSubroutine_Terminate(t *testing.T) { tests := []struct { name string obj *kcpcorev1alpha1.LogicalCluster - mockSetup func(*mocks.MockStoreIDGetter, *mocks.MockKCPClientGetter, *mocks.MockClient, *mocks.MockOpenFGAServiceClient) + mockSetup func(*mocks.MockStoreIDGetter, *mocks.MockManager, *mocks.MockCluster, *mocks.MockClient, *mocks.MockOpenFGAServiceClient) expectError bool }{ { name: "error: missing path annotation", obj: &kcpcorev1alpha1.LogicalCluster{}, - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { + mockSetup: func(_ *mocks.MockStoreIDGetter, _ *mocks.MockManager, _ *mocks.MockCluster, _ *mocks.MockClient, _ *mocks.MockOpenFGAServiceClient) { + }, + expectError: true, + }, + { + name: "error: cluster from context fails", + obj: newAccountLogicalCluster(), + mockSetup: func(_ *mocks.MockStoreIDGetter, mgr *mocks.MockManager, _ *mocks.MockCluster, _ *mocks.MockClient, _ *mocks.MockOpenFGAServiceClient) { + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(nil, assert.AnError).Once() }, expectError: true, }, { - name: "error: NewForLogicalCluster fails", + name: "error: parent account missing", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - kcpHelper.EXPECT().NewClientForLogicalCluster(mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() + mockSetup: func(_ *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, _ *mocks.MockOpenFGAServiceClient) { + accountInfo := newLocalAccountInfo(nil) + accountInfo.Spec.ParentAccount = nil + mockLocalAccountInfo(mgr, cluster, localClient, accountInfo, nil) }, expectError: true, }, { name: "error: storeIDGetter fails", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - mockParentLogicalCluster(kcpHelper, parentClient) + mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, _ *mocks.MockOpenFGAServiceClient) { + creator := "user@example.com" + mockLocalAccountInfo(mgr, cluster, localClient, newLocalAccountInfo(&creator), nil) storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("", assert.AnError) }, expectError: true, @@ -284,8 +279,9 @@ func TestAccountTuplesSubroutine_Terminate(t *testing.T) { { name: "error: ListWithKey fails", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - mockParentLogicalCluster(kcpHelper, parentClient) + mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { + creator := "user@example.com" + mockLocalAccountInfo(mgr, cluster, localClient, newLocalAccountInfo(&creator), nil) storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) fgaClient.EXPECT().Read(mock.Anything, mock.Anything).Return(nil, assert.AnError) }, @@ -294,10 +290,10 @@ func TestAccountTuplesSubroutine_Terminate(t *testing.T) { { name: "error: ListWithKey for role fails", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - mockParentLogicalCluster(kcpHelper, parentClient) + mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { + creator := "user@example.com" + mockLocalAccountInfo(mgr, cluster, localClient, newLocalAccountInfo(&creator), nil) storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) - // First Read returns a tuple whose User has the role prefix for this account. roleUser := "role:account/" + parentClusterID + "/myaccount/owner#assignee" fgaClient.EXPECT().Read(mock.Anything, mock.Anything). Return(&openfgav1.ReadResponse{ @@ -305,7 +301,6 @@ func TestAccountTuplesSubroutine_Terminate(t *testing.T) { {Key: &openfgav1.TupleKey{Object: "account:" + parentClusterID + "/myaccount", Relation: "member", User: roleUser}}, }, }, nil).Once() - // Second Read (for role references) fails. fgaClient.EXPECT().Read(mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() }, expectError: true, @@ -313,8 +308,9 @@ func TestAccountTuplesSubroutine_Terminate(t *testing.T) { { name: "error: Delete fails", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - mockParentLogicalCluster(kcpHelper, parentClient) + mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { + creator := "user@example.com" + mockLocalAccountInfo(mgr, cluster, localClient, newLocalAccountInfo(&creator), nil) storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) fgaClient.EXPECT().Read(mock.Anything, mock.Anything). Return(&openfgav1.ReadResponse{ @@ -329,8 +325,9 @@ func TestAccountTuplesSubroutine_Terminate(t *testing.T) { { name: "success: no tuples to delete", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - mockParentLogicalCluster(kcpHelper, parentClient) + mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { + creator := "user@example.com" + mockLocalAccountInfo(mgr, cluster, localClient, newLocalAccountInfo(&creator), nil) storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) fgaClient.EXPECT().Read(mock.Anything, mock.Anything). Return(&openfgav1.ReadResponse{Tuples: []*openfgav1.Tuple{}}, nil).Once() @@ -340,8 +337,9 @@ func TestAccountTuplesSubroutine_Terminate(t *testing.T) { { name: "success: tuples with role prefix deleted", obj: newAccountLogicalCluster(), - mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, kcpHelper *mocks.MockKCPClientGetter, parentClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { - mockParentLogicalCluster(kcpHelper, parentClient) + mockSetup: func(storeIDGetter *mocks.MockStoreIDGetter, mgr *mocks.MockManager, cluster *mocks.MockCluster, localClient *mocks.MockClient, fgaClient *mocks.MockOpenFGAServiceClient) { + creator := "user@example.com" + mockLocalAccountInfo(mgr, cluster, localClient, newLocalAccountInfo(&creator), nil) storeIDGetter.EXPECT().Get(mock.Anything, "myorg").Return("store-id", nil) roleUser := "role:account/" + parentClusterID + "/myaccount/owner#assignee" fgaClient.EXPECT().Read(mock.Anything, mock.Anything). @@ -350,7 +348,6 @@ func TestAccountTuplesSubroutine_Terminate(t *testing.T) { {Key: &openfgav1.TupleKey{Object: "account:" + parentClusterID + "/myaccount", Relation: "member", User: roleUser}}, }, }, nil).Once() - // Role references lookup returns no results. fgaClient.EXPECT().Read(mock.Anything, mock.Anything). Return(&openfgav1.ReadResponse{Tuples: []*openfgav1.Tuple{}}, nil).Once() fgaClient.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil) @@ -362,13 +359,14 @@ func TestAccountTuplesSubroutine_Terminate(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { storeIDGetter := mocks.NewMockStoreIDGetter(t) - kcpHelper := mocks.NewMockKCPClientGetter(t) - parentClient := mocks.NewMockClient(t) + mgr := mocks.NewMockManager(t) + cluster := mocks.NewMockCluster(t) + localClient := mocks.NewMockClient(t) fgaClient := mocks.NewMockOpenFGAServiceClient(t) - test.mockSetup(storeIDGetter, kcpHelper, parentClient, fgaClient) + test.mockSetup(storeIDGetter, mgr, cluster, localClient, fgaClient) - sub := subroutine.NewAccountTuplesSubroutine(nil, fgaClient, storeIDGetter, "creator", "parent", "account", kcpHelper) + sub := subroutine.NewAccountTuplesSubroutine(mgr, fgaClient, storeIDGetter, "creator", "parent", "account", nil) _, err := sub.Terminate(context.Background(), test.obj) if test.expectError { assert.Error(t, err) diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index 287c48b9..1a853825 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -87,27 +87,16 @@ func (w *workspaceInitializer) reconcile(ctx context.Context, obj client.Object) return subroutines.StopWithRequeue(5*time.Second, "AccountInfo not found yet, requeueing"), nil } - orgsClient, err := w.kcpClientGetter.NewClientForLogicalCluster(ctx, "root:orgs") - if err != nil { - return subroutines.OK(), fmt.Errorf("getting orgs client: %w", err) - } - var acc accountsv1alpha1.Account - if err := orgsClient.Get(ctx, client.ObjectKey{ - Name: ai.Spec.Account.Name, - }, &acc); err != nil { - return subroutines.OK(), fmt.Errorf("getting Account in platform-mesh-system: %w", err) - } - store := v1alpha1.Store{ ObjectMeta: metav1.ObjectMeta{Name: generateStoreName(lc)}, } - if acc.Spec.Creator == nil || *acc.Spec.Creator == "" { + if ai.Spec.Account.Creator == nil || *ai.Spec.Account.Creator == "" { return subroutines.OK(), fmt.Errorf("account creator is nil or empty") } tuples, err := fga.TuplesForOrganization(fga.TuplesForOrganizationInput{ BaseTuplesInput: fga.BaseTuplesInput{ - Creator: *acc.Spec.Creator, + Creator: *ai.Spec.Account.Creator, AccountOriginClusterID: ai.Spec.Account.OriginClusterId, AccountName: ai.Spec.Account.Name, CreatorRelation: w.creatorRelation, @@ -132,6 +121,11 @@ func (w *workspaceInitializer) reconcile(ctx context.Context, obj client.Object) }...) } + orgsClient, err := w.kcpClientGetter.NewClientForLogicalCluster(ctx, "root:orgs") + if err != nil { + return subroutines.OK(), fmt.Errorf("getting orgs client: %w", err) + } + if result, err := controllerutil.CreateOrUpdate(ctx, orgsClient, &store, func() error { store.Spec = v1alpha1.StoreSpec{ CoreModule: w.coreModule, diff --git a/internal/test/integration/yaml/apiresourceschema-accountinfos.core.platform-mesh.io.yaml b/internal/test/integration/yaml/apiresourceschema-accountinfos.core.platform-mesh.io.yaml index e50ed61a..0c935480 100644 --- a/internal/test/integration/yaml/apiresourceschema-accountinfos.core.platform-mesh.io.yaml +++ b/internal/test/integration/yaml/apiresourceschema-accountinfos.core.platform-mesh.io.yaml @@ -38,6 +38,10 @@ spec: properties: account: properties: + creator: + description: The initial creator of the account resource that led + to this workspace + type: string generatedClusterId: description: The GeneratedClusterId represents the cluster id of the workspace that was generated for a given account @@ -84,6 +88,10 @@ spec: type: object organization: properties: + creator: + description: The initial creator of the account resource that led + to this workspace + type: string generatedClusterId: description: The GeneratedClusterId represents the cluster id of the workspace that was generated for a given account @@ -111,6 +119,10 @@ spec: type: object parentAccount: properties: + creator: + description: The initial creator of the account resource that led + to this workspace + type: string generatedClusterId: description: The GeneratedClusterId represents the cluster id of the workspace that was generated for a given account