From 8f0829a5d6d358b311a6b79ca2f9b751fde2bbba Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Fri, 10 Oct 2025 16:40:59 +0300 Subject: [PATCH 01/35] feat: Add the object/parent/creator relation knobs needed by the initializer --- internal/config/config.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 9942f9a..0c8b060 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,7 +3,10 @@ package config // Config struct to hold the app config type Config struct { FGA struct { - Target string `mapstructure:"fga-target"` + Target string `mapstructure:"fga-target"` + ObjectType string `mapstructure:"fga-object-type" default:"core_platform-mesh_io_account"` + ParentRelation string `mapstructure:"fga-parent-relation" default:"parent"` + CreatorRelation string `mapstructure:"fga-creator-relation" default:"owner"` } `mapstructure:",squash"` APIExportEndpointSliceName string `mapstructure:"api-export-endpoint-slice-name"` CoreModulePath string `mapstructure:"core-module-path"` From f7353b42c23b7f19750489824f467d6b74cd59e9 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Fri, 10 Oct 2025 16:42:18 +0300 Subject: [PATCH 02/35] feat: Workspace initializer now pulls account + accountinfo data, waits for readiness, merges parent/owner tuples, and keeps accountInfo store IDs aligned --- internal/subroutine/workspace_initializer.go | 280 +++++++++++++++---- 1 file changed, 228 insertions(+), 52 deletions(-) diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index 21aad8a..c8ad8b1 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -4,21 +4,25 @@ import ( "context" "fmt" "os" + "regexp" "slices" "strings" "time" kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" + "github.com/kcp-dev/logicalcluster/v3" accountsv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" lifecycleruntimeobject "github.com/platform-mesh/golang-commons/controller/lifecycle/runtimeobject" lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine" "github.com/platform-mesh/golang-commons/errors" + kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/kontext" "github.com/platform-mesh/security-operator/api/v1alpha1" "github.com/platform-mesh/security-operator/internal/config" @@ -26,34 +30,49 @@ import ( const initializerName = "root:security" +type workspaceClientFactoryFunc func(path string) (client.Client, error) + +type workspaceInitializer struct { + cl client.Client + orgsClient client.Client + restCfg *rest.Config + coreModule string + fgaObjectType string + fgaParentRelation string + fgaCreatorRelation string + newWorkspaceClientFunc workspaceClientFactoryFunc +} + +var _ lifecyclesubroutine.Subroutine = &workspaceInitializer{} + func NewWorkspaceInitializer(cl, orgsClient client.Client, restCfg *rest.Config, cfg config.Config) *workspaceInitializer { coreModulePath := cfg.CoreModulePath - // read file from path - res, err := os.ReadFile(coreModulePath) + data, err := os.ReadFile(coreModulePath) if err != nil { panic(err) } - return &workspaceInitializer{ - cl: cl, - orgsClient: orgsClient, - restCfg: restCfg, - coreModule: string(res), + wi := &workspaceInitializer{ + cl: cl, + orgsClient: orgsClient, + restCfg: restCfg, + coreModule: string(data), + fgaObjectType: cfg.FGA.ObjectType, + fgaParentRelation: cfg.FGA.ParentRelation, + fgaCreatorRelation: cfg.FGA.CreatorRelation, } -} -var _ lifecyclesubroutine.Subroutine = &workspaceInitializer{} + wi.newWorkspaceClientFunc = func(path string) (client.Client, error) { + cfgCopy := rest.CopyConfig(restCfg) + cfgCopy.Host = strings.ReplaceAll(cfgCopy.Host, "/services/initializingworkspaces/root:security", "/clusters/"+path) + return client.New(cfgCopy, client.Options{Scheme: cl.Scheme()}) + } -type workspaceInitializer struct { - cl client.Client - orgsClient client.Client - restCfg *rest.Config - coreModule string + return wi } func (w *workspaceInitializer) Finalize(ctx context.Context, instance lifecycleruntimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { - // TODO: implement once finalizing workspaces are a thing return ctrl.Result{}, nil } @@ -69,58 +88,100 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance lifecycleru return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get workspace path"), true, false) } - store := v1alpha1.Store{ - ObjectMeta: metav1.ObjectMeta{Name: generateStoreName(lc)}, + account, opErr := w.getAccount(ctx, lc) + if opErr != nil { + return ctrl.Result{}, opErr } - //TODO use ctx after migrating to multi-cluster runtime - ctxWithTimeout,cancel := context.WithTimeout(context.Background(), 5*time.Second) + + ctxWithTimeout, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() - _, err := controllerutil.CreateOrUpdate(ctxWithTimeout, w.orgsClient, &store, func() error { - store.Spec = v1alpha1.StoreSpec{ - Tuples: []v1alpha1.Tuple{ - { - Object: "role:authenticated", - Relation: "assignee", - User: "user:*", - }, - { - Object: fmt.Sprintf("core_platform-mesh_io_account:%s/%s", lc.Spec.Owner.Cluster, lc.Spec.Owner.Name), - Relation: "member", - User: "role:authenticated#assignee", - }, - }, - CoreModule: w.coreModule, + wsClient, err := w.newWorkspaceClientFunc(path) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to create workspace client: %w", err), true, true) + } + + // Ensure the store resource exists for organizations so that status can be observed. + storeName := "" + if account.Spec.Type == accountsv1alpha1.AccountTypeOrg { + storeName = generateStoreName(lc) + store := &v1alpha1.Store{ObjectMeta: metav1.ObjectMeta{Name: storeName}} + _, err = controllerutil.CreateOrUpdate(ctxWithTimeout, w.orgsClient, store, func() error { + store.Spec.CoreModule = w.coreModule + return nil + }) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to create/update store: %w", err), true, true) } - return nil - }) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to create/update store: %w", err), true, true) + if store.Status.StoreID == "" { + return ctrl.Result{Requeue: true}, nil + } + + if err := w.ensureAccountInfoStoreID(ctxWithTimeout, wsClient, store.Status.StoreID); err != nil { + return ctrl.Result{}, err + } + } + + accountInfo := &accountsv1alpha1.AccountInfo{} + if err := wsClient.Get(ctxWithTimeout, client.ObjectKey{Name: "account"}, accountInfo); err != nil { + if kerrors.IsNotFound(err) { + return ctrl.Result{Requeue: true}, nil + } + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get accountInfo: %w", err), true, true) + } + + if accountInfo.Spec.Account.Name == "" || accountInfo.Spec.Account.OriginClusterId == "" { + return ctrl.Result{Requeue: true}, nil + } + + if accountInfo.Spec.FGA.Store.Id == "" { + return ctrl.Result{Requeue: true}, nil + } + + if account.Spec.Type == accountsv1alpha1.AccountTypeAccount { + if accountInfo.Spec.ParentAccount == nil || accountInfo.Spec.ParentAccount.OriginClusterId == "" || accountInfo.Spec.ParentAccount.Name == "" { + return ctrl.Result{Requeue: true}, nil + } + if accountInfo.Spec.Organization.Path == "" { + return ctrl.Result{Requeue: true}, nil + } + storeName = generateStoreNameFromPath(accountInfo.Spec.Organization.Path) + if storeName == "" { + return ctrl.Result{Requeue: true}, nil + } + } + + store := &v1alpha1.Store{ObjectMeta: metav1.ObjectMeta{Name: storeName}} + if err := w.orgsClient.Get(ctxWithTimeout, client.ObjectKeyFromObject(store), store); err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get store: %w", err), true, true) } if store.Status.StoreID == "" { - // Store is not ready yet, requeue return ctrl.Result{Requeue: true}, nil } - // Update accountInfo with storeid - wsCfg := rest.CopyConfig(w.restCfg) - wsCfg.Host = strings.ReplaceAll(wsCfg.Host, "/services/initializingworkspaces/root:security", "/clusters/"+path) - wsClient, err := client.New(wsCfg, client.Options{Scheme: w.cl.Scheme()}) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to create client: %w", err), true, true) + if accountInfo.Spec.FGA.Store.Id != store.Status.StoreID { + if err := w.ensureAccountInfoStoreID(ctxWithTimeout, wsClient, store.Status.StoreID); err != nil { + return ctrl.Result{}, err + } } - accountInfo := accountsv1alpha1.AccountInfo{ - ObjectMeta: metav1.ObjectMeta{Name: "account"}, + additionalTuples, opErr := w.buildAdditionalTuples(account, accountInfo) + if opErr != nil { + return ctrl.Result{}, opErr } - _, err = controllerutil.CreateOrUpdate(ctxWithTimeout, wsClient, &accountInfo, func() error { - accountInfo.Spec.FGA.Store.Id = store.Status.StoreID + + desiredTuples := baseTuples(w.fgaObjectType, accountInfo) + desiredTuples = append(desiredTuples, additionalTuples...) + + _, err = controllerutil.CreateOrUpdate(ctxWithTimeout, w.orgsClient, store, func() error { + store.Spec.CoreModule = w.coreModule + store.Spec.Tuples = mergeTuples(store.Spec.Tuples, desiredTuples...) return nil }) if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to create/update accountInfo: %w", err), true, true) + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to update store tuples: %w", err), true, true) } original := lc.DeepCopy() @@ -128,14 +189,108 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance lifecycleru return s == initializerName }) - err = w.cl.Status().Patch(ctx, lc, client.MergeFrom(original)) - if err != nil { + if err := w.cl.Status().Patch(ctx, lc, client.MergeFrom(original)); err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to patch out initializers: %w", err), true, true) } return ctrl.Result{}, nil } +func (w *workspaceInitializer) ensureAccountInfoStoreID(ctx context.Context, wsClient client.Client, storeID string) errors.OperatorError { + accountInfo := &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: "account"}} + _, err := controllerutil.CreateOrUpdate(ctx, wsClient, accountInfo, func() error { + accountInfo.Spec.FGA.Store.Id = storeID + return nil + }) + if err != nil { + return errors.NewOperatorError(fmt.Errorf("unable to create/update accountInfo: %w", err), true, true) + } + return nil +} + +func (w *workspaceInitializer) getAccount(ctx context.Context, lc *kcpv1alpha1.LogicalCluster) (*accountsv1alpha1.Account, errors.OperatorError) { + account := &accountsv1alpha1.Account{} + ownerCluster := logicalcluster.Name(lc.Spec.Owner.Cluster) + if err := w.cl.Get(kontext.WithCluster(ctx, ownerCluster), client.ObjectKey{Name: lc.Spec.Owner.Name}, account); err != nil { + return nil, errors.NewOperatorError(fmt.Errorf("unable to get account: %w", err), true, true) + } + return account, nil +} + +func (w *workspaceInitializer) buildAdditionalTuples(account *accountsv1alpha1.Account, accountInfo *accountsv1alpha1.AccountInfo) ([]v1alpha1.Tuple, errors.OperatorError) { + tuples := []v1alpha1.Tuple{} + + if account.Spec.Type != accountsv1alpha1.AccountTypeOrg { + parentAccountName := accountInfo.Spec.ParentAccount.Name + tuples = append(tuples, v1alpha1.Tuple{ + Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), + Relation: w.fgaParentRelation, + User: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.ParentAccount.OriginClusterId, parentAccountName), + }) + } + + if account.Spec.Creator != nil { + if !validateCreator(*account.Spec.Creator) { + return nil, errors.NewOperatorError(fmt.Errorf("creator string is in the protected service account prefix range"), false, false) + } + creator := formatUser(*account.Spec.Creator) + + tuples = append(tuples, v1alpha1.Tuple{ + Object: fmt.Sprintf("role:%s/%s/%s/owner", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), + Relation: "assignee", + User: fmt.Sprintf("user:%s", creator), + }) + + tuples = append(tuples, v1alpha1.Tuple{ + Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), + Relation: w.fgaCreatorRelation, + User: fmt.Sprintf("role:%s/%s/%s/owner#assignee", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), + }) + } + + return tuples, nil +} + +func baseTuples(objectType string, accountInfo *accountsv1alpha1.AccountInfo) []v1alpha1.Tuple { + return []v1alpha1.Tuple{ + { + Object: "role:authenticated", + Relation: "assignee", + User: "user:*", + }, + { + Object: fmt.Sprintf("%s:%s/%s", objectType, accountInfo.Spec.Account.OriginClusterId, accountInfo.Spec.Account.Name), + Relation: "member", + User: "role:authenticated#assignee", + }, + } +} + +func mergeTuples(existing []v1alpha1.Tuple, additions ...v1alpha1.Tuple) []v1alpha1.Tuple { + seen := make(map[string]struct{}) + result := make([]v1alpha1.Tuple, 0, len(existing)+len(additions)) + + for _, tuple := range existing { + key := tuple.String() + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + result = append(result, tuple) + } + + for _, tuple := range additions { + key := tuple.String() + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + result = append(result, tuple) + } + + return result +} + func generateStoreName(lc *kcpv1alpha1.LogicalCluster) string { if path, ok := lc.Annotations["kcp.io/path"]; ok { pathElements := strings.Split(path, ":") @@ -143,3 +298,24 @@ func generateStoreName(lc *kcpv1alpha1.LogicalCluster) string { } return "" } + +func generateStoreNameFromPath(path string) string { + parts := strings.Split(path, ":") + if len(parts) == 0 { + return "" + } + return parts[len(parts)-1] +} + +var saRegex = regexp.MustCompile(`^system:serviceaccount:[^:]*:[^:]*$`) + +func formatUser(user string) string { + if saRegex.MatchString(user) { + return strings.ReplaceAll(user, ":", ".") + } + return user +} + +func validateCreator(creator string) bool { + return !strings.HasPrefix(creator, "system.serviceaccount") +} From 19b6d16fa7c2643cbdef92bca898d976806b350d Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Fri, 10 Oct 2025 16:43:15 +0300 Subject: [PATCH 03/35] feat: Workspace initializer test exercises org/account success paths and a requeue scenario with fakes --- .../subroutine/workspace_initializer_test.go | 266 ++++++++++++++++++ 1 file changed, 266 insertions(+) create mode 100644 internal/subroutine/workspace_initializer_test.go diff --git a/internal/subroutine/workspace_initializer_test.go b/internal/subroutine/workspace_initializer_test.go new file mode 100644 index 0000000..2a642e7 --- /dev/null +++ b/internal/subroutine/workspace_initializer_test.go @@ -0,0 +1,266 @@ +package subroutine + +import ( + "context" + "os" + "path/filepath" + "testing" + + kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" + "github.com/kcp-dev/logicalcluster/v3" + accountsv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/platform-mesh/security-operator/api/v1alpha1" + "github.com/platform-mesh/security-operator/internal/config" +) + +func setupScheme(t *testing.T) *runtime.Scheme { + scheme := runtime.NewScheme() + require.NoError(t, kcpv1alpha1.AddToScheme(scheme)) + require.NoError(t, accountsv1alpha1.AddToScheme(scheme)) + require.NoError(t, v1alpha1.AddToScheme(scheme)) + return scheme +} + +func newTestInitializer(t *testing.T, scheme *runtime.Scheme, cl client.Client, orgsClient client.Client, cfg config.Config, wsClient client.Client) *workspaceInitializer { + restCfg := &rest.Config{Host: "https://example/services/initializingworkspaces/root:security"} + initializer := NewWorkspaceInitializer(cl, orgsClient, restCfg, cfg) + initializer.newWorkspaceClientFunc = func(string) (client.Client, error) { + return wsClient, nil + } + return initializer +} + +func TestWorkspaceInitializer_Process_Organization(t *testing.T) { + scheme := setupScheme(t) + + moduleDir := t.TempDir() + modulePath := filepath.Join(moduleDir, "core-module.fga") + require.NoError(t, os.WriteFile(modulePath, []byte("model"), 0o600)) + + cfg := config.Config{} + cfg.CoreModulePath = modulePath + cfg.FGA.ObjectType = "core_platform-mesh_io_account" + cfg.FGA.ParentRelation = "parent" + cfg.FGA.CreatorRelation = "owner" + + lc := &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + Annotations: map[string]string{ + "kcp.io/path": "root:security:org-workspace", + logicalcluster.AnnotationKey: "root:security:org-workspace", + }, + }, + Spec: kcpv1alpha1.LogicalClusterSpec{ + Owner: &kcpv1alpha1.LogicalClusterOwner{Cluster: "root:orgs", Name: "org-account"}, + }, + Status: kcpv1alpha1.LogicalClusterStatus{Initializers: []kcpv1alpha1.LogicalClusterInitializer{initializerName}}, + } + + account := &accountsv1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{Name: "org-account"}, + Spec: accountsv1alpha1.AccountSpec{ + Type: accountsv1alpha1.AccountTypeOrg, + Creator: ptr.To("creator@example.com"), + }, + } + + cl := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(lc.DeepCopy(), account). + WithStatusSubresource(&kcpv1alpha1.LogicalCluster{}). + Build() + + store := &v1alpha1.Store{ + ObjectMeta: metav1.ObjectMeta{Name: "org-workspace"}, + Status: v1alpha1.StoreStatus{StoreID: "store-id"}, + } + + orgsClient := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(store). + WithStatusSubresource(&v1alpha1.Store{}). + Build() + + wsAccountInfo := &accountsv1alpha1.AccountInfo{ + ObjectMeta: metav1.ObjectMeta{Name: "account"}, + Spec: accountsv1alpha1.AccountInfoSpec{ + FGA: accountsv1alpha1.FGAInfo{Store: accountsv1alpha1.StoreInfo{Id: "store-id"}}, + Account: accountsv1alpha1.AccountLocation{ + Name: "org-account", + OriginClusterId: "root:orgs", + }, + }, + } + + wsClient := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(wsAccountInfo). + Build() + + initializer := newTestInitializer(t, scheme, cl, orgsClient, cfg, wsClient) + result, opErr := initializer.Process(context.Background(), lc.DeepCopy()) + require.Nil(t, opErr) + assert.Equal(t, ctrl.Result{}, result) + + updatedStore := &v1alpha1.Store{} + require.NoError(t, orgsClient.Get(context.Background(), client.ObjectKey{Name: "org-workspace"}, updatedStore)) + + expectedTuples := []v1alpha1.Tuple{ + {Object: "role:authenticated", Relation: "assignee", User: "user:*"}, + {Object: "core_platform-mesh_io_account:root:orgs/org-account", Relation: "member", User: "role:authenticated#assignee"}, + {Object: "role:core_platform-mesh_io_account/root:orgs/org-account/owner", Relation: "assignee", User: "user:creator@example.com"}, + {Object: "core_platform-mesh_io_account:root:orgs/org-account", Relation: "owner", User: "role:core_platform-mesh_io_account/root:orgs/org-account/owner#assignee"}, + } + assert.ElementsMatch(t, expectedTuples, updatedStore.Spec.Tuples) + + updatedLC := &kcpv1alpha1.LogicalCluster{} + require.NoError(t, cl.Get(context.Background(), client.ObjectKey{Name: "cluster"}, updatedLC)) + assert.NotContains(t, updatedLC.Status.Initializers, initializerName) +} + +func TestWorkspaceInitializer_Process_Account(t *testing.T) { + scheme := setupScheme(t) + + moduleDir := t.TempDir() + modulePath := filepath.Join(moduleDir, "core-module.fga") + require.NoError(t, os.WriteFile(modulePath, []byte("model"), 0o600)) + + cfg := config.Config{} + cfg.CoreModulePath = modulePath + cfg.FGA.ObjectType = "core_platform-mesh_io_account" + cfg.FGA.ParentRelation = "parent" + cfg.FGA.CreatorRelation = "owner" + + lc := &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + Annotations: map[string]string{ + "kcp.io/path": "root:security:org-workspace:child-account", + logicalcluster.AnnotationKey: "root:security:org-workspace:child-account", + }, + }, + Spec: kcpv1alpha1.LogicalClusterSpec{ + Owner: &kcpv1alpha1.LogicalClusterOwner{Cluster: "root:orgs", Name: "child-account"}, + }, + Status: kcpv1alpha1.LogicalClusterStatus{Initializers: []kcpv1alpha1.LogicalClusterInitializer{initializerName}}, + } + + account := &accountsv1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{Name: "child-account"}, + Spec: accountsv1alpha1.AccountSpec{ + Type: accountsv1alpha1.AccountTypeAccount, + Creator: ptr.To("system:serviceaccount:tenant:creator"), + }, + } + + cl := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(lc.DeepCopy(), account). + WithStatusSubresource(&kcpv1alpha1.LogicalCluster{}). + Build() + + store := &v1alpha1.Store{ + ObjectMeta: metav1.ObjectMeta{Name: "org-workspace"}, + Status: v1alpha1.StoreStatus{StoreID: "store-id"}, + } + + orgsClient := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(store). + WithStatusSubresource(&v1alpha1.Store{}). + Build() + + wsAccountInfo := &accountsv1alpha1.AccountInfo{ + ObjectMeta: metav1.ObjectMeta{Name: "account"}, + Spec: accountsv1alpha1.AccountInfoSpec{ + FGA: accountsv1alpha1.FGAInfo{Store: accountsv1alpha1.StoreInfo{Id: "store-id"}}, + Account: accountsv1alpha1.AccountLocation{ + Name: "child-account", + OriginClusterId: "root:orgs", + }, + ParentAccount: &accountsv1alpha1.AccountLocation{ + Name: "org-account", + OriginClusterId: "root:orgs", + }, + Organization: accountsv1alpha1.AccountLocation{Path: "root:security:org-workspace"}, + }, + } + + wsClient := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(wsAccountInfo). + Build() + + initializer := newTestInitializer(t, scheme, cl, orgsClient, cfg, wsClient) + result, opErr := initializer.Process(context.Background(), lc.DeepCopy()) + require.Nil(t, opErr) + assert.Equal(t, ctrl.Result{}, result) + + updatedStore := &v1alpha1.Store{} + require.NoError(t, orgsClient.Get(context.Background(), client.ObjectKey{Name: "org-workspace"}, updatedStore)) + + expectedTuples := []v1alpha1.Tuple{ + {Object: "role:authenticated", Relation: "assignee", User: "user:*"}, + {Object: "core_platform-mesh_io_account:root:orgs/child-account", Relation: "member", User: "role:authenticated#assignee"}, + {Object: "core_platform-mesh_io_account:root:orgs/child-account", Relation: "parent", User: "core_platform-mesh_io_account:root:orgs/org-account"}, + {Object: "role:core_platform-mesh_io_account/root:orgs/child-account/owner", Relation: "assignee", User: "user:system.serviceaccount.tenant.creator"}, + {Object: "core_platform-mesh_io_account:root:orgs/child-account", Relation: "owner", User: "role:core_platform-mesh_io_account/root:orgs/child-account/owner#assignee"}, + } + assert.ElementsMatch(t, expectedTuples, updatedStore.Spec.Tuples) +} + +func TestWorkspaceInitializer_Process_RequeuesWhenAccountInfoMissing(t *testing.T) { + scheme := setupScheme(t) + + moduleDir := t.TempDir() + modulePath := filepath.Join(moduleDir, "core-module.fga") + require.NoError(t, os.WriteFile(modulePath, []byte("model"), 0o600)) + + cfg := config.Config{} + cfg.CoreModulePath = modulePath + cfg.FGA.ObjectType = "core_platform-mesh_io_account" + cfg.FGA.ParentRelation = "parent" + cfg.FGA.CreatorRelation = "owner" + + lc := &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + Annotations: map[string]string{ + "kcp.io/path": "root:security:org-workspace", + logicalcluster.AnnotationKey: "root:security:org-workspace", + }, + }, + Spec: kcpv1alpha1.LogicalClusterSpec{ + Owner: &kcpv1alpha1.LogicalClusterOwner{Cluster: "root:orgs", Name: "org-account"}, + }, + Status: kcpv1alpha1.LogicalClusterStatus{Initializers: []kcpv1alpha1.LogicalClusterInitializer{initializerName}}, + } + + account := &accountsv1alpha1.Account{ + ObjectMeta: metav1.ObjectMeta{Name: "org-account"}, + Spec: accountsv1alpha1.AccountSpec{Type: accountsv1alpha1.AccountTypeOrg}, + } + + cl := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(lc.DeepCopy(), account). + WithStatusSubresource(&kcpv1alpha1.LogicalCluster{}). + Build() + + store := &v1alpha1.Store{ObjectMeta: metav1.ObjectMeta{Name: "org-workspace"}} + orgsClient := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(store). + WithStatusSubresource(&v1alpha1.Store{}). + Build() + + wsClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + initializer := newTestInitializer(t, scheme, cl, orgsClient, cfg, wsClient) + result, opErr := initializer.Process(context.Background(), lc.DeepCopy()) + require.Nil(t, opErr) + assert.True(t, result.Requeue) +} From 455c8aa4554060f19964eca4464453f508792f81 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Wed, 15 Oct 2025 16:31:18 +0300 Subject: [PATCH 04/35] fix: logic reimplementation --- cmd/initializer.go | 14 +- internal/controller/initializer_controller.go | 5 +- internal/subroutine/workspace_initializer.go | 184 ++++++++++-------- 3 files changed, 117 insertions(+), 86 deletions(-) diff --git a/cmd/initializer.go b/cmd/initializer.go index 9896c23..1018230 100644 --- a/cmd/initializer.go +++ b/cmd/initializer.go @@ -6,11 +6,14 @@ import ( helmv2 "github.com/fluxcd/helm-controller/api/v2" sourcev1 "github.com/fluxcd/source-controller/api/v1" + openfgav1 "github.com/openfga/api/proto/openfga/v1" "github.com/kcp-dev/logicalcluster/v3" "github.com/kcp-dev/multicluster-provider/initializingworkspaces" pmcontext "github.com/platform-mesh/golang-commons/context" "github.com/spf13/cobra" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/rest" @@ -97,7 +100,16 @@ var initializerCmd = &cobra.Command{ appCfg.IDP.AdditionalRedirectURLs = []string{} } - if err := controller.NewLogicalClusterReconciler(log, orgClient, appCfg, inClusterClient, mgr). + conn, err := grpc.NewClient(appCfg.FGA.Target, grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + log.Error().Err(err).Msg("unable to create grpc client") + os.Exit(1) + } + defer conn.Close() + + fga := openfgav1.NewOpenFGAServiceClient(conn) + + if err := controller.NewLogicalClusterReconciler(log, orgClient, appCfg, inClusterClient, mgr, fga). SetupWithManager(mgr, defaultCfg); err != nil { setupLog.Error(err, "unable to create controller", "controller", "LogicalCluster") os.Exit(1) diff --git a/internal/controller/initializer_controller.go b/internal/controller/initializer_controller.go index 8d44f5b..4d15fd3 100644 --- a/internal/controller/initializer_controller.go +++ b/internal/controller/initializer_controller.go @@ -4,6 +4,7 @@ import ( "context" kcpcorev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" + openfgav1 "github.com/openfga/api/proto/openfga/v1" platformeshconfig "github.com/platform-mesh/golang-commons/config" "github.com/platform-mesh/golang-commons/controller/lifecycle/builder" lifecyclecontrollerruntime "github.com/platform-mesh/golang-commons/controller/lifecycle/multicluster" @@ -26,11 +27,11 @@ type LogicalClusterReconciler struct { lifecycle *lifecyclecontrollerruntime.LifecycleManager } -func NewLogicalClusterReconciler(log *logger.Logger, orgClient client.Client, cfg config.Config, inClusterClient client.Client, mgr mcmanager.Manager) *LogicalClusterReconciler { +func NewLogicalClusterReconciler(log *logger.Logger, orgClient client.Client, cfg config.Config, inClusterClient client.Client, mgr mcmanager.Manager, fga openfgav1.OpenFGAServiceClient) *LogicalClusterReconciler { return &LogicalClusterReconciler{ log: log, lifecycle: builder.NewBuilder("logicalcluster", "LogicalClusterReconciler", []lifecyclesubroutine.Subroutine{ - subroutine.NewWorkspaceInitializer(orgClient, cfg, mgr), + subroutine.NewWorkspaceInitializer(orgClient, cfg, mgr, fga), subroutine.NewWorkspaceAuthConfigurationSubroutine(orgClient, inClusterClient, cfg), subroutine.NewRealmSubroutine(inClusterClient, &cfg, cfg.BaseDomain), subroutine.NewRemoveInitializer(mgr, cfg.InitializerName), diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index c5d2d55..0a6a114 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -10,12 +10,13 @@ import ( kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" "github.com/kcp-dev/logicalcluster/v3" + openfgav1 "github.com/openfga/api/proto/openfga/v1" accountsv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" "github.com/platform-mesh/account-operator/pkg/subroutines/accountinfo" "github.com/platform-mesh/golang-commons/controller/lifecycle/runtimeobject" lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine" - "github.com/platform-mesh/golang-commons/errors" + "github.com/platform-mesh/golang-commons/fga/helpers" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" @@ -28,7 +29,12 @@ import ( "github.com/platform-mesh/security-operator/internal/config" ) -func NewWorkspaceInitializer(orgsClient client.Client, cfg config.Config, mgr mcmanager.Manager) *workspaceInitializer { +const ( + annotationOwnerWritten = "security-operator.platform-mesh.io/fga-owner-written" + annotationParentWritten = "security-operator.platform-mesh.io/fga-parent-written" +) + +func NewWorkspaceInitializer(orgsClient client.Client, cfg config.Config, mgr mcmanager.Manager, fga openfgav1.OpenFGAServiceClient) *workspaceInitializer { coreModulePath := cfg.CoreModulePath data, err := os.ReadFile(coreModulePath) @@ -40,6 +46,7 @@ func NewWorkspaceInitializer(orgsClient client.Client, cfg config.Config, mgr mc orgsClient: orgsClient, mgr: mgr, coreModule: string(data), + fga: fga, fgaObjectType: cfg.FGA.ObjectType, fgaParentRel: cfg.FGA.ParentRelation, fgaCreatorRel: cfg.FGA.CreatorRelation, @@ -52,6 +59,7 @@ type workspaceInitializer struct { orgsClient client.Client mgr mcmanager.Manager coreModule string + fga openfgav1.OpenFGAServiceClient fgaObjectType string fgaParentRel string fgaCreatorRel string @@ -111,11 +119,6 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje return ctrl.Result{}, opErr } - additionalTuples, opErr := w.buildAdditionalTuples(&account, accountInfo) - if opErr != nil { - return ctrl.Result{}, opErr - } - ctxStore := mccontext.WithCluster(ctxWithTimeout, storeClusterName.String()) allowCreate := account.Spec.Type == accountsv1alpha1.AccountTypeOrg @@ -134,8 +137,6 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje if allowCreate { store.Spec.CoreModule = w.coreModule } - desired := append(baseTuples(w.fgaObjectType, accountInfo), additionalTuples...) - store.Spec.Tuples = mergeTuples(store.Spec.Tuples, desired...) return nil }) if err != nil { @@ -156,6 +157,55 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje } } + annotations := lc.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + ownerWritten := annotations[annotationOwnerWritten] == "true" + parentWritten := annotations[annotationParentWritten] == "true" + + if account.Spec.Type != accountsv1alpha1.AccountTypeOrg && !parentWritten { + parentAccount := accountInfo.Spec.ParentAccount + if parentAccount == nil || parentAccount.Name == "" || parentAccount.OriginClusterId == "" { + return ctrl.Result{Requeue: true}, nil + } + if err := w.writeTuple(ctx, store.Status.StoreID, &openfgav1.TupleKey{ + User: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, parentAccount.OriginClusterId, parentAccount.Name), + Relation: w.fgaParentRel, + Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), + }); err != nil { + return ctrl.Result{}, err + } + parentWritten = true + } + + if !ownerWritten && account.Spec.Creator != nil { + if !validateCreator(*account.Spec.Creator) { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("creator string is in the protected service account prefix range"), false, false) + } + creator := formatUser(*account.Spec.Creator) + + if err := w.writeTuple(ctx, store.Status.StoreID, &openfgav1.TupleKey{ + User: fmt.Sprintf("user:%s", creator), + Relation: "assignee", + Object: fmt.Sprintf("role:%s/%s/%s/owner", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), + }); err != nil { + return ctrl.Result{}, err + } + if err := w.writeTuple(ctx, store.Status.StoreID, &openfgav1.TupleKey{ + User: fmt.Sprintf("role:%s/%s/%s/owner#assignee", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), + Relation: w.fgaCreatorRel, + Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), + }); err != nil { + return ctrl.Result{}, err + } + ownerWritten = true + } + + if err := w.updateAnnotations(ctx, clusterRef.GetClient(), lc, ownerWritten, parentWritten); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -194,80 +244,6 @@ func (w *workspaceInitializer) resolveStoreTarget(lc *kcpv1alpha1.LogicalCluster return logicalcluster.Name(accountInfo.Spec.Organization.Path), storeName, nil } -func (w *workspaceInitializer) buildAdditionalTuples(account *accountsv1alpha1.Account, accountInfo *accountsv1alpha1.AccountInfo) ([]v1alpha1.Tuple, errors.OperatorError) { - tuples := []v1alpha1.Tuple{} - - if account.Spec.Type != accountsv1alpha1.AccountTypeOrg { - parentAccountName := accountInfo.Spec.ParentAccount.Name - tuples = append(tuples, v1alpha1.Tuple{ - Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), - Relation: w.fgaParentRel, - User: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.ParentAccount.OriginClusterId, parentAccountName), - }) - } - - if account.Spec.Creator != nil { - if !validateCreator(*account.Spec.Creator) { - return nil, errors.NewOperatorError(fmt.Errorf("creator string is in the protected service account prefix range"), false, false) - } - creator := formatUser(*account.Spec.Creator) - - tuples = append(tuples, v1alpha1.Tuple{ - Object: fmt.Sprintf("role:%s/%s/%s/owner", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), - Relation: "assignee", - User: fmt.Sprintf("user:%s", creator), - }) - - tuples = append(tuples, v1alpha1.Tuple{ - Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), - Relation: w.fgaCreatorRel, - User: fmt.Sprintf("role:%s/%s/%s/owner#assignee", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), - }) - } - - return tuples, nil -} - -func baseTuples(objectType string, accountInfo *accountsv1alpha1.AccountInfo) []v1alpha1.Tuple { - return []v1alpha1.Tuple{ - { - Object: "role:authenticated", - Relation: "assignee", - User: "user:*", - }, - { - Object: fmt.Sprintf("%s:%s/%s", objectType, accountInfo.Spec.Account.OriginClusterId, accountInfo.Spec.Account.Name), - Relation: "member", - User: "role:authenticated#assignee", - }, - } -} - -func mergeTuples(existing []v1alpha1.Tuple, additions ...v1alpha1.Tuple) []v1alpha1.Tuple { - seen := make(map[string]struct{}, len(existing)+len(additions)) - merged := make([]v1alpha1.Tuple, 0, len(existing)+len(additions)) - - for _, tuple := range existing { - key := tuple.String() - if _, ok := seen[key]; ok { - continue - } - seen[key] = struct{}{} - merged = append(merged, tuple) - } - - for _, tuple := range additions { - key := tuple.String() - if _, ok := seen[key]; ok { - continue - } - seen[key] = struct{}{} - merged = append(merged, tuple) - } - - return merged -} - func generateStoreName(lc *kcpv1alpha1.LogicalCluster) string { if path, ok := lc.Annotations["kcp.io/path"]; ok { pathElements := strings.Split(path, ":") @@ -296,3 +272,45 @@ func formatUser(user string) string { func validateCreator(creator string) bool { return !strings.HasPrefix(creator, "system.serviceaccount") } + +func (w *workspaceInitializer) writeTuple(ctx context.Context, storeID string, tuple *openfgav1.TupleKey) errors.OperatorError { + _, err := w.fga.Write(ctx, &openfgav1.WriteRequest{ + StoreId: storeID, + Writes: &openfgav1.WriteRequestWrites{ + TupleKeys: []*openfgav1.TupleKey{tuple}, + }, + }) + + if helpers.IsDuplicateWriteError(err) { + return nil + } + if err != nil { + return errors.NewOperatorError(fmt.Errorf("unable to write FGA tuple: %w", err), true, true) + } + return nil +} + +func (w *workspaceInitializer) updateAnnotations(ctx context.Context, cl client.Client, lc *kcpv1alpha1.LogicalCluster, ownerWritten, parentWritten bool) errors.OperatorError { + currentOwner := lc.GetAnnotations()[annotationOwnerWritten] == "true" + currentParent := lc.GetAnnotations()[annotationParentWritten] == "true" + + if currentOwner == ownerWritten && currentParent == parentWritten { + return nil + } + + original := lc.DeepCopy() + if lc.Annotations == nil { + lc.Annotations = map[string]string{} + } + if ownerWritten { + lc.Annotations[annotationOwnerWritten] = "true" + } + if parentWritten { + lc.Annotations[annotationParentWritten] = "true" + } + + if err := cl.Patch(ctx, lc, client.MergeFrom(original)); err != nil { + return errors.NewOperatorError(fmt.Errorf("unable to patch logical cluster annotations: %w", err), true, true) + } + return nil +} From a2102a80b691d19504436f6076cb246626de32ab Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Wed, 15 Oct 2025 17:21:41 +0300 Subject: [PATCH 05/35] fix: initializerCfg instead of appCfg --- cmd/initializer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/initializer.go b/cmd/initializer.go index 689c810..bcb4472 100644 --- a/cmd/initializer.go +++ b/cmd/initializer.go @@ -101,7 +101,7 @@ var initializerCmd = &cobra.Command{ initializerCfg.IDP.AdditionalRedirectURLs = []string{} } - conn, err := grpc.NewClient(appCfg.FGA.Target, grpc.WithTransportCredentials(insecure.NewCredentials())) + conn, err := grpc.NewClient(initializerCfg.FGA.Target, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { log.Error().Err(err).Msg("unable to create grpc client") os.Exit(1) @@ -110,7 +110,7 @@ var initializerCmd = &cobra.Command{ fga := openfgav1.NewOpenFGAServiceClient(conn) - if err := controller.NewLogicalClusterReconciler(log, orgClient, appCfg, inClusterClient, mgr, fga). + if err := controller.NewLogicalClusterReconciler(log, orgClient, initializerCfg, inClusterClient, mgr, fga). SetupWithManager(mgr, defaultCfg); err != nil { setupLog.Error(err, "unable to create controller", "controller", "LogicalCluster") os.Exit(1) From 7a94d013ff364d9b87e6384711b0b8892efcbff3 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Wed, 15 Oct 2025 17:26:20 +0300 Subject: [PATCH 06/35] fix: gRPC client creation: use DialContext --- cmd/initializer.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/cmd/initializer.go b/cmd/initializer.go index bcb4472..decacd8 100644 --- a/cmd/initializer.go +++ b/cmd/initializer.go @@ -1,8 +1,10 @@ package cmd import ( + "context" "crypto/tls" "os" + "time" helmv2 "github.com/fluxcd/helm-controller/api/v2" sourcev1 "github.com/fluxcd/source-controller/api/v1" @@ -101,12 +103,19 @@ var initializerCmd = &cobra.Command{ initializerCfg.IDP.AdditionalRedirectURLs = []string{} } - conn, err := grpc.NewClient(initializerCfg.FGA.Target, grpc.WithTransportCredentials(insecure.NewCredentials())) + ctxDial, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + conn, err := grpc.DialContext( + ctxDial, + initializerCfg.FGA.Target, + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithBlock(), + ) if err != nil { - log.Error().Err(err).Msg("unable to create grpc client") + log.Error().Err(err).Msg("unable to dial OpenFGA") os.Exit(1) } - defer conn.Close() + defer func() { _ = conn.Close() }() fga := openfgav1.NewOpenFGAServiceClient(conn) From 1697335596d57ad23ac38d26ef9e96ff85f19025 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Wed, 15 Oct 2025 17:28:31 +0300 Subject: [PATCH 07/35] fix: switched the FGA tuple writes to use the ctxWithTimeout --- internal/subroutine/workspace_initializer.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index 0a6a114..3492da6 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -169,7 +169,7 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje if parentAccount == nil || parentAccount.Name == "" || parentAccount.OriginClusterId == "" { return ctrl.Result{Requeue: true}, nil } - if err := w.writeTuple(ctx, store.Status.StoreID, &openfgav1.TupleKey{ + if err := w.writeTuple(ctxWithTimeout, store.Status.StoreID, &openfgav1.TupleKey{ User: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, parentAccount.OriginClusterId, parentAccount.Name), Relation: w.fgaParentRel, Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), @@ -185,14 +185,14 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje } creator := formatUser(*account.Spec.Creator) - if err := w.writeTuple(ctx, store.Status.StoreID, &openfgav1.TupleKey{ + if err := w.writeTuple(ctxWithTimeout, store.Status.StoreID, &openfgav1.TupleKey{ User: fmt.Sprintf("user:%s", creator), Relation: "assignee", Object: fmt.Sprintf("role:%s/%s/%s/owner", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), }); err != nil { return ctrl.Result{}, err } - if err := w.writeTuple(ctx, store.Status.StoreID, &openfgav1.TupleKey{ + if err := w.writeTuple(ctxWithTimeout, store.Status.StoreID, &openfgav1.TupleKey{ User: fmt.Sprintf("role:%s/%s/%s/owner#assignee", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), Relation: w.fgaCreatorRel, Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), From f9b813859c301e014c1739280af04ebad118d8ce Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Wed, 15 Oct 2025 17:31:08 +0300 Subject: [PATCH 08/35] fix: updated validateCreator in workspace_initializer.go to block both the colon and dotted service account forms --- internal/subroutine/workspace_initializer.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index 3492da6..5ba6eb1 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -270,7 +270,14 @@ func formatUser(user string) string { } func validateCreator(creator string) bool { - return !strings.HasPrefix(creator, "system.serviceaccount") + // Block both canonical and formatted service account usernames + if strings.HasPrefix(creator, "system:serviceaccount:") { + return false + } + if strings.HasPrefix(creator, "system.serviceaccount.") { + return false + } + return true } func (w *workspaceInitializer) writeTuple(ctx context.Context, storeID string, tuple *openfgav1.TupleKey) errors.OperatorError { From 19346fd49ed1ebad9b634dcd9d63b3d3bf587ae8 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Wed, 15 Oct 2025 17:34:46 +0300 Subject: [PATCH 09/35] fix: abort dialContext, use NewClient instead --- cmd/initializer.go | 24 ++++++++++++++++++++---- internal/controller/store_controller.go | 2 +- internal/subroutine/tuples.go | 4 ++-- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/cmd/initializer.go b/cmd/initializer.go index decacd8..500adc7 100644 --- a/cmd/initializer.go +++ b/cmd/initializer.go @@ -15,6 +15,7 @@ import ( pmcontext "github.com/platform-mesh/golang-commons/context" "github.com/spf13/cobra" "google.golang.org/grpc" + "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials/insecure" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -105,14 +106,29 @@ var initializerCmd = &cobra.Command{ ctxDial, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() - conn, err := grpc.DialContext( - ctxDial, + conn, err := grpc.NewClient( initializerCfg.FGA.Target, grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithBlock(), ) if err != nil { - log.Error().Err(err).Msg("unable to dial OpenFGA") + log.Error().Err(err).Msg("unable to create OpenFGA client connection") + os.Exit(1) + } + // Eagerly connect and wait until Ready or timeout to fail fast on bad endpoints. + conn.Connect() + ready := false + for { + s := conn.GetState() + if s == connectivity.Ready { + ready = true + break + } + if !conn.WaitForStateChange(ctxDial, s) { + break // context expired + } + } + if !ready { + log.Error().Msg("OpenFGA connection not ready before deadline") os.Exit(1) } defer func() { _ = conn.Close() }() diff --git a/internal/controller/store_controller.go b/internal/controller/store_controller.go index c2a043c..50ee985 100644 --- a/internal/controller/store_controller.go +++ b/internal/controller/store_controller.go @@ -59,7 +59,7 @@ func (r *StoreReconciler) SetupWithManager(mgr mcmanager.Manager, cfg *platforme Watches( &corev1alpha1.AuthorizationModel{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []reconcile.Request { - model,ok := obj.(*corev1alpha1.AuthorizationModel) + model, ok := obj.(*corev1alpha1.AuthorizationModel) if !ok { return nil } diff --git a/internal/subroutine/tuples.go b/internal/subroutine/tuples.go index e04d658..998f85c 100644 --- a/internal/subroutine/tuples.go +++ b/internal/subroutine/tuples.go @@ -140,8 +140,8 @@ func (t *tupleSubroutine) Process(ctx context.Context, instance runtimeobject.Ru } storeCtx := mccontext.WithCluster(ctx, string(logicalcluster.Name(lc.Annotations[logicalcluster.AnnotationKey]))) - - storeCluster, err := t.mgr.GetCluster(ctx,obj.Spec.StoreRef.Path) + + storeCluster, err := t.mgr.GetCluster(ctx, obj.Spec.StoreRef.Path) if err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get store cluster: %w", err), true, false) } From 161669939f677530280a31470a95eb2eb23df017 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Wed, 15 Oct 2025 18:03:58 +0300 Subject: [PATCH 10/35] fix: Use TLS (and auth) for OpenFGA; avoid insecure transport --- cmd/initializer.go | 82 +++++++++++++++++++++++++++++++++++++-- internal/config/config.go | 4 ++ 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/cmd/initializer.go b/cmd/initializer.go index 500adc7..9289620 100644 --- a/cmd/initializer.go +++ b/cmd/initializer.go @@ -3,7 +3,10 @@ package cmd import ( "context" "crypto/tls" + "crypto/x509" + "io/fs" "os" + "path/filepath" "time" helmv2 "github.com/fluxcd/helm-controller/api/v2" @@ -16,6 +19,7 @@ import ( "github.com/spf13/cobra" "google.golang.org/grpc" "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -30,6 +34,49 @@ import ( "github.com/platform-mesh/security-operator/internal/controller" ) +// loadCertPool loads PEM certificates from a file or directory into a CertPool. +func loadCertPool(path string) (*x509.CertPool, error) { + pool, _ := x509.SystemCertPool() + if pool == nil { + pool = x509.NewCertPool() + } + info, err := os.Stat(path) + if err != nil { + return nil, err + } + add := func(p string) error { + pem, err := os.ReadFile(p) + if err != nil { + return err + } + if ok := pool.AppendCertsFromPEM(pem); !ok { + return fs.ErrInvalid + } + return nil + } + if info.IsDir() { + return pool, filepath.WalkDir(path, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + return nil + } + return add(p) + }) + } + return pool, add(path) +} + +// bearerToken implements PerRPCCredentials with a static bearer token. +type bearerToken string + +func (b bearerToken) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) { + return map[string]string{"authorization": "Bearer " + string(b)}, nil +} + +func (b bearerToken) RequireTransportSecurity() bool { return true } + var initializerCmd = &cobra.Command{ Use: "initializer", Short: "FGA initializer for the organization workspacetype", @@ -104,12 +151,39 @@ var initializerCmd = &cobra.Command{ initializerCfg.IDP.AdditionalRedirectURLs = []string{} } + if initializerCfg.FGA.Target == "" { + log.Error().Msg("FGA target is empty; set fga-target in configuration") + os.Exit(1) + } + + // Build transport credentials: prefer TLS unless explicitly configured as insecure + var tcreds credentials.TransportCredentials + if !initializerCfg.FGA.Insecure { + var tlsCfg tls.Config + // Load custom CA if provided + if initializerCfg.FGA.CACertPath != "" { + // best-effort CA load; fall back to system pool on failure + if pool, err := loadCertPool(initializerCfg.FGA.CACertPath); err == nil { + tlsCfg.RootCAs = pool + } else { + log.Warn().Err(err).Msg("failed to load FGA CA bundle; falling back to system roots") + } + } + tcreds = credentials.NewTLS(&tlsCfg) + } else { + tcreds = insecure.NewCredentials() + } + + // Optional per-RPC bearer token + var dialOpts []grpc.DialOption + dialOpts = append(dialOpts, grpc.WithTransportCredentials(tcreds)) + if tok := initializerCfg.FGA.BearerToken; tok != "" { + dialOpts = append(dialOpts, grpc.WithPerRPCCredentials(bearerToken(tok))) + } + ctxDial, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() - conn, err := grpc.NewClient( - initializerCfg.FGA.Target, - grpc.WithTransportCredentials(insecure.NewCredentials()), - ) + conn, err := grpc.NewClient(initializerCfg.FGA.Target, dialOpts...) if err != nil { log.Error().Err(err).Msg("unable to create OpenFGA client connection") os.Exit(1) diff --git a/internal/config/config.go b/internal/config/config.go index 502f9ea..c3dbad5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -14,6 +14,10 @@ type Config struct { ObjectType string `mapstructure:"fga-object-type" default:"core_platform-mesh_io_account"` ParentRelation string `mapstructure:"fga-parent-relation" default:"parent"` CreatorRelation string `mapstructure:"fga-creator-relation" default:"owner"` + // Security options + Insecure bool `mapstructure:"fga-insecure" default:"false"` + CACertPath string `mapstructure:"fga-ca-path"` + BearerToken string `mapstructure:"fga-bearer-token"` } `mapstructure:",squash"` APIExportEndpointSliceName string `mapstructure:"api-export-endpoint-slice-name"` CoreModulePath string `mapstructure:"core-module-path"` From f3234262d2572006cf3f6ed4ae96909e030161e2 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Wed, 15 Oct 2025 18:15:55 +0300 Subject: [PATCH 11/35] fix: added a fail-fast validation to reject the incompatible config --- cmd/initializer.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/initializer.go b/cmd/initializer.go index 9289620..faf464e 100644 --- a/cmd/initializer.go +++ b/cmd/initializer.go @@ -156,6 +156,11 @@ var initializerCmd = &cobra.Command{ os.Exit(1) } + if initializerCfg.FGA.Insecure && initializerCfg.FGA.BearerToken != "" { + log.Error().Msg("FGA bearer token requires TLS; cannot use with fga-insecure=true") + os.Exit(1) + } + // Build transport credentials: prefer TLS unless explicitly configured as insecure var tcreds credentials.TransportCredentials if !initializerCfg.FGA.Insecure { From 674e8bfd8b54b32e4da28a042f08dc52a6688cff Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 16 Oct 2025 16:21:24 +0300 Subject: [PATCH 12/35] fix: cleaned up the initializer --- cmd/initializer.go | 102 +-------------------------------------------- 1 file changed, 2 insertions(+), 100 deletions(-) diff --git a/cmd/initializer.go b/cmd/initializer.go index faf464e..1583236 100644 --- a/cmd/initializer.go +++ b/cmd/initializer.go @@ -1,13 +1,8 @@ package cmd import ( - "context" "crypto/tls" - "crypto/x509" - "io/fs" "os" - "path/filepath" - "time" helmv2 "github.com/fluxcd/helm-controller/api/v2" sourcev1 "github.com/fluxcd/source-controller/api/v1" @@ -18,8 +13,6 @@ import ( pmcontext "github.com/platform-mesh/golang-commons/context" "github.com/spf13/cobra" "google.golang.org/grpc" - "google.golang.org/grpc/connectivity" - "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -34,49 +27,6 @@ import ( "github.com/platform-mesh/security-operator/internal/controller" ) -// loadCertPool loads PEM certificates from a file or directory into a CertPool. -func loadCertPool(path string) (*x509.CertPool, error) { - pool, _ := x509.SystemCertPool() - if pool == nil { - pool = x509.NewCertPool() - } - info, err := os.Stat(path) - if err != nil { - return nil, err - } - add := func(p string) error { - pem, err := os.ReadFile(p) - if err != nil { - return err - } - if ok := pool.AppendCertsFromPEM(pem); !ok { - return fs.ErrInvalid - } - return nil - } - if info.IsDir() { - return pool, filepath.WalkDir(path, func(p string, d fs.DirEntry, err error) error { - if err != nil { - return err - } - if d.IsDir() { - return nil - } - return add(p) - }) - } - return pool, add(path) -} - -// bearerToken implements PerRPCCredentials with a static bearer token. -type bearerToken string - -func (b bearerToken) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) { - return map[string]string{"authorization": "Bearer " + string(b)}, nil -} - -func (b bearerToken) RequireTransportSecurity() bool { return true } - var initializerCmd = &cobra.Command{ Use: "initializer", Short: "FGA initializer for the organization workspacetype", @@ -156,60 +106,12 @@ var initializerCmd = &cobra.Command{ os.Exit(1) } - if initializerCfg.FGA.Insecure && initializerCfg.FGA.BearerToken != "" { - log.Error().Msg("FGA bearer token requires TLS; cannot use with fga-insecure=true") - os.Exit(1) - } - - // Build transport credentials: prefer TLS unless explicitly configured as insecure - var tcreds credentials.TransportCredentials - if !initializerCfg.FGA.Insecure { - var tlsCfg tls.Config - // Load custom CA if provided - if initializerCfg.FGA.CACertPath != "" { - // best-effort CA load; fall back to system pool on failure - if pool, err := loadCertPool(initializerCfg.FGA.CACertPath); err == nil { - tlsCfg.RootCAs = pool - } else { - log.Warn().Err(err).Msg("failed to load FGA CA bundle; falling back to system roots") - } - } - tcreds = credentials.NewTLS(&tlsCfg) - } else { - tcreds = insecure.NewCredentials() - } - - // Optional per-RPC bearer token - var dialOpts []grpc.DialOption - dialOpts = append(dialOpts, grpc.WithTransportCredentials(tcreds)) - if tok := initializerCfg.FGA.BearerToken; tok != "" { - dialOpts = append(dialOpts, grpc.WithPerRPCCredentials(bearerToken(tok))) - } - - ctxDial, cancel := context.WithTimeout(ctx, 10*time.Second) - defer cancel() - conn, err := grpc.NewClient(initializerCfg.FGA.Target, dialOpts...) + // Transport: mTLS is handled by the service mesh (Istio). + conn, err := grpc.NewClient(initializerCfg.FGA.Target, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { log.Error().Err(err).Msg("unable to create OpenFGA client connection") os.Exit(1) } - // Eagerly connect and wait until Ready or timeout to fail fast on bad endpoints. - conn.Connect() - ready := false - for { - s := conn.GetState() - if s == connectivity.Ready { - ready = true - break - } - if !conn.WaitForStateChange(ctxDial, s) { - break // context expired - } - } - if !ready { - log.Error().Msg("OpenFGA connection not ready before deadline") - os.Exit(1) - } defer func() { _ = conn.Close() }() fga := openfgav1.NewOpenFGAServiceClient(conn) From 668941385cc6ac313ea2c754ea9e67b4e088eeac Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 16 Oct 2025 16:23:35 +0300 Subject: [PATCH 13/35] fix: cleared the config --- internal/config/config.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index c3dbad5..502f9ea 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -14,10 +14,6 @@ type Config struct { ObjectType string `mapstructure:"fga-object-type" default:"core_platform-mesh_io_account"` ParentRelation string `mapstructure:"fga-parent-relation" default:"parent"` CreatorRelation string `mapstructure:"fga-creator-relation" default:"owner"` - // Security options - Insecure bool `mapstructure:"fga-insecure" default:"false"` - CACertPath string `mapstructure:"fga-ca-path"` - BearerToken string `mapstructure:"fga-bearer-token"` } `mapstructure:",squash"` APIExportEndpointSliceName string `mapstructure:"api-export-endpoint-slice-name"` CoreModulePath string `mapstructure:"core-module-path"` From 31b977054c1c6d747527da32bb3672125a25ccd9 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 16 Oct 2025 16:25:07 +0300 Subject: [PATCH 14/35] fix: removed annotations --- internal/subroutine/workspace_initializer.go | 118 ------------------- 1 file changed, 118 deletions(-) diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index 5ba6eb1..be770a4 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "os" - "regexp" "strings" "time" @@ -16,7 +15,6 @@ import ( "github.com/platform-mesh/golang-commons/controller/lifecycle/runtimeobject" lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine" "github.com/platform-mesh/golang-commons/errors" - "github.com/platform-mesh/golang-commons/fga/helpers" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" @@ -29,11 +27,6 @@ import ( "github.com/platform-mesh/security-operator/internal/config" ) -const ( - annotationOwnerWritten = "security-operator.platform-mesh.io/fga-owner-written" - annotationParentWritten = "security-operator.platform-mesh.io/fga-parent-written" -) - func NewWorkspaceInitializer(orgsClient client.Client, cfg config.Config, mgr mcmanager.Manager, fga openfgav1.OpenFGAServiceClient) *workspaceInitializer { coreModulePath := cfg.CoreModulePath @@ -157,55 +150,6 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje } } - annotations := lc.GetAnnotations() - if annotations == nil { - annotations = map[string]string{} - } - ownerWritten := annotations[annotationOwnerWritten] == "true" - parentWritten := annotations[annotationParentWritten] == "true" - - if account.Spec.Type != accountsv1alpha1.AccountTypeOrg && !parentWritten { - parentAccount := accountInfo.Spec.ParentAccount - if parentAccount == nil || parentAccount.Name == "" || parentAccount.OriginClusterId == "" { - return ctrl.Result{Requeue: true}, nil - } - if err := w.writeTuple(ctxWithTimeout, store.Status.StoreID, &openfgav1.TupleKey{ - User: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, parentAccount.OriginClusterId, parentAccount.Name), - Relation: w.fgaParentRel, - Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), - }); err != nil { - return ctrl.Result{}, err - } - parentWritten = true - } - - if !ownerWritten && account.Spec.Creator != nil { - if !validateCreator(*account.Spec.Creator) { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("creator string is in the protected service account prefix range"), false, false) - } - creator := formatUser(*account.Spec.Creator) - - if err := w.writeTuple(ctxWithTimeout, store.Status.StoreID, &openfgav1.TupleKey{ - User: fmt.Sprintf("user:%s", creator), - Relation: "assignee", - Object: fmt.Sprintf("role:%s/%s/%s/owner", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), - }); err != nil { - return ctrl.Result{}, err - } - if err := w.writeTuple(ctxWithTimeout, store.Status.StoreID, &openfgav1.TupleKey{ - User: fmt.Sprintf("role:%s/%s/%s/owner#assignee", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), - Relation: w.fgaCreatorRel, - Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, account.Name), - }); err != nil { - return ctrl.Result{}, err - } - ownerWritten = true - } - - if err := w.updateAnnotations(ctx, clusterRef.GetClient(), lc, ownerWritten, parentWritten); err != nil { - return ctrl.Result{}, err - } - return ctrl.Result{}, nil } @@ -259,65 +203,3 @@ func generateStoreNameFromPath(path string) string { } return pathElements[len(pathElements)-1] } - -var saRegex = regexp.MustCompile(`^system:serviceaccount:[^:]*:[^:]*$`) - -func formatUser(user string) string { - if saRegex.MatchString(user) { - return strings.ReplaceAll(user, ":", ".") - } - return user -} - -func validateCreator(creator string) bool { - // Block both canonical and formatted service account usernames - if strings.HasPrefix(creator, "system:serviceaccount:") { - return false - } - if strings.HasPrefix(creator, "system.serviceaccount.") { - return false - } - return true -} - -func (w *workspaceInitializer) writeTuple(ctx context.Context, storeID string, tuple *openfgav1.TupleKey) errors.OperatorError { - _, err := w.fga.Write(ctx, &openfgav1.WriteRequest{ - StoreId: storeID, - Writes: &openfgav1.WriteRequestWrites{ - TupleKeys: []*openfgav1.TupleKey{tuple}, - }, - }) - - if helpers.IsDuplicateWriteError(err) { - return nil - } - if err != nil { - return errors.NewOperatorError(fmt.Errorf("unable to write FGA tuple: %w", err), true, true) - } - return nil -} - -func (w *workspaceInitializer) updateAnnotations(ctx context.Context, cl client.Client, lc *kcpv1alpha1.LogicalCluster, ownerWritten, parentWritten bool) errors.OperatorError { - currentOwner := lc.GetAnnotations()[annotationOwnerWritten] == "true" - currentParent := lc.GetAnnotations()[annotationParentWritten] == "true" - - if currentOwner == ownerWritten && currentParent == parentWritten { - return nil - } - - original := lc.DeepCopy() - if lc.Annotations == nil { - lc.Annotations = map[string]string{} - } - if ownerWritten { - lc.Annotations[annotationOwnerWritten] = "true" - } - if parentWritten { - lc.Annotations[annotationParentWritten] = "true" - } - - if err := cl.Patch(ctx, lc, client.MergeFrom(original)); err != nil { - return errors.NewOperatorError(fmt.Errorf("unable to patch logical cluster annotations: %w", err), true, true) - } - return nil -} From 3f7ce2103d02b6f79b32a7502996375873c1d7d1 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 16 Oct 2025 18:08:44 +0300 Subject: [PATCH 15/35] feat: FGA logic is in a separate subroutine --- internal/controller/initializer_controller.go | 1 + internal/subroutine/workspace_fga.go | 154 ++++++++++++++++++ internal/subroutine/workspace_initializer.go | 34 ++-- 3 files changed, 178 insertions(+), 11 deletions(-) create mode 100644 internal/subroutine/workspace_fga.go diff --git a/internal/controller/initializer_controller.go b/internal/controller/initializer_controller.go index 4d15fd3..23aff55 100644 --- a/internal/controller/initializer_controller.go +++ b/internal/controller/initializer_controller.go @@ -32,6 +32,7 @@ func NewLogicalClusterReconciler(log *logger.Logger, orgClient client.Client, cf log: log, lifecycle: builder.NewBuilder("logicalcluster", "LogicalClusterReconciler", []lifecyclesubroutine.Subroutine{ subroutine.NewWorkspaceInitializer(orgClient, cfg, mgr, fga), + subroutine.NewWorkspaceFGASubroutine(orgClient, mgr, fga, cfg.FGA.ObjectType, cfg.FGA.ParentRelation, cfg.FGA.CreatorRelation), subroutine.NewWorkspaceAuthConfigurationSubroutine(orgClient, inClusterClient, cfg), subroutine.NewRealmSubroutine(inClusterClient, &cfg, cfg.BaseDomain), subroutine.NewRemoveInitializer(mgr, cfg.InitializerName), diff --git a/internal/subroutine/workspace_fga.go b/internal/subroutine/workspace_fga.go new file mode 100644 index 0000000..efd6879 --- /dev/null +++ b/internal/subroutine/workspace_fga.go @@ -0,0 +1,154 @@ +package subroutine + +import ( + "context" + "fmt" + "regexp" + "strings" + "time" + + kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" + "github.com/kcp-dev/logicalcluster/v3" + openfgav1 "github.com/openfga/api/proto/openfga/v1" + accountsv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" + "github.com/platform-mesh/account-operator/pkg/subroutines/accountinfo" + "github.com/platform-mesh/golang-commons/controller/lifecycle/runtimeobject" + lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine" + "github.com/platform-mesh/golang-commons/errors" + "github.com/platform-mesh/golang-commons/fga/helpers" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" +) + +type workspaceFGASubroutine struct { + orgsClient client.Client + mgr mcmanager.Manager + fga openfgav1.OpenFGAServiceClient + fgaObjectType string + fgaParentRel string + fgaCreatorRel string +} + +func NewWorkspaceFGASubroutine(orgsClient client.Client, mgr mcmanager.Manager, fga openfgav1.OpenFGAServiceClient, objectType, parentRel, creatorRel string) *workspaceFGASubroutine { + return &workspaceFGASubroutine{ + orgsClient: orgsClient, + mgr: mgr, + fga: fga, + fgaObjectType: objectType, + fgaParentRel: parentRel, + fgaCreatorRel: creatorRel, + } +} + +var _ lifecyclesubroutine.Subroutine = &workspaceFGASubroutine{} + +func (w *workspaceFGASubroutine) GetName() string { return "WorkspaceFGA" } + +func (w *workspaceFGASubroutine) Finalizers(_ runtimeobject.RuntimeObject) []string { return nil } + +func (w *workspaceFGASubroutine) Finalize(ctx context.Context, _ runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { + return ctrl.Result{}, nil +} + +func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { + lc := instance.(*kcpv1alpha1.LogicalCluster) + + clusterRef, err := w.mgr.ClusterFromContext(ctx) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get cluster from context: %w", err), true, false) + } + workspaceClient := clusterRef.GetClient() + + ctxWithTimeout, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + + accountInfo := &accountsv1alpha1.AccountInfo{} + if err := workspaceClient.Get(ctxWithTimeout, client.ObjectKey{Name: accountinfo.DefaultAccountInfoName}, accountInfo); err != nil { + return ctrl.Result{Requeue: true}, nil + } + if accountInfo.Spec.Account.Name == "" || accountInfo.Spec.Account.OriginClusterId == "" || accountInfo.Spec.FGA.Store.Id == "" { + return ctrl.Result{Requeue: true}, nil + } + + // Parent relation for non-org accounts + if accountInfo.Spec.ParentAccount != nil { + parent := accountInfo.Spec.ParentAccount + if err := w.writeTuple(ctxWithTimeout, accountInfo.Spec.FGA.Store.Id, &openfgav1.TupleKey{ + User: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, parent.OriginClusterId, parent.Name), + Relation: w.fgaParentRel, + Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, accountInfo.Spec.Account.Name), + }); err != nil { + return ctrl.Result{}, err + } + } + + // Owner/creator relations: fetch the Account to read .Spec.Creator + ownerClusterName := logicalcluster.Name(lc.Spec.Owner.Cluster) + ownerClusterRef, err := w.mgr.GetCluster(ctx, ownerClusterName.String()) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get owner cluster: %w", err), true, true) + } + var account accountsv1alpha1.Account + if err := ownerClusterRef.GetClient().Get(ctxWithTimeout, client.ObjectKey{Name: lc.Spec.Owner.Name}, &account); err != nil { + return ctrl.Result{Requeue: true}, nil + } + if account.Spec.Creator != nil && *account.Spec.Creator != "" { + creator := *account.Spec.Creator + if !validateCreator(creator) { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("creator string is in the protected service account prefix range"), false, false) + } + normalized := formatUser(creator) + if err := w.writeTuple(ctxWithTimeout, accountInfo.Spec.FGA.Store.Id, &openfgav1.TupleKey{ + User: fmt.Sprintf("user:%s", normalized), + Relation: "assignee", + Object: fmt.Sprintf("role:%s/%s/%s/owner", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, accountInfo.Spec.Account.Name), + }); err != nil { + return ctrl.Result{}, err + } + if err := w.writeTuple(ctxWithTimeout, accountInfo.Spec.FGA.Store.Id, &openfgav1.TupleKey{ + User: fmt.Sprintf("role:%s/%s/%s/owner#assignee", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, accountInfo.Spec.Account.Name), + Relation: w.fgaCreatorRel, + Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, accountInfo.Spec.Account.Name), + }); err != nil { + return ctrl.Result{}, err + } + } + + return ctrl.Result{}, nil +} + +func (w *workspaceFGASubroutine) writeTuple(ctx context.Context, storeID string, tuple *openfgav1.TupleKey) errors.OperatorError { + _, err := w.fga.Write(ctx, &openfgav1.WriteRequest{ + StoreId: storeID, + Writes: &openfgav1.WriteRequestWrites{ + TupleKeys: []*openfgav1.TupleKey{tuple}, + }, + }) + if helpers.IsDuplicateWriteError(err) { + return nil + } + if err != nil { + return errors.NewOperatorError(fmt.Errorf("unable to write FGA tuple: %w", err), true, true) + } + return nil +} + +var saRegex = regexp.MustCompile(`^system:serviceaccount:[^:]*:[^:]*$`) + +func formatUser(user string) string { + if saRegex.MatchString(user) { + return strings.ReplaceAll(user, ":", ".") + } + return user +} + +func validateCreator(creator string) bool { + if strings.HasPrefix(creator, "system:serviceaccount:") { + return false + } + if strings.HasPrefix(creator, "system.serviceaccount.") { + return false + } + return true +} diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index be770a4..399f35b 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -92,19 +92,20 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get owner account: %w", err), true, true) } - ctxWithTimeout, cancel := context.WithTimeout(ctx, 5*time.Second) - defer cancel() + // Timeout for AccountInfo retrieval + ctxGetTimeout, cancelGet := context.WithTimeout(ctx, 5*time.Second) + defer cancelGet() accountInfo := &accountsv1alpha1.AccountInfo{} - if err := workspaceClient.Get(ctxWithTimeout, client.ObjectKey{Name: accountinfo.DefaultAccountInfoName}, accountInfo); err != nil { + if err := workspaceClient.Get(ctxGetTimeout, client.ObjectKey{Name: accountinfo.DefaultAccountInfoName}, accountInfo); err != nil { if kerrors.IsNotFound(err) { - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil } return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get accountInfo: %w", err), true, true) } if accountInfo.Spec.Account.Name == "" || accountInfo.Spec.Account.OriginClusterId == "" { - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil } storeClusterName, storeName, opErr := w.resolveStoreTarget(lc, account, accountInfo) @@ -112,7 +113,10 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje return ctrl.Result{}, opErr } - ctxStore := mccontext.WithCluster(ctxWithTimeout, storeClusterName.String()) + // Fresh timeout for store operations + ctxStoreTimeout, cancelStore := context.WithTimeout(ctx, 5*time.Second) + defer cancelStore() + ctxStore := mccontext.WithCluster(ctxStoreTimeout, storeClusterName.String()) allowCreate := account.Spec.Type == accountsv1alpha1.AccountTypeOrg store := &v1alpha1.Store{} @@ -120,14 +124,15 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje if kerrors.IsNotFound(err) && allowCreate { store = &v1alpha1.Store{ObjectMeta: metav1.ObjectMeta{Name: storeName}} } else if kerrors.IsNotFound(err) { - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil } else { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get store: %w", err), true, true) } } _, err = controllerutil.CreateOrUpdate(ctxStore, w.orgsClient, store, func() error { - if allowCreate { + // Backfill CoreModule if missing for org accounts (creation or partial prior init) + if allowCreate && store.Spec.CoreModule == "" { store.Spec.CoreModule = w.coreModule } return nil @@ -141,11 +146,14 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje } if store.Status.StoreID == "" { - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil } if accountInfo.Spec.FGA.Store.Id != store.Status.StoreID { - if opErr := w.ensureAccountInfoStoreID(ctxWithTimeout, workspaceClient, store.Status.StoreID); opErr != nil { + // Fresh timeout for AccountInfo update + ctxUpdateTimeout, cancelUpdate := context.WithTimeout(ctx, 5*time.Second) + defer cancelUpdate() + if opErr := w.ensureAccountInfoStoreID(ctxUpdateTimeout, workspaceClient, store.Status.StoreID); opErr != nil { return ctrl.Result{}, opErr } } @@ -171,7 +179,11 @@ func (w *workspaceInitializer) resolveStoreTarget(lc *kcpv1alpha1.LogicalCluster if !ok { return "", "", errors.NewOperatorError(fmt.Errorf("unable to get workspace path"), true, false) } - return logicalcluster.Name(path), generateStoreName(lc), nil + storeName := generateStoreName(lc) + if storeName == "" { + return "", "", errors.NewOperatorError(fmt.Errorf("unable to generate store name from workspace path"), true, false) + } + return logicalcluster.Name(path), storeName, nil } if accountInfo.Spec.Organization.Path == "" { From 344775c9416205f213506e4a1c80763afc3957e4 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 16 Oct 2025 18:41:38 +0300 Subject: [PATCH 16/35] feat: tests for workspace_fga and some others --- internal/subroutine/store_test.go | 15 + internal/subroutine/tuples_test.go | 66 ++++ .../workspace_authorization_test.go | 17 ++ internal/subroutine/workspace_fga_test.go | 289 ++++++++++++++++++ 4 files changed, 387 insertions(+) create mode 100644 internal/subroutine/workspace_fga_test.go diff --git a/internal/subroutine/store_test.go b/internal/subroutine/store_test.go index 643160a..1487fbd 100644 --- a/internal/subroutine/store_test.go +++ b/internal/subroutine/store_test.go @@ -67,6 +67,21 @@ func TestProcess(t *testing.T) { }, nil) }, }, + { + name: "should fail if get store by ID fails", + store: &v1alpha1.Store{ + ObjectMeta: metav1.ObjectMeta{ + Name: "store", + }, + Status: v1alpha1.StoreStatus{ + StoreID: "id", + }, + }, + fgaMocks: func(fga *mocks.MockOpenFGAServiceClient) { + fga.EXPECT().GetStore(mock.Anything, &openfgav1.GetStoreRequest{StoreId: "id"}).Return(nil, errors.New("boom")) + }, + expectError: true, + }, { name: "should verify the store if .status.storeId is set", store: &v1alpha1.Store{ diff --git a/internal/subroutine/tuples_test.go b/internal/subroutine/tuples_test.go index 4ade6c4..bb68194 100644 --- a/internal/subroutine/tuples_test.go +++ b/internal/subroutine/tuples_test.go @@ -5,6 +5,7 @@ import ( "errors" "testing" + kcpcorev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" "github.com/kcp-dev/logicalcluster/v3" "github.com/platform-mesh/security-operator/api/v1alpha1" "github.com/platform-mesh/security-operator/internal/subroutine" @@ -479,3 +480,68 @@ func TestTupleFinalizationWithStore(t *testing.T) { }) } } + +func TestTupleFinalizationWithAuthorizationModel_Errors(t *testing.T) { + t.Run("cluster from context error", func(t *testing.T) { + fga := mocks.NewMockOpenFGAServiceClient(t) + manager := mocks.NewMockManager(t) + // Simulate failure to get cluster from context + manager.EXPECT().ClusterFromContext(mock.Anything).Return(nil, errors.New("cluster ctx error")) + + sub := subroutine.NewTupleSubroutine(fga, manager) + am := &v1alpha1.AuthorizationModel{} + + ctx := mccontext.WithCluster(context.Background(), string(logicalcluster.Name("a"))) + _, opErr := sub.Finalize(ctx, am) + assert.NotNil(t, opErr) + }) + + t.Run("logicalcluster get error", func(t *testing.T) { + fga := mocks.NewMockOpenFGAServiceClient(t) + manager := mocks.NewMockManager(t) + cluster := mocks.NewMockCluster(t) + k8sClient := mocks.NewMockClient(t) + + manager.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil) + cluster.EXPECT().GetClient().Return(k8sClient) + // First Get tries to fetch the LogicalCluster named "cluster" + k8sClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).Return(errors.New("get lc error")).Once() + + sub := subroutine.NewTupleSubroutine(fga, manager) + am := &v1alpha1.AuthorizationModel{} + ctx := mccontext.WithCluster(context.Background(), string(logicalcluster.Name("a"))) + _, opErr := sub.Finalize(ctx, am) + assert.NotNil(t, opErr) + }) + + t.Run("store get error", func(t *testing.T) { + fga := mocks.NewMockOpenFGAServiceClient(t) + manager := mocks.NewMockManager(t) + cluster := mocks.NewMockCluster(t) + k8sClient := mocks.NewMockClient(t) + + // AuthorizationModel with StoreRef set so we try to lookup the store + am := &v1alpha1.AuthorizationModel{} + am.Spec.StoreRef.Name = "store" + am.Spec.StoreRef.Path = "path" + + manager.EXPECT().ClusterFromContext(mock.Anything).Return(cluster, nil) + cluster.EXPECT().GetClient().Return(k8sClient) + // First Get returns LogicalCluster successfully + k8sClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, _ client.ObjectKey, o client.Object, _ ...client.GetOption) error { + // Populate lc annotation used in code path + if lc, ok := o.(*kcpcorev1alpha1.LogicalCluster); ok { + lc.Annotations = map[string]string{"kcp.io/cluster": "path"} + } + return nil + }).Once() + + // Second: attempt to fetch Store from storeCtx must fail + k8sClient.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "store"}, mock.Anything).Return(errors.New("get store error")).Once() + + sub := subroutine.NewTupleSubroutine(fga, manager) + ctx := mccontext.WithCluster(context.Background(), string(logicalcluster.Name("a"))) + _, opErr := sub.Finalize(ctx, am) + assert.NotNil(t, opErr) + }) +} diff --git a/internal/subroutine/workspace_authorization_test.go b/internal/subroutine/workspace_authorization_test.go index a67b16a..d1dcf81 100644 --- a/internal/subroutine/workspace_authorization_test.go +++ b/internal/subroutine/workspace_authorization_test.go @@ -56,6 +56,23 @@ func TestWorkspaceAuthSubroutine_Process(t *testing.T) { expectError: false, expectedResult: ctrl.Result{}, }, + { + name: "error - domain CA lookup enabled but secret get fails", + logicalCluster: &kcpv1alpha1.LogicalCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kcp.io/path": "root:orgs:test-workspace", + }, + }, + }, + cfg: config.Config{BaseDomain: "test.domain", GroupClaim: "groups", UserClaim: "email", DomainCALookup: true}, + setupMocks: func(m *mocks.MockClient) { + m.EXPECT().Get(mock.Anything, types.NamespacedName{Name: "domain-certificate-ca", Namespace: "platform-mesh-system"}, mock.Anything, mock.Anything). + Return(errors.New("secret get failed")).Once() + }, + expectError: true, + expectedResult: ctrl.Result{}, + }, { name: "success - update existing WorkspaceAuthenticationConfiguration", logicalCluster: &kcpv1alpha1.LogicalCluster{ diff --git a/internal/subroutine/workspace_fga_test.go b/internal/subroutine/workspace_fga_test.go new file mode 100644 index 0000000..7ccc10a --- /dev/null +++ b/internal/subroutine/workspace_fga_test.go @@ -0,0 +1,289 @@ +package subroutine_test + +import ( + "context" + "testing" + + kcpcorev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" + "github.com/kcp-dev/logicalcluster/v3" + openfgav1 "github.com/openfga/api/proto/openfga/v1" + accountsv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" + "github.com/platform-mesh/security-operator/internal/subroutine" + "github.com/platform-mesh/security-operator/internal/subroutine/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + mccontext "sigs.k8s.io/multicluster-runtime/pkg/context" +) + +func TestWorkspaceFGA_Requeue_WhenAccountInfoMissing(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + ctx := mccontext.WithCluster(context.Background(), "ws") + res, opErr := sub.Process(ctx, lc) + assert.Nil(t, opErr) + assert.True(t, res.Requeue) +} + +func TestWorkspaceFGA_Requeue_WhenAccountInfoIncomplete(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "" // missing + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + ctx := mccontext.WithCluster(context.Background(), "ws") + res, opErr := sub.Process(ctx, lc) + assert.Nil(t, opErr) + assert.True(t, res.Requeue) +} + +func TestWorkspaceFGA_WritesParentAndOwnerTuples(t *testing.T) { + mgr := mocks.NewMockManager(t) + + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.ParentAccount = &accountsv1alpha1.AccountLocation{Name: "org", OriginClusterId: "root:orgs"} + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + ownerCluster := mocks.NewMockCluster(t) + ownerClient := mocks.NewMockClient(t) + ownerClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + a := obj.(*accountsv1alpha1.Account) + creator := "user@example.com" + a.Spec.Creator = &creator + return nil + }, + ) + ownerCluster.EXPECT().GetClient().Return(ownerClient) + mgr.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(ownerCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Times(3) + + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} + lc.Spec.Owner.Cluster = logicalcluster.Name("ws-owner").String() + lc.Spec.Owner.Name = "acc" + ctx := mccontext.WithCluster(context.Background(), "ws") + res, opErr := sub.Process(ctx, lc) + assert.Nil(t, opErr) + assert.False(t, res.Requeue) +} + +func TestWorkspaceFGA_InvalidCreator_ReturnsError(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + ownerCluster := mocks.NewMockCluster(t) + ownerClient := mocks.NewMockClient(t) + ownerClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + a := obj.(*accountsv1alpha1.Account) + creator := "system:serviceaccount:ns:name" + a.Spec.Creator = &creator + return nil + }, + ) + ownerCluster.EXPECT().GetClient().Return(ownerClient) + mgr.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(ownerCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} + lc.Spec.Owner.Cluster = logicalcluster.Name("ws-owner").String() + lc.Spec.Owner.Name = "acc" + ctx := mccontext.WithCluster(context.Background(), "ws") + _, opErr := sub.Process(ctx, lc) + assert.NotNil(t, opErr) +} + +func TestWorkspaceFGA_OwnerAccountGetError_Requeues(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + ownerCluster := mocks.NewMockCluster(t) + ownerClient := mocks.NewMockClient(t) + ownerClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError) + ownerCluster.EXPECT().GetClient().Return(ownerClient) + mgr.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(ownerCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} + lc.Spec.Owner.Cluster = logicalcluster.Name("ws-owner").String() + lc.Spec.Owner.Name = "acc" + ctx := mccontext.WithCluster(context.Background(), "ws") + res, opErr := sub.Process(ctx, lc) + assert.Nil(t, opErr) + assert.True(t, res.Requeue) +} + +func TestWorkspaceFGA_GetClusterError_ReturnsOperatorError(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + mgr.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(nil, assert.AnError) + + fga := mocks.NewMockOpenFGAServiceClient(t) + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} + lc.Spec.Owner.Cluster = logicalcluster.Name("ws-owner").String() + lc.Spec.Owner.Name = "acc" + ctx := mccontext.WithCluster(context.Background(), "ws") + _, opErr := sub.Process(ctx, lc) + assert.NotNil(t, opErr) +} + +func TestWorkspaceFGA_OnlyParentTuple_WhenNoCreator(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.ParentAccount = &accountsv1alpha1.AccountLocation{Name: "org", OriginClusterId: "root:orgs"} + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + ownerCluster := mocks.NewMockCluster(t) + ownerClient := mocks.NewMockClient(t) + ownerClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + a := obj.(*accountsv1alpha1.Account) + a.Spec.Creator = nil // no creator set + return nil + }, + ) + ownerCluster.EXPECT().GetClient().Return(ownerClient) + mgr.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(ownerCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() + + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} + lc.Spec.Owner.Cluster = logicalcluster.Name("ws-owner").String() + lc.Spec.Owner.Name = "acc" + ctx := mccontext.WithCluster(context.Background(), "ws") + res, opErr := sub.Process(ctx, lc) + assert.Nil(t, opErr) + assert.False(t, res.Requeue) +} + +func TestWorkspaceFGA_WriteTupleError_Propagates(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.ParentAccount = &accountsv1alpha1.AccountLocation{Name: "org", OriginClusterId: "root:orgs"} + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() + + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} + lc.Spec.Owner.Cluster = logicalcluster.Name("ws-owner").String() + lc.Spec.Owner.Name = "acc" + ctx := mccontext.WithCluster(context.Background(), "ws") + _, opErr := sub.Process(ctx, lc) + assert.NotNil(t, opErr) +} From bd6958f45ab9be8595f14ef3fd73dfa68f1ad1d4 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 16 Oct 2025 19:11:31 +0300 Subject: [PATCH 17/35] fix: Replaced deprecated Requeue with RequeueAfter; Creator tuple written only once --- internal/subroutine/workspace_fga.go | 28 +++--- internal/subroutine/workspace_fga_test.go | 113 ++++++++++------------ 2 files changed, 65 insertions(+), 76 deletions(-) diff --git a/internal/subroutine/workspace_fga.go b/internal/subroutine/workspace_fga.go index efd6879..480150e 100644 --- a/internal/subroutine/workspace_fga.go +++ b/internal/subroutine/workspace_fga.go @@ -8,7 +8,6 @@ import ( "time" kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" - "github.com/kcp-dev/logicalcluster/v3" openfgav1 "github.com/openfga/api/proto/openfga/v1" accountsv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" "github.com/platform-mesh/account-operator/pkg/subroutines/accountinfo" @@ -52,7 +51,7 @@ func (w *workspaceFGASubroutine) Finalize(ctx context.Context, _ runtimeobject.R } func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { - lc := instance.(*kcpv1alpha1.LogicalCluster) + _ = instance.(*kcpv1alpha1.LogicalCluster) clusterRef, err := w.mgr.ClusterFromContext(ctx) if err != nil { @@ -65,10 +64,10 @@ func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeob accountInfo := &accountsv1alpha1.AccountInfo{} if err := workspaceClient.Get(ctxWithTimeout, client.ObjectKey{Name: accountinfo.DefaultAccountInfoName}, accountInfo); err != nil { - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil } if accountInfo.Spec.Account.Name == "" || accountInfo.Spec.Account.OriginClusterId == "" || accountInfo.Spec.FGA.Store.Id == "" { - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil } // Parent relation for non-org accounts @@ -83,18 +82,9 @@ func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeob } } - // Owner/creator relations: fetch the Account to read .Spec.Creator - ownerClusterName := logicalcluster.Name(lc.Spec.Owner.Cluster) - ownerClusterRef, err := w.mgr.GetCluster(ctx, ownerClusterName.String()) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get owner cluster: %w", err), true, true) - } - var account accountsv1alpha1.Account - if err := ownerClusterRef.GetClient().Get(ctxWithTimeout, client.ObjectKey{Name: lc.Spec.Owner.Name}, &account); err != nil { - return ctrl.Result{Requeue: true}, nil - } - if account.Spec.Creator != nil && *account.Spec.Creator != "" { - creator := *account.Spec.Creator + // Owner/creator relations: write only once using creator from AccountInfo + if accountInfo.Spec.Creator != nil && *accountInfo.Spec.Creator != "" && !accountInfo.Status.CreatorTupleWritten { + creator := *accountInfo.Spec.Creator if !validateCreator(creator) { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("creator string is in the protected service account prefix range"), false, false) } @@ -113,6 +103,12 @@ func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeob }); err != nil { return ctrl.Result{}, err } + + // Mark creator tuple as written + accountInfo.Status.CreatorTupleWritten = true + if err := workspaceClient.Status().Update(ctxWithTimeout, accountInfo); err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to update accountInfo status: %w", err), true, true) + } } return ctrl.Result{}, nil diff --git a/internal/subroutine/workspace_fga_test.go b/internal/subroutine/workspace_fga_test.go index 7ccc10a..64ac3c3 100644 --- a/internal/subroutine/workspace_fga_test.go +++ b/internal/subroutine/workspace_fga_test.go @@ -33,7 +33,7 @@ func TestWorkspaceFGA_Requeue_WhenAccountInfoMissing(t *testing.T) { ctx := mccontext.WithCluster(context.Background(), "ws") res, opErr := sub.Process(ctx, lc) assert.Nil(t, opErr) - assert.True(t, res.Requeue) + assert.True(t, res.RequeueAfter > 0, "Expected requeue with delay") } func TestWorkspaceFGA_Requeue_WhenAccountInfoIncomplete(t *testing.T) { @@ -58,7 +58,7 @@ func TestWorkspaceFGA_Requeue_WhenAccountInfoIncomplete(t *testing.T) { ctx := mccontext.WithCluster(context.Background(), "ws") res, opErr := sub.Process(ctx, lc) assert.Nil(t, opErr) - assert.True(t, res.Requeue) + assert.True(t, res.RequeueAfter > 0, "Expected requeue with delay") } func TestWorkspaceFGA_WritesParentAndOwnerTuples(t *testing.T) { @@ -66,6 +66,10 @@ func TestWorkspaceFGA_WritesParentAndOwnerTuples(t *testing.T) { wsCluster := mocks.NewMockCluster(t) wsClient := mocks.NewMockClient(t) + wsStatusWriter := mocks.NewMockStatusWriter(t) + creator := "user@example.com" + + // Mock Get for AccountInfo wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { ai := obj.(*accountsv1alpha1.AccountInfo) @@ -73,25 +77,23 @@ func TestWorkspaceFGA_WritesParentAndOwnerTuples(t *testing.T) { ai.Spec.Account.OriginClusterId = "root:orgs" ai.Spec.FGA.Store.Id = "store-1" ai.Spec.ParentAccount = &accountsv1alpha1.AccountLocation{Name: "org", OriginClusterId: "root:orgs"} + ai.Spec.Creator = &creator + ai.Status.CreatorTupleWritten = false return nil }, ) + + // Mock Status().Update() + wsClient.EXPECT().Status().Return(wsStatusWriter).Once() + wsStatusWriter.EXPECT().Update(mock.Anything, mock.MatchedBy(func(obj client.Object) bool { + ai := obj.(*accountsv1alpha1.AccountInfo) + assert.True(t, ai.Status.CreatorTupleWritten) + return true + }), mock.Anything).Return(nil).Once() + wsCluster.EXPECT().GetClient().Return(wsClient) mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) - ownerCluster := mocks.NewMockCluster(t) - ownerClient := mocks.NewMockClient(t) - ownerClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { - a := obj.(*accountsv1alpha1.Account) - creator := "user@example.com" - a.Spec.Creator = &creator - return nil - }, - ) - ownerCluster.EXPECT().GetClient().Return(ownerClient) - mgr.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(ownerCluster, nil) - fga := mocks.NewMockOpenFGAServiceClient(t) fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Times(3) @@ -104,38 +106,28 @@ func TestWorkspaceFGA_WritesParentAndOwnerTuples(t *testing.T) { ctx := mccontext.WithCluster(context.Background(), "ws") res, opErr := sub.Process(ctx, lc) assert.Nil(t, opErr) - assert.False(t, res.Requeue) + assert.Equal(t, int64(0), res.RequeueAfter.Nanoseconds(), "Expected no requeue") } func TestWorkspaceFGA_InvalidCreator_ReturnsError(t *testing.T) { mgr := mocks.NewMockManager(t) wsCluster := mocks.NewMockCluster(t) wsClient := mocks.NewMockClient(t) + creator := "system:serviceaccount:ns:name" wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { ai := obj.(*accountsv1alpha1.AccountInfo) ai.Spec.Account.Name = "acc" ai.Spec.Account.OriginClusterId = "root:orgs" ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.Creator = &creator + ai.Status.CreatorTupleWritten = false return nil }, ) wsCluster.EXPECT().GetClient().Return(wsClient) mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) - ownerCluster := mocks.NewMockCluster(t) - ownerClient := mocks.NewMockClient(t) - ownerClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { - a := obj.(*accountsv1alpha1.Account) - creator := "system:serviceaccount:ns:name" - a.Spec.Creator = &creator - return nil - }, - ) - ownerCluster.EXPECT().GetClient().Return(ownerClient) - mgr.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(ownerCluster, nil) - fga := mocks.NewMockOpenFGAServiceClient(t) sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") @@ -148,7 +140,7 @@ func TestWorkspaceFGA_InvalidCreator_ReturnsError(t *testing.T) { assert.NotNil(t, opErr) } -func TestWorkspaceFGA_OwnerAccountGetError_Requeues(t *testing.T) { +func TestWorkspaceFGA_OnlyParentTuple_WhenNoCreator(t *testing.T) { mgr := mocks.NewMockManager(t) wsCluster := mocks.NewMockCluster(t) wsClient := mocks.NewMockClient(t) @@ -158,19 +150,17 @@ func TestWorkspaceFGA_OwnerAccountGetError_Requeues(t *testing.T) { ai.Spec.Account.Name = "acc" ai.Spec.Account.OriginClusterId = "root:orgs" ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.ParentAccount = &accountsv1alpha1.AccountLocation{Name: "org", OriginClusterId: "root:orgs"} + ai.Spec.Creator = nil // no creator set return nil }, ) wsCluster.EXPECT().GetClient().Return(wsClient) mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) - ownerCluster := mocks.NewMockCluster(t) - ownerClient := mocks.NewMockClient(t) - ownerClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError) - ownerCluster.EXPECT().GetClient().Return(ownerClient) - mgr.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(ownerCluster, nil) - fga := mocks.NewMockOpenFGAServiceClient(t) + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} @@ -180,28 +170,34 @@ func TestWorkspaceFGA_OwnerAccountGetError_Requeues(t *testing.T) { ctx := mccontext.WithCluster(context.Background(), "ws") res, opErr := sub.Process(ctx, lc) assert.Nil(t, opErr) - assert.True(t, res.Requeue) + assert.Equal(t, int64(0), res.RequeueAfter.Nanoseconds(), "Expected no requeue") } -func TestWorkspaceFGA_GetClusterError_ReturnsOperatorError(t *testing.T) { +func TestWorkspaceFGA_SkipsCreatorTuple_WhenAlreadyWritten(t *testing.T) { mgr := mocks.NewMockManager(t) wsCluster := mocks.NewMockCluster(t) wsClient := mocks.NewMockClient(t) + creator := "user@example.com" + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { ai := obj.(*accountsv1alpha1.AccountInfo) ai.Spec.Account.Name = "acc" ai.Spec.Account.OriginClusterId = "root:orgs" ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.ParentAccount = &accountsv1alpha1.AccountLocation{Name: "org", OriginClusterId: "root:orgs"} + ai.Spec.Creator = &creator + ai.Status.CreatorTupleWritten = true // Already written return nil }, ) wsCluster.EXPECT().GetClient().Return(wsClient) mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) - mgr.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(nil, assert.AnError) - fga := mocks.NewMockOpenFGAServiceClient(t) + // Only one write for parent tuple, no creator tuples + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} @@ -209,52 +205,49 @@ func TestWorkspaceFGA_GetClusterError_ReturnsOperatorError(t *testing.T) { lc.Spec.Owner.Cluster = logicalcluster.Name("ws-owner").String() lc.Spec.Owner.Name = "acc" ctx := mccontext.WithCluster(context.Background(), "ws") - _, opErr := sub.Process(ctx, lc) - assert.NotNil(t, opErr) + res, opErr := sub.Process(ctx, lc) + assert.Nil(t, opErr) + assert.Equal(t, int64(0), res.RequeueAfter.Nanoseconds(), "Expected no requeue") } -func TestWorkspaceFGA_OnlyParentTuple_WhenNoCreator(t *testing.T) { +func TestWorkspaceFGA_NoParentTuple_ForOrgAccount(t *testing.T) { mgr := mocks.NewMockManager(t) wsCluster := mocks.NewMockCluster(t) wsClient := mocks.NewMockClient(t) + creator := "user@example.com" + wsStatusWriter := mocks.NewMockStatusWriter(t) + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { ai := obj.(*accountsv1alpha1.AccountInfo) - ai.Spec.Account.Name = "acc" + ai.Spec.Account.Name = "org" ai.Spec.Account.OriginClusterId = "root:orgs" ai.Spec.FGA.Store.Id = "store-1" - ai.Spec.ParentAccount = &accountsv1alpha1.AccountLocation{Name: "org", OriginClusterId: "root:orgs"} + ai.Spec.ParentAccount = nil // Org accounts don't have parent + ai.Spec.Creator = &creator + ai.Status.CreatorTupleWritten = false return nil }, ) + wsClient.EXPECT().Status().Return(wsStatusWriter).Once() + wsStatusWriter.EXPECT().Update(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() wsCluster.EXPECT().GetClient().Return(wsClient) mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) - ownerCluster := mocks.NewMockCluster(t) - ownerClient := mocks.NewMockClient(t) - ownerClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( - func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { - a := obj.(*accountsv1alpha1.Account) - a.Spec.Creator = nil // no creator set - return nil - }, - ) - ownerCluster.EXPECT().GetClient().Return(ownerClient) - mgr.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(ownerCluster, nil) - fga := mocks.NewMockOpenFGAServiceClient(t) - fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() + // Only two writes for creator tuples, no parent tuple + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Times(2) sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} lc.Spec.Owner.Cluster = logicalcluster.Name("ws-owner").String() - lc.Spec.Owner.Name = "acc" + lc.Spec.Owner.Name = "org" ctx := mccontext.WithCluster(context.Background(), "ws") res, opErr := sub.Process(ctx, lc) assert.Nil(t, opErr) - assert.False(t, res.Requeue) + assert.Equal(t, int64(0), res.RequeueAfter.Nanoseconds(), "Expected no requeue") } func TestWorkspaceFGA_WriteTupleError_Propagates(t *testing.T) { From d044b7649fdd453e79724305b82ac697e1dd775b Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 16 Oct 2025 19:13:53 +0300 Subject: [PATCH 18/35] fix: gPRC init, KCP API calls, Creator tulpe improvements --- cmd/initializer.go | 4 +- go.mod | 2 + go.sum | 2 - .../subroutine/mocks/mock_StatusWriter.go | 107 ++++++++++++++++++ internal/subroutine/workspace_initializer.go | 7 +- 5 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 internal/subroutine/mocks/mock_StatusWriter.go diff --git a/cmd/initializer.go b/cmd/initializer.go index 1583236..dff369e 100644 --- a/cmd/initializer.go +++ b/cmd/initializer.go @@ -106,10 +106,10 @@ var initializerCmd = &cobra.Command{ os.Exit(1) } - // Transport: mTLS is handled by the service mesh (Istio). + // gRPC client initialization: mTLS is handled by the service mesh (Istio) conn, err := grpc.NewClient(initializerCfg.FGA.Target, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { - log.Error().Err(err).Msg("unable to create OpenFGA client connection") + log.Error().Err(err).Msg("unable to create OpenFGA gRPC client") os.Exit(1) } defer func() { _ = conn.Close() }() diff --git a/go.mod b/go.mod index a382b72..22b9e39 100644 --- a/go.mod +++ b/go.mod @@ -126,3 +126,5 @@ require ( sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.0 // indirect ) + +replace github.com/platform-mesh/account-operator => ../account-operator diff --git a/go.sum b/go.sum index 02d57fb..4f80988 100644 --- a/go.sum +++ b/go.sum @@ -157,8 +157,6 @@ github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4 github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= -github.com/platform-mesh/account-operator v0.5.3 h1:RjV6gRipWLeYX8YmIZl5aEAa8Y+9Sw4xb0p4r3oSD/8= -github.com/platform-mesh/account-operator v0.5.3/go.mod h1:zcEtaflOGFdc/tYWbPthnKpfCyZJMiKCln7Ns5u8mCU= github.com/platform-mesh/golang-commons v0.6.3 h1:YkCUbBKeOdBNA/va/M+IueE02bRudUhGdHy7De7vhOc= github.com/platform-mesh/golang-commons v0.6.3/go.mod h1:bQLKn7KeZZdHJ+WilZPpJ+Bs4PE6FkMb8pkNL7bp2Yk= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= diff --git a/internal/subroutine/mocks/mock_StatusWriter.go b/internal/subroutine/mocks/mock_StatusWriter.go new file mode 100644 index 0000000..875c31a --- /dev/null +++ b/internal/subroutine/mocks/mock_StatusWriter.go @@ -0,0 +1,107 @@ +// Code generated manually for testing purposes. + +package mocks + +import ( + "context" + + "github.com/stretchr/testify/mock" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// MockStatusWriter is a mock implementation of client.StatusWriter +type MockStatusWriter struct { + mock.Mock +} + +func NewMockStatusWriter(t interface { + mock.TestingT + Cleanup(func()) +}) *MockStatusWriter { + m := &MockStatusWriter{} + m.Mock.Test(t) + t.Cleanup(func() { m.AssertExpectations(t) }) + return m +} + +func (m *MockStatusWriter) EXPECT() *MockStatusWriter_Expecter { + return &MockStatusWriter_Expecter{mock: &m.Mock} +} + +type MockStatusWriter_Expecter struct { + mock *mock.Mock +} + +func (m *MockStatusWriter) Create(ctx context.Context, obj client.Object, subResource client.Object, opts ...client.SubResourceCreateOption) error { + args := m.Called(ctx, obj, subResource, opts) + return args.Error(0) +} + +func (m *MockStatusWriter_Expecter) Create(ctx interface{}, obj interface{}, subResource interface{}, opts ...interface{}) *MockStatusWriter_Create_Call { + return &MockStatusWriter_Create_Call{Call: m.mock.On("Create", ctx, obj, subResource, opts)} +} + +type MockStatusWriter_Create_Call struct { + *mock.Call +} + +func (c *MockStatusWriter_Create_Call) Return(_a0 error) *MockStatusWriter_Create_Call { + c.Call.Return(_a0) + return c +} + +func (c *MockStatusWriter_Create_Call) RunAndReturn(run func(context.Context, client.Object, client.Object, ...client.SubResourceCreateOption) error) *MockStatusWriter_Create_Call { + c.Call.Return(run) + return c +} + +func (m *MockStatusWriter) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + args := m.Called(ctx, obj, opts) + return args.Error(0) +} + +func (m *MockStatusWriter_Expecter) Update(ctx interface{}, obj interface{}, opts ...interface{}) *MockStatusWriter_Update_Call { + return &MockStatusWriter_Update_Call{Call: m.mock.On("Update", append([]interface{}{ctx, obj}, opts...)...)} +} + +type MockStatusWriter_Update_Call struct { + *mock.Call +} + +func (c *MockStatusWriter_Update_Call) Return(_a0 error) *MockStatusWriter_Update_Call { + c.Call.Return(_a0) + return c +} + +func (c *MockStatusWriter_Update_Call) RunAndReturn(run func(context.Context, client.Object, ...client.SubResourceUpdateOption) error) *MockStatusWriter_Update_Call { + c.Call.Return(run) + return c +} + +func (c *MockStatusWriter_Update_Call) Once() *MockStatusWriter_Update_Call { + c.Call.Once() + return c +} + +func (m *MockStatusWriter) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { + args := m.Called(ctx, obj, patch, opts) + return args.Error(0) +} + +func (m *MockStatusWriter_Expecter) Patch(ctx interface{}, obj interface{}, patch interface{}, opts ...interface{}) *MockStatusWriter_Patch_Call { + return &MockStatusWriter_Patch_Call{Call: m.mock.On("Patch", ctx, obj, patch, opts)} +} + +type MockStatusWriter_Patch_Call struct { + *mock.Call +} + +func (c *MockStatusWriter_Patch_Call) Return(_a0 error) *MockStatusWriter_Patch_Call { + c.Call.Return(_a0) + return c +} + +func (c *MockStatusWriter_Patch_Call) RunAndReturn(run func(context.Context, client.Object, client.Patch, ...client.SubResourcePatchOption) error) *MockStatusWriter_Patch_Call { + c.Call.Return(run) + return c +} diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index 399f35b..c49a3e7 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -149,11 +149,11 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje return ctrl.Result{RequeueAfter: 2 * time.Second}, nil } - if accountInfo.Spec.FGA.Store.Id != store.Status.StoreID { + if accountInfo.Spec.FGA.Store.Id != store.Status.StoreID || accountInfo.Spec.Creator != account.Spec.Creator { // Fresh timeout for AccountInfo update ctxUpdateTimeout, cancelUpdate := context.WithTimeout(ctx, 5*time.Second) defer cancelUpdate() - if opErr := w.ensureAccountInfoStoreID(ctxUpdateTimeout, workspaceClient, store.Status.StoreID); opErr != nil { + if opErr := w.ensureAccountInfo(ctxUpdateTimeout, workspaceClient, store.Status.StoreID, account.Spec.Creator); opErr != nil { return ctrl.Result{}, opErr } } @@ -161,10 +161,11 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje return ctrl.Result{}, nil } -func (w *workspaceInitializer) ensureAccountInfoStoreID(ctx context.Context, workspaceClient client.Client, storeID string) errors.OperatorError { +func (w *workspaceInitializer) ensureAccountInfo(ctx context.Context, workspaceClient client.Client, storeID string, creator *string) errors.OperatorError { accountInfo := &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} _, err := controllerutil.CreateOrUpdate(ctx, workspaceClient, accountInfo, func() error { accountInfo.Spec.FGA.Store.Id = storeID + accountInfo.Spec.Creator = creator return nil }) if err != nil { From 25e8d84e7151fa46e248fa50f1c8d7aff894a6cd Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Fri, 17 Oct 2025 16:04:40 +0300 Subject: [PATCH 19/35] feat: removed local replace directive and use pseudo-version of account-operator for this branch --- go.mod | 4 +--- go.sum | 2 ++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 22b9e39..7bc86a5 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/kcp-dev/multicluster-provider v0.2.0 github.com/openfga/api/proto v0.0.0-20250909173124-0ac19aac54f2 github.com/openfga/language/pkg/go v0.2.0-beta.2.0.20251003203216-7c0d09a1cc5a - github.com/platform-mesh/account-operator v0.5.3 + github.com/platform-mesh/account-operator v0.5.5-0.20251017120838-b8c73d0b347e github.com/platform-mesh/golang-commons v0.6.3 github.com/rs/zerolog v1.34.0 github.com/spf13/cobra v1.10.1 @@ -126,5 +126,3 @@ require ( sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.0 // indirect ) - -replace github.com/platform-mesh/account-operator => ../account-operator diff --git a/go.sum b/go.sum index 4f80988..2f93f11 100644 --- a/go.sum +++ b/go.sum @@ -157,6 +157,8 @@ github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4 github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/platform-mesh/account-operator v0.5.5-0.20251017120838-b8c73d0b347e h1:eG3p5jn92yKjXBfkU4E+t/qvfeqZNnYvjwlX3aFknyc= +github.com/platform-mesh/account-operator v0.5.5-0.20251017120838-b8c73d0b347e/go.mod h1:Kg2hxWCRggF86d9zrjhHGvEo42B+sfL+Go2PVBs9uvo= github.com/platform-mesh/golang-commons v0.6.3 h1:YkCUbBKeOdBNA/va/M+IueE02bRudUhGdHy7De7vhOc= github.com/platform-mesh/golang-commons v0.6.3/go.mod h1:bQLKn7KeZZdHJ+WilZPpJ+Bs4PE6FkMb8pkNL7bp2Yk= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= From 65edbc4c866e61832c91e4c3ed696c9a6ecc3b34 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Fri, 17 Oct 2025 17:17:56 +0300 Subject: [PATCH 20/35] feat: new fga tests --- .../subroutine/mocks/mock_StatusWriter.go | 2 +- internal/subroutine/workspace_fga.go | 4 +- internal/subroutine/workspace_fga_test.go | 270 ++++++++++++++++++ 3 files changed, 273 insertions(+), 3 deletions(-) diff --git a/internal/subroutine/mocks/mock_StatusWriter.go b/internal/subroutine/mocks/mock_StatusWriter.go index 875c31a..a82be26 100644 --- a/internal/subroutine/mocks/mock_StatusWriter.go +++ b/internal/subroutine/mocks/mock_StatusWriter.go @@ -19,7 +19,7 @@ func NewMockStatusWriter(t interface { Cleanup(func()) }) *MockStatusWriter { m := &MockStatusWriter{} - m.Mock.Test(t) + m.Test(t) t.Cleanup(func() { m.AssertExpectations(t) }) return m } diff --git a/internal/subroutine/workspace_fga.go b/internal/subroutine/workspace_fga.go index 480150e..bfaf5f4 100644 --- a/internal/subroutine/workspace_fga.go +++ b/internal/subroutine/workspace_fga.go @@ -85,10 +85,10 @@ func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeob // Owner/creator relations: write only once using creator from AccountInfo if accountInfo.Spec.Creator != nil && *accountInfo.Spec.Creator != "" && !accountInfo.Status.CreatorTupleWritten { creator := *accountInfo.Spec.Creator - if !validateCreator(creator) { + normalized := formatUser(creator) + if !validateCreator(normalized) { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("creator string is in the protected service account prefix range"), false, false) } - normalized := formatUser(creator) if err := w.writeTuple(ctxWithTimeout, accountInfo.Spec.FGA.Store.Id, &openfgav1.TupleKey{ User: fmt.Sprintf("user:%s", normalized), Relation: "assignee", diff --git a/internal/subroutine/workspace_fga_test.go b/internal/subroutine/workspace_fga_test.go index 64ac3c3..7412cb1 100644 --- a/internal/subroutine/workspace_fga_test.go +++ b/internal/subroutine/workspace_fga_test.go @@ -280,3 +280,273 @@ func TestWorkspaceFGA_WriteTupleError_Propagates(t *testing.T) { _, opErr := sub.Process(ctx, lc) assert.NotNil(t, opErr) } + +func TestWorkspaceFGA_ClusterFromContextError_ReturnsError(t *testing.T) { + mgr := mocks.NewMockManager(t) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(nil, assert.AnError) + + fga := mocks.NewMockOpenFGAServiceClient(t) + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + ctx := mccontext.WithCluster(context.Background(), "ws") + _, opErr := sub.Process(ctx, lc) + assert.NotNil(t, opErr) +} + +func TestWorkspaceFGA_EmptyCreator_SkipsCreatorTuples(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + creator := "" + + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.ParentAccount = &accountsv1alpha1.AccountLocation{Name: "org", OriginClusterId: "root:orgs"} + ai.Spec.Creator = &creator // empty string + ai.Status.CreatorTupleWritten = false + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + // Only one write for parent tuple, no creator tuples because creator is empty + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() + + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + ctx := mccontext.WithCluster(context.Background(), "ws") + res, opErr := sub.Process(ctx, lc) + assert.Nil(t, opErr) + assert.Equal(t, int64(0), res.RequeueAfter.Nanoseconds(), "Expected no requeue") +} + +func TestWorkspaceFGA_CreatorAssigneeTupleWriteError_ReturnsError(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + creator := "user@example.com" + + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.Creator = &creator + ai.Status.CreatorTupleWritten = false + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + // First writeTuple call fails (assignee tuple) + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() + + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + ctx := mccontext.WithCluster(context.Background(), "ws") + _, opErr := sub.Process(ctx, lc) + assert.NotNil(t, opErr) +} + +func TestWorkspaceFGA_CreatorOwnerTupleWriteError_ReturnsError(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + creator := "user@example.com" + + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.Creator = &creator + ai.Status.CreatorTupleWritten = false + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + // First writeTuple call succeeds (assignee tuple), second fails (owner tuple) + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() + + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + ctx := mccontext.WithCluster(context.Background(), "ws") + _, opErr := sub.Process(ctx, lc) + assert.NotNil(t, opErr) +} + +func TestWorkspaceFGA_StatusUpdateError_ReturnsError(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + wsStatusWriter := mocks.NewMockStatusWriter(t) + creator := "user@example.com" + + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.Creator = &creator + ai.Status.CreatorTupleWritten = false + return nil + }, + ) + wsClient.EXPECT().Status().Return(wsStatusWriter).Once() + wsStatusWriter.EXPECT().Update(mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError).Once() + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + // Both writeTuple calls succeed + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Times(2) + + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + ctx := mccontext.WithCluster(context.Background(), "ws") + _, opErr := sub.Process(ctx, lc) + assert.NotNil(t, opErr) +} + +func TestWorkspaceFGA_ServiceAccountCreator_FormatsCorrectly(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + wsStatusWriter := mocks.NewMockStatusWriter(t) + // Use a regular user that doesn't match service account pattern for formatUser to return unchanged + creator := "user@example.com" + + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.Creator = &creator + ai.Status.CreatorTupleWritten = false + return nil + }, + ) + wsClient.EXPECT().Status().Return(wsStatusWriter).Once() + wsStatusWriter.EXPECT().Update(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + // Expect the user unchanged since it doesn't match service account pattern + fga.EXPECT().Write(mock.Anything, mock.MatchedBy(func(req *openfgav1.WriteRequest) bool { + if len(req.Writes.TupleKeys) == 0 { + return false + } + tuple := req.Writes.TupleKeys[0] + expectedUser := "user:user@example.com" + return tuple.User == expectedUser + })).Return(&openfgav1.WriteResponse{}, nil).Once() + fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() + + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + ctx := mccontext.WithCluster(context.Background(), "ws") + res, opErr := sub.Process(ctx, lc) + assert.Nil(t, opErr) + assert.Equal(t, int64(0), res.RequeueAfter.Nanoseconds(), "Expected no requeue") +} + +func TestWorkspaceFGA_ValidateCreator_WithDottedServiceAccount_ReturnsError(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + // Use a dotted service account format that should be rejected by validateCreator + creator := "system.serviceaccount.ns.name" + + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.Creator = &creator + ai.Status.CreatorTupleWritten = false + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + ctx := mccontext.WithCluster(context.Background(), "ws") + _, opErr := sub.Process(ctx, lc) + assert.NotNil(t, opErr) +} + +func TestWorkspaceFGA_ServiceAccountFormatted_ThenValidated(t *testing.T) { + mgr := mocks.NewMockManager(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + // Use a system service account that should be formatted from : to . and then rejected + creator := "system:serviceaccount:ns:name" + + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(ctx context.Context, key types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + ai := obj.(*accountsv1alpha1.AccountInfo) + ai.Spec.Account.Name = "acc" + ai.Spec.Account.OriginClusterId = "root:orgs" + ai.Spec.FGA.Store.Id = "store-1" + ai.Spec.Creator = &creator + ai.Status.CreatorTupleWritten = false + return nil + }, + ) + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + fga := mocks.NewMockOpenFGAServiceClient(t) + sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + + lc := &kcpcorev1alpha1.LogicalCluster{} + ctx := mccontext.WithCluster(context.Background(), "ws") + _, opErr := sub.Process(ctx, lc) + // Should get an error because after formatting system:serviceaccount:ns:name becomes system.serviceaccount.ns.name + // which is rejected by validateCreator + assert.NotNil(t, opErr) +} + +func TestWorkspaceFGA_InterfaceMethods(t *testing.T) { + sub := subroutine.NewWorkspaceFGASubroutine(nil, nil, nil, "core_platform-mesh_io_account", "parent", "owner") + + // Test GetName + assert.Equal(t, "WorkspaceFGA", sub.GetName()) + + // Test Finalizers + finalizers := sub.Finalizers(nil) + assert.Nil(t, finalizers) + + // Test Finalize + result, err := sub.Finalize(context.Background(), nil) + assert.Nil(t, err) + assert.Equal(t, int64(0), result.RequeueAfter.Nanoseconds()) +} From acf8f0d26f5658bb9ce8acf3b8252b5436133137 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Fri, 17 Oct 2025 17:39:09 +0300 Subject: [PATCH 21/35] fix: removed unused orgsClient field from workspaceFGASubroutine --- internal/controller/initializer_controller.go | 2 +- internal/subroutine/workspace_fga.go | 4 +-- internal/subroutine/workspace_fga_test.go | 34 +++++++++---------- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/internal/controller/initializer_controller.go b/internal/controller/initializer_controller.go index 23aff55..81e3c9d 100644 --- a/internal/controller/initializer_controller.go +++ b/internal/controller/initializer_controller.go @@ -32,7 +32,7 @@ func NewLogicalClusterReconciler(log *logger.Logger, orgClient client.Client, cf log: log, lifecycle: builder.NewBuilder("logicalcluster", "LogicalClusterReconciler", []lifecyclesubroutine.Subroutine{ subroutine.NewWorkspaceInitializer(orgClient, cfg, mgr, fga), - subroutine.NewWorkspaceFGASubroutine(orgClient, mgr, fga, cfg.FGA.ObjectType, cfg.FGA.ParentRelation, cfg.FGA.CreatorRelation), + subroutine.NewWorkspaceFGASubroutine(mgr, fga, cfg.FGA.ObjectType, cfg.FGA.ParentRelation, cfg.FGA.CreatorRelation), subroutine.NewWorkspaceAuthConfigurationSubroutine(orgClient, inClusterClient, cfg), subroutine.NewRealmSubroutine(inClusterClient, &cfg, cfg.BaseDomain), subroutine.NewRemoveInitializer(mgr, cfg.InitializerName), diff --git a/internal/subroutine/workspace_fga.go b/internal/subroutine/workspace_fga.go index bfaf5f4..f84ce4c 100644 --- a/internal/subroutine/workspace_fga.go +++ b/internal/subroutine/workspace_fga.go @@ -21,7 +21,6 @@ import ( ) type workspaceFGASubroutine struct { - orgsClient client.Client mgr mcmanager.Manager fga openfgav1.OpenFGAServiceClient fgaObjectType string @@ -29,9 +28,8 @@ type workspaceFGASubroutine struct { fgaCreatorRel string } -func NewWorkspaceFGASubroutine(orgsClient client.Client, mgr mcmanager.Manager, fga openfgav1.OpenFGAServiceClient, objectType, parentRel, creatorRel string) *workspaceFGASubroutine { +func NewWorkspaceFGASubroutine(mgr mcmanager.Manager, fga openfgav1.OpenFGAServiceClient, objectType, parentRel, creatorRel string) *workspaceFGASubroutine { return &workspaceFGASubroutine{ - orgsClient: orgsClient, mgr: mgr, fga: fga, fgaObjectType: objectType, diff --git a/internal/subroutine/workspace_fga_test.go b/internal/subroutine/workspace_fga_test.go index 7412cb1..1248e26 100644 --- a/internal/subroutine/workspace_fga_test.go +++ b/internal/subroutine/workspace_fga_test.go @@ -27,7 +27,7 @@ func TestWorkspaceFGA_Requeue_WhenAccountInfoMissing(t *testing.T) { mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) fga := mocks.NewMockOpenFGAServiceClient(t) - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} ctx := mccontext.WithCluster(context.Background(), "ws") @@ -52,7 +52,7 @@ func TestWorkspaceFGA_Requeue_WhenAccountInfoIncomplete(t *testing.T) { mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) fga := mocks.NewMockOpenFGAServiceClient(t) - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} ctx := mccontext.WithCluster(context.Background(), "ws") @@ -97,7 +97,7 @@ func TestWorkspaceFGA_WritesParentAndOwnerTuples(t *testing.T) { fga := mocks.NewMockOpenFGAServiceClient(t) fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Times(3) - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} @@ -129,7 +129,7 @@ func TestWorkspaceFGA_InvalidCreator_ReturnsError(t *testing.T) { mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) fga := mocks.NewMockOpenFGAServiceClient(t) - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} @@ -161,7 +161,7 @@ func TestWorkspaceFGA_OnlyParentTuple_WhenNoCreator(t *testing.T) { fga := mocks.NewMockOpenFGAServiceClient(t) fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} @@ -198,7 +198,7 @@ func TestWorkspaceFGA_SkipsCreatorTuple_WhenAlreadyWritten(t *testing.T) { // Only one write for parent tuple, no creator tuples fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} @@ -238,7 +238,7 @@ func TestWorkspaceFGA_NoParentTuple_ForOrgAccount(t *testing.T) { // Only two writes for creator tuples, no parent tuple fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Times(2) - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} @@ -270,7 +270,7 @@ func TestWorkspaceFGA_WriteTupleError_Propagates(t *testing.T) { fga := mocks.NewMockOpenFGAServiceClient(t) fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{} @@ -286,7 +286,7 @@ func TestWorkspaceFGA_ClusterFromContextError_ReturnsError(t *testing.T) { mgr.EXPECT().ClusterFromContext(mock.Anything).Return(nil, assert.AnError) fga := mocks.NewMockOpenFGAServiceClient(t) - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} ctx := mccontext.WithCluster(context.Background(), "ws") @@ -319,7 +319,7 @@ func TestWorkspaceFGA_EmptyCreator_SkipsCreatorTuples(t *testing.T) { // Only one write for parent tuple, no creator tuples because creator is empty fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} ctx := mccontext.WithCluster(context.Background(), "ws") @@ -352,7 +352,7 @@ func TestWorkspaceFGA_CreatorAssigneeTupleWriteError_ReturnsError(t *testing.T) // First writeTuple call fails (assignee tuple) fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} ctx := mccontext.WithCluster(context.Background(), "ws") @@ -385,7 +385,7 @@ func TestWorkspaceFGA_CreatorOwnerTupleWriteError_ReturnsError(t *testing.T) { fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() fga.EXPECT().Write(mock.Anything, mock.Anything).Return(nil, assert.AnError).Once() - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} ctx := mccontext.WithCluster(context.Background(), "ws") @@ -420,7 +420,7 @@ func TestWorkspaceFGA_StatusUpdateError_ReturnsError(t *testing.T) { // Both writeTuple calls succeed fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Times(2) - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} ctx := mccontext.WithCluster(context.Background(), "ws") @@ -464,7 +464,7 @@ func TestWorkspaceFGA_ServiceAccountCreator_FormatsCorrectly(t *testing.T) { })).Return(&openfgav1.WriteResponse{}, nil).Once() fga.EXPECT().Write(mock.Anything, mock.Anything).Return(&openfgav1.WriteResponse{}, nil).Once() - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} ctx := mccontext.WithCluster(context.Background(), "ws") @@ -495,7 +495,7 @@ func TestWorkspaceFGA_ValidateCreator_WithDottedServiceAccount_ReturnsError(t *t mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) fga := mocks.NewMockOpenFGAServiceClient(t) - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} ctx := mccontext.WithCluster(context.Background(), "ws") @@ -525,7 +525,7 @@ func TestWorkspaceFGA_ServiceAccountFormatted_ThenValidated(t *testing.T) { mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) fga := mocks.NewMockOpenFGAServiceClient(t) - sub := subroutine.NewWorkspaceFGASubroutine(nil, mgr, fga, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(mgr, fga, "core_platform-mesh_io_account", "parent", "owner") lc := &kcpcorev1alpha1.LogicalCluster{} ctx := mccontext.WithCluster(context.Background(), "ws") @@ -536,7 +536,7 @@ func TestWorkspaceFGA_ServiceAccountFormatted_ThenValidated(t *testing.T) { } func TestWorkspaceFGA_InterfaceMethods(t *testing.T) { - sub := subroutine.NewWorkspaceFGASubroutine(nil, nil, nil, "core_platform-mesh_io_account", "parent", "owner") + sub := subroutine.NewWorkspaceFGASubroutine(nil, nil, "core_platform-mesh_io_account", "parent", "owner") // Test GetName assert.Equal(t, "WorkspaceFGA", sub.GetName()) From 27f38dbfd2dd0a12d487d0890be56ad07fc474eb Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Mon, 20 Oct 2025 14:37:29 +0300 Subject: [PATCH 22/35] chore: removed commented lines --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index cba2c6c..1ae0852 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,6 @@ require ( github.com/openfga/api/proto v0.0.0-20250909173124-0ac19aac54f2 github.com/openfga/language/pkg/go v0.2.0-beta.2.0.20251003203216-7c0d09a1cc5a github.com/platform-mesh/account-operator v0.5.5-0.20251017120838-b8c73d0b347e - // TODO: use this line instead of the pseudo version. github.com/platform-mesh/account-operator v0.5.5 github.com/platform-mesh/golang-commons v0.6.4 github.com/rs/zerolog v1.34.0 github.com/spf13/cobra v1.10.1 diff --git a/go.sum b/go.sum index e960964..ad81f7c 100644 --- a/go.sum +++ b/go.sum @@ -159,8 +159,6 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/platform-mesh/account-operator v0.5.5-0.20251017120838-b8c73d0b347e h1:eG3p5jn92yKjXBfkU4E+t/qvfeqZNnYvjwlX3aFknyc= github.com/platform-mesh/account-operator v0.5.5-0.20251017120838-b8c73d0b347e/go.mod h1:Kg2hxWCRggF86d9zrjhHGvEo42B+sfL+Go2PVBs9uvo= -// TODO: use this line instead of the pseudo version. github.com/platform-mesh/account-operator v0.5.5 h1:xCz/GXNRwLG0sruUAWbHJk1r064CN3IeYVmTcrDwYE8= -// TODO: use this line instead of the pseudo version. github.com/platform-mesh/account-operator v0.5.5/go.mod h1:TN8OqDS3XYbskz/ci+SSixc4QJQIMINOZMIFAAoR9qU= github.com/platform-mesh/golang-commons v0.6.4 h1:/IfrO5XOy0dsi0icn7UztP8WfowsliCDBqrkcuYM7RI= github.com/platform-mesh/golang-commons v0.6.4/go.mod h1:fDKAPQFeH2ZzYRDNpFYISptYLpk6KrEp/icfWqL5gW4= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= From 38c75088de63ebd8b6c4d755c3e44148d5a7be12 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Mon, 20 Oct 2025 19:50:23 +0300 Subject: [PATCH 23/35] fix: Validate that owner cluster is specified --- internal/subroutine/workspace_initializer.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index c49a3e7..5bbc67d 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -78,6 +78,13 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje } workspaceClient := clusterRef.GetClient() + // Validate that owner cluster is specified + if lc.Spec.Owner.Cluster == "" { + return ctrl.Result{}, errors.NewOperatorError( + fmt.Errorf("spec.owner.cluster is empty for LogicalCluster %s", lc.Name), + true, true) + } + ownerClusterName := logicalcluster.Name(lc.Spec.Owner.Cluster) ownerClusterRef, err := w.mgr.GetCluster(ctx, ownerClusterName.String()) if err != nil { From b8b1b91b3ee2e6cc8775ab3187aec2af651ab298 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Tue, 21 Oct 2025 16:55:19 +0300 Subject: [PATCH 24/35] feat: move FGA initialization to security-operator initializer --- internal/subroutine/workspace_initializer.go | 125 +++++++++++++----- .../subroutine/workspace_initializer_test.go | 89 +++++++++++++ 2 files changed, 180 insertions(+), 34 deletions(-) create mode 100644 internal/subroutine/workspace_initializer_test.go diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index 5bbc67d..725accf 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -72,6 +72,12 @@ func (w *workspaceInitializer) GetName() string { return "WorkspaceInitializer" func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { lc := instance.(*kcpv1alpha1.LogicalCluster) + // Check if workspace is still initializing + if lc.Status.Phase != kcpv1alpha1.LogicalClusterPhaseInitializing { + fmt.Printf("[DEBUG] Workspace no longer initializing (phase=%s), skipping\n", lc.Status.Phase) + return ctrl.Result{}, nil + } + clusterRef, err := w.mgr.ClusterFromContext(ctx) if err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get cluster from context: %w", err), true, false) @@ -85,61 +91,93 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje true, true) } - ownerClusterName := logicalcluster.Name(lc.Spec.Owner.Cluster) - ownerClusterRef, err := w.mgr.GetCluster(ctx, ownerClusterName.String()) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get owner cluster: %w", err), true, true) - } - + // Use orgsClient directly since lc.Spec.Owner.Cluster contains short cluster ID + // which cannot be resolved via mgr.GetCluster() var account accountsv1alpha1.Account - if err := ownerClusterRef.GetClient().Get(ctx, client.ObjectKey{Name: lc.Spec.Owner.Name}, &account); err != nil { + if err := w.orgsClient.Get(ctx, client.ObjectKey{Name: lc.Spec.Owner.Name}, &account); err != nil { if kerrors.IsNotFound(err) { + fmt.Printf("[DEBUG] Account %s not found yet, requeuing\n", lc.Spec.Owner.Name) return ctrl.Result{Requeue: true}, nil } + fmt.Printf("[ERROR] Failed to get account %s: %v\n", lc.Spec.Owner.Name, err) return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get owner account: %w", err), true, true) } + fmt.Printf("[DEBUG] Successfully fetched account: %s, type: %s\n", account.Name, account.Spec.Type) - // Timeout for AccountInfo retrieval + // Timeout for AccountInfo retrieval/creation ctxGetTimeout, cancelGet := context.WithTimeout(ctx, 5*time.Second) defer cancelGet() - accountInfo := &accountsv1alpha1.AccountInfo{} - if err := workspaceClient.Get(ctxGetTimeout, client.ObjectKey{Name: accountinfo.DefaultAccountInfoName}, accountInfo); err != nil { - if kerrors.IsNotFound(err) { - return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + // Ensure AccountInfo exists (create if missing) so account-operator can populate it + fmt.Printf("[DEBUG] Creating/updating AccountInfo in workspace\n") + accountInfo := &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} + op, err := controllerutil.CreateOrUpdate(ctxGetTimeout, workspaceClient, accountInfo, func() error { + // Set Creator immediately when creating AccountInfo to avoid race with account-operator + if accountInfo.Spec.Creator == nil && account.Spec.Creator != nil { + creatorValue := *account.Spec.Creator + accountInfo.Spec.Creator = &creatorValue + fmt.Printf("[DEBUG] Setting Creator to: %s during AccountInfo creation\n", creatorValue) } - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get accountInfo: %w", err), true, true) + return nil + }) + if op != controllerutil.OperationResultNone { + creatorVal := "" + if accountInfo.Spec.Creator != nil { + creatorVal = *accountInfo.Spec.Creator + } + fmt.Printf("[DEBUG] After CreateOrUpdate (op=%s): Creator=%s\n", op, creatorVal) } + if err != nil { + // If APIBinding not ready yet, return error to retry whole reconcile + if strings.Contains(err.Error(), "no matches for kind") { + fmt.Printf("[DEBUG] CRD not ready yet (no matches for kind), will retry reconcile\n") + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("APIBinding not ready: %w", err), true, false) + } + fmt.Printf("[ERROR] Failed to create/update AccountInfo: %v\n", err) + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to ensure accountInfo exists: %w", err), true, true) + } + fmt.Printf("[DEBUG] AccountInfo operation: %s\n", op) - if accountInfo.Spec.Account.Name == "" || accountInfo.Spec.Account.OriginClusterId == "" { - return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + // Only create Store for org accounts during initialization + // For account-type accounts, Store already exists in parent org + if account.Spec.Type != accountsv1alpha1.AccountTypeOrg { + fmt.Printf("[DEBUG] Account type is '%s', skipping Store creation (using parent org Store)\n", account.Spec.Type) + return ctrl.Result{}, nil } - storeClusterName, storeName, opErr := w.resolveStoreTarget(lc, account, accountInfo) - if opErr != nil { - return ctrl.Result{}, opErr + // Resolve Store name and location for org accounts + fmt.Printf("[DEBUG] Resolving store for org account\n") + path, ok := lc.Annotations["kcp.io/path"] + if !ok { + fmt.Printf("[ERROR] Missing kcp.io/path annotation\n") + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get workspace path"), true, false) } + storeName := generateStoreName(lc) + if storeName == "" { + fmt.Printf("[ERROR] Failed to generate store name from workspace path\n") + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to generate store name from workspace path"), true, false) + } + storeClusterName := logicalcluster.Name(path) + fmt.Printf("[DEBUG] Resolved store: cluster=%s, name=%s\n", storeClusterName, storeName) // Fresh timeout for store operations ctxStoreTimeout, cancelStore := context.WithTimeout(ctx, 5*time.Second) defer cancelStore() ctxStore := mccontext.WithCluster(ctxStoreTimeout, storeClusterName.String()) - allowCreate := account.Spec.Type == accountsv1alpha1.AccountTypeOrg + // Create Store for org account store := &v1alpha1.Store{} if err := w.orgsClient.Get(ctxStore, client.ObjectKey{Name: storeName}, store); err != nil { - if kerrors.IsNotFound(err) && allowCreate { - store = &v1alpha1.Store{ObjectMeta: metav1.ObjectMeta{Name: storeName}} - } else if kerrors.IsNotFound(err) { - return ctrl.Result{RequeueAfter: 2 * time.Second}, nil - } else { + if !kerrors.IsNotFound(err) { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get store: %w", err), true, true) } + // Store doesn't exist, create it + store = &v1alpha1.Store{ObjectMeta: metav1.ObjectMeta{Name: storeName}} } _, err = controllerutil.CreateOrUpdate(ctxStore, w.orgsClient, store, func() error { - // Backfill CoreModule if missing for org accounts (creation or partial prior init) - if allowCreate && store.Spec.CoreModule == "" { + // Set CoreModule for store + if store.Spec.CoreModule == "" { store.Spec.CoreModule = w.coreModule } return nil @@ -148,23 +186,42 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to create/update store: %w", err), true, true) } + // Re-fetch to get store status if err := w.orgsClient.Get(ctxStore, client.ObjectKey{Name: storeName}, store); err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to refresh store status: %w", err), true, true) } if store.Status.StoreID == "" { - return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + fmt.Printf("[DEBUG] Store not ready yet (StoreID empty), will retry reconcile\n") + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("Store not ready"), true, false) } - - if accountInfo.Spec.FGA.Store.Id != store.Status.StoreID || accountInfo.Spec.Creator != account.Spec.Creator { - // Fresh timeout for AccountInfo update - ctxUpdateTimeout, cancelUpdate := context.WithTimeout(ctx, 5*time.Second) - defer cancelUpdate() - if opErr := w.ensureAccountInfo(ctxUpdateTimeout, workspaceClient, store.Status.StoreID, account.Spec.Creator); opErr != nil { - return ctrl.Result{}, opErr + fmt.Printf("[DEBUG] Store ready with ID: %s\n", store.Status.StoreID) + + // Update AccountInfo with Store ID and Creator + fmt.Printf("[DEBUG] Updating AccountInfo with StoreID=%s and Creator\n", store.Status.StoreID) + ctxUpdateTimeout, cancelUpdate := context.WithTimeout(ctx, 5*time.Second) + defer cancelUpdate() + + accountInfo = &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} + _, err = controllerutil.CreateOrUpdate(ctxUpdateTimeout, workspaceClient, accountInfo, func() error { + accountInfo.Spec.FGA.Store.Id = store.Status.StoreID + // Copy creator value (not pointer) to avoid issues with pointer sharing + if account.Spec.Creator != nil { + creatorValue := *account.Spec.Creator + accountInfo.Spec.Creator = &creatorValue + fmt.Printf("[DEBUG] Setting Creator to: %s (from account.Spec.Creator)\n", creatorValue) + } else { + fmt.Printf("[DEBUG] account.Spec.Creator is nil, not setting Creator\n") } + return nil + }) + if err != nil { + fmt.Printf("[ERROR] Failed to update AccountInfo: %v\n", err) + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to update accountInfo: %w", err), true, true) } + fmt.Printf("[DEBUG] AccountInfo updated successfully\n") + fmt.Printf("[SUCCESS] WorkspaceInitializer completed successfully for org workspace\n") return ctrl.Result{}, nil } diff --git a/internal/subroutine/workspace_initializer_test.go b/internal/subroutine/workspace_initializer_test.go new file mode 100644 index 0000000..6d1af3a --- /dev/null +++ b/internal/subroutine/workspace_initializer_test.go @@ -0,0 +1,89 @@ +package subroutine_test + +import ( + "context" + "os" + "testing" + + kcpcorev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" + "github.com/platform-mesh/security-operator/internal/config" + "github.com/platform-mesh/security-operator/internal/subroutine" + "github.com/platform-mesh/security-operator/internal/subroutine/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + mccontext "sigs.k8s.io/multicluster-runtime/pkg/context" +) + +func TestWorkspaceInitializer_ErrorWhenOwnerClusterEmpty(t *testing.T) { + // Create temp file for core module + tmpFile, err := os.CreateTemp("", "coreModule*.fga") + assert.NoError(t, err) + defer os.Remove(tmpFile.Name()) + tmpFile.WriteString("model\n schema 1.1") + tmpFile.Close() + + mgr := mocks.NewMockManager(t) + orgsClient := mocks.NewMockClient(t) + fga := mocks.NewMockOpenFGAServiceClient(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + + cfg := config.Config{ + CoreModulePath: tmpFile.Name(), + } + cfg.FGA.ObjectType = "core_platform-mesh_io_account" + cfg.FGA.ParentRelation = "parent" + cfg.FGA.CreatorRelation = "owner" + + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + + sub := subroutine.NewWorkspaceInitializer(orgsClient, cfg, mgr, fga) + + lc := &kcpcorev1alpha1.LogicalCluster{} + lc.Name = "test-workspace" + lc.Spec.Owner.Cluster = "" // Empty cluster + + ctx := mccontext.WithCluster(context.Background(), "ws") + _, opErr := sub.Process(ctx, lc) + + assert.NotNil(t, opErr, "Expected error when owner.cluster is empty") +} + +func TestWorkspaceInitializer_ErrorWhenOwnerClusterNotFound(t *testing.T) { + // Create temp file for core module + tmpFile, err := os.CreateTemp("", "coreModule*.fga") + assert.NoError(t, err) + defer os.Remove(tmpFile.Name()) + tmpFile.WriteString("model\n schema 1.1") + tmpFile.Close() + + mgr := mocks.NewMockManager(t) + orgsClient := mocks.NewMockClient(t) + fga := mocks.NewMockOpenFGAServiceClient(t) + wsCluster := mocks.NewMockCluster(t) + wsClient := mocks.NewMockClient(t) + + cfg := config.Config{ + CoreModulePath: tmpFile.Name(), + } + cfg.FGA.ObjectType = "core_platform-mesh_io_account" + cfg.FGA.ParentRelation = "parent" + cfg.FGA.CreatorRelation = "owner" + + wsCluster.EXPECT().GetClient().Return(wsClient) + mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) + mgr.EXPECT().GetCluster(mock.Anything, "root:orgs").Return(nil, assert.AnError) + + sub := subroutine.NewWorkspaceInitializer(orgsClient, cfg, mgr, fga) + + lc := &kcpcorev1alpha1.LogicalCluster{} + lc.Name = "test-workspace" + lc.Spec.Owner.Cluster = "root:orgs" + lc.Spec.Owner.Name = "org1" + + ctx := mccontext.WithCluster(context.Background(), "ws") + _, opErr := sub.Process(ctx, lc) + + assert.NotNil(t, opErr, "Expected error when owner cluster not found") +} From bb1b94a0778c5d8e1175f8621850ba72e0a26a6d Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Wed, 22 Oct 2025 21:10:54 +0300 Subject: [PATCH 25/35] fix: Added store populated by accountinfo; write creator fga tuples; fixed apis_kcp_io_apibinding type; store crd definition --- data/coreModule.fga | 88 ++++++++++++++++--- internal/subroutine/authorization_model.go | 7 ++ .../authorization_model_generation.go | 3 + internal/subroutine/workspace_initializer.go | 16 ++-- 4 files changed, 96 insertions(+), 18 deletions(-) diff --git a/data/coreModule.fga b/data/coreModule.fga index d85d09e..cae243b 100644 --- a/data/coreModule.fga +++ b/data/coreModule.fga @@ -6,24 +6,90 @@ type role relations define assignee: [user,user:*] -type account +type core_platform-mesh_io_account relations + define parent: [core_platform-mesh_io_account] - define parent: [account] - define owner: [role#assignee] - define member: [role#assignee] or owner + define owner: [role#assignee] or owner from parent + define member: [role#assignee] or owner or member from parent - define get: member or get from parent - define update: member or update from parent - define delete: owner or delete from parent + define get: member + define update: member + define patch: member + define delete: owner define create_core_platform-mesh_io_accounts: member define list_core_platform-mesh_io_accounts: member define watch_core_platform-mesh_io_accounts: member # org and account specific - define watch: member or watch from parent + define watch: member - # org specific - define create: member or create from parent - define list: member or list from parent \ No newline at end of file + define create_core_namespaces: member + define list_core_namespaces: member + define watch_core_namespaces: member + + define create_core_platform-mesh_io_accountinfos: member + define list_core_platform-mesh_io_accountinfos: member + define watch_core_platform-mesh_io_accountinfos: member + + define create_apis_kcp_io_apibindings: owner + define list__apis_kcp_io_apibindings: member + define watch_apis_kcp_io_apibindings: member + + # IAM specific + define manage_iam_roles: owner + define get_iam_roles: member + define get_iam_users: member + +type core_namespace + relations + define parent: [core_platform-mesh_io_account] + + define member: member from parent + define owner: owner from parent + + define get: member + define watch: member + + define update: member + define patch: member + define delete: member + + # IAM specific + define manage_iam_roles: owner + define get_iam_roles: member + define get_iam_users: member + +type core_platform-mesh_io_accountinfo + relations + define parent: [core_platform-mesh_io_account] + + define member: member from parent + define owner: owner from parent + + define get: member + define watch: member + + # IAM specific + define manage_iam_roles: owner + define get_iam_roles: member + define get_iam_users: member + +type apis_kcp_io_apibinding + relations + define parent: [core_platform-mesh_io_account] + + define member: member from parent + define owner: owner from parent + + define get: member + define watch: member + define update: owner + define patch: owner + define delete: owner + + # IAM specific + define manage_iam_roles: owner + define get_iam_roles: member + define get_iam_users: member diff --git a/internal/subroutine/authorization_model.go b/internal/subroutine/authorization_model.go index bccf876..274c097 100644 --- a/internal/subroutine/authorization_model.go +++ b/internal/subroutine/authorization_model.go @@ -141,6 +141,13 @@ func (a *authorizationModelSubroutine) Process(ctx context.Context, instance run } + // DEBUG: Log the model being written + modelJSON, _ := protojson.Marshal(authorizationModel) + modelDSL, err := language.TransformJSONStringToDSL(string(modelJSON)) + if err == nil && modelDSL != nil { + log.Info().Str("store", store.Name).Str("model", *modelDSL).Msg("Writing authorization model") + } + res, err := a.fga.WriteAuthorizationModel(ctx, &openfgav1.WriteAuthorizationModelRequest{ StoreId: store.Status.StoreID, TypeDefinitions: authorizationModel.TypeDefinitions, diff --git a/internal/subroutine/authorization_model_generation.go b/internal/subroutine/authorization_model_generation.go index 9c23b5c..df10e76 100644 --- a/internal/subroutine/authorization_model_generation.go +++ b/internal/subroutine/authorization_model_generation.go @@ -59,6 +59,9 @@ type {{ .Group }}_{{ .Singular }} relations define parent: [{{ if eq .Scope "Namespaced" }}core_namespace{{ else }}core_platform-mesh_io_account{{ end }}] + define member: member from parent + define owner: owner from parent + define get: member from parent define update: member from parent define delete: member from parent diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index 725accf..4d7bc29 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -72,10 +72,8 @@ func (w *workspaceInitializer) GetName() string { return "WorkspaceInitializer" func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { lc := instance.(*kcpv1alpha1.LogicalCluster) - // Check if workspace is still initializing if lc.Status.Phase != kcpv1alpha1.LogicalClusterPhaseInitializing { - fmt.Printf("[DEBUG] Workspace no longer initializing (phase=%s), skipping\n", lc.Status.Phase) - return ctrl.Result{}, nil + fmt.Printf("[DEBUG] Workspace phase=%s, ensuring resources remain consistent\n", lc.Status.Phase) } clusterRef, err := w.mgr.ClusterFromContext(ctx) @@ -142,7 +140,7 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje // For account-type accounts, Store already exists in parent org if account.Spec.Type != accountsv1alpha1.AccountTypeOrg { fmt.Printf("[DEBUG] Account type is '%s', skipping Store creation (using parent org Store)\n", account.Spec.Type) - return ctrl.Result{}, nil + return ctrl.Result{RequeueAfter: time.Minute}, nil } // Resolve Store name and location for org accounts @@ -175,9 +173,10 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje store = &v1alpha1.Store{ObjectMeta: metav1.ObjectMeta{Name: storeName}} } + coreModuleUpdated := false _, err = controllerutil.CreateOrUpdate(ctxStore, w.orgsClient, store, func() error { - // Set CoreModule for store - if store.Spec.CoreModule == "" { + if store.Spec.CoreModule != w.coreModule { + coreModuleUpdated = true store.Spec.CoreModule = w.coreModule } return nil @@ -185,6 +184,9 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje if err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to create/update store: %w", err), true, true) } + if coreModuleUpdated { + fmt.Printf("[DEBUG] Store %s core module refreshed from ConfigMap contents\n", storeName) + } // Re-fetch to get store status if err := w.orgsClient.Get(ctxStore, client.ObjectKey{Name: storeName}, store); err != nil { @@ -222,7 +224,7 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje fmt.Printf("[DEBUG] AccountInfo updated successfully\n") fmt.Printf("[SUCCESS] WorkspaceInitializer completed successfully for org workspace\n") - return ctrl.Result{}, nil + return ctrl.Result{RequeueAfter: time.Minute}, nil } func (w *workspaceInitializer) ensureAccountInfo(ctx context.Context, workspaceClient client.Client, storeID string, creator *string) errors.OperatorError { From 8c0d9e47a63c6debea7582ee95b6a6867160cd04 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 23 Oct 2025 12:53:05 +0300 Subject: [PATCH 26/35] fix: fixed tests --- internal/subroutine/workspace_initializer.go | 14 +++--- .../subroutine/workspace_initializer_test.go | 48 ++----------------- 2 files changed, 10 insertions(+), 52 deletions(-) diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index 4d7bc29..dee0c7b 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -76,19 +76,19 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje fmt.Printf("[DEBUG] Workspace phase=%s, ensuring resources remain consistent\n", lc.Status.Phase) } - clusterRef, err := w.mgr.ClusterFromContext(ctx) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get cluster from context: %w", err), true, false) - } - workspaceClient := clusterRef.GetClient() - - // Validate that owner cluster is specified + // Validate that owner cluster is specified before getting workspace client if lc.Spec.Owner.Cluster == "" { return ctrl.Result{}, errors.NewOperatorError( fmt.Errorf("spec.owner.cluster is empty for LogicalCluster %s", lc.Name), true, true) } + clusterRef, err := w.mgr.ClusterFromContext(ctx) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get cluster from context: %w", err), true, false) + } + workspaceClient := clusterRef.GetClient() + // Use orgsClient directly since lc.Spec.Owner.Cluster contains short cluster ID // which cannot be resolved via mgr.GetCluster() var account accountsv1alpha1.Account diff --git a/internal/subroutine/workspace_initializer_test.go b/internal/subroutine/workspace_initializer_test.go index 6d1af3a..34abbc8 100644 --- a/internal/subroutine/workspace_initializer_test.go +++ b/internal/subroutine/workspace_initializer_test.go @@ -10,7 +10,6 @@ import ( "github.com/platform-mesh/security-operator/internal/subroutine" "github.com/platform-mesh/security-operator/internal/subroutine/mocks" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" mccontext "sigs.k8s.io/multicluster-runtime/pkg/context" ) @@ -25,8 +24,6 @@ func TestWorkspaceInitializer_ErrorWhenOwnerClusterEmpty(t *testing.T) { mgr := mocks.NewMockManager(t) orgsClient := mocks.NewMockClient(t) fga := mocks.NewMockOpenFGAServiceClient(t) - wsCluster := mocks.NewMockCluster(t) - wsClient := mocks.NewMockClient(t) cfg := config.Config{ CoreModulePath: tmpFile.Name(), @@ -35,55 +32,16 @@ func TestWorkspaceInitializer_ErrorWhenOwnerClusterEmpty(t *testing.T) { cfg.FGA.ParentRelation = "parent" cfg.FGA.CreatorRelation = "owner" - wsCluster.EXPECT().GetClient().Return(wsClient) - mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) - sub := subroutine.NewWorkspaceInitializer(orgsClient, cfg, mgr, fga) lc := &kcpcorev1alpha1.LogicalCluster{} lc.Name = "test-workspace" - lc.Spec.Owner.Cluster = "" // Empty cluster - - ctx := mccontext.WithCluster(context.Background(), "ws") - _, opErr := sub.Process(ctx, lc) - - assert.NotNil(t, opErr, "Expected error when owner.cluster is empty") -} - -func TestWorkspaceInitializer_ErrorWhenOwnerClusterNotFound(t *testing.T) { - // Create temp file for core module - tmpFile, err := os.CreateTemp("", "coreModule*.fga") - assert.NoError(t, err) - defer os.Remove(tmpFile.Name()) - tmpFile.WriteString("model\n schema 1.1") - tmpFile.Close() - - mgr := mocks.NewMockManager(t) - orgsClient := mocks.NewMockClient(t) - fga := mocks.NewMockOpenFGAServiceClient(t) - wsCluster := mocks.NewMockCluster(t) - wsClient := mocks.NewMockClient(t) - - cfg := config.Config{ - CoreModulePath: tmpFile.Name(), + lc.Spec.Owner = &kcpcorev1alpha1.LogicalClusterOwner{ + Cluster: "", // Empty cluster } - cfg.FGA.ObjectType = "core_platform-mesh_io_account" - cfg.FGA.ParentRelation = "parent" - cfg.FGA.CreatorRelation = "owner" - - wsCluster.EXPECT().GetClient().Return(wsClient) - mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) - mgr.EXPECT().GetCluster(mock.Anything, "root:orgs").Return(nil, assert.AnError) - - sub := subroutine.NewWorkspaceInitializer(orgsClient, cfg, mgr, fga) - - lc := &kcpcorev1alpha1.LogicalCluster{} - lc.Name = "test-workspace" - lc.Spec.Owner.Cluster = "root:orgs" - lc.Spec.Owner.Name = "org1" ctx := mccontext.WithCluster(context.Background(), "ws") _, opErr := sub.Process(ctx, lc) - assert.NotNil(t, opErr, "Expected error when owner cluster not found") + assert.NotNil(t, opErr, "Expected error when owner.cluster is empty") } From c7507f4e998dbf44825fe2893d8bb6037dd9fb8d Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 23 Oct 2025 12:54:03 +0300 Subject: [PATCH 27/35] fix: Protect against nil w.fga to avoid a crash if wiring is misconfigured --- internal/subroutine/workspace_fga.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/subroutine/workspace_fga.go b/internal/subroutine/workspace_fga.go index f84ce4c..b4a8c4b 100644 --- a/internal/subroutine/workspace_fga.go +++ b/internal/subroutine/workspace_fga.go @@ -49,6 +49,10 @@ func (w *workspaceFGASubroutine) Finalize(ctx context.Context, _ runtimeobject.R } func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { + if w.fga == nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("OpenFGA client is nil"), false, false) + } + _ = instance.(*kcpv1alpha1.LogicalCluster) clusterRef, err := w.mgr.ClusterFromContext(ctx) From afe0e7626bd0a37010c4a71d91d0c82cc23d21f5 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 23 Oct 2025 13:06:35 +0300 Subject: [PATCH 28/35] fix: Changed the error handling of workspaceClient.Get to distinguish NotFound from other errors --- internal/subroutine/workspace_fga.go | 8 +++++++- internal/subroutine/workspace_fga_test.go | 5 ++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/subroutine/workspace_fga.go b/internal/subroutine/workspace_fga.go index b4a8c4b..20b4ee0 100644 --- a/internal/subroutine/workspace_fga.go +++ b/internal/subroutine/workspace_fga.go @@ -15,6 +15,7 @@ import ( lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine" "github.com/platform-mesh/golang-commons/errors" "github.com/platform-mesh/golang-commons/fga/helpers" + apierrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" @@ -66,7 +67,12 @@ func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeob accountInfo := &accountsv1alpha1.AccountInfo{} if err := workspaceClient.Get(ctxWithTimeout, client.ObjectKey{Name: accountinfo.DefaultAccountInfoName}, accountInfo); err != nil { - return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + if apierrors.IsNotFound(err) { + // AccountInfo not created yet by workspace_initializer, wait and retry + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + } + // Other errors (permissions, network, etc) should be surfaced + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get AccountInfo: %w", err), true, true) } if accountInfo.Spec.Account.Name == "" || accountInfo.Spec.Account.OriginClusterId == "" || accountInfo.Spec.FGA.Store.Id == "" { return ctrl.Result{RequeueAfter: 2 * time.Second}, nil diff --git a/internal/subroutine/workspace_fga_test.go b/internal/subroutine/workspace_fga_test.go index 1248e26..356dd1a 100644 --- a/internal/subroutine/workspace_fga_test.go +++ b/internal/subroutine/workspace_fga_test.go @@ -12,6 +12,8 @@ import ( "github.com/platform-mesh/security-operator/internal/subroutine/mocks" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" mccontext "sigs.k8s.io/multicluster-runtime/pkg/context" @@ -22,7 +24,8 @@ func TestWorkspaceFGA_Requeue_WhenAccountInfoMissing(t *testing.T) { wsCluster := mocks.NewMockCluster(t) wsClient := mocks.NewMockClient(t) - wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError) + notFoundErr := &apierrors.StatusError{ErrStatus: metav1.Status{Reason: metav1.StatusReasonNotFound}} + wsClient.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(notFoundErr) wsCluster.EXPECT().GetClient().Return(wsClient) mgr.EXPECT().ClusterFromContext(mock.Anything).Return(wsCluster, nil) From 728f0ca2f39a6859243b997de06d690e31f58667 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 23 Oct 2025 13:40:24 +0300 Subject: [PATCH 29/35] fix: removed specific contexts with timeout --- internal/subroutine/workspace_fga.go | 13 +++++-------- internal/subroutine/workspace_initializer.go | 16 +++------------- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/internal/subroutine/workspace_fga.go b/internal/subroutine/workspace_fga.go index 20b4ee0..32cc794 100644 --- a/internal/subroutine/workspace_fga.go +++ b/internal/subroutine/workspace_fga.go @@ -62,11 +62,8 @@ func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeob } workspaceClient := clusterRef.GetClient() - ctxWithTimeout, cancel := context.WithTimeout(ctx, 10*time.Second) - defer cancel() - accountInfo := &accountsv1alpha1.AccountInfo{} - if err := workspaceClient.Get(ctxWithTimeout, client.ObjectKey{Name: accountinfo.DefaultAccountInfoName}, accountInfo); err != nil { + if err := workspaceClient.Get(ctx, client.ObjectKey{Name: accountinfo.DefaultAccountInfoName}, accountInfo); err != nil { if apierrors.IsNotFound(err) { // AccountInfo not created yet by workspace_initializer, wait and retry return ctrl.Result{RequeueAfter: 2 * time.Second}, nil @@ -81,7 +78,7 @@ func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeob // Parent relation for non-org accounts if accountInfo.Spec.ParentAccount != nil { parent := accountInfo.Spec.ParentAccount - if err := w.writeTuple(ctxWithTimeout, accountInfo.Spec.FGA.Store.Id, &openfgav1.TupleKey{ + if err := w.writeTuple(ctx, accountInfo.Spec.FGA.Store.Id, &openfgav1.TupleKey{ User: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, parent.OriginClusterId, parent.Name), Relation: w.fgaParentRel, Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, accountInfo.Spec.Account.Name), @@ -97,14 +94,14 @@ func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeob if !validateCreator(normalized) { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("creator string is in the protected service account prefix range"), false, false) } - if err := w.writeTuple(ctxWithTimeout, accountInfo.Spec.FGA.Store.Id, &openfgav1.TupleKey{ + if err := w.writeTuple(ctx, accountInfo.Spec.FGA.Store.Id, &openfgav1.TupleKey{ User: fmt.Sprintf("user:%s", normalized), Relation: "assignee", Object: fmt.Sprintf("role:%s/%s/%s/owner", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, accountInfo.Spec.Account.Name), }); err != nil { return ctrl.Result{}, err } - if err := w.writeTuple(ctxWithTimeout, accountInfo.Spec.FGA.Store.Id, &openfgav1.TupleKey{ + if err := w.writeTuple(ctx, accountInfo.Spec.FGA.Store.Id, &openfgav1.TupleKey{ User: fmt.Sprintf("role:%s/%s/%s/owner#assignee", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, accountInfo.Spec.Account.Name), Relation: w.fgaCreatorRel, Object: fmt.Sprintf("%s:%s/%s", w.fgaObjectType, accountInfo.Spec.Account.OriginClusterId, accountInfo.Spec.Account.Name), @@ -114,7 +111,7 @@ func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeob // Mark creator tuple as written accountInfo.Status.CreatorTupleWritten = true - if err := workspaceClient.Status().Update(ctxWithTimeout, accountInfo); err != nil { + if err := workspaceClient.Status().Update(ctx, accountInfo); err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to update accountInfo status: %w", err), true, true) } } diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index dee0c7b..d893aa7 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -102,14 +102,10 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje } fmt.Printf("[DEBUG] Successfully fetched account: %s, type: %s\n", account.Name, account.Spec.Type) - // Timeout for AccountInfo retrieval/creation - ctxGetTimeout, cancelGet := context.WithTimeout(ctx, 5*time.Second) - defer cancelGet() - // Ensure AccountInfo exists (create if missing) so account-operator can populate it fmt.Printf("[DEBUG] Creating/updating AccountInfo in workspace\n") accountInfo := &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} - op, err := controllerutil.CreateOrUpdate(ctxGetTimeout, workspaceClient, accountInfo, func() error { + op, err := controllerutil.CreateOrUpdate(ctx, workspaceClient, accountInfo, func() error { // Set Creator immediately when creating AccountInfo to avoid race with account-operator if accountInfo.Spec.Creator == nil && account.Spec.Creator != nil { creatorValue := *account.Spec.Creator @@ -158,10 +154,7 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje storeClusterName := logicalcluster.Name(path) fmt.Printf("[DEBUG] Resolved store: cluster=%s, name=%s\n", storeClusterName, storeName) - // Fresh timeout for store operations - ctxStoreTimeout, cancelStore := context.WithTimeout(ctx, 5*time.Second) - defer cancelStore() - ctxStore := mccontext.WithCluster(ctxStoreTimeout, storeClusterName.String()) + ctxStore := mccontext.WithCluster(ctx, storeClusterName.String()) // Create Store for org account store := &v1alpha1.Store{} @@ -201,11 +194,8 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje // Update AccountInfo with Store ID and Creator fmt.Printf("[DEBUG] Updating AccountInfo with StoreID=%s and Creator\n", store.Status.StoreID) - ctxUpdateTimeout, cancelUpdate := context.WithTimeout(ctx, 5*time.Second) - defer cancelUpdate() - accountInfo = &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} - _, err = controllerutil.CreateOrUpdate(ctxUpdateTimeout, workspaceClient, accountInfo, func() error { + _, err = controllerutil.CreateOrUpdate(ctx, workspaceClient, accountInfo, func() error { accountInfo.Spec.FGA.Store.Id = store.Status.StoreID // Copy creator value (not pointer) to avoid issues with pointer sharing if account.Spec.Creator != nil { From f88b94b82a248a7c5221baf0232473e06e1012cc Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 23 Oct 2025 13:51:45 +0300 Subject: [PATCH 30/35] fix: removed leftovers --- internal/subroutine/workspace_initializer.go | 70 +++----------------- 1 file changed, 8 insertions(+), 62 deletions(-) diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index d893aa7..0894f12 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -36,26 +36,20 @@ func NewWorkspaceInitializer(orgsClient client.Client, cfg config.Config, mgr mc } return &workspaceInitializer{ - orgsClient: orgsClient, - mgr: mgr, - coreModule: string(data), - fga: fga, - fgaObjectType: cfg.FGA.ObjectType, - fgaParentRel: cfg.FGA.ParentRelation, - fgaCreatorRel: cfg.FGA.CreatorRelation, + orgsClient: orgsClient, + mgr: mgr, + coreModule: string(data), + fga: fga, } } var _ lifecyclesubroutine.Subroutine = &workspaceInitializer{} type workspaceInitializer struct { - orgsClient client.Client - mgr mcmanager.Manager - coreModule string - fga openfgav1.OpenFGAServiceClient - fgaObjectType string - fgaParentRel string - fgaCreatorRel string + orgsClient client.Client + mgr mcmanager.Manager + coreModule string + fga openfgav1.OpenFGAServiceClient } func (w *workspaceInitializer) Finalize(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { @@ -217,46 +211,6 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje return ctrl.Result{RequeueAfter: time.Minute}, nil } -func (w *workspaceInitializer) ensureAccountInfo(ctx context.Context, workspaceClient client.Client, storeID string, creator *string) errors.OperatorError { - accountInfo := &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} - _, err := controllerutil.CreateOrUpdate(ctx, workspaceClient, accountInfo, func() error { - accountInfo.Spec.FGA.Store.Id = storeID - accountInfo.Spec.Creator = creator - return nil - }) - if err != nil { - return errors.NewOperatorError(fmt.Errorf("unable to create/update accountInfo: %w", err), true, true) - } - return nil -} - -func (w *workspaceInitializer) resolveStoreTarget(lc *kcpv1alpha1.LogicalCluster, account accountsv1alpha1.Account, accountInfo *accountsv1alpha1.AccountInfo) (logicalcluster.Name, string, errors.OperatorError) { - if account.Spec.Type == accountsv1alpha1.AccountTypeOrg { - path, ok := lc.Annotations["kcp.io/path"] - if !ok { - return "", "", errors.NewOperatorError(fmt.Errorf("unable to get workspace path"), true, false) - } - storeName := generateStoreName(lc) - if storeName == "" { - return "", "", errors.NewOperatorError(fmt.Errorf("unable to generate store name from workspace path"), true, false) - } - return logicalcluster.Name(path), storeName, nil - } - - if accountInfo.Spec.Organization.Path == "" { - return "", "", errors.NewOperatorError(fmt.Errorf("organization path not yet set"), true, false) - } - storeName := generateStoreNameFromPath(accountInfo.Spec.Organization.Path) - if storeName == "" { - return "", "", errors.NewOperatorError(fmt.Errorf("unable to derive store name from organization path"), true, false) - } - if accountInfo.Spec.ParentAccount == nil || accountInfo.Spec.ParentAccount.Name == "" || accountInfo.Spec.ParentAccount.OriginClusterId == "" { - return "", "", errors.NewOperatorError(fmt.Errorf("parent account information not ready"), true, false) - } - - return logicalcluster.Name(accountInfo.Spec.Organization.Path), storeName, nil -} - func generateStoreName(lc *kcpv1alpha1.LogicalCluster) string { if path, ok := lc.Annotations["kcp.io/path"]; ok { pathElements := strings.Split(path, ":") @@ -264,11 +218,3 @@ func generateStoreName(lc *kcpv1alpha1.LogicalCluster) string { } return "" } - -func generateStoreNameFromPath(path string) string { - pathElements := strings.Split(path, ":") - if len(pathElements) == 0 { - return "" - } - return pathElements[len(pathElements)-1] -} From e8230cb90518c75c4779fe0b74769e48aa7d0947 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 23 Oct 2025 14:51:11 +0300 Subject: [PATCH 31/35] Enabled FGA authorization for non-org accounts; some other changes --- data/coreModule.fga | 2 +- internal/subroutine/workspace_fga.go | 7 +- internal/subroutine/workspace_fga_test.go | 4 +- internal/subroutine/workspace_initializer.go | 66 ++++++++++++++++++- .../subroutine/workspace_initializer_test.go | 8 ++- 5 files changed, 74 insertions(+), 13 deletions(-) diff --git a/data/coreModule.fga b/data/coreModule.fga index cae243b..dcb3b3d 100644 --- a/data/coreModule.fga +++ b/data/coreModule.fga @@ -34,7 +34,7 @@ type core_platform-mesh_io_account define watch_core_platform-mesh_io_accountinfos: member define create_apis_kcp_io_apibindings: owner - define list__apis_kcp_io_apibindings: member + define list_apis_kcp_io_apibindings: member define watch_apis_kcp_io_apibindings: member # IAM specific diff --git a/internal/subroutine/workspace_fga.go b/internal/subroutine/workspace_fga.go index 32cc794..f89282d 100644 --- a/internal/subroutine/workspace_fga.go +++ b/internal/subroutine/workspace_fga.go @@ -5,7 +5,6 @@ import ( "fmt" "regexp" "strings" - "time" kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" openfgav1 "github.com/openfga/api/proto/openfga/v1" @@ -65,14 +64,14 @@ func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeob accountInfo := &accountsv1alpha1.AccountInfo{} if err := workspaceClient.Get(ctx, client.ObjectKey{Name: accountinfo.DefaultAccountInfoName}, accountInfo); err != nil { if apierrors.IsNotFound(err) { - // AccountInfo not created yet by workspace_initializer, wait and retry - return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + // AccountInfo not created yet by workspace_initializer, requeue immediately + return ctrl.Result{RequeueAfter: 1}, nil } // Other errors (permissions, network, etc) should be surfaced return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("failed to get AccountInfo: %w", err), true, true) } if accountInfo.Spec.Account.Name == "" || accountInfo.Spec.Account.OriginClusterId == "" || accountInfo.Spec.FGA.Store.Id == "" { - return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + return ctrl.Result{RequeueAfter: 1}, nil } // Parent relation for non-org accounts diff --git a/internal/subroutine/workspace_fga_test.go b/internal/subroutine/workspace_fga_test.go index 356dd1a..2337c8c 100644 --- a/internal/subroutine/workspace_fga_test.go +++ b/internal/subroutine/workspace_fga_test.go @@ -36,7 +36,7 @@ func TestWorkspaceFGA_Requeue_WhenAccountInfoMissing(t *testing.T) { ctx := mccontext.WithCluster(context.Background(), "ws") res, opErr := sub.Process(ctx, lc) assert.Nil(t, opErr) - assert.True(t, res.RequeueAfter > 0, "Expected requeue with delay") + assert.Equal(t, 1, int(res.RequeueAfter), "Expected immediate requeue") } func TestWorkspaceFGA_Requeue_WhenAccountInfoIncomplete(t *testing.T) { @@ -61,7 +61,7 @@ func TestWorkspaceFGA_Requeue_WhenAccountInfoIncomplete(t *testing.T) { ctx := mccontext.WithCluster(context.Background(), "ws") res, opErr := sub.Process(ctx, lc) assert.Nil(t, opErr) - assert.True(t, res.RequeueAfter > 0, "Expected requeue with delay") + assert.Equal(t, 1, int(res.RequeueAfter), "Expected immediate requeue") } func TestWorkspaceFGA_WritesParentAndOwnerTuples(t *testing.T) { diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index 0894f12..9130099 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -119,7 +119,7 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje // If APIBinding not ready yet, return error to retry whole reconcile if strings.Contains(err.Error(), "no matches for kind") { fmt.Printf("[DEBUG] CRD not ready yet (no matches for kind), will retry reconcile\n") - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("APIBinding not ready: %w", err), true, false) + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("apiBinding not ready: %w", err), true, false) } fmt.Printf("[ERROR] Failed to create/update AccountInfo: %v\n", err) return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to ensure accountInfo exists: %w", err), true, true) @@ -129,7 +129,59 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje // Only create Store for org accounts during initialization // For account-type accounts, Store already exists in parent org if account.Spec.Type != accountsv1alpha1.AccountTypeOrg { - fmt.Printf("[DEBUG] Account type is '%s', skipping Store creation (using parent org Store)\n", account.Spec.Type) + fmt.Printf("[DEBUG] Account type is '%s', need to populate Store ID from parent org\n", account.Spec.Type) + + // Re-fetch AccountInfo to get latest state populated by account-operator + accountInfo = &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} + if err := workspaceClient.Get(ctx, client.ObjectKey{Name: accountinfo.DefaultAccountInfoName}, accountInfo); err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get accountInfo: %w", err), true, true) + } + + // Wait for account-operator to populate organization path + if accountInfo.Spec.Organization.Path == "" { + fmt.Printf("[DEBUG] Organization path not yet set by account-operator, requeuing\n") + return ctrl.Result{RequeueAfter: time.Second}, nil + } + + // Resolve parent org's Store name from organization path + storeName := generateStoreNameFromPath(accountInfo.Spec.Organization.Path) + if storeName == "" { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to derive store name from organization path"), true, false) + } + storeClusterName := logicalcluster.Name(accountInfo.Spec.Organization.Path) + fmt.Printf("[DEBUG] Resolved parent org store: cluster=%s, name=%s\n", storeClusterName, storeName) + + ctxStore := mccontext.WithCluster(ctx, storeClusterName.String()) + + // Get parent org's Store + store := &v1alpha1.Store{} + if err := w.orgsClient.Get(ctxStore, client.ObjectKey{Name: storeName}, store); err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get parent org store: %w", err), true, true) + } + + if store.Status.StoreID == "" { + fmt.Printf("[DEBUG] Parent org Store not ready yet (StoreID empty), requeuing\n") + return ctrl.Result{RequeueAfter: time.Second}, nil + } + fmt.Printf("[DEBUG] Parent org Store ready with ID: %s\n", store.Status.StoreID) + + // Update AccountInfo with parent org's Store ID + accountInfo = &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} + _, err = controllerutil.CreateOrUpdate(ctx, workspaceClient, accountInfo, func() error { + accountInfo.Spec.FGA.Store.Id = store.Status.StoreID + // Also set Creator if available + if account.Spec.Creator != nil { + creatorValue := *account.Spec.Creator + accountInfo.Spec.Creator = &creatorValue + fmt.Printf("[DEBUG] Setting Creator to: %s (from account.Spec.Creator)\n", creatorValue) + } + return nil + }) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to update accountInfo with store ID: %w", err), true, true) + } + fmt.Printf("[DEBUG] AccountInfo updated with parent org Store ID\n") + return ctrl.Result{RequeueAfter: time.Minute}, nil } @@ -182,7 +234,7 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje if store.Status.StoreID == "" { fmt.Printf("[DEBUG] Store not ready yet (StoreID empty), will retry reconcile\n") - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("Store not ready"), true, false) + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("store not ready"), true, false) } fmt.Printf("[DEBUG] Store ready with ID: %s\n", store.Status.StoreID) @@ -218,3 +270,11 @@ func generateStoreName(lc *kcpv1alpha1.LogicalCluster) string { } return "" } + +func generateStoreNameFromPath(path string) string { + pathElements := strings.Split(path, ":") + if len(pathElements) == 0 { + return "" + } + return pathElements[len(pathElements)-1] +} diff --git a/internal/subroutine/workspace_initializer_test.go b/internal/subroutine/workspace_initializer_test.go index 34abbc8..fbde883 100644 --- a/internal/subroutine/workspace_initializer_test.go +++ b/internal/subroutine/workspace_initializer_test.go @@ -17,9 +17,11 @@ func TestWorkspaceInitializer_ErrorWhenOwnerClusterEmpty(t *testing.T) { // Create temp file for core module tmpFile, err := os.CreateTemp("", "coreModule*.fga") assert.NoError(t, err) - defer os.Remove(tmpFile.Name()) - tmpFile.WriteString("model\n schema 1.1") - tmpFile.Close() + defer func() { _ = os.Remove(tmpFile.Name()) }() + _, err = tmpFile.WriteString("model\n schema 1.1") + assert.NoError(t, err) + err = tmpFile.Close() + assert.NoError(t, err) mgr := mocks.NewMockManager(t) orgsClient := mocks.NewMockClient(t) From c849a54b71b3f57875adedb7d88bf7c6339d7e24 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 23 Oct 2025 15:50:07 +0300 Subject: [PATCH 32/35] fix: Removed duplicate relation definitions --- internal/subroutine/authorization_model_generation.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/subroutine/authorization_model_generation.go b/internal/subroutine/authorization_model_generation.go index 05c9b1f..46a1ea9 100644 --- a/internal/subroutine/authorization_model_generation.go +++ b/internal/subroutine/authorization_model_generation.go @@ -58,11 +58,8 @@ extend type core_namespace type {{ .Group }}_{{ .Singular }} relations define parent: [{{ if eq .Scope "Namespaced" }}core_namespace{{ else }}core_platform-mesh_io_account{{ end }}] - define member: [role#assignee] or owner or member from parent - define owner: [role#assignee] or owner from parent - - define member: member - define owner: owner + define member: member from parent + define owner: owner from parent define statusUpdate: member define statusPatch: member From ef99bc1e817be96f838d731cd367a56fa5edaf3f Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Thu, 23 Oct 2025 16:07:12 +0300 Subject: [PATCH 33/35] fix: Cleaned up the code from debug messages and unused lines --- internal/controller/initializer_controller.go | 2 +- internal/subroutine/invite_test.go | 2 +- internal/subroutine/workspace_fga.go | 4 +- internal/subroutine/workspace_initializer.go | 56 +------------------ .../subroutine/workspace_initializer_test.go | 3 +- 5 files changed, 9 insertions(+), 58 deletions(-) diff --git a/internal/controller/initializer_controller.go b/internal/controller/initializer_controller.go index ca859d3..54e4b62 100644 --- a/internal/controller/initializer_controller.go +++ b/internal/controller/initializer_controller.go @@ -31,7 +31,7 @@ func NewLogicalClusterReconciler(log *logger.Logger, orgClient client.Client, cf return &LogicalClusterReconciler{ log: log, lifecycle: builder.NewBuilder("logicalcluster", "LogicalClusterReconciler", []lifecyclesubroutine.Subroutine{ - subroutine.NewWorkspaceInitializer(orgClient, cfg, mgr, fga), + subroutine.NewWorkspaceInitializer(orgClient, cfg, mgr), subroutine.NewWorkspaceFGASubroutine(mgr, fga, cfg.FGA.ObjectType, cfg.FGA.ParentRelation, cfg.FGA.CreatorRelation), subroutine.NewWorkspaceAuthConfigurationSubroutine(orgClient, inClusterClient, cfg), subroutine.NewRealmSubroutine(inClusterClient, &cfg, cfg.BaseDomain), diff --git a/internal/subroutine/invite_test.go b/internal/subroutine/invite_test.go index 72d3114..17404b6 100644 --- a/internal/subroutine/invite_test.go +++ b/internal/subroutine/invite_test.go @@ -71,7 +71,7 @@ func TestInviteSubroutine_Process(t *testing.T) { expectedResult ctrl.Result }{ { - name: "Empty workspace name - early return", + name: "Empty workspace name - early return", setupMocks: func(orgsClient *mocks.MockClient, mgr *mocks.MockManager, cluster *mocks.MockCluster) {}, lc: &kcpv1alpha1.LogicalCluster{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/subroutine/workspace_fga.go b/internal/subroutine/workspace_fga.go index f89282d..0d00221 100644 --- a/internal/subroutine/workspace_fga.go +++ b/internal/subroutine/workspace_fga.go @@ -53,7 +53,9 @@ func (w *workspaceFGASubroutine) Process(ctx context.Context, instance runtimeob return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("OpenFGA client is nil"), false, false) } - _ = instance.(*kcpv1alpha1.LogicalCluster) + if _, ok := instance.(*kcpv1alpha1.LogicalCluster); !ok { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unexpected instance type %T", instance), false, false) + } clusterRef, err := w.mgr.ClusterFromContext(ctx) if err != nil { diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index 9130099..bae028b 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -9,7 +9,6 @@ import ( kcpv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" "github.com/kcp-dev/logicalcluster/v3" - openfgav1 "github.com/openfga/api/proto/openfga/v1" accountsv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" "github.com/platform-mesh/account-operator/pkg/subroutines/accountinfo" "github.com/platform-mesh/golang-commons/controller/lifecycle/runtimeobject" @@ -27,7 +26,7 @@ import ( "github.com/platform-mesh/security-operator/internal/config" ) -func NewWorkspaceInitializer(orgsClient client.Client, cfg config.Config, mgr mcmanager.Manager, fga openfgav1.OpenFGAServiceClient) *workspaceInitializer { +func NewWorkspaceInitializer(orgsClient client.Client, cfg config.Config, mgr mcmanager.Manager) *workspaceInitializer { coreModulePath := cfg.CoreModulePath data, err := os.ReadFile(coreModulePath) @@ -39,7 +38,6 @@ func NewWorkspaceInitializer(orgsClient client.Client, cfg config.Config, mgr mc orgsClient: orgsClient, mgr: mgr, coreModule: string(data), - fga: fga, } } @@ -49,7 +47,6 @@ type workspaceInitializer struct { orgsClient client.Client mgr mcmanager.Manager coreModule string - fga openfgav1.OpenFGAServiceClient } func (w *workspaceInitializer) Finalize(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { @@ -66,10 +63,6 @@ func (w *workspaceInitializer) GetName() string { return "WorkspaceInitializer" func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { lc := instance.(*kcpv1alpha1.LogicalCluster) - if lc.Status.Phase != kcpv1alpha1.LogicalClusterPhaseInitializing { - fmt.Printf("[DEBUG] Workspace phase=%s, ensuring resources remain consistent\n", lc.Status.Phase) - } - // Validate that owner cluster is specified before getting workspace client if lc.Spec.Owner.Cluster == "" { return ctrl.Result{}, errors.NewOperatorError( @@ -88,49 +81,32 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje var account accountsv1alpha1.Account if err := w.orgsClient.Get(ctx, client.ObjectKey{Name: lc.Spec.Owner.Name}, &account); err != nil { if kerrors.IsNotFound(err) { - fmt.Printf("[DEBUG] Account %s not found yet, requeuing\n", lc.Spec.Owner.Name) return ctrl.Result{Requeue: true}, nil } - fmt.Printf("[ERROR] Failed to get account %s: %v\n", lc.Spec.Owner.Name, err) return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get owner account: %w", err), true, true) } - fmt.Printf("[DEBUG] Successfully fetched account: %s, type: %s\n", account.Name, account.Spec.Type) // Ensure AccountInfo exists (create if missing) so account-operator can populate it - fmt.Printf("[DEBUG] Creating/updating AccountInfo in workspace\n") accountInfo := &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} - op, err := controllerutil.CreateOrUpdate(ctx, workspaceClient, accountInfo, func() error { + _, err = controllerutil.CreateOrUpdate(ctx, workspaceClient, accountInfo, func() error { // Set Creator immediately when creating AccountInfo to avoid race with account-operator if accountInfo.Spec.Creator == nil && account.Spec.Creator != nil { creatorValue := *account.Spec.Creator accountInfo.Spec.Creator = &creatorValue - fmt.Printf("[DEBUG] Setting Creator to: %s during AccountInfo creation\n", creatorValue) } return nil }) - if op != controllerutil.OperationResultNone { - creatorVal := "" - if accountInfo.Spec.Creator != nil { - creatorVal = *accountInfo.Spec.Creator - } - fmt.Printf("[DEBUG] After CreateOrUpdate (op=%s): Creator=%s\n", op, creatorVal) - } if err != nil { // If APIBinding not ready yet, return error to retry whole reconcile if strings.Contains(err.Error(), "no matches for kind") { - fmt.Printf("[DEBUG] CRD not ready yet (no matches for kind), will retry reconcile\n") return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("apiBinding not ready: %w", err), true, false) } - fmt.Printf("[ERROR] Failed to create/update AccountInfo: %v\n", err) return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to ensure accountInfo exists: %w", err), true, true) } - fmt.Printf("[DEBUG] AccountInfo operation: %s\n", op) // Only create Store for org accounts during initialization // For account-type accounts, Store already exists in parent org if account.Spec.Type != accountsv1alpha1.AccountTypeOrg { - fmt.Printf("[DEBUG] Account type is '%s', need to populate Store ID from parent org\n", account.Spec.Type) - // Re-fetch AccountInfo to get latest state populated by account-operator accountInfo = &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} if err := workspaceClient.Get(ctx, client.ObjectKey{Name: accountinfo.DefaultAccountInfoName}, accountInfo); err != nil { @@ -139,7 +115,6 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje // Wait for account-operator to populate organization path if accountInfo.Spec.Organization.Path == "" { - fmt.Printf("[DEBUG] Organization path not yet set by account-operator, requeuing\n") return ctrl.Result{RequeueAfter: time.Second}, nil } @@ -149,7 +124,6 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to derive store name from organization path"), true, false) } storeClusterName := logicalcluster.Name(accountInfo.Spec.Organization.Path) - fmt.Printf("[DEBUG] Resolved parent org store: cluster=%s, name=%s\n", storeClusterName, storeName) ctxStore := mccontext.WithCluster(ctx, storeClusterName.String()) @@ -160,10 +134,8 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje } if store.Status.StoreID == "" { - fmt.Printf("[DEBUG] Parent org Store not ready yet (StoreID empty), requeuing\n") return ctrl.Result{RequeueAfter: time.Second}, nil } - fmt.Printf("[DEBUG] Parent org Store ready with ID: %s\n", store.Status.StoreID) // Update AccountInfo with parent org's Store ID accountInfo = &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} @@ -173,32 +145,26 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje if account.Spec.Creator != nil { creatorValue := *account.Spec.Creator accountInfo.Spec.Creator = &creatorValue - fmt.Printf("[DEBUG] Setting Creator to: %s (from account.Spec.Creator)\n", creatorValue) } return nil }) if err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to update accountInfo with store ID: %w", err), true, true) } - fmt.Printf("[DEBUG] AccountInfo updated with parent org Store ID\n") return ctrl.Result{RequeueAfter: time.Minute}, nil } // Resolve Store name and location for org accounts - fmt.Printf("[DEBUG] Resolving store for org account\n") path, ok := lc.Annotations["kcp.io/path"] if !ok { - fmt.Printf("[ERROR] Missing kcp.io/path annotation\n") return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to get workspace path"), true, false) } storeName := generateStoreName(lc) if storeName == "" { - fmt.Printf("[ERROR] Failed to generate store name from workspace path\n") return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to generate store name from workspace path"), true, false) } storeClusterName := logicalcluster.Name(path) - fmt.Printf("[DEBUG] Resolved store: cluster=%s, name=%s\n", storeClusterName, storeName) ctxStore := mccontext.WithCluster(ctx, storeClusterName.String()) @@ -212,20 +178,13 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje store = &v1alpha1.Store{ObjectMeta: metav1.ObjectMeta{Name: storeName}} } - coreModuleUpdated := false _, err = controllerutil.CreateOrUpdate(ctxStore, w.orgsClient, store, func() error { - if store.Spec.CoreModule != w.coreModule { - coreModuleUpdated = true - store.Spec.CoreModule = w.coreModule - } + store.Spec.CoreModule = w.coreModule return nil }) if err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to create/update store: %w", err), true, true) } - if coreModuleUpdated { - fmt.Printf("[DEBUG] Store %s core module refreshed from ConfigMap contents\n", storeName) - } // Re-fetch to get store status if err := w.orgsClient.Get(ctxStore, client.ObjectKey{Name: storeName}, store); err != nil { @@ -233,13 +192,10 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje } if store.Status.StoreID == "" { - fmt.Printf("[DEBUG] Store not ready yet (StoreID empty), will retry reconcile\n") return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("store not ready"), true, false) } - fmt.Printf("[DEBUG] Store ready with ID: %s\n", store.Status.StoreID) // Update AccountInfo with Store ID and Creator - fmt.Printf("[DEBUG] Updating AccountInfo with StoreID=%s and Creator\n", store.Status.StoreID) accountInfo = &accountsv1alpha1.AccountInfo{ObjectMeta: metav1.ObjectMeta{Name: accountinfo.DefaultAccountInfoName}} _, err = controllerutil.CreateOrUpdate(ctx, workspaceClient, accountInfo, func() error { accountInfo.Spec.FGA.Store.Id = store.Status.StoreID @@ -247,19 +203,13 @@ func (w *workspaceInitializer) Process(ctx context.Context, instance runtimeobje if account.Spec.Creator != nil { creatorValue := *account.Spec.Creator accountInfo.Spec.Creator = &creatorValue - fmt.Printf("[DEBUG] Setting Creator to: %s (from account.Spec.Creator)\n", creatorValue) - } else { - fmt.Printf("[DEBUG] account.Spec.Creator is nil, not setting Creator\n") } return nil }) if err != nil { - fmt.Printf("[ERROR] Failed to update AccountInfo: %v\n", err) return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to update accountInfo: %w", err), true, true) } - fmt.Printf("[DEBUG] AccountInfo updated successfully\n") - fmt.Printf("[SUCCESS] WorkspaceInitializer completed successfully for org workspace\n") return ctrl.Result{RequeueAfter: time.Minute}, nil } diff --git a/internal/subroutine/workspace_initializer_test.go b/internal/subroutine/workspace_initializer_test.go index fbde883..11c6c83 100644 --- a/internal/subroutine/workspace_initializer_test.go +++ b/internal/subroutine/workspace_initializer_test.go @@ -25,7 +25,6 @@ func TestWorkspaceInitializer_ErrorWhenOwnerClusterEmpty(t *testing.T) { mgr := mocks.NewMockManager(t) orgsClient := mocks.NewMockClient(t) - fga := mocks.NewMockOpenFGAServiceClient(t) cfg := config.Config{ CoreModulePath: tmpFile.Name(), @@ -34,7 +33,7 @@ func TestWorkspaceInitializer_ErrorWhenOwnerClusterEmpty(t *testing.T) { cfg.FGA.ParentRelation = "parent" cfg.FGA.CreatorRelation = "owner" - sub := subroutine.NewWorkspaceInitializer(orgsClient, cfg, mgr, fga) + sub := subroutine.NewWorkspaceInitializer(orgsClient, cfg, mgr) lc := &kcpcorev1alpha1.LogicalCluster{} lc.Name = "test-workspace" From 67c236417fa371320ace015dfcad38a4126e35b1 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Fri, 24 Oct 2025 13:29:37 +0300 Subject: [PATCH 34/35] fix: updated test data --- internal/subroutine/authorization_model_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/subroutine/authorization_model_test.go b/internal/subroutine/authorization_model_test.go index ea15cb9..39941c9 100644 --- a/internal/subroutine/authorization_model_test.go +++ b/internal/subroutine/authorization_model_test.go @@ -73,10 +73,12 @@ type core_namespace define get_iam_roles: member define get_iam_users: member define manage_iam_roles: owner - define member: [role#assignee] or owner or member from parent - define owner: [role#assignee] or owner from parent + define member: member from parent + define owner: owner from parent define parent: [core_platform-mesh_io_account] define patch: member + define statusPatch: member + define statusUpdate: member define update: member define watch: member ` @@ -220,7 +222,7 @@ func TestAuthorizationModelProcess(t *testing.T) { Contents: extensionModel, }, { - Name: "internal_core_types_namespaces.fga", + Name: "internal_core_types_namespaces.fga", Contents: `module namespaces extend type core_platform-mesh_io_account @@ -232,13 +234,15 @@ extend type core_platform-mesh_io_account type core_namespace relations define parent: [core_platform-mesh_io_account] - define member: [role#assignee] or owner or member from parent - define owner: [role#assignee] or owner from parent + define member: member from parent + define owner: owner from parent define get: member define update: member define delete: member define patch: member + define statusPatch: member + define statusUpdate: member define watch: member define manage_iam_roles: owner From 7779cce8d24a19024ba8cc5aad4dd1501c4026c8 Mon Sep 17 00:00:00 2001 From: Arturs Jefimovs Date: Fri, 24 Oct 2025 13:37:20 +0300 Subject: [PATCH 35/35] fix: authorization_model.go uses the same simplified inheritance logic via from parent --- internal/subroutine/authorization_model.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/subroutine/authorization_model.go b/internal/subroutine/authorization_model.go index 8a3bb4b..d2baf46 100644 --- a/internal/subroutine/authorization_model.go +++ b/internal/subroutine/authorization_model.go @@ -33,7 +33,7 @@ const ( var ( privilegedGroupVersions = []string{"rbac.authorization.k8s.io/v1"} - groupVersions = []string{"authentication.k8s.io/v1", "authorization.k8s.io/v1", "v1"} + groupVersions = []string{"authentication.k8s.io/v1", "authorization.k8s.io/v1", "v1"} privilegedTemplate = template.Must(template.New("model").Parse(`module internal_core_types_{{ .Name }} @@ -56,13 +56,15 @@ extend type core_namespace type {{ .Group }}_{{ .Singular }} relations define parent: [{{ if eq .Scope "Namespaced" }}core_namespace{{ else }}core_platform-mesh_io_account{{ end }}] - define member: [role#assignee] or owner or member from parent - define owner: [role#assignee] or owner from parent + define member: member from parent + define owner: owner from parent define get: member define update: owner define delete: owner define patch: owner + define statusPatch: member + define statusUpdate: member define watch: member define manage_iam_roles: owner