diff --git a/internal/services/configstore/api/user.go b/internal/services/configstore/api/user.go index 1bc3bc1..8fec6d6 100644 --- a/internal/services/configstore/api/user.go +++ b/internal/services/configstore/api/user.go @@ -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 } diff --git a/internal/services/configstore/command/command.go b/internal/services/configstore/command/command.go index 7ef2715..baf0523 100644 --- a/internal/services/configstore/command/command.go +++ b/internal/services/configstore/command/command.go @@ -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) diff --git a/internal/services/configstore/configstore_test.go b/internal/services/configstore/configstore_test.go index b0e1e38..6cbda98 100644 --- a/internal/services/configstore/configstore_test.go +++ b/internal/services/configstore/configstore_test.go @@ -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) diff --git a/internal/util/errors.go b/internal/util/errors.go index 379b981..3d5cd3d 100644 --- a/internal/util/errors.go +++ b/internal/util/errors.go @@ -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 +}