From fb5783691df348b2f8fc28ea9aeb189a9c9e08aa Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Thu, 9 Jan 2025 10:40:18 +0100 Subject: [PATCH] Zanzana: Fix reconciliation of fixed roles (#98696) Remove "globalReconciler" and reuse the same one but only run them for cluster namespace --- .../accesscontrol/dualwrite/collectors.go | 16 ++- .../dualwrite/global_reconciler.go | 112 ------------------ .../accesscontrol/dualwrite/reconciler.go | 13 +- pkg/services/authz/zanzana/zanzana.go | 2 +- 4 files changed, 22 insertions(+), 121 deletions(-) delete mode 100644 pkg/services/accesscontrol/dualwrite/global_reconciler.go diff --git a/pkg/services/accesscontrol/dualwrite/collectors.go b/pkg/services/accesscontrol/dualwrite/collectors.go index 929c0e2a062..c5fee947a73 100644 --- a/pkg/services/accesscontrol/dualwrite/collectors.go +++ b/pkg/services/accesscontrol/dualwrite/collectors.go @@ -414,8 +414,8 @@ func rolePermissionsCollector(store db.DB) legacyTupleCollector { } } -func fixedRolePermissionsCollector(store db.DB) globalTupleCollector { - return func(ctx context.Context) (map[string]map[string]*openfgav1.TupleKey, error) { +func fixedRolePermissionsCollector(store db.DB) legacyTupleCollector { + return func(ctx context.Context, _ int64) (map[string]map[string]*openfgav1.TupleKey, error) { var query = ` SELECT r.uid as role_uid, p.action, p.kind, p.identifier FROM permission p @@ -457,6 +457,18 @@ func fixedRolePermissionsCollector(store db.DB) globalTupleCollector { tuples[tuple.Object] = make(map[string]*openfgav1.TupleKey) } + // For resource actions on folders we need to merge the tuples into one with combined + // group_resources. + if zanzana.IsFolderResourceTuple(tuple) { + key := tupleStringWithoutCondition(tuple) + if t, ok := tuples[tuple.Object][key]; ok { + zanzana.MergeFolderResourceTuples(t, tuple) + } else { + tuples[tuple.Object][key] = tuple + } + continue + } + tuples[tuple.Object][tuple.String()] = tuple } diff --git a/pkg/services/accesscontrol/dualwrite/global_reconciler.go b/pkg/services/accesscontrol/dualwrite/global_reconciler.go deleted file mode 100644 index 04b381cffa6..00000000000 --- a/pkg/services/accesscontrol/dualwrite/global_reconciler.go +++ /dev/null @@ -1,112 +0,0 @@ -package dualwrite - -import ( - "context" - "fmt" - - openfgav1 "github.com/openfga/api/proto/openfga/v1" - - authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" - "github.com/grafana/grafana/pkg/services/authz/zanzana" -) - -type globalTupleCollector func(ctx context.Context) (map[string]map[string]*openfgav1.TupleKey, error) - -type globalReconciler struct { - name string - globalCollector globalTupleCollector - zanzana zanzanaTupleCollector - client zanzana.Client -} - -func newGlobalReconciler(name string, globalCollector globalTupleCollector, zanzana zanzanaTupleCollector, client zanzana.Client) globalReconciler { - return globalReconciler{name, globalCollector, zanzana, client} -} - -func (r globalReconciler) reconcile(ctx context.Context) error { - namespace := zanzana.ClusterNamespace - - // 1. Fetch grafana resources stored in grafana db. - res, err := r.globalCollector(ctx) - if err != nil { - return fmt.Errorf("failed to collect legacy tuples for %s: %w", r.name, err) - } - - var ( - writes = []*openfgav1.TupleKey{} - deletes = []*openfgav1.TupleKeyWithoutCondition{} - ) - - for object, tuples := range res { - // 2. Fetch all tuples for given object. - // Due to limitations in open fga api we need to collect tuples per object - zanzanaTuples, err := r.zanzana(ctx, r.client, object, namespace) - if err != nil { - return fmt.Errorf("failed to collect zanzanaa tuples for %s: %w", r.name, err) - } - - // 3. Check if tuples from grafana db exists in zanzana and if not add them to writes - for key, t := range tuples { - stored, ok := zanzanaTuples[key] - if !ok { - writes = append(writes, t) - continue - } - - // 4. For folder resource tuples we also need to compare the stored group_resources - if zanzana.IsFolderResourceTuple(t) && t.String() != stored.String() { - deletes = append(deletes, &openfgav1.TupleKeyWithoutCondition{ - User: t.User, - Relation: t.Relation, - Object: t.Object, - }) - - writes = append(writes, t) - } - } - - // 5. Check if tuple from zanzana don't exists in grafana db, if not add them to deletes. - for key, tuple := range zanzanaTuples { - _, ok := tuples[key] - if !ok { - deletes = append(deletes, &openfgav1.TupleKeyWithoutCondition{ - User: tuple.User, - Relation: tuple.Relation, - Object: tuple.Object, - }) - } - } - } - - if len(writes) == 0 && len(deletes) == 0 { - return nil - } - - if len(deletes) > 0 { - err := batch(deletes, 100, func(items []*openfgav1.TupleKeyWithoutCondition) error { - return r.client.Write(ctx, &authzextv1.WriteRequest{ - Namespace: namespace, - Deletes: &authzextv1.WriteRequestDeletes{TupleKeys: zanzana.ToAuthzExtTupleKeysWithoutCondition(items)}, - }) - }) - - if err != nil { - return err - } - } - - if len(writes) > 0 { - err := batch(writes, 100, func(items []*openfgav1.TupleKey) error { - return r.client.Write(ctx, &authzextv1.WriteRequest{ - Namespace: namespace, - Writes: &authzextv1.WriteRequestWrites{TupleKeys: zanzana.ToAuthzExtTupleKeys(items)}, - }) - }) - - if err != nil { - return err - } - } - - return nil -} diff --git a/pkg/services/accesscontrol/dualwrite/reconciler.go b/pkg/services/accesscontrol/dualwrite/reconciler.go index 88461ed17bf..ac2c979c4d6 100644 --- a/pkg/services/accesscontrol/dualwrite/reconciler.go +++ b/pkg/services/accesscontrol/dualwrite/reconciler.go @@ -32,8 +32,9 @@ type ZanzanaReconciler struct { lock *serverlock.ServerLockService // reconcilers are migrations that tries to reconcile the state of grafana db to zanzana store. // These are run periodically to try to maintain a consistent state. - reconcilers []resourceReconciler - globalReconcilers []globalReconciler + reconcilers []resourceReconciler + // globalReconcilers are reconcilers that should only run for cluster namespace + globalReconcilers []resourceReconciler } func NewZanzanaReconciler(cfg *setting.Cfg, client zanzana.Client, store db.DB, lock *serverlock.ServerLockService, folderService folder.Service) *ZanzanaReconciler { @@ -93,11 +94,11 @@ func NewZanzanaReconciler(cfg *setting.Cfg, client zanzana.Client, store db.DB, client, ), }, - globalReconcilers: []globalReconciler{ - newGlobalReconciler( + globalReconcilers: []resourceReconciler{ + newResourceReconciler( "fixed role pemissions", fixedRolePermissionsCollector(store), - zanzanaCollector([]string{zanzana.RelationAssignee}), + zanzanaCollector(zanzana.RelationsFolder), client, ), }, @@ -146,7 +147,7 @@ func (r *ZanzanaReconciler) reconcile(ctx context.Context) { runGlobal := func(ctx context.Context) { for _, reconciler := range r.globalReconcilers { r.log.Debug("Performing zanzana reconciliation", "reconciler", reconciler.name) - if err := reconciler.reconcile(ctx); err != nil { + if err := reconciler.reconcile(ctx, zanzana.ClusterNamespace); err != nil { r.log.Warn("Failed to perform reconciliation for resource", "err", err) } } diff --git a/pkg/services/authz/zanzana/zanzana.go b/pkg/services/authz/zanzana/zanzana.go index 696f75c1e82..26b82d0151a 100644 --- a/pkg/services/authz/zanzana/zanzana.go +++ b/pkg/services/authz/zanzana/zanzana.go @@ -49,8 +49,8 @@ const ( var ( RelationsFolder = common.RelationsFolder - RelationsFolderResource = common.RelationsFolder RelationsResouce = common.RelationsResource + RelationsFolderResource = common.RelationsFolderResource ) const (