configstore: use ErrBadRequest

This commit is contained in:
Simone Gotti 2019-03-12 15:12:19 +01:00
parent f09602cdc3
commit 7d105f1232
4 changed files with 81 additions and 53 deletions

View File

@ -34,6 +34,19 @@ type UserHandler struct {
readDB *readdb.ReadDB
}
func httpError(w http.ResponseWriter, err error) bool {
if err != nil {
if util.IsErrBadRequest(err) {
http.Error(w, err.Error(), http.StatusBadRequest)
} else {
http.Error(w, "", http.StatusInternalServerError)
}
return true
}
return false
}
func NewUserHandler(logger *zap.Logger, readDB *readdb.ReadDB) *UserHandler {
return &UserHandler{log: logger.Sugar(), readDB: readDB}
}
@ -123,9 +136,8 @@ func (h *CreateUserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
user, err := h.ch.CreateUser(ctx, &req)
if err != nil {
if httpError(w, err) {
h.log.Errorf("err: %+v", err)
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
@ -152,10 +164,9 @@ func (h *DeleteUserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
vars := mux.Vars(r)
userName := vars["username"]
if err := h.ch.DeleteUser(ctx, userName); err != nil {
err := h.ch.DeleteUser(ctx, userName)
if httpError(w, err) {
h.log.Errorf("err: %+v", err)
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
}
@ -325,9 +336,8 @@ func (h *CreateUserLAHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
UserAccessToken: req.UserAccessToken,
}
user, err := h.ch.CreateUserLA(ctx, creq)
if err != nil {
if httpError(w, err) {
h.log.Errorf("err: %+v", err)
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
@ -353,10 +363,9 @@ func (h *DeleteUserLAHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
userName := vars["username"]
laID := vars["laid"]
if err := h.ch.DeleteUserLA(ctx, userName, laID); err != nil {
err := h.ch.DeleteUserLA(ctx, userName, laID)
if httpError(w, err) {
h.log.Errorf("err: %+v", err)
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
}
@ -400,9 +409,8 @@ func (h *UpdateUserLAHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
UserAccessToken: req.UserAccessToken,
}
user, err := h.ch.UpdateUserLA(ctx, creq)
if err != nil {
if httpError(w, err) {
h.log.Errorf("err: %+v", err)
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
@ -443,9 +451,8 @@ func (h *CreateUserTokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques
}
token, err := h.ch.CreateUserToken(ctx, userName, req.TokenName)
if err != nil {
if httpError(w, err) {
h.log.Errorf("err: %+v", err)
http.Error(w, err.Error(), http.StatusBadRequest)
return
}

View File

@ -46,16 +46,16 @@ func NewCommandHandler(logger *zap.Logger, readDB *readdb.ReadDB, wal *wal.WalMa
func (s *CommandHandler) CreateProject(ctx context.Context, project *types.Project) (*types.Project, error) {
if project.Name == "" {
return nil, errors.Errorf("project name required")
return nil, util.NewErrBadRequest(errors.Errorf("project name required"))
}
if project.OwnerType == "" {
return nil, errors.Errorf("project ownertype required")
return nil, util.NewErrBadRequest(errors.Errorf("project ownertype required"))
}
if project.OwnerID == "" {
return nil, errors.Errorf("project ownerid required")
return nil, util.NewErrBadRequest(errors.Errorf("project ownerid required"))
}
if !types.IsValidOwnerType(project.OwnerType) {
return nil, errors.Errorf("invalid project ownertype %q", project.OwnerType)
return nil, util.NewErrBadRequest(errors.Errorf("invalid project ownertype %q", project.OwnerType))
}
var cgt *wal.ChangeGroupsUpdateToken
@ -77,7 +77,7 @@ func (s *CommandHandler) CreateProject(ctx context.Context, project *types.Proje
return err
}
if user == nil {
return errors.Errorf("user id %q doesn't exist", project.OwnerID)
return util.NewErrBadRequest(errors.Errorf("user id %q doesn't exist", project.OwnerID))
}
case types.OwnerTypeOrganization:
org, err := s.readDB.GetOrg(tx, project.OwnerID)
@ -85,7 +85,7 @@ func (s *CommandHandler) CreateProject(ctx context.Context, project *types.Proje
return err
}
if org == nil {
return errors.Errorf("organization id %q doesn't exist", project.OwnerID)
return util.NewErrBadRequest(errors.Errorf("organization id %q doesn't exist", project.OwnerID))
}
}
// check duplicate project name
@ -94,7 +94,7 @@ func (s *CommandHandler) CreateProject(ctx context.Context, project *types.Proje
return err
}
if p != nil {
return errors.Errorf("project with name %q for %s with id %q already exists", p.Name, project.OwnerType, project.OwnerID)
return util.NewErrBadRequest(errors.Errorf("project with name %q for %s with id %q already exists", p.Name, project.OwnerType, project.OwnerID))
}
return nil
})
@ -140,7 +140,7 @@ func (s *CommandHandler) DeleteProject(ctx context.Context, projectID string) er
return err
}
if project == nil {
return errors.Errorf("project %q doesn't exist", projectID)
return util.NewErrBadRequest(errors.Errorf("project %q doesn't exist", projectID))
}
return nil
})
@ -161,7 +161,7 @@ func (s *CommandHandler) DeleteProject(ctx context.Context, projectID string) er
func (s *CommandHandler) CreateUser(ctx context.Context, user *types.User) (*types.User, error) {
if user.UserName == "" {
return nil, errors.Errorf("user name required")
return nil, util.NewErrBadRequest(errors.Errorf("user name required"))
}
var cgt *wal.ChangeGroupsUpdateToken
@ -181,7 +181,7 @@ func (s *CommandHandler) CreateUser(ctx context.Context, user *types.User) (*typ
return err
}
if u != nil {
return errors.Errorf("user with name %q already exists", u.UserName)
return util.NewErrBadRequest(errors.Errorf("user with name %q already exists", u.UserName))
}
return nil
})
@ -227,7 +227,7 @@ func (s *CommandHandler) DeleteUser(ctx context.Context, userName string) error
return err
}
if user == nil {
return errors.Errorf("user %q doesn't exist", userName)
return util.NewErrBadRequest(errors.Errorf("user %q doesn't exist", userName))
}
return nil
})
@ -260,10 +260,10 @@ type CreateUserLARequest struct {
func (s *CommandHandler) CreateUserLA(ctx context.Context, req *CreateUserLARequest) (*types.LinkedAccount, error) {
if req.UserName == "" {
return nil, errors.Errorf("user name required")
return nil, util.NewErrBadRequest(errors.Errorf("user name required"))
}
if req.RemoteSourceName == "" {
return nil, errors.Errorf("remote source name required")
return nil, util.NewErrBadRequest(errors.Errorf("remote source name required"))
}
var user *types.User
@ -279,7 +279,7 @@ func (s *CommandHandler) CreateUserLA(ctx context.Context, req *CreateUserLARequ
return err
}
if user == nil {
return errors.Errorf("user %q doesn't exist", req.UserName)
return util.NewErrBadRequest(errors.Errorf("user %q doesn't exist", req.UserName))
}
cgNames := []string{user.ID}
@ -293,7 +293,7 @@ func (s *CommandHandler) CreateUserLA(ctx context.Context, req *CreateUserLARequ
return err
}
if rs == nil {
return errors.Errorf("remote source %q doesn't exist", req.RemoteSourceName)
return util.NewErrBadRequest(errors.Errorf("remote source %q doesn't exist", req.RemoteSourceName))
}
return nil
})
@ -335,10 +335,10 @@ func (s *CommandHandler) CreateUserLA(ctx context.Context, req *CreateUserLARequ
func (s *CommandHandler) DeleteUserLA(ctx context.Context, userName, laID string) error {
if userName == "" {
return errors.Errorf("user name required")
return util.NewErrBadRequest(errors.Errorf("user name required"))
}
if laID == "" {
return errors.Errorf("user linked account id required")
return util.NewErrBadRequest(errors.Errorf("user linked account id required"))
}
var user *types.User
@ -353,7 +353,7 @@ func (s *CommandHandler) DeleteUserLA(ctx context.Context, userName, laID string
return err
}
if user == nil {
return errors.Errorf("user %q doesn't exist", userName)
return util.NewErrBadRequest(errors.Errorf("user %q doesn't exist", userName))
}
cgNames := []string{user.ID}
@ -370,7 +370,7 @@ func (s *CommandHandler) DeleteUserLA(ctx context.Context, userName, laID string
_, ok := user.LinkedAccounts[laID]
if !ok {
return errors.Errorf("linked account id %q for user %q doesn't exist", laID, userName)
return util.NewErrBadRequest(errors.Errorf("linked account id %q for user %q doesn't exist", laID, userName))
}
delete(user.LinkedAccounts, laID)
@ -403,7 +403,7 @@ type UpdateUserLARequest struct {
func (s *CommandHandler) UpdateUserLA(ctx context.Context, req *UpdateUserLARequest) (*types.LinkedAccount, error) {
if req.UserName == "" {
return nil, errors.Errorf("user name required")
return nil, util.NewErrBadRequest(errors.Errorf("user name required"))
}
var user *types.User
@ -419,7 +419,7 @@ func (s *CommandHandler) UpdateUserLA(ctx context.Context, req *UpdateUserLARequ
return err
}
if user == nil {
return errors.Errorf("user %q doesn't exist", req.UserName)
return util.NewErrBadRequest(errors.Errorf("user %q doesn't exist", req.UserName))
}
cgNames := []string{user.ID}
@ -430,7 +430,7 @@ func (s *CommandHandler) UpdateUserLA(ctx context.Context, req *UpdateUserLARequ
la, ok := user.LinkedAccounts[req.LinkedAccountID]
if !ok {
return errors.Errorf("linked account id %q for user %q doesn't exist", req.LinkedAccountID, user.UserName)
return util.NewErrBadRequest(errors.Errorf("linked account id %q for user %q doesn't exist", req.LinkedAccountID, user.UserName))
}
rs, err = s.readDB.GetRemoteSource(tx, la.RemoteSourceID)
@ -438,7 +438,7 @@ func (s *CommandHandler) UpdateUserLA(ctx context.Context, req *UpdateUserLARequ
return err
}
if rs == nil {
return errors.Errorf("remote source with id %q doesn't exist", la.RemoteSourceID)
return util.NewErrBadRequest(errors.Errorf("remote source with id %q doesn't exist", la.RemoteSourceID))
}
return nil
})
@ -472,7 +472,7 @@ func (s *CommandHandler) UpdateUserLA(ctx context.Context, req *UpdateUserLARequ
func (s *CommandHandler) CreateUserToken(ctx context.Context, userName, tokenName string) (string, error) {
if userName == "" {
return "", errors.Errorf("user name required")
return "", util.NewErrBadRequest(errors.Errorf("user name required"))
}
var user *types.User
@ -487,7 +487,7 @@ func (s *CommandHandler) CreateUserToken(ctx context.Context, userName, tokenNam
return err
}
if user == nil {
return errors.Errorf("user %q doesn't exist", userName)
return util.NewErrBadRequest(errors.Errorf("user %q doesn't exist", userName))
}
cgNames := []string{user.ID}
@ -528,7 +528,7 @@ func (s *CommandHandler) CreateUserToken(ctx context.Context, userName, tokenNam
func (s *CommandHandler) CreateRemoteSource(ctx context.Context, remoteSource *types.RemoteSource) (*types.RemoteSource, error) {
if remoteSource.Name == "" {
return nil, errors.Errorf("remotesource name required")
return nil, util.NewErrBadRequest(errors.Errorf("remotesource name required"))
}
var cgt *wal.ChangeGroupsUpdateToken
@ -548,7 +548,7 @@ func (s *CommandHandler) CreateRemoteSource(ctx context.Context, remoteSource *t
return err
}
if u != nil {
return errors.Errorf("remoteSource %q already exists", u.Name)
return util.NewErrBadRequest(errors.Errorf("remoteSource %q already exists", u.Name))
}
return nil
})
@ -594,7 +594,7 @@ func (s *CommandHandler) DeleteRemoteSource(ctx context.Context, remoteSourceNam
return err
}
if remoteSource == nil {
return errors.Errorf("remotesource %q doesn't exist", remoteSourceName)
return util.NewErrBadRequest(errors.Errorf("remotesource %q doesn't exist", remoteSourceName))
}
return nil
})
@ -616,7 +616,7 @@ func (s *CommandHandler) DeleteRemoteSource(ctx context.Context, remoteSourceNam
func (s *CommandHandler) CreateOrg(ctx context.Context, org *types.Organization) (*types.Organization, error) {
if org.Name == "" {
return nil, errors.Errorf("org name required")
return nil, util.NewErrBadRequest(errors.Errorf("org name required"))
}
var cgt *wal.ChangeGroupsUpdateToken
@ -636,7 +636,7 @@ func (s *CommandHandler) CreateOrg(ctx context.Context, org *types.Organization)
return err
}
if u != nil {
return errors.Errorf("org %q already exists", u.Name)
return util.NewErrBadRequest(errors.Errorf("org %q already exists", u.Name))
}
return nil
})
@ -683,7 +683,7 @@ func (s *CommandHandler) DeleteOrg(ctx context.Context, orgName string) error {
return err
}
if org == nil {
return errors.Errorf("org %q doesn't exist", orgName)
return util.NewErrBadRequest(errors.Errorf("org %q doesn't exist", orgName))
}
// get org projects
projects, err = s.readDB.GetOwnerProjects(tx, org.ID, "", 0, false)

View File

@ -52,7 +52,7 @@ func shutdownEtcd(tetcd *testutil.TestEmbeddedEtcd) {
}
}
func setupConfigstore(t *testing.T, ctx context.Context, dir string) (*ConfigStore, *testutil.TestEtcd) {
func setupConfigstore(t *testing.T, ctx context.Context, dir string) (*ConfigStore, *testutil.TestEmbeddedEtcd) {
etcdDir, err := ioutil.TempDir(dir, "etcd")
tetcd := setupEtcd(t, etcdDir)
@ -320,7 +320,7 @@ func TestUser(t *testing.T) {
time.Sleep(2 * time.Second)
t.Run("create duplicated user", func(t *testing.T) {
expectedErr := fmt.Sprintf("user with name %q already exists", "user01")
expectedErr := fmt.Sprintf("bad request: user with name %q already exists", "user01")
_, err := cs.ch.CreateUser(ctx, &types.User{UserName: "user01"})
if err == nil {
t.Fatalf("expected error %v, got nil err", expectedErr)
@ -403,42 +403,42 @@ func TestProject(t *testing.T) {
}
})
t.Run("create duplicated project for user", func(t *testing.T) {
expectedErr := fmt.Sprintf("project with name %q for user with id %q already exists", "project01", user.ID)
expectedErr := fmt.Sprintf("bad request: project with name %q for user with id %q already exists", "project01", user.ID)
_, err := cs.ch.CreateProject(ctx, &types.Project{Name: "project01", OwnerType: "user", OwnerID: user.ID})
if err.Error() != expectedErr {
t.Fatalf("expected err %v, got err: %v", expectedErr, err)
}
})
t.Run("create duplicated project for org", func(t *testing.T) {
expectedErr := fmt.Sprintf("project with name %q for organization with id %q already exists", "project01", org.ID)
expectedErr := fmt.Sprintf("bad request: project with name %q for organization with id %q already exists", "project01", org.ID)
_, err := cs.ch.CreateProject(ctx, &types.Project{Name: "project01", OwnerType: "organization", OwnerID: org.ID})
if err.Error() != expectedErr {
t.Fatalf("expected err %v, got err: %v", expectedErr, err)
}
})
t.Run("create project with owner as unexistent user", func(t *testing.T) {
expectedErr := `user id "unexistentid" doesn't exist`
expectedErr := `bad request: user id "unexistentid" doesn't exist`
_, err := cs.ch.CreateProject(ctx, &types.Project{Name: "project01", OwnerType: "user", OwnerID: "unexistentid"})
if err.Error() != expectedErr {
t.Fatalf("expected err %v, got err: %v", expectedErr, err)
}
})
t.Run("create project with owner as unexistent org", func(t *testing.T) {
expectedErr := `organization id "unexistentid" doesn't exist`
expectedErr := `bad request: organization id "unexistentid" doesn't exist`
_, err := cs.ch.CreateProject(ctx, &types.Project{Name: "project01", OwnerType: "organization", OwnerID: "unexistentid"})
if err.Error() != expectedErr {
t.Fatalf("expected err %v, got err: %v", expectedErr, err)
}
})
t.Run("create project without ownertype specified", func(t *testing.T) {
expectedErr := "project ownertype required"
expectedErr := "bad request: project ownertype required"
_, err := cs.ch.CreateProject(ctx, &types.Project{Name: "project01"})
if err.Error() != expectedErr {
t.Fatalf("expected err %v, got err: %v", expectedErr, err)
}
})
t.Run("create project without ownerid specified", func(t *testing.T) {
expectedErr := "project ownerid required"
expectedErr := "bad request: project ownerid required"
_, err := cs.ch.CreateProject(ctx, &types.Project{Name: "project01", OwnerType: "organization"})
if err.Error() != expectedErr {
t.Fatalf("expected err %v, got err: %v", expectedErr, err)

View File

@ -15,9 +15,11 @@
package util
import (
"fmt"
"strings"
)
// Errors is an error that contains multiple errors
type Errors struct {
Errs []error
}
@ -54,3 +56,22 @@ func (e *Errors) Equal(e2 error) bool {
return CompareStringSliceNoOrder(errs1, errs2)
}
// ErrBadRequest represent an error caused by a bad command request
// it's used to differentiate an internal error from an user error
type ErrBadRequest struct {
Err error
}
func (e *ErrBadRequest) Error() string {
return fmt.Sprintf("bad request: %s", e.Err.Error())
}
func NewErrBadRequest(err error) *ErrBadRequest {
return &ErrBadRequest{Err: err}
}
func IsErrBadRequest(err error) bool {
_, ok := err.(*ErrBadRequest)
return ok
}