Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use full consistent reads #200

Merged
merged 1 commit into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions cmd/createrole.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,12 @@ func createRole(ctx context.Context, cfg *config.AppConfig) {
logger.Fatalw("error creating subject resource", "error", err)
}

role, _, err := engine.CreateRole(ctx, resource, actions)
role, err := engine.CreateRole(ctx, resource, actions)
if err != nil {
logger.Fatalw("error creating role", "error", err)
}

_, err = engine.AssignSubjectRole(ctx, subjectResource, role)
if err != nil {
if err = engine.AssignSubjectRole(ctx, subjectResource, role); err != nil {
logger.Fatalw("error creating role", "error", err)
}

Expand Down
14 changes: 6 additions & 8 deletions internal/api/assignments.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (r *Router) assignmentCreate(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)
}

resource, err := r.engine.GetRoleResource(ctx, roleResource, "")
resource, err := r.engine.GetRoleResource(ctx, roleResource)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err)
}
Expand All @@ -63,8 +63,7 @@ func (r *Router) assignmentCreate(c echo.Context) error {
ID: roleID,
}

_, err = r.engine.AssignSubjectRole(ctx, assigneeResource, role)
if err != nil {
if err = r.engine.AssignSubjectRole(ctx, assigneeResource, role); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error creating resource").SetInternal(err)
}

Expand Down Expand Up @@ -96,7 +95,7 @@ func (r *Router) assignmentsList(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)
}

resource, err := r.engine.GetRoleResource(ctx, roleResource, "")
resource, err := r.engine.GetRoleResource(ctx, roleResource)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err)
}
Expand All @@ -109,7 +108,7 @@ func (r *Router) assignmentsList(c echo.Context) error {
ID: roleID,
}

assignments, err := r.engine.ListAssignments(ctx, role, "")
assignments, err := r.engine.ListAssignments(ctx, role)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error listing assignments").SetInternal(err)
}
Expand Down Expand Up @@ -169,7 +168,7 @@ func (r *Router) assignmentDelete(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)
}

resource, err := r.engine.GetRoleResource(ctx, roleResource, "")
resource, err := r.engine.GetRoleResource(ctx, roleResource)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err)
}
Expand All @@ -182,8 +181,7 @@ func (r *Router) assignmentDelete(c echo.Context) error {
ID: roleID,
}

_, err = r.engine.UnassignSubjectRole(ctx, assigneeResource, role)
if err != nil {
if err = r.engine.UnassignSubjectRole(ctx, assigneeResource, role); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error deleting assignment").SetInternal(err)
}

Expand Down
4 changes: 2 additions & 2 deletions internal/api/relationships.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (r *Router) relationshipListFrom(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "error listing relationships").SetInternal(err)
}

rels, err := r.engine.ListRelationshipsFrom(ctx, resource, "")
rels, err := r.engine.ListRelationshipsFrom(ctx, resource)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error listing relationships").SetInternal(err)
}
Expand Down Expand Up @@ -62,7 +62,7 @@ func (r *Router) relationshipListTo(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "error listing relationships").SetInternal(err)
}

rels, err := r.engine.ListRelationshipsTo(ctx, resource, "")
rels, err := r.engine.ListRelationshipsTo(ctx, resource)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error listing relationships").SetInternal(err)
}
Expand Down
15 changes: 7 additions & 8 deletions internal/api/roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (r *Router) roleCreate(c echo.Context) error {
return err
}

role, _, err := r.engine.CreateRole(ctx, resource, reqBody.Actions)
role, err := r.engine.CreateRole(ctx, resource, reqBody.Actions)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error creating resource").SetInternal(err)
}
Expand Down Expand Up @@ -85,7 +85,7 @@ func (r *Router) roleGet(c echo.Context) error {

// Roles belong to resources by way of the actions they can perform; do the permissions
// check on the role resource.
resource, err := r.engine.GetRoleResource(ctx, roleResource, "")
resource, err := r.engine.GetRoleResource(ctx, roleResource)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)
}
Expand All @@ -96,7 +96,7 @@ func (r *Router) roleGet(c echo.Context) error {
return err
}

