From 436638afff18f01ab2e112e907e300a171eee7ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20Echterh=C3=B6lter?= Date: Fri, 17 Jan 2025 16:16:04 +0100 Subject: [PATCH] fix: Fixing ParentAccount lookup and adjusting tests --- internal/controller/account_controller.go | 4 +- pkg/subroutines/extensions.go | 64 ++- pkg/subroutines/fga.go | 21 +- pkg/subroutines/fga_test.go | 641 ++++++++++++++++++---- 4 files changed, 588 insertions(+), 142 deletions(-) diff --git a/internal/controller/account_controller.go b/internal/controller/account_controller.go index 872e7b5..ff9d7e2 100644 --- a/internal/controller/account_controller.go +++ b/internal/controller/account_controller.go @@ -70,9 +70,9 @@ func NewAccountReconciler(log *logger.Logger, mgr ctrl.Manager, cfg config.Confi srv := service.NewService(mgr.GetClient(), cfg.Subroutines.FGA.RootNamespace) - cl := openfgav1.NewOpenFGAServiceClient(conn) + fgaClient := openfgav1.NewOpenFGAServiceClient(conn) - subs = append(subs, subroutines.NewFGASubroutine(cl, srv, cfg.Subroutines.FGA.RootNamespace, cfg.Subroutines.FGA.CreatorRelation, cfg.Subroutines.FGA.ParentRelation, cfg.Subroutines.FGA.ObjectType)) + subs = append(subs, subroutines.NewFGASubroutine(mgr.GetClient(), fgaClient, srv, cfg.Subroutines.FGA.RootNamespace, cfg.Subroutines.FGA.CreatorRelation, cfg.Subroutines.FGA.ParentRelation, cfg.Subroutines.FGA.ObjectType)) } return &AccountReconciler{ lifecycle: lifecycle.NewLifecycleManager(log, operatorName, accountReconcilerName, mgr.GetClient(), subs).WithSpreadingReconciles().WithConditionManagement(), diff --git a/pkg/subroutines/extensions.go b/pkg/subroutines/extensions.go index 0da4c0c..807bdf2 100644 --- a/pkg/subroutines/extensions.go +++ b/pkg/subroutines/extensions.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "strings" "text/template" @@ -196,41 +197,58 @@ func collectExtensions(ctx context.Context, cl client.Client, lookupNamespace st } func getParentAccount(ctx context.Context, cl client.Client, ns string) (*v1alpha1.Account, *string, error) { + if _, ok := kontext.ClusterFrom(ctx); ok { + return getParentAccountWithKcp(ctx, cl) + } else { + return getParentAccountByNs(ctx, cl, ns) + } +} - if cluster, ok := kontext.ClusterFrom(ctx); ok && !cluster.Empty() { +func getParentAccountWithKcp(ctx context.Context, cl client.Client) (*v1alpha1.Account, *string, error) { - wsCtx := kontext.WithCluster(ctx, logicalcluster.Name("")) - list := &tenancyv1alpha1.WorkspaceList{} + cluster, ok := kontext.ClusterFrom(ctx) + if !ok { + return nil, nil, fmt.Errorf("no cluster context found, this is a configuration error") + } - err := cl.List(wsCtx, list) - if err != nil { - return nil, nil, err - } + if cluster.Empty() { + return nil, nil, fmt.Errorf("no cluster in context is empty") + } - for _, ws := range list.Items { - if ws.Spec.Cluster != cluster.String() { - continue - } + wsCtx := kontext.WithCluster(ctx, "") + list := &tenancyv1alpha1.WorkspaceList{} + + err := cl.List(wsCtx, list) + if err != nil { + return nil, nil, err + } - clusterName := ws.Annotations[logicalcluster.AnnotationKey] + for _, ws := range list.Items { + if ws.Spec.Cluster != cluster.String() { + continue + } - parentCtx := kontext.WithCluster(ctx, logicalcluster.Name(clusterName)) + clusterName := ws.Annotations[logicalcluster.AnnotationKey] - parentAccount := v1alpha1.Account{} - err = cl.Get(parentCtx, types.NamespacedName{ - Name: ws.Annotations[v1alpha1.NamespaceAccountOwnerLabel], - Namespace: ns, - }, &parentAccount) - if err != nil { - return nil, nil, err - } + parentCtx := kontext.WithCluster(ctx, logicalcluster.Name(clusterName)) - return &parentAccount, &clusterName, nil + parentAccount := v1alpha1.Account{} + err = cl.Get(parentCtx, types.NamespacedName{ + Name: ws.Annotations[v1alpha1.NamespaceAccountOwnerLabel], + Namespace: ws.Annotations[v1alpha1.NamespaceAccountOwnerNamespaceLabel], + }, &parentAccount) + if err != nil { + return nil, nil, err } - return nil, nil, ErrNoParentAvailable + return &parentAccount, &clusterName, nil } + return nil, nil, ErrNoParentAvailable +} + +func getParentAccountByNs(ctx context.Context, cl client.Client, ns string) (*v1alpha1.Account, *string, error) { + var namespace v1.Namespace err := cl.Get(ctx, types.NamespacedName{Name: ns}, &namespace) if kerrors.IsNotFound(err) { diff --git a/pkg/subroutines/fga.go b/pkg/subroutines/fga.go index 695ab4d..c332ffe 100644 --- a/pkg/subroutines/fga.go +++ b/pkg/subroutines/fga.go @@ -22,8 +22,8 @@ import ( ) type FGASubroutine struct { - client openfgav1.OpenFGAServiceClient - k8sClient client.Client + fgaClient openfgav1.OpenFGAServiceClient + client client.Client srv service.Servicer rootNamespace string objectType string @@ -31,9 +31,10 @@ type FGASubroutine struct { creatorRelation string } -func NewFGASubroutine(cl openfgav1.OpenFGAServiceClient, s service.Servicer, rootNamespace, creatorRelation, parentRealtion, objectType string) *FGASubroutine { +func NewFGASubroutine(cl client.Client, fgaClient openfgav1.OpenFGAServiceClient, s service.Servicer, rootNamespace, creatorRelation, parentRealtion, objectType string) *FGASubroutine { return &FGASubroutine{ client: cl, + fgaClient: fgaClient, srv: s, rootNamespace: rootNamespace, creatorRelation: creatorRelation, @@ -61,8 +62,9 @@ func (e *FGASubroutine) Process(ctx context.Context, runtimeObj lifecycle.Runtim writes := []*openfgav1.TupleKey{} + // Determine parent account to create parent relation if account.GetNamespace() != e.rootNamespace { - parent, err := e.srv.GetAccountForNamespace(ctx, account.GetNamespace()) + parent, _, err := getParentAccount(ctx, e.client, account.GetNamespace()) if err != nil { log.Error().Err(err).Msg("Couldn't get parent account") return ctrl.Result{}, errors.NewOperatorError(err, true, true) @@ -75,6 +77,7 @@ func (e *FGASubroutine) Process(ctx context.Context, runtimeObj lifecycle.Runtim }) } + // Assign creator to the account if account.Spec.Creator != nil { if valid := validateCreator(*account.Spec.Creator); !valid { log.Error().Err(err).Str("creator", *account.Spec.Creator).Msg("creator string is in the protected service account prefix range") @@ -97,7 +100,7 @@ func (e *FGASubroutine) Process(ctx context.Context, runtimeObj lifecycle.Runtim for _, writeTuple := range writes { - _, err = e.client.Write(ctx, &openfgav1.WriteRequest{ + _, err = e.fgaClient.Write(ctx, &openfgav1.WriteRequest{ StoreId: storeId, Writes: &openfgav1.WriteRequestWrites{ TupleKeys: []*openfgav1.TupleKey{writeTuple}, @@ -131,7 +134,7 @@ func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj lifecycle.Runti deletes := []*openfgav1.TupleKeyWithoutCondition{} if account.GetNamespace() != e.rootNamespace { - parent, err := e.srv.GetAccountForNamespace(ctx, account.GetNamespace()) + parent, _, err := getParentAccount(ctx, e.client, account.GetNamespace()) if err != nil { log.Error().Err(err).Msg("Couldn't get parent account") return ctrl.Result{}, errors.NewOperatorError(err, true, true) @@ -161,7 +164,7 @@ func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj lifecycle.Runti for _, deleteTuple := range deletes { - _, err = e.client.Write(ctx, &openfgav1.WriteRequest{ + _, err = e.fgaClient.Write(ctx, &openfgav1.WriteRequest{ StoreId: storeId, Deletes: &openfgav1.WriteRequestDeletes{ TupleKeys: []*openfgav1.TupleKeyWithoutCondition{deleteTuple}, @@ -191,7 +194,7 @@ func (e *FGASubroutine) getStoreId(ctx context.Context, account *v1alpha1.Accoun lookupNamespace := account.Namespace lookupCtx := ctx for { - parent, newClusterContext, err := getParentAccount(lookupCtx, e.k8sClient, lookupNamespace) + parent, newClusterContext, err := getParentAccount(lookupCtx, e.client, lookupNamespace) if errors.Is(err, ErrNoParentAvailable) { break } @@ -208,7 +211,7 @@ func (e *FGASubroutine) getStoreId(ctx context.Context, account *v1alpha1.Accoun } } - storeId, err := helpers.GetStoreIDForTenant(ctx, e.client, firstLevelAccountName) + storeId, err := helpers.GetStoreIDForTenant(ctx, e.fgaClient, firstLevelAccountName) if err != nil { return "", err } diff --git a/pkg/subroutines/fga_test.go b/pkg/subroutines/fga_test.go index f2fce0d..fef705a 100644 --- a/pkg/subroutines/fga_test.go +++ b/pkg/subroutines/fga_test.go @@ -2,6 +2,10 @@ package subroutines_test import ( "context" + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "testing" openfgav1 "github.com/openfga/api/proto/openfga/v1" @@ -13,6 +17,7 @@ import ( "k8s.io/utils/ptr" "github.com/openmfp/account-operator/api/v1alpha1" + corev1alpha1 "github.com/openmfp/account-operator/api/v1alpha1" "github.com/openmfp/account-operator/pkg/subroutines" "github.com/openmfp/account-operator/pkg/subroutines/mocks" ) @@ -35,29 +40,61 @@ func newFgaError(c openfgav1.ErrorCode, m string) *fgaError { } func TestCreatorSubroutine_GetName(t *testing.T) { - routine := subroutines.NewFGASubroutine(nil, nil, "", "", "", "") + routine := subroutines.NewFGASubroutine(nil, nil, nil, "", "", "", "") assert.Equal(t, "CreatorSubroutine", routine.GetName()) } func TestCreatorSubroutine_Finalizers(t *testing.T) { - routine := subroutines.NewFGASubroutine(nil, nil, "", "", "", "") + routine := subroutines.NewFGASubroutine(nil, nil, nil, "", "", "", "") assert.Equal(t, []string{"account.core.openmfp.io/fga"}, routine.Finalizers()) } -func getStoreMocks(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - k8ServiceMock.EXPECT(). - GetFirstLevelAccountForNamespace(context.Background(), mock.Anything). - Return(&v1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{ - Name: "tenant1", - }, - }, nil) +func getStoreMocks(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + + *ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, + }, + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) + + *account = v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{ + Name: "first-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, + }, + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + return nil + }).Once() - openFGAServiceClientMock.EXPECT(). - ListStores(context.Background(), mock.Anything). - Return(&openfgav1.ListStoresResponse{ - Stores: []*openfgav1.Store{{Id: "1", Name: "tenant-tenant1"}}, - }, nil).Maybe() + openFGAServiceClientMock.EXPECT().ListStores(context.Background(), mock.Anything).Return(&openfgav1.ListStoresResponse{Stores: []*openfgav1.Store{{Id: "1", Name: "tenant-first-level"}}}, nil).Maybe() } func TestCreatorSubroutine_Process(t *testing.T) { @@ -68,7 +105,7 @@ func TestCreatorSubroutine_Process(t *testing.T) { name string expectedError bool account *v1alpha1.Account - setupMocks func(*mocks.OpenFGAServiceClient, *mocks.K8Service) + setupMocks func(*mocks.OpenFGAServiceClient, *mocks.K8Service, *mocks.Client) }{ { name: "should_skip_processing_if_subroutine_ran_before", @@ -92,8 +129,45 @@ func TestCreatorSubroutine_Process(t *testing.T) { Namespace: "test-namespace", }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - k8ServiceMock.EXPECT().GetFirstLevelAccountForNamespace(mock.Anything, mock.Anything).Return(nil, assert.AnError) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + + *ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, + }, + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) + + *account = v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, + }, + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) + openFGAServiceClientMock.EXPECT().ListStores(mock.Anything, mock.Anything).Return(nil, assert.AnError) }, }, { @@ -105,9 +179,21 @@ func TestCreatorSubroutine_Process(t *testing.T) { Namespace: "test-namespace", }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) - k8ServiceMock.EXPECT().GetAccountForNamespace(mock.Anything, mock.Anything).Return(nil, assert.AnError) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + + *ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, + }, + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError) }, }, { @@ -119,16 +205,45 @@ func TestCreatorSubroutine_Process(t *testing.T) { Namespace: "test-namespace", }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + getStoreMocks(openFGAServiceClientMock, k8ServiceMock, clientMock) + + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + *ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, + }, + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) - k8ServiceMock.EXPECT(). - GetAccountForNamespace(mock.Anything, mock.Anything). - Return(&v1alpha1.Account{ + *account = v1alpha1.Account{ ObjectMeta: metav1.ObjectMeta{ - Name: "parent-account", + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, }, - }, nil) + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). @@ -144,16 +259,45 @@ func TestCreatorSubroutine_Process(t *testing.T) { Namespace: "test-namespace", }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + getStoreMocks(openFGAServiceClientMock, k8ServiceMock, clientMock) - k8ServiceMock.EXPECT(). - GetAccountForNamespace(mock.Anything, mock.Anything). - Return(&v1alpha1.Account{ + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + *ns = corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: "parent-account", + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, }, - }, nil) + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) + + *account = v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, + }, + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). @@ -168,16 +312,45 @@ func TestCreatorSubroutine_Process(t *testing.T) { Namespace: "test-namespace", }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + getStoreMocks(openFGAServiceClientMock, k8ServiceMock, clientMock) + + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + *ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, + }, + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) - k8ServiceMock.EXPECT(). - GetAccountForNamespace(mock.Anything, mock.Anything). - Return(&v1alpha1.Account{ + *account = v1alpha1.Account{ ObjectMeta: metav1.ObjectMeta{ - Name: "parent-account", + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, }, - }, nil) + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). @@ -195,16 +368,45 @@ func TestCreatorSubroutine_Process(t *testing.T) { Creator: ptr.To("system:serviceaccount:some-namespace:some-service-account"), }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + getStoreMocks(openFGAServiceClientMock, k8ServiceMock, clientMock) - k8ServiceMock.EXPECT(). - GetAccountForNamespace(mock.Anything, mock.Anything). - Return(&v1alpha1.Account{ + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + *ns = corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: "parent-account", + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, + }, + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) + + *account = v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, }, - }, nil) + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). @@ -230,16 +432,45 @@ func TestCreatorSubroutine_Process(t *testing.T) { Creator: ptr.To("system.serviceaccount.some-namespace.some-service-account"), }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + getStoreMocks(openFGAServiceClientMock, k8ServiceMock, clientMock) + + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + *ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, + }, + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) - k8ServiceMock.EXPECT(). - GetAccountForNamespace(mock.Anything, mock.Anything). - Return(&v1alpha1.Account{ + *account = v1alpha1.Account{ ObjectMeta: metav1.ObjectMeta{ - Name: "parent-account", + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, }, - }, nil) + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) }, }, { @@ -253,16 +484,45 @@ func TestCreatorSubroutine_Process(t *testing.T) { Creator: &creator, }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + getStoreMocks(openFGAServiceClientMock, k8ServiceMock, clientMock) - k8ServiceMock.EXPECT(). - GetAccountForNamespace(mock.Anything, mock.Anything). - Return(&v1alpha1.Account{ + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + *ns = corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: "parent-account", + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, }, - }, nil) + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) + + *account = v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, + }, + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). @@ -276,14 +536,15 @@ func TestCreatorSubroutine_Process(t *testing.T) { openFGAClient := mocks.NewOpenFGAServiceClient(t) accountClient := mocks.NewK8Service(t) + clientMock := mocks.NewClient(t) if test.setupMocks != nil { - test.setupMocks(openFGAClient, accountClient) + test.setupMocks(openFGAClient, accountClient, clientMock) } - routine := subroutines.NewFGASubroutine(openFGAClient, accountClient, namespace, "owner", "parent", "account") - - _, err := routine.Process(context.Background(), test.account) + routine := subroutines.NewFGASubroutine(clientMock, openFGAClient, accountClient, namespace, "owner", "parent", "account") + ctx := context.Background() + _, err := routine.Process(ctx, test.account) if test.expectedError { assert.NotNil(t, err) } else { @@ -302,7 +563,7 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { name string expectedError bool account *v1alpha1.Account - setupMocks func(*mocks.OpenFGAServiceClient, *mocks.K8Service) + setupMocks func(*mocks.OpenFGAServiceClient, *mocks.K8Service, *mocks.Client) }{ { name: "should_fail_if_get_store_id_fails", @@ -313,8 +574,45 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { Namespace: "test-namespace", }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - k8ServiceMock.EXPECT().GetFirstLevelAccountForNamespace(mock.Anything, mock.Anything).Return(nil, assert.AnError) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + + *ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, + }, + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) + + *account = v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, + }, + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) + openFGAServiceClientMock.EXPECT().ListStores(mock.Anything, mock.Anything).Return(nil, assert.AnError) }, }, { @@ -326,9 +624,21 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { Namespace: "test-namespace", }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) - k8ServiceMock.EXPECT().GetAccountForNamespace(mock.Anything, mock.Anything).Return(nil, assert.AnError) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + + *ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, + }, + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError) }, }, { @@ -340,20 +650,47 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { Namespace: "test-namespace", }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + + getStoreMocks(openFGAServiceClientMock, k8ServiceMock, clientMock) - k8ServiceMock.EXPECT(). - GetAccountForNamespace(mock.Anything, mock.Anything). - Return(&v1alpha1.Account{ + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + *ns = corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: "parent-account", + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, }, - }, nil) + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) - openFGAServiceClientMock.EXPECT(). - Write(mock.Anything, mock.Anything). - Return(nil, assert.AnError) + *account = v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, + }, + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) + openFGAServiceClientMock.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, assert.AnError) }, }, @@ -365,16 +702,45 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { Namespace: "test-namespace", }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + getStoreMocks(openFGAServiceClientMock, k8ServiceMock, clientMock) + + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + *ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, + }, + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) - k8ServiceMock.EXPECT(). - GetAccountForNamespace(mock.Anything, mock.Anything). - Return(&v1alpha1.Account{ + *account = v1alpha1.Account{ ObjectMeta: metav1.ObjectMeta{ - Name: "parent-account", + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, }, - }, nil) + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). @@ -389,16 +755,45 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { Namespace: "test-namespace", }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + getStoreMocks(openFGAServiceClientMock, k8ServiceMock, clientMock) - k8ServiceMock.EXPECT(). - GetAccountForNamespace(mock.Anything, mock.Anything). - Return(&v1alpha1.Account{ + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + *ns = corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: "parent-account", + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, }, - }, nil) + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) + + *account = v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, + }, + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). @@ -416,16 +811,45 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { Creator: &creator, }, }, - setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service) { - getStoreMocks(openFGAServiceClientMock, k8ServiceMock) + setupMocks: func(openFGAServiceClientMock *mocks.OpenFGAServiceClient, k8ServiceMock *mocks.K8Service, clientMock *mocks.Client) { + getStoreMocks(openFGAServiceClientMock, k8ServiceMock, clientMock) - k8ServiceMock.EXPECT(). - GetAccountForNamespace(mock.Anything, mock.Anything). - Return(&v1alpha1.Account{ + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + ns := o.(*corev1.Namespace) + *ns = corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: "parent-account", + Labels: map[string]string{ + corev1alpha1.NamespaceAccountOwnerLabel: "first-level", + corev1alpha1.NamespaceAccountOwnerNamespaceLabel: "first-level", + }, }, - }, nil) + } + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, nn types.NamespacedName, o client.Object, opts ...client.GetOption) error { + account := o.(*v1alpha1.Account) + + *account = v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fist-level", + Namespace: "first-level", + }, + Spec: v1alpha1.AccountSpec{ + Extensions: []v1alpha1.Extension{ + { + TypeMeta: metav1.TypeMeta{ + Kind: "AccountExtension", + APIVersion: "core.openmfp.io/v1alpha1", + }, + SpecGoTemplate: apiextensionsv1.JSON{}, + }, + }, + }, + } + + return nil + }).Once() + clientMock.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(nil) openFGAServiceClientMock.EXPECT(). Write(mock.Anything, mock.Anything). @@ -439,14 +863,15 @@ func TestCreatorSubroutine_Finalize(t *testing.T) { openFGAClient := mocks.NewOpenFGAServiceClient(t) accountClient := mocks.NewK8Service(t) + k8sClient := mocks.NewClient(t) if test.setupMocks != nil { - test.setupMocks(openFGAClient, accountClient) + test.setupMocks(openFGAClient, accountClient, k8sClient) } - routine := subroutines.NewFGASubroutine(openFGAClient, accountClient, namespace, "owner", "parent", "account") - - _, err := routine.Finalize(context.Background(), test.account) + routine := subroutines.NewFGASubroutine(k8sClient, openFGAClient, accountClient, namespace, "owner", "parent", "account") + ctx := context.Background() + _, err := routine.Finalize(ctx, test.account) if test.expectedError { assert.NotNil(t, err) } else {