diff --git a/pkg/registry/apis/iam/resource_permission_hooks.go b/pkg/registry/apis/iam/resource_permission_hooks.go index e725726b2ec..010bb40049b 100644 --- a/pkg/registry/apis/iam/resource_permission_hooks.go +++ b/pkg/registry/apis/iam/resource_permission_hooks.go @@ -2,7 +2,6 @@ package iam import ( "context" - "errors" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -14,8 +13,6 @@ import ( ) var ( - errEmptyName = errors.New("name cannot be empty") - defaultWriteTimeout = 15 * time.Second ) diff --git a/pkg/registry/apis/iam/team_binding_hooks.go b/pkg/registry/apis/iam/team_binding_hooks.go index dcad58cae0a..298237df70a 100644 --- a/pkg/registry/apis/iam/team_binding_hooks.go +++ b/pkg/registry/apis/iam/team_binding_hooks.go @@ -10,42 +10,8 @@ import ( iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" v1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" - "github.com/grafana/grafana/pkg/services/authz/zanzana" ) -// convertTeamBindingToTuple converts a TeamBinding to a v1 TupleKey format -// TeamBinding represents a user's membership in a team with a specific permission level -func convertTeamBindingToTuple(tb *iamv0.TeamBinding) (*v1.TupleKey, error) { - if tb.Spec.Subject.Name == "" { - return nil, errEmptyName - } - - if tb.Spec.TeamRef.Name == "" { - return nil, errEmptyName - } - - // Map permission to relation - var relation string - switch tb.Spec.Permission { - case iamv0.TeamBindingTeamPermissionAdmin: - relation = zanzana.RelationTeamAdmin - case iamv0.TeamBindingTeamPermissionMember: - relation = zanzana.RelationTeamMember - default: - // Default to member if unknown permission - relation = zanzana.RelationTeamMember - } - - // Create tuple: user:{subjectUID} has {relation} relation to team:{teamUID} - tuple := &v1.TupleKey{ - User: zanzana.NewTupleEntry(zanzana.TypeUser, tb.Spec.Subject.Name, ""), - Relation: relation, - Object: zanzana.NewTupleEntry(zanzana.TypeTeam, tb.Spec.TeamRef.Name, ""), - } - - return tuple, nil -} - // AfterTeamBindingCreate is a post-create hook that writes the team binding to Zanzana (openFGA) func (b *IdentityAccessManagementAPIBuilder) AfterTeamBindingCreate(obj runtime.Object, _ *metav1.CreateOptions) { if b.zClient == nil { @@ -79,20 +45,6 @@ func (b *IdentityAccessManagementAPIBuilder) AfterTeamBindingCreate(obj runtime. hooksOperationCounter.WithLabelValues(resourceType, operation, status).Inc() }() - tuple, err := convertTeamBindingToTuple(tb) - if err != nil { - b.logger.Error("failed to convert team binding to tuple", - "namespace", tb.Namespace, - "name", tb.Name, - "subject", tb.Spec.Subject.Name, - "teamRef", tb.Spec.TeamRef.Name, - "permission", tb.Spec.Permission, - "err", err, - ) - status = "failure" - return - } - b.logger.Debug("writing team binding to zanzana", "namespace", tb.Namespace, "name", tb.Name, @@ -104,12 +56,21 @@ func (b *IdentityAccessManagementAPIBuilder) AfterTeamBindingCreate(obj runtime. ctx, cancel := context.WithTimeout(context.Background(), defaultWriteTimeout) defer cancel() - err = b.zClient.Write(ctx, &v1.WriteRequest{ + err := b.zClient.Mutate(ctx, &v1.MutateRequest{ Namespace: tb.Namespace, - Writes: &v1.WriteRequestWrites{ - TupleKeys: []*v1.TupleKey{tuple}, + Operations: []*v1.MutateOperation{ + { + Operation: &v1.MutateOperation_CreateTeamBinding{ + CreateTeamBinding: &v1.CreateTeamBindingOperation{ + SubjectName: tb.Spec.Subject.Name, + TeamName: tb.Spec.TeamRef.Name, + Permission: string(tb.Spec.Permission), + }, + }, + }, }, }) + if err != nil { status = "failure" b.logger.Error("failed to write team binding to zanzana", @@ -159,34 +120,28 @@ func (b *IdentityAccessManagementAPIBuilder) BeginTeamBindingUpdate(ctx context. return nil, nil } - // Convert old team binding to tuple for deletion - var oldTuple *v1.TupleKey - var oldErr error - if oldTB.Spec.Subject.Name != "" && oldTB.Spec.TeamRef.Name != "" { - oldTuple, oldErr = convertTeamBindingToTuple(oldTB) - if oldErr != nil { - b.logger.Error("failed to convert old team binding to tuple", - "namespace", oldTB.Namespace, - "name", oldTB.Name, - "err", oldErr, - ) - return nil, nil - } - } - - // Convert new team binding to tuple for writing - var newTuple *v1.TupleKey - var newErr error - if newTB.Spec.Subject.Name != "" && newTB.Spec.TeamRef.Name != "" { - newTuple, newErr = convertTeamBindingToTuple(newTB) - if newErr != nil { - b.logger.Error("failed to convert new team binding to tuple", - "namespace", newTB.Namespace, - "name", newTB.Name, - "err", newErr, - ) - return nil, nil - } + operations := make([]*v1.MutateOperation, 0, 2) + operations = append(operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_DeleteTeamBinding{ + DeleteTeamBinding: &v1.DeleteTeamBindingOperation{ + SubjectName: oldTB.Spec.Subject.Name, + TeamName: oldTB.Spec.TeamRef.Name, + Permission: string(oldTB.Spec.Permission), + }, + }, + }) + operations = append(operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_CreateTeamBinding{ + CreateTeamBinding: &v1.CreateTeamBindingOperation{ + SubjectName: newTB.Spec.Subject.Name, + TeamName: newTB.Spec.TeamRef.Name, + Permission: string(newTB.Spec.Permission), + }, + }, + }) + if len(operations) == 0 { + b.logger.Debug("no updates to team binding in zanzana", "namespace", newTB.Namespace, "name", newTB.Name) + return func(ctx context.Context, success bool) {}, nil } // Return a finish function that performs the zanzana write only on success @@ -224,57 +179,22 @@ func (b *IdentityAccessManagementAPIBuilder) BeginTeamBindingUpdate(ctx context. ctx, cancel := context.WithTimeout(context.Background(), defaultWriteTimeout) defer cancel() - // Prepare write request - req := &v1.WriteRequest{ - Namespace: newTB.Namespace, - } - - // Add delete for old tuple - if oldTuple != nil && oldErr == nil { - deleteTuple := toTupleKeysWithoutCondition([]*v1.TupleKey{oldTuple}) - req.Deletes = &v1.WriteRequestDeletes{ - TupleKeys: deleteTuple, - } - b.logger.Debug("deleting existing team binding from zanzana", - "namespace", newTB.Namespace, - "subject", oldTB.Spec.Subject.Name, - "teamRef", oldTB.Spec.TeamRef.Name, - ) - } - - // Add write for new tuple - if newTuple != nil && newErr == nil { - req.Writes = &v1.WriteRequestWrites{ - TupleKeys: []*v1.TupleKey{newTuple}, - } - b.logger.Debug("writing new team binding to zanzana", - "namespace", newTB.Namespace, - "subject", newTB.Spec.Subject.Name, - "teamRef", newTB.Spec.TeamRef.Name, - ) - } - // Only make the request if there are deletes or writes - if (req.Deletes != nil && len(req.Deletes.TupleKeys) > 0) || (req.Writes != nil && len(req.Writes.TupleKeys) > 0) { - err := b.zClient.Write(ctx, req) - if err != nil { - status = "failure" - b.logger.Error("failed to update team binding in zanzana", - "err", err, - "namespace", newTB.Namespace, - "name", newTB.Name, - ) - } else { - // Record successful tuple operations - if oldTuple != nil && oldErr == nil { - hooksTuplesCounter.WithLabelValues("teambinding", "update", "delete").Inc() - } - if newTuple != nil && newErr == nil { - hooksTuplesCounter.WithLabelValues("teambinding", "update", "write").Inc() - } - } + err := b.zClient.Mutate(ctx, &v1.MutateRequest{ + Namespace: newTB.Namespace, + Operations: operations, + }) + if err != nil { + status = "failure" + b.logger.Error("failed to update team binding in zanzana", + "err", err, + "namespace", newTB.Namespace, + "name", newTB.Name, + ) } else { - b.logger.Debug("no tuples to update in zanzana", "namespace", newTB.Namespace, "name", newTB.Name) + // Record successful tuple operations + hooksTuplesCounter.WithLabelValues("teambinding", "update", "delete").Inc() + hooksTuplesCounter.WithLabelValues("teambinding", "update", "write").Inc() } }() }, nil @@ -313,22 +233,6 @@ func (b *IdentityAccessManagementAPIBuilder) AfterTeamBindingDelete(obj runtime. hooksOperationCounter.WithLabelValues(resourceType, operation, status).Inc() }() - tuple, err := convertTeamBindingToTuple(tb) - if err != nil { - b.logger.Error("failed to convert team binding to tuple for deletion", - "namespace", tb.Namespace, - "name", tb.Name, - "subject", tb.Spec.Subject.Name, - "teamRef", tb.Spec.TeamRef.Name, - "err", err, - ) - status = "failure" - return - } - - // Convert tuple to TupleKeyWithoutCondition for deletion - deleteTuple := toTupleKeysWithoutCondition([]*v1.TupleKey{tuple}) - b.logger.Debug("deleting team binding from zanzana", "namespace", tb.Namespace, "name", tb.Name, @@ -340,10 +244,18 @@ func (b *IdentityAccessManagementAPIBuilder) AfterTeamBindingDelete(obj runtime. ctx, cancel := context.WithTimeout(context.Background(), defaultWriteTimeout) defer cancel() - err = b.zClient.Write(ctx, &v1.WriteRequest{ + err := b.zClient.Mutate(ctx, &v1.MutateRequest{ Namespace: tb.Namespace, - Deletes: &v1.WriteRequestDeletes{ - TupleKeys: deleteTuple, + Operations: []*v1.MutateOperation{ + { + Operation: &v1.MutateOperation_DeleteTeamBinding{ + DeleteTeamBinding: &v1.DeleteTeamBindingOperation{ + SubjectName: tb.Spec.Subject.Name, + TeamName: tb.Spec.TeamRef.Name, + Permission: string(tb.Spec.Permission), + }, + }, + }, }, }) if err != nil { diff --git a/pkg/registry/apis/iam/team_binding_hooks_test.go b/pkg/registry/apis/iam/team_binding_hooks_test.go index 2f56ba12c87..6b41e8359da 100644 --- a/pkg/registry/apis/iam/team_binding_hooks_test.go +++ b/pkg/registry/apis/iam/team_binding_hooks_test.go @@ -2,16 +2,18 @@ package iam import ( "context" + "slices" "sync" "testing" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/stretchr/testify/require" + iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/infra/log" v1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" - "github.com/stretchr/testify/require" ) func TestAfterTeamBindingCreate(t *testing.T) { @@ -40,30 +42,29 @@ func TestAfterTeamBindingCreate(t *testing.T) { }, } - testMemberBinding := func(ctx context.Context, req *v1.WriteRequest) error { + testMemberBinding := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) - require.NotNil(t, req.Writes) - require.Len(t, req.Writes.TupleKeys, 1) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 1) require.Equal(t, "org-1", req.Namespace) - require.Nil(t, req.Deletes) - expectedTuple := &v1.TupleKey{ - User: "user:user-1", - Relation: "member", - Object: "team:team-1", + expectedOperation := &v1.MutateOperation{ + Operation: &v1.MutateOperation_CreateTeamBinding{ + CreateTeamBinding: &v1.CreateTeamBindingOperation{ + SubjectName: "user-1", + TeamName: "team-1", + Permission: "member", + }, + }, } - actualTuple := req.Writes.TupleKeys[0] - require.Equal(t, expectedTuple.User, actualTuple.User) - require.Equal(t, expectedTuple.Relation, actualTuple.Relation) - require.Equal(t, expectedTuple.Object, actualTuple.Object) - require.Nil(t, actualTuple.Condition) + require.True(t, containsTeamBindingOperation(req.Operations, expectedOperation)) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testMemberBinding} + b.zClient = &FakeZanzanaClient{mutateCallback: testMemberBinding} b.AfterTeamBindingCreate(&teamBinding, nil) wg.Wait() }) @@ -87,30 +88,29 @@ func TestAfterTeamBindingCreate(t *testing.T) { }, } - testAdminBinding := func(ctx context.Context, req *v1.WriteRequest) error { + testAdminBinding := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) - require.NotNil(t, req.Writes) - require.Len(t, req.Writes.TupleKeys, 1) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 1) require.Equal(t, "org-2", req.Namespace) - require.Nil(t, req.Deletes) - expectedTuple := &v1.TupleKey{ - User: "user:user-2", - Relation: "admin", - Object: "team:team-2", + expectedOperation := &v1.MutateOperation{ + Operation: &v1.MutateOperation_CreateTeamBinding{ + CreateTeamBinding: &v1.CreateTeamBindingOperation{ + SubjectName: "user-2", + TeamName: "team-2", + Permission: "admin", + }, + }, } - actualTuple := req.Writes.TupleKeys[0] - require.Equal(t, expectedTuple.User, actualTuple.User) - require.Equal(t, expectedTuple.Relation, actualTuple.Relation) - require.Equal(t, expectedTuple.Object, actualTuple.Object) - require.Nil(t, actualTuple.Condition) + require.True(t, containsTeamBindingOperation(req.Operations, expectedOperation)) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testAdminBinding} + b.zClient = &FakeZanzanaClient{mutateCallback: testAdminBinding} b.AfterTeamBindingCreate(&teamBinding, nil) wg.Wait() }) @@ -141,40 +141,6 @@ func TestAfterTeamBindingCreate(t *testing.T) { // Should not panic or error when zClient is nil builder.AfterTeamBindingCreate(&teamBinding, nil) }) - - t.Run("should handle conversion error gracefully", func(t *testing.T) { - // TeamBinding with empty subject name should fail conversion - teamBinding := iamv0.TeamBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "binding-4", - Namespace: "org-4", - }, - Spec: iamv0.TeamBindingSpec{ - Subject: iamv0.TeamBindingspecSubject{ - Name: "", // Empty name should cause error - }, - TeamRef: iamv0.TeamBindingTeamRef{ - Name: "team-4", - }, - Permission: iamv0.TeamBindingTeamPermissionMember, - }, - } - - writeCalled := false - testErrorHandling := func(ctx context.Context, req *v1.WriteRequest) error { - writeCalled = true - // Should not be called due to conversion error - require.Fail(t, "Write should not be called when conversion fails") - return nil - } - - b.zClient = &FakeZanzanaClient{writeCallback: testErrorHandling} - b.AfterTeamBindingCreate(&teamBinding, nil) - // Wait a bit to ensure the goroutine has time to process - // The goroutine will complete but won't call the write callback - time.Sleep(100 * time.Millisecond) - require.False(t, writeCalled, "Write callback should not be called when conversion fails") - }) } func TestBeginTeamBindingUpdate(t *testing.T) { @@ -218,33 +184,37 @@ func TestBeginTeamBindingUpdate(t *testing.T) { }, } - testPermissionUpdate := func(ctx context.Context, req *v1.WriteRequest) error { + testPermissionUpdate := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) require.Equal(t, "org-1", req.Namespace) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 2) - // Should delete old member permission - require.NotNil(t, req.Deletes) - require.Len(t, req.Deletes.TupleKeys, 1) - require.Equal( - t, - req.Deletes.TupleKeys[0], - &v1.TupleKeyWithoutCondition{User: "user:user-1", Relation: "member", Object: "team:team-1"}, - ) + require.True(t, containsTeamBindingOperation(req.Operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_DeleteTeamBinding{ + DeleteTeamBinding: &v1.DeleteTeamBindingOperation{ + SubjectName: "user-1", + TeamName: "team-1", + Permission: "member", + }, + }, + })) - // Should write new admin permission - require.NotNil(t, req.Writes) - require.Len(t, req.Writes.TupleKeys, 1) - require.Equal( - t, - req.Writes.TupleKeys[0], - &v1.TupleKey{User: "user:user-1", Relation: "admin", Object: "team:team-1"}, - ) + require.True(t, containsTeamBindingOperation(req.Operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_CreateTeamBinding{ + CreateTeamBinding: &v1.CreateTeamBindingOperation{ + SubjectName: "user-1", + TeamName: "team-1", + Permission: "admin", + }, + }, + })) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testPermissionUpdate} + b.zClient = &FakeZanzanaClient{mutateCallback: testPermissionUpdate} finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil) require.NoError(t, err) @@ -288,33 +258,36 @@ func TestBeginTeamBindingUpdate(t *testing.T) { }, } - testUserUpdate := func(ctx context.Context, req *v1.WriteRequest) error { + testUserUpdate := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) require.Equal(t, "org-2", req.Namespace) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 2) - // Should delete old user binding - require.NotNil(t, req.Deletes) - require.Len(t, req.Deletes.TupleKeys, 1) - require.Equal( - t, - req.Deletes.TupleKeys[0], - &v1.TupleKeyWithoutCondition{User: "user:user-1", Relation: "member", Object: "team:team-1"}, - ) - - // Should write new user binding - require.NotNil(t, req.Writes) - require.Len(t, req.Writes.TupleKeys, 1) - require.Equal( - t, - req.Writes.TupleKeys[0], - &v1.TupleKey{User: "user:user-2", Relation: "member", Object: "team:team-1"}, - ) + require.True(t, containsTeamBindingOperation(req.Operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_DeleteTeamBinding{ + DeleteTeamBinding: &v1.DeleteTeamBindingOperation{ + SubjectName: "user-1", + TeamName: "team-1", + Permission: "member", + }, + }, + })) + require.True(t, containsTeamBindingOperation(req.Operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_CreateTeamBinding{ + CreateTeamBinding: &v1.CreateTeamBindingOperation{ + SubjectName: "user-2", + TeamName: "team-1", + Permission: "member", + }, + }, + })) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testUserUpdate} + b.zClient = &FakeZanzanaClient{mutateCallback: testUserUpdate} finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil) require.NoError(t, err) @@ -358,33 +331,35 @@ func TestBeginTeamBindingUpdate(t *testing.T) { }, } - testTeamUpdate := func(ctx context.Context, req *v1.WriteRequest) error { + testTeamUpdate := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) require.Equal(t, "org-3", req.Namespace) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 2) - // Should delete old team binding - require.NotNil(t, req.Deletes) - require.Len(t, req.Deletes.TupleKeys, 1) - require.Equal( - t, - req.Deletes.TupleKeys[0], - &v1.TupleKeyWithoutCondition{User: "user:user-1", Relation: "admin", Object: "team:team-1"}, - ) - - // Should write new team binding - require.NotNil(t, req.Writes) - require.Len(t, req.Writes.TupleKeys, 1) - require.Equal( - t, - req.Writes.TupleKeys[0], - &v1.TupleKey{User: "user:user-1", Relation: "admin", Object: "team:team-2"}, - ) - + require.True(t, containsTeamBindingOperation(req.Operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_DeleteTeamBinding{ + DeleteTeamBinding: &v1.DeleteTeamBindingOperation{ + SubjectName: "user-1", + TeamName: "team-1", + Permission: "admin", + }, + }, + })) + require.True(t, containsTeamBindingOperation(req.Operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_CreateTeamBinding{ + CreateTeamBinding: &v1.CreateTeamBindingOperation{ + SubjectName: "user-1", + TeamName: "team-2", + Permission: "admin", + }, + }, + })) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testTeamUpdate} + b.zClient = &FakeZanzanaClient{mutateCallback: testTeamUpdate} finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil) require.NoError(t, err) @@ -427,13 +402,13 @@ func TestBeginTeamBindingUpdate(t *testing.T) { }, } - testNoWriteOnFailure := func(ctx context.Context, req *v1.WriteRequest) error { + testNoMutateOnFailure := func(ctx context.Context, req *v1.MutateRequest) error { // Should not be called when success=false - require.Fail(t, "Write should not be called when update fails") + require.Fail(t, "Mutate should not be called when update fails") return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testNoWriteOnFailure} + b.zClient = &FakeZanzanaClient{mutateCallback: testNoMutateOnFailure} finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil) require.NoError(t, err) @@ -441,7 +416,7 @@ func TestBeginTeamBindingUpdate(t *testing.T) { // Call finish function with success=false finishFunc(context.Background(), false) - // No wait needed since write should not be called + // No wait needed since mutate should not be called }) t.Run("should not write to zanzana when zClient is nil", func(t *testing.T) { @@ -497,7 +472,7 @@ func TestBeginTeamBindingUpdate(t *testing.T) { }, Spec: iamv0.TeamBindingSpec{ Subject: iamv0.TeamBindingspecSubject{ - Name: "", // Empty name - conversion will be skipped + Name: "", // Empty name will cause server-side error on delete }, TeamRef: iamv0.TeamBindingTeamRef{ Name: "team-1", @@ -522,27 +497,42 @@ func TestBeginTeamBindingUpdate(t *testing.T) { }, } - testEmptyOldBinding := func(ctx context.Context, req *v1.WriteRequest) error { + testEmptyOldBinding := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) require.Equal(t, "org-6", req.Namespace) + require.NotNil(t, req.Operations) - // Should not delete old binding (it was skipped due to empty name) - require.Nil(t, req.Deletes) + // Should have both delete and create operations + // The delete will have empty subject and fail server-side validation + require.Len(t, req.Operations, 2) - // Should write new binding - require.NotNil(t, req.Writes) - require.Len(t, req.Writes.TupleKeys, 1) - require.Equal( - t, - req.Writes.TupleKeys[0], - &v1.TupleKey{User: "user:user-2", Relation: "member", Object: "team:team-1"}, - ) + // First operation is delete with empty subject + require.True(t, containsTeamBindingOperation(req.Operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_DeleteTeamBinding{ + DeleteTeamBinding: &v1.DeleteTeamBindingOperation{ + SubjectName: "", + TeamName: "team-1", + Permission: "member", + }, + }, + })) + + // Second operation is create with valid data + require.True(t, containsTeamBindingOperation(req.Operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_CreateTeamBinding{ + CreateTeamBinding: &v1.CreateTeamBindingOperation{ + SubjectName: "user-2", + TeamName: "team-1", + Permission: "member", + }, + }, + })) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testEmptyOldBinding} + b.zClient = &FakeZanzanaClient{mutateCallback: testEmptyOldBinding} finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil) require.NoError(t, err) @@ -585,22 +575,22 @@ func TestBeginTeamBindingUpdate(t *testing.T) { }, } - writeCalled := false - testNoWriteOnNoChange := func(ctx context.Context, req *v1.WriteRequest) error { - writeCalled = true - require.Fail(t, "Write should not be called when bindings are identical") + mutateCalled := false + testNoMutateOnNoChange := func(ctx context.Context, req *v1.MutateRequest) error { + mutateCalled = true + require.Fail(t, "Mutate should not be called when bindings are identical") return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testNoWriteOnNoChange} + b.zClient = &FakeZanzanaClient{mutateCallback: testNoMutateOnNoChange} finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil) require.NoError(t, err) require.Nil(t, finishFunc) // Should return nil when bindings are identical - // Verify write was never called + // Verify mutate was never called time.Sleep(100 * time.Millisecond) - require.False(t, writeCalled, "Write callback should not be called when bindings are identical") + require.False(t, mutateCalled, "Mutate callback should not be called when bindings are identical") }) t.Run("should return nil finish func when new binding has empty subject name", func(t *testing.T) { @@ -636,22 +626,22 @@ func TestBeginTeamBindingUpdate(t *testing.T) { }, } - writeCalled := false - testNoWriteOnInvalidBinding := func(ctx context.Context, req *v1.WriteRequest) error { - writeCalled = true - require.Fail(t, "Write should not be called when new binding has empty subject name") + mutateCalled := false + testNoMutateOnInvalidBinding := func(ctx context.Context, req *v1.MutateRequest) error { + mutateCalled = true + require.Fail(t, "Mutate should not be called when new binding has empty subject name") return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testNoWriteOnInvalidBinding} + b.zClient = &FakeZanzanaClient{mutateCallback: testNoMutateOnInvalidBinding} finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil) require.NoError(t, err) require.Nil(t, finishFunc) // Should return nil when new binding has empty subject name - // Verify write was never called + // Verify mutate was never called time.Sleep(100 * time.Millisecond) - require.False(t, writeCalled, "Write callback should not be called when new binding has empty subject name") + require.False(t, mutateCalled, "Mutate callback should not be called when new binding has empty subject name") }) t.Run("should return nil finish func when new binding has empty team ref name", func(t *testing.T) { @@ -687,22 +677,22 @@ func TestBeginTeamBindingUpdate(t *testing.T) { }, } - writeCalled := false - testNoWriteOnInvalidBinding := func(ctx context.Context, req *v1.WriteRequest) error { - writeCalled = true - require.Fail(t, "Write should not be called when new binding has empty team ref name") + mutateCalled := false + testNoMutateOnInvalidBinding := func(ctx context.Context, req *v1.MutateRequest) error { + mutateCalled = true + require.Fail(t, "Mutate should not be called when new binding has empty team ref name") return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testNoWriteOnInvalidBinding} + b.zClient = &FakeZanzanaClient{mutateCallback: testNoMutateOnInvalidBinding} finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil) require.NoError(t, err) require.Nil(t, finishFunc) // Should return nil when new binding has empty team ref name - // Verify write was never called + // Verify mutate was never called time.Sleep(100 * time.Millisecond) - require.False(t, writeCalled, "Write callback should not be called when new binding has empty team ref name") + require.False(t, mutateCalled, "Mutate callback should not be called when new binding has empty team ref name") }) } @@ -732,26 +722,28 @@ func TestAfterTeamBindingDelete(t *testing.T) { }, } - testMemberDelete := func(ctx context.Context, req *v1.WriteRequest) error { + testMemberDelete := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) require.Equal(t, "org-1", req.Namespace) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 1) - // Should have deletes but no writes - require.NotNil(t, req.Deletes) - require.Len(t, req.Deletes.TupleKeys, 1) - require.Nil(t, req.Writes) - - require.Equal( - t, - req.Deletes.TupleKeys[0], - &v1.TupleKeyWithoutCondition{User: "user:user-1", Relation: "member", Object: "team:team-1"}, - ) + expectedOperation := &v1.MutateOperation{ + Operation: &v1.MutateOperation_DeleteTeamBinding{ + DeleteTeamBinding: &v1.DeleteTeamBindingOperation{ + SubjectName: "user-1", + TeamName: "team-1", + Permission: "member", + }, + }, + } + require.True(t, containsTeamBindingOperation(req.Operations, expectedOperation)) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testMemberDelete} + b.zClient = &FakeZanzanaClient{mutateCallback: testMemberDelete} b.AfterTeamBindingDelete(&teamBinding, nil) wg.Wait() }) @@ -775,26 +767,28 @@ func TestAfterTeamBindingDelete(t *testing.T) { }, } - testAdminDelete := func(ctx context.Context, req *v1.WriteRequest) error { + testAdminDelete := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) require.Equal(t, "org-2", req.Namespace) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 1) - // Should have deletes but no writes - require.NotNil(t, req.Deletes) - require.Len(t, req.Deletes.TupleKeys, 1) - require.Nil(t, req.Writes) - - require.Equal( - t, - req.Deletes.TupleKeys[0], - &v1.TupleKeyWithoutCondition{User: "user:user-2", Relation: "admin", Object: "team:team-2"}, - ) + expectedOperation := &v1.MutateOperation{ + Operation: &v1.MutateOperation_DeleteTeamBinding{ + DeleteTeamBinding: &v1.DeleteTeamBindingOperation{ + SubjectName: "user-2", + TeamName: "team-2", + Permission: "admin", + }, + }, + } + require.True(t, containsTeamBindingOperation(req.Operations, expectedOperation)) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testAdminDelete} + b.zClient = &FakeZanzanaClient{mutateCallback: testAdminDelete} b.AfterTeamBindingDelete(&teamBinding, nil) wg.Wait() }) @@ -826,8 +820,9 @@ func TestAfterTeamBindingDelete(t *testing.T) { builder.AfterTeamBindingDelete(&teamBinding, nil) }) - t.Run("should handle conversion error gracefully", func(t *testing.T) { - // TeamBinding with empty team ref name should fail conversion + t.Run("should handle empty team name gracefully", func(t *testing.T) { + wg.Add(1) + // TeamBinding with empty team ref name will be sent to server which will return error teamBinding := iamv0.TeamBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "binding-4", @@ -838,129 +833,60 @@ func TestAfterTeamBindingDelete(t *testing.T) { Name: "user-4", }, TeamRef: iamv0.TeamBindingTeamRef{ - Name: "", // Empty name should cause error + Name: "", // Empty name will cause server-side error }, Permission: iamv0.TeamBindingTeamPermissionMember, }, } - writeCalled := false - testErrorHandling := func(ctx context.Context, req *v1.WriteRequest) error { - writeCalled = true - // Should not be called due to conversion error - require.Fail(t, "Write should not be called when conversion fails") + testErrorHandling := func(ctx context.Context, req *v1.MutateRequest) error { + defer wg.Done() + require.NotNil(t, req) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 1) + require.Equal(t, "org-4", req.Namespace) + + // Operation will have empty team name, which would fail server-side validation + require.True(t, containsTeamBindingOperation(req.Operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_DeleteTeamBinding{ + DeleteTeamBinding: &v1.DeleteTeamBindingOperation{ + SubjectName: "user-4", + TeamName: "", + Permission: "member", + }, + }, + })) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testErrorHandling} + b.zClient = &FakeZanzanaClient{mutateCallback: testErrorHandling} b.AfterTeamBindingDelete(&teamBinding, nil) - // Wait a bit to ensure the goroutine has time to process - // The goroutine will complete but won't call the write callback - time.Sleep(100 * time.Millisecond) - require.False(t, writeCalled, "Write callback should not be called when conversion fails") + wg.Wait() }) } -func TestConvertTeamBindingToTuple(t *testing.T) { - t.Run("should convert member permission correctly", func(t *testing.T) { - tb := &iamv0.TeamBinding{ - Spec: iamv0.TeamBindingSpec{ - Subject: iamv0.TeamBindingspecSubject{ - Name: "user-1", - }, - TeamRef: iamv0.TeamBindingTeamRef{ - Name: "team-1", - }, - Permission: iamv0.TeamBindingTeamPermissionMember, - }, +func containsTeamBindingOperation(operations []*v1.MutateOperation, operation *v1.MutateOperation) bool { + return slices.ContainsFunc(operations, func(o *v1.MutateOperation) bool { + switch operation.Operation.(type) { + case *v1.MutateOperation_DeleteTeamBinding: + deleteOperation := operation.Operation.(*v1.MutateOperation_DeleteTeamBinding) + deleteO, ok := o.Operation.(*v1.MutateOperation_DeleteTeamBinding) + if !ok { + return false + } + return deleteO.DeleteTeamBinding.SubjectName == deleteOperation.DeleteTeamBinding.SubjectName && + deleteO.DeleteTeamBinding.TeamName == deleteOperation.DeleteTeamBinding.TeamName && + deleteO.DeleteTeamBinding.Permission == deleteOperation.DeleteTeamBinding.Permission + case *v1.MutateOperation_CreateTeamBinding: + createOperation := operation.Operation.(*v1.MutateOperation_CreateTeamBinding) + createO, ok := o.Operation.(*v1.MutateOperation_CreateTeamBinding) + if !ok { + return false + } + return createO.CreateTeamBinding.SubjectName == createOperation.CreateTeamBinding.SubjectName && + createO.CreateTeamBinding.TeamName == createOperation.CreateTeamBinding.TeamName && + createO.CreateTeamBinding.Permission == createOperation.CreateTeamBinding.Permission } - - tuple, err := convertTeamBindingToTuple(tb) - require.NoError(t, err) - require.NotNil(t, tuple) - require.Equal(t, "user:user-1", tuple.User) - require.Equal(t, "member", tuple.Relation) - require.Equal(t, "team:team-1", tuple.Object) - require.Nil(t, tuple.Condition) - }) - - t.Run("should convert admin permission correctly", func(t *testing.T) { - tb := &iamv0.TeamBinding{ - Spec: iamv0.TeamBindingSpec{ - Subject: iamv0.TeamBindingspecSubject{ - Name: "user-2", - }, - TeamRef: iamv0.TeamBindingTeamRef{ - Name: "team-2", - }, - Permission: iamv0.TeamBindingTeamPermissionAdmin, - }, - } - - tuple, err := convertTeamBindingToTuple(tb) - require.NoError(t, err) - require.NotNil(t, tuple) - require.Equal(t, "user:user-2", tuple.User) - require.Equal(t, "admin", tuple.Relation) - require.Equal(t, "team:team-2", tuple.Object) - require.Nil(t, tuple.Condition) - }) - - t.Run("should return error for empty subject name", func(t *testing.T) { - tb := &iamv0.TeamBinding{ - Spec: iamv0.TeamBindingSpec{ - Subject: iamv0.TeamBindingspecSubject{ - Name: "", - }, - TeamRef: iamv0.TeamBindingTeamRef{ - Name: "team-1", - }, - Permission: iamv0.TeamBindingTeamPermissionMember, - }, - } - - tuple, err := convertTeamBindingToTuple(tb) - require.Error(t, err) - require.Nil(t, tuple) - require.Equal(t, errEmptyName, err) - }) - - t.Run("should return error for empty team ref name", func(t *testing.T) { - tb := &iamv0.TeamBinding{ - Spec: iamv0.TeamBindingSpec{ - Subject: iamv0.TeamBindingspecSubject{ - Name: "user-1", - }, - TeamRef: iamv0.TeamBindingTeamRef{ - Name: "", - }, - Permission: iamv0.TeamBindingTeamPermissionMember, - }, - } - - tuple, err := convertTeamBindingToTuple(tb) - require.Error(t, err) - require.Nil(t, tuple) - require.Equal(t, errEmptyName, err) - }) - - t.Run("should default to member for unknown permission", func(t *testing.T) { - tb := &iamv0.TeamBinding{ - Spec: iamv0.TeamBindingSpec{ - Subject: iamv0.TeamBindingspecSubject{ - Name: "user-1", - }, - TeamRef: iamv0.TeamBindingTeamRef{ - Name: "team-1", - }, - Permission: "unknown", // Invalid permission - }, - } - - tuple, err := convertTeamBindingToTuple(tb) - require.NoError(t, err) - require.NotNil(t, tuple) - // Should default to member relation - require.Equal(t, "member", tuple.Relation) + return false }) }