Merge pull request #1436 from alesstimec/openfga-cleanup

Adds cleanup of orphaned OpenFGA tuples.

cmd/jimmsrv/main.go

@@ -203,6 +203,9 @@ func start(ctx context.Context, s *service.Service) error {
 			}
 			return nil
 		})
+		s.Go(func() error {
+			return jimmsvc.OpenFGACleanup(ctx, time.NewTicker(6*time.Hour).C)
+		})
 	}
 
 	if isLeader {

cmd/jimmsrv/service/service.go

@@ -255,6 +255,22 @@ func (s *Service) MonitorResources(ctx context.Context) {
 	}
 }
 
+// OpenFGACleanup starts a goroutine that cleans up any orphaned tuples from OpenFGA.
+func (s *Service) OpenFGACleanup(ctx context.Context, trigger <-chan time.Time) error {
+	for {
+		select {
+		case <-trigger:
+			err := s.jimm.OpenFGACleanup(ctx)
+			if err != nil {
+				zapctx.Error(ctx, "openfga cleanup", zap.Error(err))
+				continue
+			}
+		case <-ctx.Done():
+			return nil
+		}
+	}
+}
+
 // Cleanup cleans up resources that need to be released on shutdown.
 func (s *Service) Cleanup() {
 	// Iterating over clean up function in reverse-order to avoid early clean ups.

internal/jimm/access.go

@@ -803,3 +803,62 @@ func (j *JIMM) ListGroups(ctx context.Context, user *openfga.User, pagination pa
 	}
 	return groups, nil
 }
+
+// OpenFGACleanup queries OpenFGA for all existing tuples, tries to resolve each tuple and removes those
+// that JIMM cannot resolved - orphaned tuples. JIMM not being able to resolve a tuple means that the
+// corresponding entity has been removed from JIMM's database.
+//
+// This approach to cleaning up tuples is intended to be temporary while we implement
+// a better approach to eventual consistency of JIMM's database objects and OpenFGA tuples.
+func (j *JIMM) OpenFGACleanup(ctx context.Context) error {
+	var (
+		continuationToken string
+		err               error
+		tuples            []ofga.Tuple
+	)
+	for {
+		tuples, continuationToken, err = j.OpenFGAClient.ReadRelatedObjects(ctx, openfga.Tuple{}, 20, continuationToken)
+		if err != nil {
+			zapctx.Error(ctx, "reading all tuples", zap.Error(err))
+			return err
+		}
+
+		orphanedTuples := j.orphanedTuples(ctx, tuples...)
+		if len(orphanedTuples) > 0 {
+			zapctx.Debug(ctx, "removing orphaned tuples", zap.Any("tuples", orphanedTuples))
+			err = j.OpenFGAClient.RemoveRelation(ctx, orphanedTuples...)
+			if err != nil {
+				zapctx.Warn(ctx, "failed to clean up orphaned tuples", zap.Error(err))
+			}
+		}
+		if continuationToken == "" {
+			return nil
+		}
+		select {
+		case <-ctx.Done():
+			return nil
+		default:
+		}
+	}
+}
+
+func (j *JIMM) orphanedTuples(ctx context.Context, tuples ...openfga.Tuple) []openfga.Tuple {
+	orphanedTuples := []openfga.Tuple{}
+	for _, tuple := range tuples {
+		_, err := j.ToJAASTag(ctx, tuple.Object, true)
+		if err != nil {
+			if errors.ErrorCode(err) == errors.CodeNotFound {
+				orphanedTuples = append(orphanedTuples, tuple)
+				continue
+			}
+		}
+		_, err = j.ToJAASTag(ctx, tuple.Target, true)
+		if err != nil {
+			if errors.ErrorCode(err) == errors.CodeNotFound {
+				orphanedTuples = append(orphanedTuples, tuple)
+				continue
+			}
+		}
+	}
+	return orphanedTuples
+}

internal/jimm/access_test.go

@@ -1110,3 +1110,103 @@ func TestListGroups(t *testing.T) {
 	c.Assert(groups[3].Name, qt.Equals, "test-group1")
 	c.Assert(groups[4].Name, qt.Equals, "test-group2")
 }
+
+func TestOpenFGACleanup(t *testing.T) {
+	c := qt.New(t)
+	ctx := context.Background()
+
+	ofgaClient, _, _, err := jimmtest.SetupTestOFGAClient(c.Name())
+	c.Assert(err, qt.IsNil)
+
+	now := time.Now().UTC().Round(time.Millisecond)
+	j := &jimm.JIMM{
+		UUID: uuid.NewString(),
+		Database: db.Database{
+			DB: jimmtest.PostgresDB(c, func() time.Time { return now }),
+		},
+		OpenFGAClient: ofgaClient,
+	}
+
+	err = j.Database.Migrate(ctx, false)
+	c.Assert(err, qt.IsNil)
+
+	// run cleanup on an empty authorizaton store
+	err = j.OpenFGACleanup(ctx)
+	c.Assert(err, qt.IsNil)
+
+	type createTagFunction func(int) *ofga.Entity
+
+	var (
+		createStringTag = func(kind openfga.Kind) createTagFunction {
+			return func(i int) *ofga.Entity {
+				return &ofga.Entity{
+					Kind: kind,
+					ID:   fmt.Sprintf("%s-%d", petname.Generate(2, "-"), i),
+				}
+			}
+		}
+
+		createUUIDTag = func(kind openfga.Kind) createTagFunction {
+			return func(i int) *ofga.Entity {
+				return &ofga.Entity{
+					Kind: kind,
+					ID:   uuid.NewString(),
+				}
+			}
+		}
+	)
+
+	tagTests := []struct {
+		createObjectTag createTagFunction
+		relation        string
+		createTargetTag createTagFunction
+	}{{
+		createObjectTag: createStringTag(openfga.UserType),
+		relation:        "member",
+		createTargetTag: createStringTag(openfga.GroupType),
+	}, {
+		createObjectTag: createStringTag(openfga.UserType),
+		relation:        "administrator",
+		createTargetTag: createUUIDTag(openfga.ControllerType),
+	}, {
+		createObjectTag: createStringTag(openfga.UserType),
+		relation:        "reader",
+		createTargetTag: createUUIDTag(openfga.ModelType),
+	}, {
+		createObjectTag: createStringTag(openfga.UserType),
+		relation:        "administrator",
+		createTargetTag: createStringTag(openfga.CloudType),
+	}, {
+		createObjectTag: createStringTag(openfga.UserType),
+		relation:        "consumer",
+		createTargetTag: createUUIDTag(openfga.ApplicationOfferType),
+	}}
+
+	orphanedTuples := []ofga.Tuple{}
+	for i := 0; i < 100; i++ {
+		for _, test := range tagTests {
+			objectTag := test.createObjectTag(i)
+			targetTag := test.createTargetTag(i)
+
+			tuple := openfga.Tuple{
+				Object:   objectTag,
+				Relation: ofga.Relation(test.relation),
+				Target:   targetTag,
+			}
+			err = ofgaClient.AddRelation(ctx, tuple)
+			c.Assert(err, qt.IsNil)
+
+			orphanedTuples = append(orphanedTuples, tuple)
+		}
+	}
+
+	err = j.OpenFGACleanup(ctx)
+	c.Assert(err, qt.IsNil)
+
+	for _, tuple := range orphanedTuples {
+		c.Logf("checking relation for %+v", tuple)
+		ok, err := ofgaClient.CheckRelation(ctx, tuple, false)
+		c.Assert(err, qt.IsNil)
+		c.Assert(ok, qt.IsFalse)
+	}
+}