role, err := r.engine.GetRole(ctx, roleResource, "")
role, err := r.engine.GetRole(ctx, roleResource)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err)
}
Expand Down Expand Up @@ -134,7 +134,7 @@ func (r *Router) rolesList(c echo.Context) error {
return err
}

roles, err := r.engine.ListRoles(ctx, resource, "")
roles, err := r.engine.ListRoles(ctx, resource)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting role").SetInternal(err)
}
Expand Down Expand Up @@ -178,7 +178,7 @@ func (r *Router) roleDelete(c echo.Context) error {

// Roles belong to resources by way of the actions they can perform; do the permissions
// check on the role resource.
resource, err := r.engine.GetRoleResource(ctx, roleResource, "")
resource, err := r.engine.GetRoleResource(ctx, roleResource)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "error getting resource").SetInternal(err)
}
Expand All @@ -187,8 +187,7 @@ func (r *Router) roleDelete(c echo.Context) error {
return err
}

_, err = r.engine.DeleteRole(ctx, roleResource, "")
if err != nil {
if err = r.engine.DeleteRole(ctx, roleResource); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error deleting resource").SetInternal(err)
}

Expand Down Expand Up @@ -222,7 +221,7 @@ func (r *Router) roleGetResource(c echo.Context) error {

// There's a little irony here in that getting a role's resource here is required to actually
// do the permissions check.
resource, err := r.engine.GetRoleResource(ctx, roleResource, "")
resource, err := r.engine.GetRoleResource(ctx, roleResource)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err)
}
Expand Down
6 changes: 2 additions & 4 deletions internal/pubsub/subscriber.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,17 +160,15 @@ func (s *Subscriber) processEvent(msg events.Request[events.AuthRelationshipRequ

func (s *Subscriber) createRelationships(ctx context.Context, relationships []types.Relationship) error {
// Attempt to create the relationships in SpiceDB.
_, err := s.qe.CreateRelationships(ctx, relationships)
if err != nil {
if err := s.qe.CreateRelationships(ctx, relationships); err != nil {
return fmt.Errorf("%w: error creating relationships", err)
}

return nil
}

func (s *Subscriber) deleteRelationships(ctx context.Context, relationships []types.Relationship) error {
_, err := s.qe.DeleteRelationships(ctx, relationships...)
if err != nil {
if err := s.qe.DeleteRelationships(ctx, relationships...); err != nil {
return err
}

Expand Down
6 changes: 3 additions & 3 deletions internal/pubsub/subscriber_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestNATS(t *testing.T) {
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
var engine mock.Engine
engine.On("CreateRelationships").Return("", nil)
engine.On("CreateRelationships").Return(nil)

return context.WithValue(ctx, contextKeyEngine, &engine)
},
Expand All @@ -130,7 +130,7 @@ func TestNATS(t *testing.T) {
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
var engine mock.Engine
engine.On("CreateRelationships").Return("", io.ErrUnexpectedEOF)
engine.On("CreateRelationships").Return(io.ErrUnexpectedEOF)

return context.WithValue(ctx, contextKeyEngine, &engine)
},
Expand Down Expand Up @@ -166,7 +166,7 @@ func TestNATS(t *testing.T) {
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
var engine mock.Engine
engine.Namespace = "gooddelete"
engine.On("DeleteRelationships").Return("", nil)
engine.On("DeleteRelationships").Return(nil)

return context.WithValue(ctx, contextKeyEngine, &engine)
},
Expand Down
40 changes: 20 additions & 20 deletions internal/query/mock/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,24 @@ type Engine struct {
}

// AssignSubjectRole does nothing but satisfies the Engine interface.
func (e *Engine) AssignSubjectRole(ctx context.Context, subject types.Resource, role types.Role) (string, error) {
return "", nil
func (e *Engine) AssignSubjectRole(ctx context.Context, subject types.Resource, role types.Role) error {
return nil
}

// UnassignSubjectRole does nothing but satisfies the Engine interface.
func (e *Engine) UnassignSubjectRole(ctx context.Context, subject types.Resource, role types.Role) (string, error) {
return "", nil
func (e *Engine) UnassignSubjectRole(ctx context.Context, subject types.Resource, role types.Role) error {
return nil
}

// CreateRelationships does nothing but satisfies the Engine interface.
func (e *Engine) CreateRelationships(ctx context.Context, rels []types.Relationship) (string, error) {
func (e *Engine) CreateRelationships(ctx context.Context, rels []types.Relationship) error {
args := e.Called()

return args.String(0), args.Error(1)
return args.Error(0)
}

// CreateRole creates a Role object and does not persist it anywhere.
func (e *Engine) CreateRole(ctx context.Context, res types.Resource, actions []string) (types.Role, string, error) {
func (e *Engine) CreateRole(ctx context.Context, res types.Resource, actions []string) (types.Role, error) {
// Copy actions instead of using the given slice
outActions := make([]string, len(actions))

Expand All @@ -54,58 +54,58 @@ func (e *Engine) CreateRole(ctx context.Context, res types.Resource, actions []s
Actions: outActions,
}

return role, "", nil
return role, nil
}

// GetRole returns nothing but satisfies the Engine interface.
func (e *Engine) GetRole(ctx context.Context, roleResource types.Resource, queryToken string) (types.Role, error) {
func (e *Engine) GetRole(ctx context.Context, roleResource types.Resource) (types.Role, error) {
return types.Role{}, nil
}

// GetRoleResource returns nothing but satisfies the Engine interface.
func (e *Engine) GetRoleResource(ctx context.Context, roleResource types.Resource, queryToken string) (types.Resource, error) {
func (e *Engine) GetRoleResource(ctx context.Context, roleResource types.Resource) (types.Resource, error) {
return types.Resource{}, nil
}

// ListAssignments returns nothing but satisfies the Engine interface.
func (e *Engine) ListAssignments(ctx context.Context, role types.Role, queryToken string) ([]types.Resource, error) {
func (e *Engine) ListAssignments(ctx context.Context, role types.Role) ([]types.Resource, error) {
return nil, nil
}

// ListRelationshipsFrom returns nothing but satisfies the Engine interface.
func (e *Engine) ListRelationshipsFrom(ctx context.Context, resource types.Resource, queryToken string) ([]types.Relationship, error) {
func (e *Engine) ListRelationshipsFrom(ctx context.Context, resource types.Resource) ([]types.Relationship, error) {
return nil, nil
}

// ListRelationshipsTo returns nothing but satisfies the Engine interface.
func (e *Engine) ListRelationshipsTo(ctx context.Context, resource types.Resource, queryToken string) ([]types.Relationship, error) {
func (e *Engine) ListRelationshipsTo(ctx context.Context, resource types.Resource) ([]types.Relationship, error) {
return nil, nil
}

// ListRoles returns nothing but satisfies the Engine interface.
func (e *Engine) ListRoles(ctx context.Context, resource types.Resource, queryToken string) ([]types.Role, error) {
func (e *Engine) ListRoles(ctx context.Context, resource types.Resource) ([]types.Role, error) {
return nil, nil
}

// DeleteRelationships does nothing but satisfies the Engine interface.
func (e *Engine) DeleteRelationships(ctx context.Context, relationships ...types.Relationship) (string, error) {
func (e *Engine) DeleteRelationships(ctx context.Context, relationships ...types.Relationship) error {
args := e.Called()

return args.String(0), args.Error(1)
return args.Error(0)
}

// DeleteRole does nothing but satisfies the Engine interface.
func (e *Engine) DeleteRole(ctx context.Context, roleResource types.Resource, queryToken string) (string, error) {
func (e *Engine) DeleteRole(ctx context.Context, roleResource types.Resource) error {
args := e.Called()

return args.String(0), args.Error(1)
return args.Error(0)
}

// DeleteResourceRelationships does nothing but satisfies the Engine interface.
func (e *Engine) DeleteResourceRelationships(ctx context.Context, resource types.Resource) (string, error) {
func (e *Engine) DeleteResourceRelationships(ctx context.Context, resource types.Resource) error {
args := e.Called()

return args.String(0), args.Error(1)
return args.Error(0)
}

// NewResourceFromID creates a new resource object based on the given ID.
Expand Down
Loading
Loading