Zanzana: Fix duplicated writes in one request (#114900)
* Zanzana: Fix duplicated writes * add tests
This commit is contained in:
@@ -6,8 +6,10 @@ import (
|
||||
"fmt"
|
||||
"time"
|
||||
|
||||
authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1"
|
||||
openfgav1 "github.com/openfga/api/proto/openfga/v1"
|
||||
"go.opentelemetry.io/otel/codes"
|
||||
|
||||
authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1"
|
||||
)
|
||||
|
||||
type OperationGroup string
|
||||
@@ -119,3 +121,65 @@ func groupByOperation(operations []*authzextv1.MutateOperation) (map[OperationGr
|
||||
|
||||
return grouped, nil
|
||||
}
|
||||
|
||||
func deduplicateTupleKeys(writeTuples []*openfgav1.TupleKey, deleteTuples []*openfgav1.TupleKeyWithoutCondition) ([]*openfgav1.TupleKey, []*openfgav1.TupleKeyWithoutCondition) {
|
||||
deduplicatedWriteTuples := make([]*openfgav1.TupleKey, 0)
|
||||
deduplicatedDeleteTuples := make([]*openfgav1.TupleKeyWithoutCondition, 0)
|
||||
|
||||
writeTupleMap := make(map[string]bool)
|
||||
|
||||
for _, writeTuple := range writeTuples {
|
||||
id := getTupleKeyID(writeTuple)
|
||||
if !writeTupleMap[id] {
|
||||
writeTupleMap[id] = true
|
||||
deduplicatedWriteTuples = append(deduplicatedWriteTuples, writeTuple)
|
||||
}
|
||||
}
|
||||
|
||||
// Prioritize writes over deletes. Deletes do not have a condition, so we don't know if write tuple is different from delete one.
|
||||
for _, deleteTuple := range deleteTuples {
|
||||
id := getTupleKeyID(deleteTuple)
|
||||
if !writeTupleMap[id] {
|
||||
writeTupleMap[id] = true
|
||||
deduplicatedDeleteTuples = append(deduplicatedDeleteTuples, deleteTuple)
|
||||
}
|
||||
}
|
||||
|
||||
return deduplicatedWriteTuples, deduplicatedDeleteTuples
|
||||
}
|
||||
|
||||
func (s *Server) writeTuples(ctx context.Context, store *storeInfo, writeTuples []*openfgav1.TupleKey, deleteTuples []*openfgav1.TupleKeyWithoutCondition) error {
|
||||
writeReq := &openfgav1.WriteRequest{
|
||||
StoreId: store.ID,
|
||||
AuthorizationModelId: store.ModelID,
|
||||
}
|
||||
|
||||
writeTuples, deleteTuples = deduplicateTupleKeys(writeTuples, deleteTuples)
|
||||
|
||||
if len(writeTuples) > 0 {
|
||||
writeReq.Writes = &openfgav1.WriteRequestWrites{
|
||||
TupleKeys: writeTuples,
|
||||
OnDuplicate: "ignore",
|
||||
}
|
||||
}
|
||||
|
||||
if len(deleteTuples) > 0 {
|
||||
writeReq.Deletes = &openfgav1.WriteRequestDeletes{
|
||||
TupleKeys: deleteTuples,
|
||||
OnMissing: "ignore",
|
||||
}
|
||||
}
|
||||
|
||||
_, err := s.openfga.Write(ctx, writeReq)
|
||||
return err
|
||||
}
|
||||
|
||||
type TupleKey interface {
|
||||
GetUser() string
|
||||
GetRelation() string
|
||||
GetObject() string
|
||||
}
|
||||
|
||||
func getTupleKeyID(t TupleKey) string {
|
||||
return fmt.Sprintf("%s:%s:%s", t.GetUser(), t.GetRelation(), t.GetObject())
|
||||
}
|
||||
|
||||
@@ -52,24 +52,7 @@ func (s *Server) mutateFolders(ctx context.Context, store *storeInfo, operations
|
||||
return nil
|
||||
}
|
||||
|
||||
writeReq := &openfgav1.WriteRequest{
|
||||
StoreId: store.ID,
|
||||
AuthorizationModelId: store.ModelID,
|
||||
}
|
||||
if len(writeTuples) > 0 {
|
||||
writeReq.Writes = &openfgav1.WriteRequestWrites{
|
||||
TupleKeys: writeTuples,
|
||||
OnDuplicate: "ignore",
|
||||
}
|
||||
}
|
||||
if len(deleteTuples) > 0 {
|
||||
writeReq.Deletes = &openfgav1.WriteRequestDeletes{
|
||||
TupleKeys: deleteTuples,
|
||||
OnMissing: "ignore",
|
||||
}
|
||||
}
|
||||
|
||||
_, err := s.openfga.Write(ctx, writeReq)
|
||||
err := s.writeTuples(ctx, store, writeTuples, deleteTuples)
|
||||
if err != nil {
|
||||
s.logger.Error("failed to write folder tuples", "error", err)
|
||||
return err
|
||||
|
||||
@@ -50,24 +50,7 @@ func (s *Server) mutateOrgRoles(ctx context.Context, store *storeInfo, operation
|
||||
return nil
|
||||
}
|
||||
|
||||
writeReq := &openfgav1.WriteRequest{
|
||||
StoreId: store.ID,
|
||||
AuthorizationModelId: store.ModelID,
|
||||
}
|
||||
if len(writeTuples) > 0 {
|
||||
writeReq.Writes = &openfgav1.WriteRequestWrites{
|
||||
TupleKeys: writeTuples,
|
||||
OnDuplicate: "ignore",
|
||||
}
|
||||
}
|
||||
if len(deleteTuples) > 0 {
|
||||
writeReq.Deletes = &openfgav1.WriteRequestDeletes{
|
||||
TupleKeys: deleteTuples,
|
||||
OnMissing: "ignore",
|
||||
}
|
||||
}
|
||||
|
||||
_, err := s.openfga.Write(ctx, writeReq)
|
||||
err := s.writeTuples(ctx, store, writeTuples, deleteTuples)
|
||||
if err != nil {
|
||||
s.logger.Error("failed to write user org role tuples", "error", err)
|
||||
return err
|
||||
|
||||
@@ -47,24 +47,7 @@ func (s *Server) mutateResourcePermissions(ctx context.Context, store *storeInfo
|
||||
}
|
||||
}
|
||||
|
||||
writeReq := &openfgav1.WriteRequest{
|
||||
StoreId: store.ID,
|
||||
AuthorizationModelId: store.ModelID,
|
||||
}
|
||||
if len(writeTuples) > 0 {
|
||||
writeReq.Writes = &openfgav1.WriteRequestWrites{
|
||||
TupleKeys: writeTuples,
|
||||
OnDuplicate: "ignore",
|
||||
}
|
||||
}
|
||||
if len(deleteTuples) > 0 {
|
||||
writeReq.Deletes = &openfgav1.WriteRequestDeletes{
|
||||
TupleKeys: deleteTuples,
|
||||
OnMissing: "ignore",
|
||||
}
|
||||
}
|
||||
|
||||
_, err := s.openfga.Write(ctx, writeReq)
|
||||
err := s.writeTuples(ctx, store, writeTuples, deleteTuples)
|
||||
if err != nil {
|
||||
s.logger.Error("failed to write resource permission tuples", "error", err)
|
||||
return err
|
||||
|
||||
@@ -44,24 +44,7 @@ func (s *Server) mutateRoleBindings(ctx context.Context, store *storeInfo, opera
|
||||
}
|
||||
}
|
||||
|
||||
writeReq := &openfgav1.WriteRequest{
|
||||
StoreId: store.ID,
|
||||
AuthorizationModelId: store.ModelID,
|
||||
}
|
||||
if len(writeTuples) > 0 {
|
||||
writeReq.Writes = &openfgav1.WriteRequestWrites{
|
||||
TupleKeys: writeTuples,
|
||||
OnDuplicate: "ignore",
|
||||
}
|
||||
}
|
||||
if len(deleteTuples) > 0 {
|
||||
writeReq.Deletes = &openfgav1.WriteRequestDeletes{
|
||||
TupleKeys: deleteTuples,
|
||||
OnMissing: "ignore",
|
||||
}
|
||||
}
|
||||
|
||||
_, err := s.openfga.Write(ctx, writeReq)
|
||||
err := s.writeTuples(ctx, store, writeTuples, deleteTuples)
|
||||
if err != nil {
|
||||
s.logger.Error("failed to write resource role binding tuples", "error", err)
|
||||
return err
|
||||
|
||||
@@ -41,24 +41,7 @@ func (s *Server) mutateRoles(ctx context.Context, store *storeInfo, operations [
|
||||
}
|
||||
}
|
||||
|
||||
writeReq := &openfgav1.WriteRequest{
|
||||
StoreId: store.ID,
|
||||
AuthorizationModelId: store.ModelID,
|
||||
}
|
||||
if len(writeTuples) > 0 {
|
||||
writeReq.Writes = &openfgav1.WriteRequestWrites{
|
||||
TupleKeys: writeTuples,
|
||||
OnDuplicate: "ignore",
|
||||
}
|
||||
}
|
||||
if len(deleteTuples) > 0 {
|
||||
writeReq.Deletes = &openfgav1.WriteRequestDeletes{
|
||||
TupleKeys: deleteTuples,
|
||||
OnMissing: "ignore",
|
||||
}
|
||||
}
|
||||
|
||||
_, err := s.openfga.Write(ctx, writeReq)
|
||||
err := s.writeTuples(ctx, store, writeTuples, deleteTuples)
|
||||
if err != nil {
|
||||
s.logger.Error("failed to write resource role binding tuples", "error", err)
|
||||
return err
|
||||
|
||||
@@ -43,24 +43,7 @@ func (s *Server) mutateTeamBindings(ctx context.Context, store *storeInfo, opera
|
||||
}
|
||||
}
|
||||
|
||||
writeReq := &openfgav1.WriteRequest{
|
||||
StoreId: store.ID,
|
||||
AuthorizationModelId: store.ModelID,
|
||||
}
|
||||
if len(writeTuples) > 0 {
|
||||
writeReq.Writes = &openfgav1.WriteRequestWrites{
|
||||
TupleKeys: writeTuples,
|
||||
OnDuplicate: "ignore",
|
||||
}
|
||||
}
|
||||
if len(deleteTuples) > 0 {
|
||||
writeReq.Deletes = &openfgav1.WriteRequestDeletes{
|
||||
TupleKeys: deleteTuples,
|
||||
OnMissing: "ignore",
|
||||
}
|
||||
}
|
||||
|
||||
_, err := s.openfga.Write(ctx, writeReq)
|
||||
err := s.writeTuples(ctx, store, writeTuples, deleteTuples)
|
||||
if err != nil {
|
||||
s.logger.Error("failed to write resource role binding tuples", "error", err)
|
||||
return err
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
|
||||
openfgav1 "github.com/openfga/api/proto/openfga/v1"
|
||||
"github.com/stretchr/testify/require"
|
||||
"google.golang.org/protobuf/types/known/structpb"
|
||||
|
||||
iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1"
|
||||
v1 "github.com/grafana/grafana/pkg/services/authz/proto/v1"
|
||||
@@ -133,3 +134,66 @@ func testMutate(t *testing.T, srv *Server) {
|
||||
require.Len(t, res.Tuples, 0)
|
||||
})
|
||||
}
|
||||
|
||||
func TestDeduplicateTupleKeys(t *testing.T) {
|
||||
t.Run("should deduplicate write tuples", func(t *testing.T) {
|
||||
writeTuples := []*openfgav1.TupleKey{
|
||||
{User: "user:1", Relation: "get", Object: "object:1"},
|
||||
{User: "user:1", Relation: "get", Object: "object:2"},
|
||||
}
|
||||
deleteTuples := []*openfgav1.TupleKeyWithoutCondition{
|
||||
{User: "user:1", Relation: "get", Object: "object:1"},
|
||||
{User: "user:2", Relation: "get", Object: "object:2"},
|
||||
}
|
||||
|
||||
deduplicatedWriteTuples, deduplicatedDeleteTuples := deduplicateTupleKeys(writeTuples, deleteTuples)
|
||||
require.Len(t, deduplicatedWriteTuples, 2)
|
||||
require.ElementsMatch(t, deduplicatedWriteTuples, []*openfgav1.TupleKey{
|
||||
{User: "user:1", Relation: "get", Object: "object:1"},
|
||||
{User: "user:1", Relation: "get", Object: "object:2"},
|
||||
})
|
||||
|
||||
require.Len(t, deduplicatedDeleteTuples, 1)
|
||||
require.ElementsMatch(t, deduplicatedDeleteTuples, []*openfgav1.TupleKeyWithoutCondition{
|
||||
{User: "user:2", Relation: "get", Object: "object:2"},
|
||||
})
|
||||
})
|
||||
|
||||
t.Run("should deduplicate write tuples with conditions", func(t *testing.T) {
|
||||
writeTuples := []*openfgav1.TupleKey{
|
||||
{User: "user:1", Relation: "get", Object: "object:1", Condition: &openfgav1.RelationshipCondition{Name: "condition:1", Context: &structpb.Struct{Fields: map[string]*structpb.Value{
|
||||
"field:1": structpb.NewStringValue("value:1"),
|
||||
}}}},
|
||||
{User: "user:1", Relation: "get", Object: "object:2"},
|
||||
}
|
||||
deleteTuples := []*openfgav1.TupleKeyWithoutCondition{
|
||||
{User: "user:1", Relation: "get", Object: "object:1"},
|
||||
}
|
||||
|
||||
deduplicatedWriteTuples, deduplicatedDeleteTuples := deduplicateTupleKeys(writeTuples, deleteTuples)
|
||||
require.Len(t, deduplicatedWriteTuples, 2)
|
||||
require.ElementsMatch(t, deduplicatedWriteTuples, []*openfgav1.TupleKey{
|
||||
{User: "user:1", Relation: "get", Object: "object:1", Condition: &openfgav1.RelationshipCondition{Name: "condition:1", Context: &structpb.Struct{Fields: map[string]*structpb.Value{
|
||||
"field:1": structpb.NewStringValue("value:1"),
|
||||
}}}},
|
||||
{User: "user:1", Relation: "get", Object: "object:2"},
|
||||
})
|
||||
|
||||
require.Len(t, deduplicatedDeleteTuples, 0)
|
||||
})
|
||||
|
||||
t.Run("should do nothing for no duplicates", func(t *testing.T) {
|
||||
writeTuples := []*openfgav1.TupleKey{
|
||||
{User: "user:1", Relation: "get", Object: "object:1"},
|
||||
}
|
||||
deleteTuples := []*openfgav1.TupleKeyWithoutCondition{
|
||||
{User: "user:2", Relation: "get", Object: "object:2"},
|
||||
}
|
||||
|
||||
deduplicatedWriteTuples, deduplicatedDeleteTuples := deduplicateTupleKeys(writeTuples, deleteTuples)
|
||||
require.Len(t, deduplicatedWriteTuples, 1)
|
||||
require.ElementsMatch(t, deduplicatedWriteTuples, writeTuples)
|
||||
require.Len(t, deduplicatedDeleteTuples, 1)
|
||||
require.ElementsMatch(t, deduplicatedDeleteTuples, deleteTuples)
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user