From 844a7332b9032fed016e9aaf7076eebbf2f6460c Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Tue, 6 Jan 2026 16:19:25 -0700 Subject: [PATCH] Zanzana: Add orphan cleanup to reconciler (#115775) --- .../dualwrite/resource_reconciler.go | 123 +++++++++++++++++- .../resource_reconciler_orphan_test.go | 110 ++++++++++++++++ 2 files changed, 227 insertions(+), 6 deletions(-) create mode 100644 pkg/services/accesscontrol/dualwrite/resource_reconciler_orphan_test.go diff --git a/pkg/services/accesscontrol/dualwrite/resource_reconciler.go b/pkg/services/accesscontrol/dualwrite/resource_reconciler.go index 63a4f8b25eb..0adf365ebde 100644 --- a/pkg/services/accesscontrol/dualwrite/resource_reconciler.go +++ b/pkg/services/accesscontrol/dualwrite/resource_reconciler.go @@ -3,11 +3,13 @@ package dualwrite import ( "context" "fmt" + "strings" openfgav1 "github.com/openfga/api/proto/openfga/v1" claims "github.com/grafana/authlib/types" + dashboardV1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" "github.com/grafana/grafana/pkg/services/authz/zanzana" ) @@ -19,14 +21,30 @@ type legacyTupleCollector func(ctx context.Context, orgID int64) (map[string]map type zanzanaTupleCollector func(ctx context.Context, client zanzana.Client, object string, namespace string) (map[string]*openfgav1.TupleKey, error) type resourceReconciler struct { - name string - legacy legacyTupleCollector - zanzana zanzanaTupleCollector - client zanzana.Client + name string + legacy legacyTupleCollector + zanzana zanzanaTupleCollector + client zanzana.Client + orphanObjectPrefix string + orphanRelations []string } -func newResourceReconciler(name string, legacy legacyTupleCollector, zanzana zanzanaTupleCollector, client zanzana.Client) resourceReconciler { - return resourceReconciler{name, legacy, zanzana, client} +func newResourceReconciler(name string, legacy legacyTupleCollector, zanzanaCollector zanzanaTupleCollector, client zanzana.Client) resourceReconciler { + r := resourceReconciler{name: name, legacy: legacy, zanzana: zanzanaCollector, client: client} + + // we only need to worry about orphaned tuples for reconcilers that use the managed permissions collector (i.e. dashboards & folders) + switch name { + case "managed folder permissions": + // prefix for folders is `folder:` + r.orphanObjectPrefix = zanzana.NewObjectEntry(zanzana.TypeFolder, "", "", "", "") + r.orphanRelations = append([]string{}, zanzana.RelationsFolder...) + case "managed dashboard permissions": + // prefix for dashboards will be `resource:dashboard.grafana.app/dashboards/` + r.orphanObjectPrefix = fmt.Sprintf("%s/", zanzana.NewObjectEntry(zanzana.TypeResource, dashboardV1.APIGroup, dashboardV1.DASHBOARD_RESOURCE, "", "")) + r.orphanRelations = append([]string{}, zanzana.RelationsResouce...) + } + + return r } func (r resourceReconciler) reconcile(ctx context.Context, namespace string) error { @@ -35,6 +53,15 @@ func (r resourceReconciler) reconcile(ctx context.Context, namespace string) err return err } + // 0. Fetch all tuples currently stored in Zanzana. This will be used later on + // to cleanup orphaned tuples. + // This order needs to be kept (fetching from Zanzana first) to avoid accidentally + // cleaning up new tuples that were added after the legacy tuples were fetched. + allTuplesInZanzana, err := r.readAllTuples(ctx, namespace) + if err != nil { + return fmt.Errorf("failed to read all tuples from zanzana for %s: %w", r.name, err) + } + // 1. Fetch grafana resources stored in grafana db. res, err := r.legacy(ctx, info.OrgID) if err != nil { @@ -87,6 +114,14 @@ func (r resourceReconciler) reconcile(ctx context.Context, namespace string) err } } + // when the last managed permission for a resource is removed, the legacy results will no + // longer contain any tuples for that resource. this process cleans it up when applicable. + orphans, err := r.collectOrphanDeletes(ctx, namespace, allTuplesInZanzana, res) + if err != nil { + return fmt.Errorf("failed to collect orphan deletes (%s): %w", r.name, err) + } + deletes = append(deletes, orphans...) + if len(writes) == 0 && len(deletes) == 0 { return nil } @@ -119,3 +154,79 @@ func (r resourceReconciler) reconcile(ctx context.Context, namespace string) err return nil } + +// collectOrphanDeletes collects tuples that are no longer present in the legacy results +// but still are present in zanzana. when that is the case, we need to delete the tuple from +// zanzana. this will happen when the last managed permission for a resource is removed. +// this is only used for dashboards and folders, as those are the only resources that use the managed permissions collector. +func (r resourceReconciler) collectOrphanDeletes( + ctx context.Context, + namespace string, + allTuplesInZanzana []*authzextv1.Tuple, + legacyReturnedTuples map[string]map[string]*openfgav1.TupleKey, +) ([]*openfgav1.TupleKeyWithoutCondition, error) { + if r.orphanObjectPrefix == "" || len(r.orphanRelations) == 0 { + return []*openfgav1.TupleKeyWithoutCondition{}, nil + } + + seen := map[string]struct{}{} + out := []*openfgav1.TupleKeyWithoutCondition{} + + // what relation types we are interested in cleaning up + relationsToCleanup := map[string]struct{}{} + for _, rel := range r.orphanRelations { + relationsToCleanup[rel] = struct{}{} + } + + for _, tuple := range allTuplesInZanzana { + if tuple == nil || tuple.Key == nil { + continue + } + // only cleanup the particular relation types we are interested in + if _, ok := relationsToCleanup[tuple.Key.Relation]; !ok { + continue + } + // only cleanup the particular object types we are interested in (either dashboards or folders) + if !strings.HasPrefix(tuple.Key.Object, r.orphanObjectPrefix) { + continue + } + // if legacy returned this object, it's not orphaned + if _, ok := legacyReturnedTuples[tuple.Key.Object]; ok { + continue + } + // keep track of the tuples we have already seen and marked for deletion + key := fmt.Sprintf("%s|%s|%s", tuple.Key.User, tuple.Key.Relation, tuple.Key.Object) + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + out = append(out, &openfgav1.TupleKeyWithoutCondition{ + User: tuple.Key.User, + Relation: tuple.Key.Relation, + Object: tuple.Key.Object, + }) + } + + return out, nil +} + +func (r resourceReconciler) readAllTuples(ctx context.Context, namespace string) ([]*authzextv1.Tuple, error) { + var ( + out []*authzextv1.Tuple + continueToken string + ) + for { + res, err := r.client.Read(ctx, &authzextv1.ReadRequest{ + Namespace: namespace, + ContinuationToken: continueToken, + }) + if err != nil { + return nil, err + } + out = append(out, res.Tuples...) + continueToken = res.ContinuationToken + if continueToken == "" { + return out, nil + } + } +} diff --git a/pkg/services/accesscontrol/dualwrite/resource_reconciler_orphan_test.go b/pkg/services/accesscontrol/dualwrite/resource_reconciler_orphan_test.go new file mode 100644 index 00000000000..8daf06a8a77 --- /dev/null +++ b/pkg/services/accesscontrol/dualwrite/resource_reconciler_orphan_test.go @@ -0,0 +1,110 @@ +package dualwrite + +import ( + "context" + "testing" + + authlib "github.com/grafana/authlib/types" + openfgav1 "github.com/openfga/api/proto/openfga/v1" + "github.com/stretchr/testify/require" + + authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" + "github.com/grafana/grafana/pkg/services/authz/zanzana" +) + +type fakeZanzanaClient struct { + readTuples []*authzextv1.Tuple + writeReqs []*authzextv1.WriteRequest +} + +func (f *fakeZanzanaClient) Read(ctx context.Context, req *authzextv1.ReadRequest) (*authzextv1.ReadResponse, error) { + return &authzextv1.ReadResponse{ + Tuples: f.readTuples, + ContinuationToken: "", + }, nil +} + +func (f *fakeZanzanaClient) Write(ctx context.Context, req *authzextv1.WriteRequest) error { + f.writeReqs = append(f.writeReqs, req) + return nil +} + +func (f *fakeZanzanaClient) BatchCheck(ctx context.Context, req *authzextv1.BatchCheckRequest) (*authzextv1.BatchCheckResponse, error) { + return &authzextv1.BatchCheckResponse{}, nil +} + +func (f *fakeZanzanaClient) Mutate(ctx context.Context, req *authzextv1.MutateRequest) error { + return nil +} + +func (f *fakeZanzanaClient) Query(ctx context.Context, req *authzextv1.QueryRequest) (*authzextv1.QueryResponse, error) { + return &authzextv1.QueryResponse{}, nil +} + +func (f *fakeZanzanaClient) Check(ctx context.Context, info authlib.AuthInfo, req authlib.CheckRequest, folder string) (authlib.CheckResponse, error) { + return authlib.CheckResponse{Allowed: true}, nil +} + +func (f *fakeZanzanaClient) Compile(ctx context.Context, info authlib.AuthInfo, req authlib.ListRequest) (authlib.ItemChecker, authlib.Zookie, error) { + return func(name, folder string) bool { return true }, authlib.NoopZookie{}, nil +} + +func TestResourceReconciler_OrphanedManagedDashboardTuplesAreDeleted(t *testing.T) { + legacy := func(ctx context.Context, orgID int64) (map[string]map[string]*openfgav1.TupleKey, error) { + return map[string]map[string]*openfgav1.TupleKey{}, nil + } + zCollector := func(ctx context.Context, client zanzana.Client, object string, namespace string) (map[string]*openfgav1.TupleKey, error) { + return map[string]*openfgav1.TupleKey{}, nil + } + + fake := &fakeZanzanaClient{} + r := newResourceReconciler("managed dashboard permissions", legacy, zCollector, fake) + + require.NotEmpty(t, r.orphanObjectPrefix) + require.NotEmpty(t, r.orphanRelations) + + relAllowed := r.orphanRelations[0] + objAllowed := r.orphanObjectPrefix + "dash-uid-1" + + fake.readTuples = []*authzextv1.Tuple{ + // should be removed + { + Key: &authzextv1.TupleKey{ + User: "user:1", + Relation: relAllowed, + Object: objAllowed, + }, + }, + + // same relation but different object type/prefix - should stay + { + Key: &authzextv1.TupleKey{ + User: "user:1", + Relation: relAllowed, + Object: "folder:some-folder", + }, + }, + // same prefix but different relation - should stay + { + Key: &authzextv1.TupleKey{ + User: "user:1", + Relation: zanzana.RelationParent, + Object: objAllowed, + }, + }, + } + + err := r.reconcile(context.Background(), authlib.OrgNamespaceFormatter(1)) + require.NoError(t, err) + + require.Len(t, fake.writeReqs, 1) + wr := fake.writeReqs[0] + require.NotNil(t, wr.Deletes) + require.Nil(t, wr.Writes) + + require.Len(t, wr.Deletes.TupleKeys, 1) + del := wr.Deletes.TupleKeys[0] + require.Equal(t, "user:1", del.User) + require.Equal(t, relAllowed, del.Relation) + require.Equal(t, objAllowed, del.Object) +}