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.

Sign up for GitHub

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

Jump to bottom

RBAC V2 - Roles APIs #245

Merged
merged 3 commits into from
Apr 22, 2024
Merged

RBAC V2 - Roles APIs #245

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
34551eb
add roles query and APIs
bailinhe Apr 15, 2024
c357346
add changes per review
bailinhe Apr 18, 2024
086c827
Apply suggestions from code review
bailinhe Apr 19, 2024
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
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ ci: | golint test coverage ## Setup dev database and run tests.
.PHONY: test
test: ## Runs unit tests.
@echo Running unit tests...
@go test -v -timeout 30s -cover -short -tags testtools ./...
@go test -v -timeout 120s -cover -short -tags testtools ./...

.PHONY: coverage
coverage: ## Generates a test coverage report.
@echo Generating coverage report...
@go test -timeout 30s -tags testtools ./... -coverprofile=coverage.out -covermode=atomic
@go test -timeout 120s -tags testtools ./... -coverprofile=coverage.out -covermode=atomic
@go tool cover -func=coverage.out
@go tool cover -html=coverage.out

Expand Down
6 changes: 2 additions & 4 deletions cmd/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ import (
"go.infratographer.com/permissions-api/internal/storage"
)

var (
apiDefaultListen = "0.0.0.0:7602"
)
var apiDefaultListen = "0.0.0.0:7602"

var serverCmd = &cobra.Command{
Use: "server",
Expand Down Expand Up @@ -89,7 +87,7 @@ func serve(_ context.Context, cfg *config.AppConfig) {
logger.Fatalw("invalid spicedb policy", "error", err)
}

engine, err := query.NewEngine("infratographer", spiceClient, kv, store, query.WithPolicy(policy))
engine, err := query.NewEngine("infratographer", spiceClient, kv, store, query.WithPolicy(policy), query.WithLogger(logger))
if err != nil {
logger.Fatalw("error creating engine", "error", err)
}
Expand Down
20 changes: 10 additions & 10 deletions internal/api/assignments_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestAssignmentCreate(t *testing.T) {
"subject_id": "bad-id",
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand All @@ -67,7 +67,7 @@ func TestAssignmentCreate(t *testing.T) {
"subject_id": "idntusr-abc123",
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand All @@ -92,7 +92,7 @@ func TestAssignmentCreate(t *testing.T) {
"subject_id": "idntusr-def456",
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand All @@ -119,7 +119,7 @@ func TestAssignmentCreate(t *testing.T) {
"subject_id": "idntusr-def456",
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand Down Expand Up @@ -211,7 +211,7 @@ func TestAssignmentsList(t *testing.T) {
{
Name: "RoleResourceNotFound",
Input: "/api/v1/roles/permrol-abc123/assignments",
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand All @@ -233,7 +233,7 @@ func TestAssignmentsList(t *testing.T) {
{
Name: "AssignmentsRetrieved",
Input: "/api/v1/roles/permrol-abc123/assignments",
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand Down Expand Up @@ -327,7 +327,7 @@ func TestAssignmentDelete(t *testing.T) {
"subject_id": "bad-id",
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand All @@ -352,7 +352,7 @@ func TestAssignmentDelete(t *testing.T) {
"subject_id": "idntusr-abc123",
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand All @@ -377,7 +377,7 @@ func TestAssignmentDelete(t *testing.T) {
"subject_id": "idntusr-def456",
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand All @@ -404,7 +404,7 @@ func TestAssignmentDelete(t *testing.T) {
"subject_id": "idntusr-def456",
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand Down
33 changes: 33 additions & 0 deletions internal/api/response.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@ package api

import (
"errors"
"fmt"
"net/http"

"github.com/labstack/echo/v4"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"go.infratographer.com/permissions-api/internal/query"
"go.infratographer.com/permissions-api/internal/storage"
)

// ErrorResponse represents the data that the server will return on any given call
Expand All @@ -20,3 +29,27 @@ var (
// ErrResourceAlreadyExists is returned when the resource already exists
ErrResourceAlreadyExists = errors.New("resource already exists")
)

func (r *Router) errorResponse(basemsg string, err error) *echo.HTTPError {
msg := fmt.Sprintf("%s: %s", basemsg, err.Error())
httpstatus := http.StatusInternalServerError

switch {
case
errors.Is(err, storage.ErrRoleNameTaken),
errors.Is(err, query.ErrInvalidType),
errors.Is(err, query.ErrInvalidArgument),
status.Code(err) == codes.InvalidArgument,
status.Code(err) == codes.FailedPrecondition:
jnschaeffer marked this conversation as resolved.
Show resolved Hide resolved
httpstatus = http.StatusBadRequest
case
errors.Is(err, storage.ErrNoRoleFound),
errors.Is(err, query.ErrRoleNotFound),
errors.Is(err, query.ErrRoleBindingNotFound):
httpstatus = http.StatusNotFound
default:
msg = basemsg
}

return echo.NewHTTPError(httpstatus, msg).SetInternal(err)
}
20 changes: 10 additions & 10 deletions internal/api/roles_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestRoleCreate(t *testing.T) {
},
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand Down Expand Up @@ -82,7 +82,7 @@ func TestRoleCreate(t *testing.T) {
},
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand Down Expand Up @@ -114,7 +114,7 @@ func TestRoleCreate(t *testing.T) {
},
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand Down Expand Up @@ -267,7 +267,7 @@ func TestRoleUpdate(t *testing.T) {
},
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand Down Expand Up @@ -300,7 +300,7 @@ func TestRoleUpdate(t *testing.T) {
},
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand Down Expand Up @@ -333,7 +333,7 @@ func TestRoleUpdate(t *testing.T) {
},
},
},
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand Down Expand Up @@ -473,7 +473,7 @@ func TestRoleGet(t *testing.T) {
{
Name: "RoleResourceNotFound",
Input: "/api/v1/roles/permrol-abc123",
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand Down Expand Up @@ -593,7 +593,7 @@ func TestRoleDelete(t *testing.T) {
{
Name: "UnknownError",
Input: "/api/v1/roles/permrol-abc123",
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand All @@ -617,7 +617,7 @@ func TestRoleDelete(t *testing.T) {
{
Name: "RoleResourceNotFound",
Input: "/api/v1/roles/permrol-abc123",
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand All @@ -639,7 +639,7 @@ func TestRoleDelete(t *testing.T) {
{
Name: "RoleDeleted",
Input: "/api/v1/roles/permrol-abc123",
SetupFn: func(ctx context.Context, t *testing.T) context.Context {
SetupFn: func(ctx context.Context, _ *testing.T) context.Context {
engine := mock.Engine{
Namespace: "test",
}
Expand Down
Loading
Loading