diff --git a/Taskfile.yaml b/Taskfile.yaml index 54447cd4..284da704 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -6,8 +6,8 @@ vars: ENVTEST_K8S_VERSION: "1.31.0" ENVTEST_VERSION: release-0.19 CRD_DIRECTORY: config/crd/bases - KCP_APIGEN_VERSION: v0.29.0 - KCP_VERSION: 0.29.0 + KCP_APIGEN_VERSION: v0.30.1 + KCP_VERSION: 0.30.1 GOLANGCI_LINT_VERSION: v2.8.0 GOARCH: sh: go env GOARCH diff --git a/cmd/initializer.go b/cmd/initializer.go index aa960aed..bcdac21e 100644 --- a/cmd/initializer.go +++ b/cmd/initializer.go @@ -8,6 +8,7 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1" openfgav1 "github.com/openfga/api/proto/openfga/v1" "github.com/platform-mesh/security-operator/internal/controller" + "github.com/platform-mesh/security-operator/internal/fga" "github.com/platform-mesh/security-operator/internal/predicates" "github.com/spf13/cobra" "google.golang.org/grpc" @@ -118,14 +119,20 @@ var initializerCmd = &cobra.Command{ return err } defer func() { _ = conn.Close() }() - fga := openfgav1.NewOpenFGAServiceClient(conn) + fgaClient := openfgav1.NewOpenFGAServiceClient(conn) + storeIDGetter := fga.NewCachingStoreIDGetter( + fgaClient, + initializerCfg.FGA.StoreIDCacheTTL, + cmd.Context(), + log, + ) mcc, err := mcclient.New(kcpCfg, client.Options{Scheme: scheme}) if err != nil { log.Error().Err(err).Msg("Failed to create multicluster client") os.Exit(1) } - if err := controller.NewAccountLogicalClusterReconciler(log, initializerCfg, fga, mcc, mgr). + if err := controller.NewAccountLogicalClusterReconciler(log, initializerCfg, fgaClient, storeIDGetter, mcc, mgr). SetupWithManager(mgr, defaultCfg, predicate.Not(predicates.LogicalClusterIsAccountTypeOrg())); err != nil { setupLog.Error(err, "unable to create controller", "controller", "AccountLogicalCluster") os.Exit(1) diff --git a/cmd/terminator.go b/cmd/terminator.go index ba7e05f8..cd062628 100644 --- a/cmd/terminator.go +++ b/cmd/terminator.go @@ -7,6 +7,7 @@ import ( openfgav1 "github.com/openfga/api/proto/openfga/v1" iclient "github.com/platform-mesh/security-operator/internal/client" "github.com/platform-mesh/security-operator/internal/controller" + "github.com/platform-mesh/security-operator/internal/fga" "github.com/platform-mesh/security-operator/internal/predicates" "github.com/platform-mesh/security-operator/internal/terminatingworkspaces" "github.com/spf13/cobra" @@ -104,9 +105,15 @@ var terminatorCmd = &cobra.Command{ os.Exit(1) } defer func() { _ = conn.Close() }() - fga := openfgav1.NewOpenFGAServiceClient(conn) + fgaClient := openfgav1.NewOpenFGAServiceClient(conn) + storeIDGetter := fga.NewCachingStoreIDGetter( + fgaClient, + terminatorCfg.FGA.StoreIDCacheTTL, + cmd.Context(), + log, + ) - if err := controller.NewAccountLogicalClusterReconciler(log, terminatorCfg, fga, mcc, mgr). + if err := controller.NewAccountLogicalClusterReconciler(log, terminatorCfg, fgaClient, storeIDGetter, mcc, mgr). SetupWithManager(mgr, defaultCfg, predicate.Not(predicates.LogicalClusterIsAccountTypeOrg())); err != nil { log.Error().Err(err).Msg("Unable to create AccountLogicalClusterTerminator") os.Exit(1) diff --git a/go.mod b/go.mod index cca59597..40f141cf 100644 --- a/go.mod +++ b/go.mod @@ -1,13 +1,15 @@ module github.com/platform-mesh/security-operator -go 1.25.7 +go 1.26 require ( github.com/coreos/go-oidc v2.5.0+incompatible + github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc github.com/fluxcd/helm-controller/api v1.5.2 github.com/fluxcd/source-controller/api v1.8.1 github.com/go-logr/logr v1.4.3 github.com/google/gnostic-models v0.7.1 + github.com/jellydator/ttlcache/v3 v3.4.0 github.com/kcp-dev/logicalcluster/v3 v3.0.5 github.com/kcp-dev/multicluster-provider v0.5.1 github.com/kcp-dev/sdk v0.30.0 @@ -39,7 +41,6 @@ require ( github.com/beorn7/perks v1.0.1 // indirect github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect - github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.13.0 // indirect github.com/envoyproxy/protoc-gen-validate v1.3.3 // indirect github.com/evanphx/json-patch/v5 v5.9.11 // indirect diff --git a/go.sum b/go.sum index bae61cdc..60944748 100644 --- a/go.sum +++ b/go.sum @@ -134,6 +134,8 @@ github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/jellydator/ttlcache/v3 v3.4.0 h1:YS4P125qQS0tNhtL6aeYkheEaB/m8HCqdMMP4mnWdTY= +github.com/jellydator/ttlcache/v3 v3.4.0/go.mod h1:Hw9EgjymziQD3yGsQdf1FqFdpp7YjFMd4Srg5EJlgD4= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/kcp-dev/apimachinery/v2 v2.30.0 h1:bj7lVVPJj5UnQFCWhXVAKC+eNaIMKGGxpq+fE5edRU0= diff --git a/internal/config/config.go b/internal/config/config.go index 823449de..caf0e90a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,6 +2,7 @@ package config import ( "os" + "time" "github.com/spf13/pflag" ) @@ -30,6 +31,7 @@ type FGAConfig struct { ObjectType string ParentRelation string CreatorRelation string + StoreIDCacheTTL time.Duration } type KCPConfig struct { @@ -85,6 +87,7 @@ func NewConfig() Config { ObjectType: "core_platform-mesh_io_account", ParentRelation: "parent", CreatorRelation: "owner", + StoreIDCacheTTL: 24 * time.Hour, }, KCP: KCPConfig{ Kubeconfig: "/api-kubeconfig/kubeconfig", @@ -119,6 +122,7 @@ func NewConfig() Config { func (c *Config) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&c.FGA.Target, "fga-target", c.FGA.Target, "Set the OpenFGA API target") + fs.DurationVar(&c.FGA.StoreIDCacheTTL, "fga-store-id-cache-ttl", c.FGA.StoreIDCacheTTL, "TTL for the OpenFGA store ID cache (e.g. 5m, 1h)") fs.StringVar(&c.FGA.ObjectType, "fga-object-type", c.FGA.ObjectType, "Set the OpenFGA object type for account tuples") fs.StringVar(&c.FGA.ParentRelation, "fga-parent-relation", c.FGA.ParentRelation, "Set the OpenFGA parent relation name") fs.StringVar(&c.FGA.CreatorRelation, "fga-creator-relation", c.FGA.CreatorRelation, "Set the OpenFGA creator relation name") diff --git a/internal/controller/accountlogicalcluster_controller.go b/internal/controller/accountlogicalcluster_controller.go index f9641695..62e87210 100644 --- a/internal/controller/accountlogicalcluster_controller.go +++ b/internal/controller/accountlogicalcluster_controller.go @@ -10,6 +10,7 @@ import ( lifecyclesubroutine "github.com/platform-mesh/golang-commons/controller/lifecycle/subroutine" "github.com/platform-mesh/golang-commons/logger" "github.com/platform-mesh/security-operator/internal/config" + "github.com/platform-mesh/security-operator/internal/fga" "github.com/platform-mesh/security-operator/internal/subroutine" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -28,11 +29,11 @@ type AccountLogicalClusterReconciler struct { mclifecycle *multicluster.LifecycleManager } -func NewAccountLogicalClusterReconciler(log *logger.Logger, cfg config.Config, fga openfgav1.OpenFGAServiceClient, mcc mcclient.ClusterClient, mgr mcmanager.Manager) *AccountLogicalClusterReconciler { +func NewAccountLogicalClusterReconciler(log *logger.Logger, cfg config.Config, fgaClient openfgav1.OpenFGAServiceClient, storeIDGetter fga.StoreIDGetter, mcc mcclient.ClusterClient, mgr mcmanager.Manager) *AccountLogicalClusterReconciler { return &AccountLogicalClusterReconciler{ log: log, mclifecycle: builder.NewBuilder("security", "AccountLogicalClusterReconciler", []lifecyclesubroutine.Subroutine{ - subroutine.NewAccountTuplesSubroutine(mcc, mgr, fga, cfg.FGA.CreatorRelation, cfg.FGA.ParentRelation, cfg.FGA.ObjectType), + subroutine.NewAccountTuplesSubroutine(mcc, mgr, fgaClient, storeIDGetter, cfg.FGA.CreatorRelation, cfg.FGA.ParentRelation, cfg.FGA.ObjectType), }, log). WithReadOnly(). WithStaticThenExponentialRateLimiter(). diff --git a/internal/fga/storeid_getter.go b/internal/fga/storeid_getter.go new file mode 100644 index 00000000..f2b86ab8 --- /dev/null +++ b/internal/fga/storeid_getter.go @@ -0,0 +1,120 @@ +package fga + +import ( + "context" + "fmt" + "time" + + "github.com/jellydator/ttlcache/v3" + openfgav1 "github.com/openfga/api/proto/openfga/v1" + "github.com/platform-mesh/golang-commons/logger" + "google.golang.org/protobuf/types/known/wrapperspb" +) + +// StoreIDGetter should return the OpenFGA store ID for a store name. +type StoreIDGetter interface { + Get(ctx context.Context, storeName string) (string, error) +} + +// CachingStoreIDGetter maps store names to IDs by listing stores in OpenFGA but keeps +// a local cache to avoid frequent list calls. +type CachingStoreIDGetter struct { + cache *ttlcache.Cache[string, string] + loader *storeIDLoader + logger *logger.Logger +} + +func NewCachingStoreIDGetter(fga openfgav1.OpenFGAServiceClient, ttl time.Duration, loadCtx context.Context, log *logger.Logger) *CachingStoreIDGetter { + loader := &storeIDLoader{fga: fga, loadCtx: loadCtx} + + cache := ttlcache.New( + ttlcache.WithTTL[string, string](ttl), + ttlcache.WithLoader(loader), + ) + cache.OnInsertion(func(_ context.Context, item *ttlcache.Item[string, string]) { + log.Debug(). + Str("store", item.Key()). + Str("id", item.Value()). + Msg("StoreID cache inserted item") + }) + cache.OnUpdate(func(_ context.Context, item *ttlcache.Item[string, string]) { + log.Debug(). + Str("store", item.Key()). + Str("id", item.Value()). + Msg("StoreID cache updated item") + }) + cache.OnEviction(func(_ context.Context, reason ttlcache.EvictionReason, item *ttlcache.Item[string, string]) { + log.Debug(). + Str("store", item.Key()). + Str("id", item.Value()). + Str("reason", fmt.Sprint(reason)). + Msg("StoreID cache evicted item") + }) + + return &CachingStoreIDGetter{ + cache: cache, + loader: loader, + logger: log, + } +} + +// Get returns the store ID for the given store name. +func (m *CachingStoreIDGetter) Get(ctx context.Context, storeName string) (string, error) { + item := m.cache.Get(storeName) + if err := m.loader.Err(); err != nil { + return "", fmt.Errorf("populating cache: %w", err) + } + + if item != nil { + return item.Value(), nil + } + + return "", fmt.Errorf("store %q not found", storeName) +} + +type storeIDLoader struct { + fga openfgav1.OpenFGAServiceClient + loadErrer error + loadCtx context.Context +} + +// Load lists all stores from OpenFGA, adds them to the cache, and returns the +// requested store's item or nil if not found. Caller is supposed to check +// Err(). Implements ttlcache.Loader. +func (l *storeIDLoader) Load(c *ttlcache.Cache[string, string], storeName string) *ttlcache.Item[string, string] { + var continuationToken string + var wantedItem *ttlcache.Item[string, string] + + for { + resp, err := l.fga.ListStores(l.loadCtx, &openfgav1.ListStoresRequest{ + PageSize: wrapperspb.Int32(100), + ContinuationToken: continuationToken, + }) + if err != nil { + l.loadErrer = fmt.Errorf("listing Stores in OpenFGA: %w", err) + return nil + } + + for _, store := range resp.GetStores() { + if item := c.Set(store.GetName(), store.GetId(), ttlcache.DefaultTTL); item.Key() == storeName { + wantedItem = item + } + } + + continuationToken = resp.GetContinuationToken() + if continuationToken == "" { + break + } + } + + return wantedItem +} + +// Err returns the last error occured during Load. See [0] for why it works like +// this. +// [0] https://github.com/jellydator/ttlcache/issues/74#issuecomment-1133012806 +func (l *storeIDLoader) Err() error { + return l.loadErrer +} + +var _ StoreIDGetter = (*CachingStoreIDGetter)(nil) diff --git a/internal/fga/storeid_getter_test.go b/internal/fga/storeid_getter_test.go new file mode 100644 index 00000000..0cb65fad --- /dev/null +++ b/internal/fga/storeid_getter_test.go @@ -0,0 +1,87 @@ +package fga + +import ( + "context" + "errors" + "testing" + "time" + + openfgav1 "github.com/openfga/api/proto/openfga/v1" + "github.com/platform-mesh/golang-commons/logger/testlogger" + "github.com/platform-mesh/security-operator/internal/subroutine/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestCachingStoreIDGetter_Get(t *testing.T) { + t.Run("returns store ID from OpenFGA on cache miss", func(t *testing.T) { + client := mocks.NewMockOpenFGAServiceClient(t) + client.EXPECT().ListStores(mock.Anything, mock.Anything).Return(&openfgav1.ListStoresResponse{ + Stores: []*openfgav1.Store{ + {Name: "foo", Id: "DEADBEEF"}, + }, + }, nil).Once() + + log := testlogger.New() + getter := NewCachingStoreIDGetter(client, 5*time.Minute, context.Background(), log.Logger) + + id, err := getter.Get(context.Background(), "foo") + require.NoError(t, err) + assert.Equal(t, "DEADBEEF", id) + }) + + t.Run("returns cached value on subsequent calls without calling OpenFGA", func(t *testing.T) { + client := mocks.NewMockOpenFGAServiceClient(t) + client.EXPECT().ListStores(mock.Anything, mock.Anything).Return(&openfgav1.ListStoresResponse{ + Stores: []*openfgav1.Store{ + {Name: "foo", Id: "DEADBEEF"}, + }, + }, nil).Once() + + loadCtx := context.Background() + log := testlogger.New() + getter := NewCachingStoreIDGetter(client, 5*time.Minute, loadCtx, log.Logger) + + id1, err := getter.Get(context.Background(), "foo") + require.NoError(t, err) + assert.Equal(t, "DEADBEEF", id1) + + id2, err := getter.Get(context.Background(), "foo") + require.NoError(t, err) + assert.Equal(t, "DEADBEEF", id2) + + client.AssertExpectations(t) + }) + + t.Run("returns error when store not found in OpenFGA", func(t *testing.T) { + client := mocks.NewMockOpenFGAServiceClient(t) + client.EXPECT().ListStores(mock.Anything, mock.Anything).Return(&openfgav1.ListStoresResponse{ + Stores: []*openfgav1.Store{ + {Name: "other-store", Id: "OTHER-ID"}, + }, + }, nil).Once() + + loadCtx := context.Background() + log := testlogger.New() + getter := NewCachingStoreIDGetter(client, 5*time.Minute, loadCtx, log.Logger) + + id, err := getter.Get(context.Background(), "missing-store") + assert.Error(t, err) + assert.Contains(t, err.Error(), "store \"missing-store\" not found") + assert.Empty(t, id) + }) + + t.Run("returns error when ListStores fails", func(t *testing.T) { + client := mocks.NewMockOpenFGAServiceClient(t) + client.EXPECT().ListStores(mock.Anything, mock.Anything).Return(nil, errors.New("connection refused")).Once() + + loadCtx := context.Background() + log := testlogger.New() + getter := NewCachingStoreIDGetter(client, 5*time.Minute, loadCtx, log.Logger) + + id, err := getter.Get(context.Background(), "foo") + assert.Error(t, err) + assert.Empty(t, id) + }) +} diff --git a/pkg/fga/tuple_manager.go b/internal/fga/tuple_manager.go similarity index 100% rename from pkg/fga/tuple_manager.go rename to internal/fga/tuple_manager.go diff --git a/pkg/fga/tuple_manager_test.go b/internal/fga/tuple_manager_test.go similarity index 89% rename from pkg/fga/tuple_manager_test.go rename to internal/fga/tuple_manager_test.go index 1250f79b..bb9cf3b6 100644 --- a/pkg/fga/tuple_manager_test.go +++ b/internal/fga/tuple_manager_test.go @@ -182,7 +182,7 @@ func TestTupleManager_Delete_verifies_tuple_contents(t *testing.T) { func TestIsTupleOfAccountFilter_returnsFalseForAllTuplesWhenGeneratedClusterIdEmpty(t *testing.T) { _, ai := testAccountAndInfo("test-account", "") - filter := IsTupleOfAccountFilter(ai) + filter := IsTupleOfAccountFilter(ai.Spec.Account.GeneratedClusterId) // Any tuple should be rejected when GeneratedClusterId is empty tuples := []v1alpha1.Tuple{ @@ -198,12 +198,42 @@ func TestIsTupleOfAccountFilter_returnsFalseForAllTuplesWhenGeneratedClusterIdEm func TestIsTupleOfAccountFilter_deleteRemovesGeneratedTuples(t *testing.T) { // Use distinct GeneratedClusterIds so the filter matches only one account's tuples acc, ai := testAccountAndInfo("test-account", "1mj722nrt4jo3ggn") - accountTuples, err := InitialTuplesForAccount(acc, ai, "creator", "parent", "account") + var creator string + if acc.Spec.Creator != nil { + creator = *acc.Spec.Creator + } + accountTuples, err := InitialTuplesForAccount(InitialTuplesForAccountInput{ + BaseTuplesInput: BaseTuplesInput{ + Creator: creator, + AccountOriginClusterID: ai.Spec.Account.OriginClusterId, + AccountName: ai.Spec.Account.Name, + CreatorRelation: "creator", + ObjectType: "account", + }, + ParentOriginClusterID: ai.Spec.ParentAccount.OriginClusterId, + ParentName: ai.Spec.ParentAccount.Name, + ParentRelation: "parent", + }) require.NoError(t, err) // Tuples for a second account (should NOT be deleted when we delete test-account's tuples) acc2, ai2 := testAccountAndInfo("other-account", "1yrj2fwqtxcxbm1v") - otherTuples, err := InitialTuplesForAccount(acc2, ai2, "creator", "parent", "account") + var creator2 string + if acc2.Spec.Creator != nil { + creator2 = *acc2.Spec.Creator + } + otherTuples, err := InitialTuplesForAccount(InitialTuplesForAccountInput{ + BaseTuplesInput: BaseTuplesInput{ + Creator: creator2, + AccountOriginClusterID: ai2.Spec.Account.OriginClusterId, + AccountName: ai2.Spec.Account.Name, + CreatorRelation: "creator", + ObjectType: "account", + }, + ParentOriginClusterID: ai2.Spec.ParentAccount.OriginClusterId, + ParentName: ai2.Spec.ParentAccount.Name, + ParentRelation: "parent", + }) require.NoError(t, err) // allTuples: database managed by mocks (Write appends/deletes, Read returns current state) @@ -242,7 +272,7 @@ func TestIsTupleOfAccountFilter_deleteRemovesGeneratedTuples(t *testing.T) { require.Len(t, allTuples, len(tuplesToApply), "database should contain all applied tuples") // 2. ListWithFilter: should return only account tuples - filtered, err := mgr.ListWithFilter(context.Background(), IsTupleOfAccountFilter(ai)) + filtered, err := mgr.ListWithFilter(context.Background(), IsTupleOfAccountFilter(ai.Spec.Account.GeneratedClusterId)) require.NoError(t, err) require.Len(t, filtered, len(accountTuples), "filter should return only account tuples") diff --git a/pkg/fga/tuples.go b/internal/fga/tuples.go similarity index 53% rename from pkg/fga/tuples.go rename to internal/fga/tuples.go index 19b5195f..bf4597c8 100644 --- a/pkg/fga/tuples.go +++ b/internal/fga/tuples.go @@ -6,34 +6,51 @@ import ( "strings" openfgav1 "github.com/openfga/api/proto/openfga/v1" - accountv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" "github.com/platform-mesh/security-operator/api/v1alpha1" ) +type BaseTuplesInput struct { + Creator string + AccountOriginClusterID string + AccountName string + CreatorRelation string + ObjectType string +} + +type TuplesForOrganizationInput struct { + BaseTuplesInput +} + +type InitialTuplesForAccountInput struct { + BaseTuplesInput + ParentOriginClusterID string + ParentName string + ParentRelation string +} + // InitialTuplesForAccount returns FGA tuples for an account not of type // organization. -func InitialTuplesForAccount(acc accountv1alpha1.Account, ai accountv1alpha1.AccountInfo, creatorRelation, parentRelation, objectType string) ([]v1alpha1.Tuple, error) { - base, err := baseTuples(acc, ai, creatorRelation, objectType) +func InitialTuplesForAccount(in InitialTuplesForAccountInput) ([]v1alpha1.Tuple, error) { + base, err := baseTuples(in.BaseTuplesInput) if err != nil { return nil, err } tuples := append(base, v1alpha1.Tuple{ - User: renderAccountEntity(objectType, ai.Spec.ParentAccount.OriginClusterId, ai.Spec.ParentAccount.Name), - Relation: parentRelation, - Object: renderAccountEntity(objectType, ai.Spec.Account.OriginClusterId, ai.Spec.Account.Name), + User: renderAccountEntity(in.ObjectType, in.ParentOriginClusterID, in.ParentName), + Relation: in.ParentRelation, + Object: renderAccountEntity(in.ObjectType, in.AccountOriginClusterID, in.AccountName), }) return tuples, nil } // TuplesForOrganization returns FGA tuples for an Account of type organization. -func TuplesForOrganization(acc accountv1alpha1.Account, ai accountv1alpha1.AccountInfo, creatorRelation, objectType string) ([]v1alpha1.Tuple, error) { - return baseTuples(acc, ai, creatorRelation, objectType) +func TuplesForOrganization(in TuplesForOrganizationInput) ([]v1alpha1.Tuple, error) { + return baseTuples(in.BaseTuplesInput) } // IsTupleOfAccountFilter returns a filter determining whether a tuple is tied // to the given account, i.e. contains its cluster id. -func IsTupleOfAccountFilter(ai accountv1alpha1.AccountInfo) TupleFilter { - generatedClusterID := ai.Spec.Account.GeneratedClusterId +func IsTupleOfAccountFilter(generatedClusterID string) TupleFilter { return func(t v1alpha1.Tuple) bool { return generatedClusterID != "" && (strings.Contains(t.Object, generatedClusterID) || strings.Contains(t.User, generatedClusterID)) } @@ -41,34 +58,35 @@ func IsTupleOfAccountFilter(ai accountv1alpha1.AccountInfo) TupleFilter { // ReferencingAccountTupleKey returns a key that can be used to List tuples that // reference a given account. -func ReferencingAccountTupleKey(objectType string, ai accountv1alpha1.AccountInfo) *openfgav1.ReadRequestTupleKey { +func ReferencingAccountTupleKey(objectType, accountOriginClusterID, accountName string) *openfgav1.ReadRequestTupleKey { return &openfgav1.ReadRequestTupleKey{ - Object: renderAccountEntity(objectType, ai.Spec.Account.OriginClusterId, ai.Spec.Account.Name), + Object: renderAccountEntity(objectType, accountOriginClusterID, accountName), } } // ReferencingOwnerRoleTupleKey returns a key that can be used to List tuples // that reference the owner role of a given account. -func ReferencingOwnerRoleTupleKey(objectType string, ai accountv1alpha1.AccountInfo) *openfgav1.ReadRequestTupleKey { +func ReferencingOwnerRoleTupleKey(objectType, accountOriginClusterID, accountName string) *openfgav1.ReadRequestTupleKey { return &openfgav1.ReadRequestTupleKey{ - Object: renderOwnerRole(objectType, ai.Spec.Account.OriginClusterId, ai.Spec.Account.Name), + Object: renderOwnerRole(objectType, accountOriginClusterID, accountName), } } -func baseTuples(acc accountv1alpha1.Account, ai accountv1alpha1.AccountInfo, creatorRelation, objectType string) ([]v1alpha1.Tuple, error) { - if acc.Spec.Creator == nil { - return nil, errors.New("account creator is nil") + +func baseTuples(in BaseTuplesInput) ([]v1alpha1.Tuple, error) { + if in.Creator == "" { + return nil, errors.New("account creator is empty") } return []v1alpha1.Tuple{ { - User: renderCreatorUser(*acc.Spec.Creator), + User: renderCreatorUser(in.Creator), Relation: "assignee", - Object: renderOwnerRole(objectType, ai.Spec.Account.OriginClusterId, ai.Spec.Account.Name), + Object: renderOwnerRole(in.ObjectType, in.AccountOriginClusterID, in.AccountName), }, { - User: renderOwnerRoleAssigneeGroup(objectType, ai.Spec.Account.OriginClusterId, ai.Spec.Account.Name), - Relation: creatorRelation, - Object: renderAccountEntity(objectType, ai.Spec.Account.OriginClusterId, ai.Spec.Account.Name), + User: renderOwnerRoleAssigneeGroup(in.ObjectType, in.AccountOriginClusterID, in.AccountName), + Relation: in.CreatorRelation, + Object: renderAccountEntity(in.ObjectType, in.AccountOriginClusterID, in.AccountName), }, }, nil } diff --git a/internal/fga/tuples_test.go b/internal/fga/tuples_test.go new file mode 100644 index 00000000..4983cf19 --- /dev/null +++ b/internal/fga/tuples_test.go @@ -0,0 +1,97 @@ +package fga + +import ( + "testing" + + "github.com/platform-mesh/security-operator/api/v1alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + accountName = "one" + parentAccountName = "default" + generatedClusterID = "1mj722nrt4jo3ggn" + originClusterID = "14uc34987epvgggc" + creator = "new@example.com" + creatorRelation = "owner" + parentRelation = "parent" + objectType = "core_platform-mesh_io_account" +) + +func TestInitialTuplesForAccount(t *testing.T) { + in := InitialTuplesForAccountInput{ + BaseTuplesInput: BaseTuplesInput{ + Creator: creator, + AccountOriginClusterID: originClusterID, + AccountName: accountName, + CreatorRelation: creatorRelation, + ObjectType: objectType, + }, + ParentOriginClusterID: originClusterID, + ParentName: parentAccountName, + ParentRelation: parentRelation, + } + tuples, err := InitialTuplesForAccount(in) + require.NoError(t, err) + require.Len(t, tuples, 3) + + // Tuple 1: creator gets assignee on owner role + assert.Equal(t, v1alpha1.Tuple{ + Object: "role:core_platform-mesh_io_account/14uc34987epvgggc/one/owner", + Relation: "assignee", + User: "user:new@example.com", + }, tuples[0]) + + // Tuple 2: owner role has creator relation on account + assert.Equal(t, v1alpha1.Tuple{ + Object: "core_platform-mesh_io_account:14uc34987epvgggc/one", + Relation: "owner", + User: "role:core_platform-mesh_io_account/14uc34987epvgggc/one/owner#assignee", + }, tuples[1]) + + // Tuple 3: parent account has parent relation on account + assert.Equal(t, v1alpha1.Tuple{ + Object: "core_platform-mesh_io_account:14uc34987epvgggc/one", + Relation: "parent", + User: "core_platform-mesh_io_account:14uc34987epvgggc/default", + }, tuples[2]) +} + +func TestInitialTuplesForAccount_formatUser(t *testing.T) { + in := InitialTuplesForAccountInput{ + BaseTuplesInput: BaseTuplesInput{ + Creator: "system:serviceaccount:ns:name", + AccountOriginClusterID: originClusterID, + AccountName: accountName, + CreatorRelation: creatorRelation, + ObjectType: objectType, + }, + ParentOriginClusterID: originClusterID, + ParentName: parentAccountName, + ParentRelation: parentRelation, + } + tuples, err := InitialTuplesForAccount(in) + require.NoError(t, err) + require.Len(t, tuples, 3) + + assert.Equal(t, "user:system.serviceaccount.ns.name", tuples[0].User) +} + +func TestInitialTuplesForAccount_nilCreator(t *testing.T) { + in := InitialTuplesForAccountInput{ + BaseTuplesInput: BaseTuplesInput{ + Creator: "", + AccountOriginClusterID: originClusterID, + AccountName: accountName, + CreatorRelation: creatorRelation, + ObjectType: objectType, + }, + ParentOriginClusterID: originClusterID, + ParentName: parentAccountName, + ParentRelation: parentRelation, + } + _, err := InitialTuplesForAccount(in) + assert.Error(t, err) + assert.Contains(t, err.Error(), "creator is empty") +} diff --git a/internal/platformmesh/account_path.go b/internal/platformmesh/account_path.go new file mode 100644 index 00000000..b70f3787 --- /dev/null +++ b/internal/platformmesh/account_path.go @@ -0,0 +1,67 @@ +package platformmeshpath + +import ( + "fmt" + "strings" + + "github.com/kcp-dev/logicalcluster/v3" + kcpcore "github.com/kcp-dev/sdk/apis/core" + kcpcorev1alpha1 "github.com/kcp-dev/sdk/apis/core/v1alpha1" +) + +const ( + rootWorkspace = "root" + orgsWorkspace = "orgs" + + kcpWorkpaceSeparator = ":" +) + +// IsPlatformMeshAccountPath returns whether a value is a platform-mesh account +// path, i.e. a canonical KCP workspace path child to the platform-mesh account +// workspace tree "root:orgs". +func IsPlatformMeshAccountPath(value string) bool { + _, valid := logicalcluster.NewValidatedPath(value) + parts := strings.Split(value, kcpWorkpaceSeparator) + + return valid && len(parts) > 2 && parts[0] == rootWorkspace && parts[1] == orgsWorkspace +} + +// AccountPath represents a logicalcluster.Path that is assumed to be the path +// of a platform-mesh Account, i.e. conforms to the conditions of the +// IsPlatformMeshAccountPath function. +type AccountPath struct { + logicalcluster.Path +} + +func NewAccountPath(value string) (AccountPath, error) { + if !IsPlatformMeshAccountPath(value) { + return AccountPath{}, fmt.Errorf("%s is not a valid platform mesh path", value) + } + + return AccountPath{ + Path: logicalcluster.NewPath(value), + }, nil +} + +func NewAccountPathFromLogicalCluster(lc *kcpcorev1alpha1.LogicalCluster) (AccountPath, error) { + p, ok := lc.Annotations[kcpcore.LogicalClusterPathAnnotationKey] + if !ok { + return AccountPath{}, fmt.Errorf("LogicalCluster does not contain %s annotation", kcpcore.LogicalClusterPathAnnotationKey) + } + + return NewAccountPath(p) +} + +// IsOrg returns true if the AccountPath is an organisation. +func (a AccountPath) IsOrg() bool { + parts := strings.Split(a.String(), kcpWorkpaceSeparator) + return len(parts) == 3 +} + +// Org returns the AccountPath's parent organisation. +func (a AccountPath) Org() AccountPath { + parts := strings.Split(a.String(), kcpWorkpaceSeparator) + return AccountPath{ + Path: logicalcluster.NewPath(strings.Join(parts[:3], kcpWorkpaceSeparator)), + } +} diff --git a/internal/platformmesh/account_path_test.go b/internal/platformmesh/account_path_test.go new file mode 100644 index 00000000..9d4669f0 --- /dev/null +++ b/internal/platformmesh/account_path_test.go @@ -0,0 +1,107 @@ +package platformmeshpath + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewAccountPath(t *testing.T) { + t.Run("returns no error for org path", func(t *testing.T) { + path, err := NewAccountPath("root:orgs:default") + require.NoError(t, err) + assert.Equal(t, "root:orgs:default", path.String()) + }) + + t.Run("returns no error for account path", func(t *testing.T) { + path, err := NewAccountPath("root:orgs:default:testaccount") + require.NoError(t, err) + assert.Equal(t, "root:orgs:default:testaccount", path.String()) + }) + + t.Run("returns no error for subaccount path", func(t *testing.T) { + path, err := NewAccountPath("root:orgs:default:testaccount") + require.NoError(t, err) + assert.Equal(t, "root:orgs:default:testaccount", path.String()) + }) + + t.Run("returns error for invalid path", func(t *testing.T) { + _, err := NewAccountPath("invalid-path") + assert.Error(t, err) + assert.Contains(t, err.Error(), "not a valid platform mesh path") + }) + + t.Run("returns error for path out of orgs workspace", func(t *testing.T) { + _, err := NewAccountPath("root:platform-mesh-system") + assert.Error(t, err) + }) + + t.Run("returns error for orgs workspace", func(t *testing.T) { + _, err := NewAccountPath("root:orgs") + assert.Error(t, err) + }) +} + +func TestAccountPath_IsOrg(t *testing.T) { + t.Run("returns true for org path", func(t *testing.T) { + path, err := NewAccountPath("root:orgs:default") + require.NoError(t, err) + assert.True(t, path.IsOrg()) + }) + + t.Run("returns false for workspace path", func(t *testing.T) { + path, err := NewAccountPath("root:orgs:default:testaccount") + require.NoError(t, err) + assert.False(t, path.IsOrg()) + }) + + t.Run("returns false for nested workspace path", func(t *testing.T) { + path, err := NewAccountPath("root:orgs:default:testaccount:subaccount") + require.NoError(t, err) + assert.False(t, path.IsOrg()) + }) +} + +func TestAccountPath_Org(t *testing.T) { + t.Run("returns self for org path", func(t *testing.T) { + path, err := NewAccountPath("root:orgs:default") + require.NoError(t, err) + org := path.Org() + assert.Equal(t, "root:orgs:default", org.String()) + }) + + t.Run("returns parent org for workspace path", func(t *testing.T) { + path, err := NewAccountPath("root:orgs:default:testaccount") + require.NoError(t, err) + org := path.Org() + assert.Equal(t, "root:orgs:default", org.String()) + }) + + t.Run("returns root org for deeply nested path", func(t *testing.T) { + path, err := NewAccountPath("root:orgs:default:testaccount:subaccount") + require.NoError(t, err) + org := path.Org() + assert.Equal(t, "root:orgs:default", org.String()) + }) +} + +func TestIsPlatformMeshAccountPath(t *testing.T) { + tests := []struct { + name string + path string + expected bool + }{ + {"valid org path", "root:orgs:default", true}, + {"valid workspace path", "root:orgs:default:testaccount", true}, + {"invalid - wrong prefix", "root:other:default", false}, + {"invalid - too few segments", "root:orgs", false}, + {"invalid - single segment", "root", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := IsPlatformMeshAccountPath(tt.path) + assert.Equal(t, tt.expected, got) + }) + } +} diff --git a/internal/subroutine/account_tuples.go b/internal/subroutine/account_tuples.go index dcf3bf5d..fc3a8829 100644 --- a/internal/subroutine/account_tuples.go +++ b/internal/subroutine/account_tuples.go @@ -13,30 +13,24 @@ import ( "github.com/platform-mesh/golang-commons/logger" "github.com/platform-mesh/security-operator/api/v1alpha1" iclient "github.com/platform-mesh/security-operator/internal/client" - "github.com/platform-mesh/security-operator/pkg/fga" + "github.com/platform-mesh/security-operator/internal/fga" + platformmeshpath "github.com/platform-mesh/security-operator/internal/platformmesh" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - mccontext "sigs.k8s.io/multicluster-runtime/pkg/context" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" - kerrors "k8s.io/apimachinery/pkg/api/errors" - "github.com/kcp-dev/logicalcluster/v3" mcclient "github.com/kcp-dev/multicluster-provider/client" - kcpcore "github.com/kcp-dev/sdk/apis/core" kcpcorev1alpha1 "github.com/kcp-dev/sdk/apis/core/v1alpha1" ) -const accountTuplesTerminatorFinalizer = "core.platform-mesh.io/account-tuples-terminator" - // AccountTuplesSubroutine creates FGA tuples for Accounts not of the // "org"-type when initializing, and deletes them when terminating. type AccountTuplesSubroutine struct { - mgr mcmanager.Manager - mcc mcclient.ClusterClient - fga openfgav1.OpenFGAServiceClient - + mgr mcmanager.Manager + mcc mcclient.ClusterClient + fga openfgav1.OpenFGAServiceClient + storeIDGetter fga.StoreIDGetter objectType string parentRelation string creatorRelation string @@ -51,33 +45,63 @@ func (s *AccountTuplesSubroutine) Process(ctx context.Context, instance runtimeo // Initialize implements lifecycle.Initializer. func (s *AccountTuplesSubroutine) Initialize(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { lc := instance.(*kcpcorev1alpha1.LogicalCluster) - acc, ai, opErr := AccountAndInfoForLogicalCluster(ctx, s.mgr, lc) - if opErr != nil { - return ctrl.Result{}, opErr + + accountPath, err := platformmeshpath.NewAccountPathFromLogicalCluster(lc) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("getting AccountPath from LogicalCluster: %w", err), true, true) } - if updated := controllerutil.AddFinalizer(&ai, accountTuplesTerminatorFinalizer); updated { - lcID, ok := mccontext.ClusterFrom(ctx) - if !ok { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("cluster name not found in context"), true, true) - } + storeID, err := s.storeIDGetter.Get(ctx, accountPath.Org().Base()) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("getting store ID: %w", err), true, true) + } - lcClient, err := iclient.NewForLogicalCluster(s.mgr.GetLocalManager().GetConfig(), s.mgr.GetLocalManager().GetScheme(), logicalcluster.Name(lcID)) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("getting client: %w", err), true, true) - } + // Determine the parent's LogicalCluster ID + parentPath, _ := accountPath.Parent() + parentAccountClusterID, err := clusterIDFromLogicalClusterForPath(ctx, s.mgr, parentPath) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("getting parent account's LogicalCluster: %w", err), true, true) + } - if err := lcClient.Update(ctx, &ai); err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("updating AccountInfo to set finalizer: %w", err), true, true) - } + // Determine the grandparent's LogicalClusterID + grandParentPath, _ := parentPath.Parent() + grandParentAccountClusterID, err := clusterIDFromLogicalClusterForPath(ctx, s.mgr, grandParentPath) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("getting parent account's LogicalCluster: %w", err), true, true) } - // Ensure the necessary tuples in OpenFGA. - tuples, err := fga.InitialTuplesForAccount(acc, ai, s.creatorRelation, s.parentRelation, s.objectType) + // Retrieve the Account resource out of the parent workspace to determine + // the creator + parentAccountClient, err := iclient.NewForLogicalCluster(s.mgr.GetLocalManager().GetConfig(), s.mgr.GetLocalManager().GetScheme(), logicalcluster.Name(parentPath.String())) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("getting client for parent account cluster: %w", err), true, true) + } + var acc accountsv1alpha1.Account + if err := parentAccountClient.Get(ctx, client.ObjectKey{ + Name: accountPath.Base(), + }, &acc); err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("getting Account in parent account cluster: %w", err), true, true) + } + if acc.Spec.Creator == nil || *acc.Spec.Creator == "" { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("account creator is nil or empty"), true, true) + } + + tuples, err := fga.InitialTuplesForAccount(fga.InitialTuplesForAccountInput{ + BaseTuplesInput: fga.BaseTuplesInput{ + Creator: *acc.Spec.Creator, + AccountOriginClusterID: parentAccountClusterID, + AccountName: accountPath.Base(), + CreatorRelation: s.creatorRelation, + ObjectType: s.objectType, + }, + ParentOriginClusterID: grandParentAccountClusterID, + ParentName: parentPath.Base(), + ParentRelation: s.parentRelation, + }) if err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("building tuples for account: %w", err), true, true) } - if err := fga.NewTupleManager(s.fga, ai.Spec.FGA.Store.Id, fga.AuthorizationModelIDLatest, logger.LoadLoggerFromContext(ctx)).Apply(ctx, tuples); err != nil { + if err := fga.NewTupleManager(s.fga, storeID, fga.AuthorizationModelIDLatest, logger.LoadLoggerFromContext(ctx)).Apply(ctx, tuples); err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("applying tuples for Account: %w", err), true, true) } @@ -87,14 +111,27 @@ func (s *AccountTuplesSubroutine) Initialize(ctx context.Context, instance runti // Terminate implements lifecycle.Terminator. func (s *AccountTuplesSubroutine) Terminate(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, errors.OperatorError) { lc := instance.(*kcpcorev1alpha1.LogicalCluster) - _, ai, opErr := AccountAndInfoForLogicalCluster(ctx, s.mgr, lc) - if opErr != nil { - return ctrl.Result{}, opErr + + accountPath, err := platformmeshpath.NewAccountPathFromLogicalCluster(lc) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("getting AccountPath from LogicalCluster: %w", err), true, true) + } + parentPath, _ := accountPath.Parent() + + // Determine the parent's LogicalClusterID + parentClusterID, err := clusterIDFromLogicalClusterForPath(ctx, s.mgr, parentPath) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("getting parent account's LogicalCluster: %w", err), true, true) + } + + storeID, err := s.storeIDGetter.Get(ctx, accountPath.Org().Base()) + if err != nil { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("getting store ID: %w", err), true, true) } // List tuples that reference the account. - tm := fga.NewTupleManager(s.fga, ai.Spec.FGA.Store.Id, fga.AuthorizationModelIDLatest, logger.LoadLoggerFromContext(ctx)) - accountReferenceTuples, err := tm.ListWithKey(ctx, fga.ReferencingAccountTupleKey(s.objectType, ai)) + tm := fga.NewTupleManager(s.fga, storeID, fga.AuthorizationModelIDLatest, logger.LoadLoggerFromContext(ctx)) + accountReferenceTuples, err := tm.ListWithKey(ctx, fga.ReferencingAccountTupleKey(s.objectType, parentClusterID, accountPath.Base())) if err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("listing tuples referencing Account: %w", err), true, true) } @@ -102,7 +139,7 @@ func (s *AccountTuplesSubroutine) Terminate(ctx context.Context, instance runtim accountTuples = append(accountTuples, accountReferenceTuples...) // From tuples referencing the account, parse potential roles specific to the account. - rolePrefix := fga.RenderRolePrefix(s.objectType, ai.Spec.Account.OriginClusterId, ai.Spec.Account.Name) + rolePrefix := fga.RenderRolePrefix(s.objectType, parentClusterID, accountPath.Base()) for _, t := range accountReferenceTuples { if strings.HasPrefix(t.User, rolePrefix) { role := strings.TrimSuffix(t.User, "#assignee") @@ -119,23 +156,6 @@ func (s *AccountTuplesSubroutine) Terminate(ctx context.Context, instance runtim return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("deleting tuples for Account: %w", err), true, true) } - // Remove finalizer from AccountInfo. - if updated := controllerutil.RemoveFinalizer(&ai, accountTuplesTerminatorFinalizer); updated { - lcID, ok := mccontext.ClusterFrom(ctx) - if !ok { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("cluster name not found in context"), true, true) - } - - lcClient, err := iclient.NewForLogicalCluster(s.mgr.GetLocalManager().GetConfig(), s.mgr.GetLocalManager().GetScheme(), logicalcluster.Name(lcID)) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("getting client: %w", err), true, true) - } - - if err := lcClient.Update(ctx, &ai); err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("updating AccountInfo to remove finalizer: %w", err), true, true) - } - } - return ctrl.Result{}, nil } @@ -152,11 +172,12 @@ func (s *AccountTuplesSubroutine) Finalizers(_ runtimeobject.RuntimeObject) []st // GetName implements lifecycle.Subroutine. func (s *AccountTuplesSubroutine) GetName() string { return "AccountTuplesSubroutine" } -func NewAccountTuplesSubroutine(mcc mcclient.ClusterClient, mgr mcmanager.Manager, fga openfgav1.OpenFGAServiceClient, creatorRelation, parentRelation, objectType string) *AccountTuplesSubroutine { +func NewAccountTuplesSubroutine(mcc mcclient.ClusterClient, mgr mcmanager.Manager, fga openfgav1.OpenFGAServiceClient, storeIDGetter fga.StoreIDGetter, creatorRelation, parentRelation, objectType string) *AccountTuplesSubroutine { return &AccountTuplesSubroutine{ mgr: mgr, mcc: mcc, fga: fga, + storeIDGetter: storeIDGetter, creatorRelation: creatorRelation, parentRelation: parentRelation, objectType: objectType, @@ -169,45 +190,25 @@ var ( _ lifecyclesubroutine.Terminator = &AccountTuplesSubroutine{} ) -// AccountAndInfoForLogicalCluster fetches the AccountInfo from the -// LogicalCluster and the corresponding Account from the parent account's -// workspace. -func AccountAndInfoForLogicalCluster(ctx context.Context, mgr mcmanager.Manager, lc *kcpcorev1alpha1.LogicalCluster) (accountsv1alpha1.Account, accountsv1alpha1.AccountInfo, errors.OperatorError) { - if lc.Annotations[kcpcore.LogicalClusterPathAnnotationKey] == "" { - return accountsv1alpha1.Account{}, accountsv1alpha1.AccountInfo{}, errors.NewOperatorError(fmt.Errorf("annotation on LogicalCluster is not set"), true, true) - } - lcID, ok := mccontext.ClusterFrom(ctx) - if !ok { - return accountsv1alpha1.Account{}, accountsv1alpha1.AccountInfo{}, errors.NewOperatorError(fmt.Errorf("cluster name not found in context"), true, true) - } +// clusterIDFromLogicalClusterForPath retrieves the LogicalCluster of a given +// path and returns its cluster ID. +func clusterIDFromLogicalClusterForPath(ctx context.Context, mgr mcmanager.Manager, p logicalcluster.Path) (string, error) { + var lc kcpcorev1alpha1.LogicalCluster - // The AccountInfo in the logical cluster belongs to the Account the - // Workspace was created for - lcClient, err := iclient.NewForLogicalCluster(mgr.GetLocalManager().GetConfig(), mgr.GetLocalManager().GetScheme(), logicalcluster.Name(lcID)) + clusterClient, err := iclient.NewForLogicalCluster(mgr.GetLocalManager().GetConfig(), mgr.GetLocalManager().GetScheme(), logicalcluster.Name(p.String())) if err != nil { - return accountsv1alpha1.Account{}, accountsv1alpha1.AccountInfo{}, errors.NewOperatorError(fmt.Errorf("getting client: %w", err), true, true) + return "", fmt.Errorf("getting account cluster client: %w", err) } - var ai accountsv1alpha1.AccountInfo - if err := lcClient.Get(ctx, client.ObjectKey{ - Name: "account", - }, &ai); err != nil && !kerrors.IsNotFound(err) { - return accountsv1alpha1.Account{}, accountsv1alpha1.AccountInfo{}, errors.NewOperatorError(fmt.Errorf("getting AccountInfo for LogicalCluster: %w", err), true, true) - } else if kerrors.IsNotFound(err) { - return accountsv1alpha1.Account{}, accountsv1alpha1.AccountInfo{}, errors.NewOperatorError(fmt.Errorf("AccountInfo not found"), true, true) + if err := clusterClient.Get(ctx, client.ObjectKey{ + Name: "cluster", + }, &lc); err != nil { + return "", fmt.Errorf("getting account's LogicalCluster: %w", err) } - // The actual Account resource belonging to the Workspace needs to be - // fetched from the parent Account's Workspace - parentAccountClient, err := iclient.NewForLogicalCluster(mgr.GetLocalManager().GetConfig(), mgr.GetLocalManager().GetScheme(), logicalcluster.Name(ai.Spec.ParentAccount.Path)) - if err != nil { - return accountsv1alpha1.Account{}, accountsv1alpha1.AccountInfo{}, errors.NewOperatorError(fmt.Errorf("getting parent account cluster client: %w", err), true, true) - } - var acc accountsv1alpha1.Account - if err := parentAccountClient.Get(ctx, client.ObjectKey{ - Name: ai.Spec.Account.Name, - }, &acc); err != nil { - return accountsv1alpha1.Account{}, accountsv1alpha1.AccountInfo{}, errors.NewOperatorError(fmt.Errorf("getting Account in parent account cluster: %w", err), true, true) + clusterID, ok := lc.Annotations["kcp.io/cluster"] + if !ok || clusterID == "" { + return "", fmt.Errorf("cluster-annotation kcp.io/cluster on LogicalCluster is not set") } - return acc, ai, nil + return clusterID, nil } diff --git a/internal/subroutine/tuples.go b/internal/subroutine/tuples.go index 4b18b6d1..fe73672c 100644 --- a/internal/subroutine/tuples.go +++ b/internal/subroutine/tuples.go @@ -11,7 +11,7 @@ import ( "github.com/platform-mesh/golang-commons/errors" "github.com/platform-mesh/golang-commons/logger" securityv1alpha1 "github.com/platform-mesh/security-operator/api/v1alpha1" - "github.com/platform-mesh/security-operator/pkg/fga" + "github.com/platform-mesh/security-operator/internal/fga" ctrl "sigs.k8s.io/controller-runtime" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" diff --git a/internal/subroutine/workspace_initializer.go b/internal/subroutine/workspace_initializer.go index dba9b64c..b0adfcc5 100644 --- a/internal/subroutine/workspace_initializer.go +++ b/internal/subroutine/workspace_initializer.go @@ -14,7 +14,7 @@ import ( "github.com/platform-mesh/security-operator/api/v1alpha1" iclient "github.com/platform-mesh/security-operator/internal/client" "github.com/platform-mesh/security-operator/internal/config" - "github.com/platform-mesh/security-operator/pkg/fga" + "github.com/platform-mesh/security-operator/internal/fga" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -119,7 +119,18 @@ func (w *workspaceInitializer) Initialize(ctx context.Context, instance runtimeo ObjectMeta: metav1.ObjectMeta{Name: generateStoreName(lc)}, } - tuples, err := fga.TuplesForOrganization(acc, ai, w.creatorRelation, w.objectType) + if acc.Spec.Creator == nil || *acc.Spec.Creator == "" { + return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("account creator is nil or empty"), true, true) + } + tuples, err := fga.TuplesForOrganization(fga.TuplesForOrganizationInput{ + BaseTuplesInput: fga.BaseTuplesInput{ + Creator: *acc.Spec.Creator, + AccountOriginClusterID: ai.Spec.Account.OriginClusterId, + AccountName: ai.Spec.Account.Name, + CreatorRelation: w.creatorRelation, + ObjectType: w.objectType, + }, + }) if err != nil { return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("building tuples for organization: %w", err), true, true) } @@ -162,22 +173,6 @@ func (w *workspaceInitializer) Initialize(ctx context.Context, instance runtimeo return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("store id is empty"), true, false) } - cluster, 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) - } - - accountInfo := accountsv1alpha1.AccountInfo{ - ObjectMeta: metav1.ObjectMeta{Name: "account"}, - } - _, err = controllerutil.CreateOrUpdate(ctx, cluster.GetClient(), &accountInfo, func() error { - accountInfo.Spec.FGA.Store.Id = store.Status.StoreID - return nil - }) - if err != nil { - return ctrl.Result{}, errors.NewOperatorError(fmt.Errorf("unable to create/update accountInfo: %w", err), true, true) - } - return ctrl.Result{}, nil } diff --git a/pkg/fga/tuples_test.go b/pkg/fga/tuples_test.go deleted file mode 100644 index 8c0722ea..00000000 --- a/pkg/fga/tuples_test.go +++ /dev/null @@ -1,128 +0,0 @@ -package fga - -import ( - "testing" - - accountv1alpha1 "github.com/platform-mesh/account-operator/api/v1alpha1" - "github.com/platform-mesh/security-operator/api/v1alpha1" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -const ( - accountName = "one" - accountInfoName = "account" - parentAccountName = "default" - generatedClusterID = "1mj722nrt4jo3ggn" - originClusterID = "14uc34987epvgggc" - creator = "new@example.com" - creatorRelation = "owner" - parentRelation = "parent" - objectType = "core_platform-mesh_io_account" -) - -func TestInitialTuplesForAccount(t *testing.T) { - creatorVal := creator - acc := accountv1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{Name: accountName}, - Spec: accountv1alpha1.AccountSpec{ - Creator: &creatorVal, - }, - } - ai := accountv1alpha1.AccountInfo{ - ObjectMeta: metav1.ObjectMeta{Name: accountInfoName}, - Spec: accountv1alpha1.AccountInfoSpec{ - Account: accountv1alpha1.AccountLocation{ - Name: accountName, - GeneratedClusterId: generatedClusterID, - OriginClusterId: originClusterID, - }, - ParentAccount: &accountv1alpha1.AccountLocation{ - Name: parentAccountName, - OriginClusterId: originClusterID, - }, - }, - } - - tuples, err := InitialTuplesForAccount(acc, ai, creatorRelation, parentRelation, objectType) - require.NoError(t, err) - require.Len(t, tuples, 3) - - // Tuple 1: creator gets assignee on owner role - assert.Equal(t, v1alpha1.Tuple{ - Object: "role:core_platform-mesh_io_account/14uc34987epvgggc/one/owner", - Relation: "assignee", - User: "user:new@example.com", - }, tuples[0]) - - // Tuple 2: owner role has creator relation on account - assert.Equal(t, v1alpha1.Tuple{ - Object: "core_platform-mesh_io_account:14uc34987epvgggc/one", - Relation: "owner", - User: "role:core_platform-mesh_io_account/14uc34987epvgggc/one/owner#assignee", - }, tuples[1]) - - // Tuple 3: parent account has parent relation on account - assert.Equal(t, v1alpha1.Tuple{ - Object: "core_platform-mesh_io_account:14uc34987epvgggc/one", - Relation: "parent", - User: "core_platform-mesh_io_account:14uc34987epvgggc/default", - }, tuples[2]) -} - -func TestInitialTuplesForAccount_formatUser(t *testing.T) { - creator := "system:serviceaccount:ns:name" - acc := accountv1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{Name: accountName}, - Spec: accountv1alpha1.AccountSpec{ - Creator: &creator, - }, - } - ai := accountv1alpha1.AccountInfo{ - ObjectMeta: metav1.ObjectMeta{Name: accountInfoName}, - Spec: accountv1alpha1.AccountInfoSpec{ - Account: accountv1alpha1.AccountLocation{ - Name: accountName, - GeneratedClusterId: generatedClusterID, - OriginClusterId: originClusterID, - }, - ParentAccount: &accountv1alpha1.AccountLocation{ - Name: parentAccountName, - OriginClusterId: originClusterID, - }, - }, - } - - tuples, err := InitialTuplesForAccount(acc, ai, creatorRelation, parentRelation, objectType) - require.NoError(t, err) - require.Len(t, tuples, 3) - - assert.Equal(t, "user:system.serviceaccount.ns.name", tuples[0].User) -} - -func TestInitialTuplesForAccount_nilCreator(t *testing.T) { - acc := accountv1alpha1.Account{ - ObjectMeta: metav1.ObjectMeta{Name: accountName}, - Spec: accountv1alpha1.AccountSpec{}, - } - ai := accountv1alpha1.AccountInfo{ - ObjectMeta: metav1.ObjectMeta{Name: accountInfoName}, - Spec: accountv1alpha1.AccountInfoSpec{ - Account: accountv1alpha1.AccountLocation{ - Name: accountName, - GeneratedClusterId: generatedClusterID, - OriginClusterId: originClusterID, - }, - ParentAccount: &accountv1alpha1.AccountLocation{ - Name: parentAccountName, - OriginClusterId: originClusterID, - }, - }, - } - - _, err := InitialTuplesForAccount(acc, ai, creatorRelation, parentRelation, objectType) - assert.Error(t, err) - assert.Contains(t, err.Error(), "creator is nil") -}