diff --git a/Dockerfile b/Dockerfile index be950ab..55c4c2d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build the manager binary -FROM golang:1.24.6-bullseye AS builder +FROM golang:1.24.8 AS builder WORKDIR /workspace diff --git a/api/v1alpha1/account_types.go b/api/v1alpha1/account_types.go index 3842dd8..1b9f452 100644 --- a/api/v1alpha1/account_types.go +++ b/api/v1alpha1/account_types.go @@ -24,8 +24,10 @@ import ( type AccountType string const ( - AccountTypeOrg AccountType = "org" - AccountTypeAccount AccountType = "account" + AccountTypeOrg AccountType = "org" + AccountTypeAccount AccountType = "account" + NamespaceAccountOwnerLabel AccountType = "account.core.platform-mesh.io/owner" + NamespaceAccountOwnerNamespaceLabel AccountType = "account.core.platform-mesh.io/owner-namespace" ) // AccountSpec defines the desired state of Account diff --git a/go.mod b/go.mod index 90b10b2..3b03eeb 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/platform-mesh/account-operator -go 1.24.5 +go 1.24.8 replace ( k8s.io/api => k8s.io/api v0.34.1 diff --git a/pkg/subroutines/accountinfo/accountinfo.go b/pkg/subroutines/accountinfo/accountinfo.go index 8610d08..6112148 100644 --- a/pkg/subroutines/accountinfo/accountinfo.go +++ b/pkg/subroutines/accountinfo/accountinfo.go @@ -54,9 +54,40 @@ func (r *AccountInfoSubroutine) Finalizers(_ runtimeobject.RuntimeObject) []stri func (r *AccountInfoSubroutine) Finalize(ctx context.Context, ro runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { cn := clusteredname.MustGetClusteredName(ctx, ro) - // The account info object is relevant input for other finalizers, removing the accountinfo finalizer at last + requeue := true + if ts := ro.GetDeletionTimestamp(); ts != nil { + oneMinAgo := v1.Now().Add(-1 * time.Minute) + if ts.Time.Before(oneMinAgo) { + requeue = false + } + } + if len(ro.GetFinalizers()) > 1 { - return ctrl.Result{RequeueAfter: r.limiter.When(cn)}, nil + if requeue { + return ctrl.Result{RequeueAfter: r.limiter.When(cn)}, nil + } + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("finalizer not removed yet"), true, false) + } + + clusterRef, err := r.mgr.GetCluster(ctx, string(cn.ClusterID)) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(err, true, true) + } + clusterClient := clusterRef.GetClient() + + accountList := &v1alpha1.AccountList{} + if err := clusterClient.List( + ctx, + accountList, + client.MatchingLabels(map[string]string{string(v1alpha1.NamespaceAccountOwnerLabel): ro.GetName()}), + ); err != nil { + return ctrl.Result{}, errors.NewOperatorError(err, true, true) + } + if len(accountList.Items) > 0 { + if requeue { + return ctrl.Result{RequeueAfter: r.limiter.When(cn)}, nil + } + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("finalizer not removed yet"), true, false) } r.limiter.Forget(cn) diff --git a/pkg/subroutines/fga.go b/pkg/subroutines/fga.go index ddb85cb..81b4fe7 100644 --- a/pkg/subroutines/fga.go +++ b/pkg/subroutines/fga.go @@ -7,14 +7,11 @@ import ( "strings" "time" - kcpcorev1alpha "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" - kcptenancyv1alpha "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1" openfgav1 "github.com/openfga/api/proto/openfga/v1" "github.com/platform-mesh/golang-commons/controller/lifecycle/runtimeobject" "github.com/platform-mesh/golang-commons/errors" "github.com/platform-mesh/golang-commons/fga/helpers" "github.com/platform-mesh/golang-commons/logger" - "k8s.io/apimachinery/pkg/api/meta" "k8s.io/client-go/util/workqueue" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -32,27 +29,31 @@ type FGASubroutine struct { objectType string parentRelation string creatorRelation string - - limiter workqueue.TypedRateLimiter[clusteredname.ClusteredName] + limiter workqueue.TypedRateLimiter[clusteredname.ClusteredName] } func NewFGASubroutine(mgr mcmanager.Manager, fgaClient openfgav1.OpenFGAServiceClient, creatorRelation, parentRelation, objectType string) *FGASubroutine { + exp := workqueue.NewTypedItemExponentialFailureRateLimiter[clusteredname.ClusteredName](1*time.Second, 120*time.Second) return &FGASubroutine{ mgr: mgr, fgaClient: fgaClient, creatorRelation: creatorRelation, parentRelation: parentRelation, objectType: objectType, - limiter: workqueue.NewTypedItemExponentialFailureRateLimiter[clusteredname.ClusteredName](1*time.Second, 120*time.Second), + limiter: exp, } } func (e *FGASubroutine) Process(ctx context.Context, ro runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { - account := ro.(*v1alpha1.Account) - cn := clusteredname.MustGetClusteredName(ctx, ro) + log := logger.LoadLoggerFromContext(ctx) + log.Debug().Msg("Skipping FGASubroutine.Process during initialization; handled by workspace initializer") + return ctrl.Result{}, nil +} +func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { + account := runtimeObj.(*v1alpha1.Account) log := logger.LoadLoggerFromContext(ctx) - log.Debug().Msg("Starting creator subroutine process() function") + cn := clusteredname.MustGetClusteredName(ctx, runtimeObj) clusterName, ok := mccontext.ClusterFrom(ctx) if !ok { @@ -65,119 +66,20 @@ func (e *FGASubroutine) Process(ctx context.Context, ro runtimeobject.RuntimeObj } clusterClient := clusterRef.GetClient() - accountWorkspace := &kcptenancyv1alpha.Workspace{} - if err := clusterClient.Get(ctx, client.ObjectKey{Name: account.Name}, accountWorkspace); err != nil { - return ctrl.Result{}, errors.NewOperatorError(err, true, true) - } - - if accountWorkspace.Status.Phase != kcpcorev1alpha.LogicalClusterPhaseReady { - log.Info().Msg("workspace is not ready yet, retry") - return ctrl.Result{RequeueAfter: e.limiter.When(cn)}, nil - } - - accountCluster, err := e.mgr.GetCluster(ctx, accountWorkspace.Spec.Cluster) - if err != nil { + childAccounts := &v1alpha1.AccountList{} + if err := clusterClient.List( + ctx, + childAccounts, + client.MatchingLabels(map[string]string{string(v1alpha1.NamespaceAccountOwnerLabel): account.Name}), + ); err != nil { return ctrl.Result{}, errors.NewOperatorError(err, true, true) } - accountClusterClient := accountCluster.GetClient() - - accountInfo, err := e.getAccountInfo(ctx, accountClusterClient) - if err != nil { - log.Error().Err(err).Msg("Couldn't get Store Id") - return ctrl.Result{}, errors.NewOperatorError(err, true, true) - } - - if accountInfo.Spec.FGA.Store.Id == "" { - log.Error().Msg("FGA Store Id is empty") - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("FGA Store Id is empty"), true, true) - } - - if accountInfo.Spec.Account.GeneratedClusterId == "" { - log.Error().Msg("account cluster id is empty") - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("account cluster id is empty"), true, true) - } - - if account.Spec.Type != v1alpha1.AccountTypeOrg && accountInfo.Spec.ParentAccount.GeneratedClusterId == "" { - log.Error().Msg("parent account cluster id is empty") - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("parent account cluster id is empty"), true, true) - } - - writes := []*openfgav1.TupleKey{} - - // Parent Name - if account.Spec.Type != v1alpha1.AccountTypeOrg { - parentAccountName := accountInfo.Spec.ParentAccount.Name - - // Determine parent account to create parent relation - writes = append(writes, &openfgav1.TupleKey{ - User: fmt.Sprintf("%s:%s/%s", e.objectType, accountInfo.Spec.ParentAccount.OriginClusterId, parentAccountName), - Relation: e.parentRelation, - Object: fmt.Sprintf("%s:%s/%s", e.objectType, accountInfo.Spec.Account.OriginClusterId, account.GetName()), - }) - } - - // Assign creator to the account - creatorTuplesWritten := meta.IsStatusConditionTrue(account.Status.Conditions, fmt.Sprintf("%s_Ready", e.GetName())) - if account.Spec.Creator != nil && !creatorTuplesWritten { - if valid := validateCreator(*account.Spec.Creator); !valid { - log.Error().Str("creator", *account.Spec.Creator).Msg("creator string is in the protected service account prefix range") - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("creator in protected service account range"), false, false) - } - creator := formatUser(*account.Spec.Creator) - - writes = append(writes, &openfgav1.TupleKey{ - User: fmt.Sprintf("user:%s", creator), - Relation: "assignee", - Object: fmt.Sprintf("role:%s/%s/%s/owner", e.objectType, accountInfo.Spec.Account.OriginClusterId, account.Name), - }) - - writes = append(writes, &openfgav1.TupleKey{ - User: fmt.Sprintf("role:%s/%s/%s/owner#assignee", e.objectType, accountInfo.Spec.Account.OriginClusterId, account.Name), - Relation: e.creatorRelation, - Object: fmt.Sprintf("%s:%s/%s", e.objectType, accountInfo.Spec.Account.OriginClusterId, account.Name), - }) - } - - for _, writeTuple := range writes { - _, err = e.fgaClient.Write(ctx, &openfgav1.WriteRequest{ - StoreId: accountInfo.Spec.FGA.Store.Id, - Writes: &openfgav1.WriteRequestWrites{ - TupleKeys: []*openfgav1.TupleKey{writeTuple}, - }, - }) - - if helpers.IsDuplicateWriteError(err) { - log.Info().Err(err).Msg("Open FGA writeTuple failed due to invalid input (possible duplicate)") - err = nil - } - - if err != nil { - log.Error().Err(err).Msg("Open FGA writeTuple failed") - return ctrl.Result{}, errors.NewOperatorError(err, true, true) - } + if len(childAccounts.Items) > 0 { + delay := e.limiter.When(cn) + return ctrl.Result{RequeueAfter: delay}, nil } - e.limiter.Forget(cn) - return ctrl.Result{}, nil -} - -func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { - account := runtimeObj.(*v1alpha1.Account) - log := logger.LoadLoggerFromContext(ctx) - - // Skip fga account finalization for organizations because the store is removed completely if account.Spec.Type != v1alpha1.AccountTypeOrg { - clusterName, ok := mccontext.ClusterFrom(ctx) - if !ok { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("cluster client not available: ensure context carries cluster information"), true, true) - } - - clusterRef, err := e.mgr.GetCluster(ctx, clusterName) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(err, true, true) - } - clusterClient := clusterRef.GetClient() - accountInfo, err := e.getAccountInfo(ctx, clusterClient) if err != nil { log.Error().Err(err).Msg("Couldn't get Store Id") @@ -190,15 +92,12 @@ func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj runtimeobject.R } deletes := []*openfgav1.TupleKeyWithoutCondition{} - if account.Spec.Type != v1alpha1.AccountTypeOrg { - parentAccountName := accountInfo.Spec.Account.Name - - deletes = append(deletes, &openfgav1.TupleKeyWithoutCondition{ - User: fmt.Sprintf("%s:%s/%s", e.objectType, accountInfo.Spec.ParentAccount.OriginClusterId, parentAccountName), - Relation: e.parentRelation, - Object: fmt.Sprintf("%s:%s/%s", e.objectType, accountInfo.Spec.Account.OriginClusterId, account.GetName()), - }) - } + parentAccountName := accountInfo.Spec.Account.Name + deletes = append(deletes, &openfgav1.TupleKeyWithoutCondition{ + User: fmt.Sprintf("%s:%s/%s", e.objectType, accountInfo.Spec.ParentAccount.OriginClusterId, parentAccountName), + Relation: e.parentRelation, + Object: fmt.Sprintf("%s:%s/%s", e.objectType, accountInfo.Spec.Account.OriginClusterId, account.GetName()), + }) if account.Spec.Creator != nil { creator := formatUser(*account.Spec.Creator) @@ -216,7 +115,6 @@ func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj runtimeobject.R } for _, deleteTuple := range deletes { - _, err = e.fgaClient.Write(ctx, &openfgav1.WriteRequest{ StoreId: accountInfo.Spec.FGA.Store.Id, Deletes: &openfgav1.WriteRequestDeletes{ @@ -228,15 +126,14 @@ func (e *FGASubroutine) Finalize(ctx context.Context, runtimeObj runtimeobject.R log.Info().Err(err).Msg("Open FGA write failed due to invalid input (possibly trying to deleteTuple nonexisting entry)") err = nil } - if err != nil { log.Error().Err(err).Msg("Open FGA write failed") return ctrl.Result{}, errors.NewOperatorError(err, true, true) } - } } + e.limiter.Forget(cn) return ctrl.Result{}, nil } diff --git a/pkg/subroutines/fga_test.go b/pkg/subroutines/fga_test.go new file mode 100644 index 0000000..055e301 --- /dev/null +++ b/pkg/subroutines/fga_test.go @@ -0,0 +1,119 @@ +package subroutines + +import ( + "context" + "testing" + "time" + + openfgav1 "github.com/openfga/api/proto/openfga/v1" + pmconfig "github.com/platform-mesh/golang-commons/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + mccontext "sigs.k8s.io/multicluster-runtime/pkg/context" + + "github.com/platform-mesh/account-operator/api/v1alpha1" + "github.com/platform-mesh/account-operator/internal/config" + "github.com/platform-mesh/account-operator/pkg/subroutines/accountinfo" + "github.com/platform-mesh/account-operator/pkg/subroutines/mocks" +) + +func TestFGASubroutine_Finalizers(t *testing.T) { + routine := NewFGASubroutine(nil, nil, "owner", "parent", "core_platform-mesh_io_account") + assert.Equal(t, []string{"account.core.platform-mesh.io/fga"}, routine.Finalizers(nil)) +} + +func TestFGASubroutine_ProcessNoOp(t *testing.T) { + routine := NewFGASubroutine(nil, nil, "owner", "parent", "core_platform-mesh_io_account") + res, err := routine.Process(context.Background(), &v1alpha1.Account{}) + assert.Nil(t, err) + assert.Equal(t, ctrl.Result{}, res) +} + +func TestFGASubroutine_Finalize_RequeuesWhenChildAccountsExist(t *testing.T) { + mgr := mocks.NewManager(t) + cluster := mocks.NewCluster(t) + clientMock := mocks.NewClient(t) + + mgr.EXPECT().GetCluster(mock.Anything, "root:orgs").Return(cluster, nil) + cluster.EXPECT().GetClient().Return(clientMock) + + clientMock.EXPECT().List(mock.Anything, mock.AnythingOfType("*github.com/platform-mesh/account-operator/api/v1alpha1.AccountList"), mock.Anything).RunAndReturn( + func(ctx context.Context, list client.ObjectList, _ ...client.ListOption) error { + l := list.(*v1alpha1.AccountList) + l.Items = append(l.Items, v1alpha1.Account{}) + return nil + }, + ) + + fgaMock := mocks.NewOpenFGAServiceClient(t) + routine := NewFGASubroutine(mgr, fgaMock, "owner", "parent", "core_platform-mesh_io_account") + + baseCtx := pmconfig.SetConfigInContext(context.Background(), config.OperatorConfig{}) + ctx := mccontext.WithCluster(baseCtx, "root:orgs") + + account := &v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Finalizers: []string{"account.core.platform-mesh.io/fga"}}, + Spec: v1alpha1.AccountSpec{Type: v1alpha1.AccountTypeAccount}, + } + + res, err := routine.Finalize(ctx, account) + require.Nil(t, err) + assert.Greater(t, res.RequeueAfter, 0*time.Second) +} + +func TestFGASubroutine_Finalize_RemovesTuples(t *testing.T) { + mgr := mocks.NewManager(t) + cluster := mocks.NewCluster(t) + clientMock := mocks.NewClient(t) + + mgr.EXPECT().GetCluster(mock.Anything, "root:orgs").Return(cluster, nil) + cluster.EXPECT().GetClient().Return(clientMock) + + clientMock.EXPECT().List(mock.Anything, mock.AnythingOfType("*github.com/platform-mesh/account-operator/api/v1alpha1.AccountList"), mock.Anything).RunAndReturn( + func(ctx context.Context, list client.ObjectList, _ ...client.ListOption) error { + return nil + }, + ) + + clientMock.EXPECT().Get(mock.Anything, types.NamespacedName{Name: accountinfo.DefaultAccountInfoName}, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, _ ...client.GetOption) error { + info := obj.(*v1alpha1.AccountInfo) + info.Spec = v1alpha1.AccountInfoSpec{ + Account: v1alpha1.AccountLocation{ + Name: "demo", + OriginClusterId: "root:orgs", + GeneratedClusterId: "account-cluster", + }, + ParentAccount: &v1alpha1.AccountLocation{ + Name: "parent", + OriginClusterId: "root:orgs", + }, + FGA: v1alpha1.FGAInfo{Store: v1alpha1.StoreInfo{Id: "store-1"}}, + } + return nil + }, + ) + + fgaMock := mocks.NewOpenFGAServiceClient(t) + fgaMock.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Times(2) + + routine := NewFGASubroutine(mgr, fgaMock, "owner", "parent", "core_platform-mesh_io_account") + + baseCtx := pmconfig.SetConfigInContext(context.Background(), config.OperatorConfig{}) + ctx := mccontext.WithCluster(baseCtx, "root:orgs") + + account := &v1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Finalizers: []string{"account.core.platform-mesh.io/fga"}}, + Spec: v1alpha1.AccountSpec{Type: v1alpha1.AccountTypeAccount, Creator: ptr.To("alice")}, + } + + res, err := routine.Finalize(ctx, account) + require.Nil(t, err) + assert.Equal(t, ctrl.Result{}, res) +}