diff --git a/internal/query/errors.go b/internal/query/errors.go index 07acb3bd..f43f1f2f 100644 --- a/internal/query/errors.go +++ b/internal/query/errors.go @@ -57,10 +57,6 @@ var ( // request attempts to use a resource that does not support role binding v2 ErrResourceDoesNotSupportRoleBindingV2 = fmt.Errorf("%w: resource does not support role binding v2", ErrInvalidArgument) - // ErrCreateRoleBindingWithNoSubjects represents an error when a role - // binding is created with no subjects - ErrCreateRoleBindingWithNoSubjects = fmt.Errorf("%w: role binding must have at least one subject", ErrInvalidArgument) - // ErrRoleBindingHasNoRelationships represents an internal error when a // role binding has no relationships ErrRoleBindingHasNoRelationships = errors.New("role binding has no relationships") diff --git a/internal/query/relations.go b/internal/query/relations.go index 58480bf0..be387a2a 100644 --- a/internal/query/relations.go +++ b/internal/query/relations.go @@ -362,8 +362,8 @@ func (e *engine) CreateRole(ctx context.Context, actor, res types.Resource, role // diff determines which entities needs to be added and removed. // If no new entity are provided it is assumed no changes are requested. -func diff(current, incoming []string) ([]string, []string) { - if len(incoming) == 0 { +func diff(current, incoming []string, allowEmptyIncoming bool) ([]string, []string) { + if len(incoming) == 0 && !allowEmptyIncoming { return nil, nil } @@ -445,7 +445,7 @@ func (e *engine) UpdateRole(ctx context.Context, actor, roleResource types.Resou newName = role.Name } - addActions, remActions := diff(role.Actions, newActions) + addActions, remActions := diff(role.Actions, newActions, false) // If no changes, return existing role with no changes. if newName == role.Name && len(addActions) == 0 && len(remActions) == 0 { diff --git a/internal/query/rolebindings.go b/internal/query/rolebindings.go index 80b00349..7250bc41 100644 --- a/internal/query/rolebindings.go +++ b/internal/query/rolebindings.go @@ -103,14 +103,6 @@ func (e *engine) CreateRoleBinding( ) defer span.End() - if len(subjects) == 0 { - err := ErrCreateRoleBindingWithNoSubjects - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - - return types.RoleBinding{}, err - } - if err := e.isRoleBindable(ctx, roleResource, resource); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -467,7 +459,7 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, actor, rb types.Resource newSubjectIDs[i] = subj.SubjectResource.ID } - add, remove := diff(current, incoming) + add, remove := diff(current, incoming, true) // return if there are no changes if (len(add) + len(remove)) == 0 { diff --git a/internal/query/rolebindings_test.go b/internal/query/rolebindings_test.go index d0f294b9..d9412db3 100644 --- a/internal/query/rolebindings_test.go +++ b/internal/query/rolebindings_test.go @@ -121,17 +121,6 @@ func TestCreateRoleBinding(t *testing.T) { assert.ErrorIs(t, res.Err, ErrRoleNotFound) }, }, - { - Name: "CreateRoleBindingWithNoSubjects", - Input: input{ - resource: root, - role: roleRes, - }, - CheckFn: func(ctx context.Context, t *testing.T, tr testingx.TestResult[types.RoleBinding]) { - assert.ErrorIs(t, tr.Err, ErrInvalidArgument) - assert.ErrorIs(t, tr.Err, ErrCreateRoleBindingWithNoSubjects) - }, - }, { Name: "CreateRoleBindingSuccess", Input: input{ @@ -405,6 +394,17 @@ func TestUpdateRoleBinding(t *testing.T) { }, Sync: true, }, + { + Name: "UpdateRoleBindingRemoveAllSubjects", + Input: input{ + rb: rbRes, + subj: []types.RoleBindingSubject{}, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.NoError(t, res.Err) + assert.Len(t, res.Success.SubjectIDs, 0) + }, + }, } testFn := func(ctx context.Context, in input) testingx.TestResult[types.RoleBinding] { diff --git a/internal/query/roles_v2.go b/internal/query/roles_v2.go index bdfa9287..5ef55924 100644 --- a/internal/query/roles_v2.go +++ b/internal/query/roles_v2.go @@ -260,7 +260,7 @@ func (e *engine) UpdateRoleV2(ctx context.Context, actor, roleResource types.Res newName = role.Name } - addActions, rmActions := diff(role.Actions, newActions) + addActions, rmActions := diff(role.Actions, newActions, false) // If no changes, return existing role if newName == role.Name && len(addActions) == 0 && len(rmActions) == 0 {