diff --git a/internal/services/configstore/api/api.go b/internal/services/configstore/api/api.go index 97378c9..594c580 100644 --- a/internal/services/configstore/api/api.go +++ b/internal/services/configstore/api/api.go @@ -30,7 +30,10 @@ type ErrorResponse struct { } func ErrorResponseFromError(err error) *ErrorResponse { - if util.IsErrBadRequest(err) { + switch { + case util.IsErrBadRequest(err): + fallthrough + case util.IsErrNotFound(err): return &ErrorResponse{Message: err.Error()} } @@ -39,24 +42,28 @@ func ErrorResponseFromError(err error) *ErrorResponse { } func httpError(w http.ResponseWriter, err error) bool { - if err != nil { - response := ErrorResponseFromError(err) - resj, merr := json.Marshal(response) - if merr != nil { - w.WriteHeader(http.StatusInternalServerError) - return true - } - if util.IsErrBadRequest(err) { - w.WriteHeader(http.StatusBadRequest) - w.Write(resj) - } else { - w.WriteHeader(http.StatusInternalServerError) - w.Write(resj) - } - return true + if err == nil { + return false } - return false + response := ErrorResponseFromError(err) + resj, merr := json.Marshal(response) + if merr != nil { + w.WriteHeader(http.StatusInternalServerError) + return true + } + switch { + case util.IsErrBadRequest(err): + w.WriteHeader(http.StatusBadRequest) + w.Write(resj) + case util.IsErrNotFound(err): + w.WriteHeader(http.StatusNotFound) + w.Write(resj) + default: + w.WriteHeader(http.StatusInternalServerError) + w.Write(resj) + } + return true } func httpResponse(w http.ResponseWriter, code int, res interface{}) error { diff --git a/internal/services/configstore/api/client.go b/internal/services/configstore/api/client.go index 70482d6..3292499 100644 --- a/internal/services/configstore/api/client.go +++ b/internal/services/configstore/api/client.go @@ -81,16 +81,14 @@ func (c *Client) getResponse(ctx context.Context, method, path string, query url defer resp.Body.Close() data, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, err + return resp, err } - if len(data) <= 1 { - return resp, errors.New(resp.Status) + errMap := make(map[string]interface{}) + if err = json.Unmarshal(data, &errMap); err != nil { + return resp, fmt.Errorf("unknown api error (code: %d): %s", resp.StatusCode, string(data)) } - - // TODO(sgotti) use a json error response - - return resp, errors.New(string(data)) + return resp, errors.New(errMap["message"].(string)) } return resp, nil diff --git a/internal/services/configstore/api/org.go b/internal/services/configstore/api/org.go index 2786f37..2fd5484 100644 --- a/internal/services/configstore/api/org.go +++ b/internal/services/configstore/api/org.go @@ -19,10 +19,12 @@ import ( "net/http" "strconv" + "github.com/pkg/errors" "github.com/sorintlab/agola/internal/db" "github.com/sorintlab/agola/internal/services/configstore/command" "github.com/sorintlab/agola/internal/services/configstore/readdb" "github.com/sorintlab/agola/internal/services/types" + "github.com/sorintlab/agola/internal/util" "github.com/gorilla/mux" "go.uber.org/zap" @@ -54,7 +56,7 @@ func (h *OrgHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } if org == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("org %q doesn't exist", orgID))) return } @@ -89,7 +91,7 @@ func (h *OrgByNameHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } if org == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("org %q doesn't exist", orgName))) return } @@ -113,7 +115,7 @@ func (h *CreateOrgHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var req types.Organization d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -176,12 +178,12 @@ func (h *OrgsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var err error limit, err = strconv.Atoi(limitS) if err != nil { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Wrapf(err, "cannot parse limit"))) return } } if limit < 0 { - http.Error(w, "limit must be greater or equal than 0", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("limit must be greater or equal than 0"))) return } if limit > MaxOrgsLimit { diff --git a/internal/services/configstore/api/project.go b/internal/services/configstore/api/project.go index d203dfa..09b5837 100644 --- a/internal/services/configstore/api/project.go +++ b/internal/services/configstore/api/project.go @@ -19,11 +19,13 @@ import ( "net/http" "net/url" + "github.com/pkg/errors" "github.com/sorintlab/agola/internal/db" "github.com/sorintlab/agola/internal/services/configstore/command" "github.com/sorintlab/agola/internal/services/configstore/common" "github.com/sorintlab/agola/internal/services/configstore/readdb" "github.com/sorintlab/agola/internal/services/types" + "github.com/sorintlab/agola/internal/util" "github.com/gorilla/mux" "go.uber.org/zap" @@ -42,13 +44,13 @@ func (h *ProjectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) projectRef, err := url.PathUnescape(vars["projectref"]) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } projectRefType, err := common.ParseRef(projectRef) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -70,7 +72,7 @@ func (h *ProjectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } if project == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("project %q doesn't exist", projectRef))) return } @@ -95,7 +97,7 @@ func (h *CreateProjectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) var req types.Project d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -125,7 +127,7 @@ func (h *DeleteProjectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) vars := mux.Vars(r) projectRef, err := url.PathUnescape(vars["projectref"]) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } diff --git a/internal/services/configstore/api/projectgroup.go b/internal/services/configstore/api/projectgroup.go index 6ea1c67..9ea067e 100644 --- a/internal/services/configstore/api/projectgroup.go +++ b/internal/services/configstore/api/projectgroup.go @@ -19,10 +19,12 @@ import ( "net/http" "net/url" + "github.com/pkg/errors" "github.com/sorintlab/agola/internal/db" "github.com/sorintlab/agola/internal/services/configstore/command" "github.com/sorintlab/agola/internal/services/configstore/readdb" "github.com/sorintlab/agola/internal/services/types" + "github.com/sorintlab/agola/internal/util" "github.com/gorilla/mux" "go.uber.org/zap" @@ -41,7 +43,7 @@ func (h *ProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) vars := mux.Vars(r) projectGroupRef, err := url.PathUnescape(vars["projectgroupref"]) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -58,7 +60,7 @@ func (h *ProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) } if projectGroup == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("project group %q doesn't exist", projectGroupRef))) return } @@ -80,7 +82,7 @@ func (h *ProjectGroupProjectsHandler) ServeHTTP(w http.ResponseWriter, r *http.R vars := mux.Vars(r) projectGroupRef, err := url.PathUnescape(vars["projectgroupref"]) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -96,7 +98,7 @@ func (h *ProjectGroupProjectsHandler) ServeHTTP(w http.ResponseWriter, r *http.R } if projectGroup == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("project group %q doesn't exist", projectGroupRef))) return } @@ -130,7 +132,7 @@ func (h *ProjectGroupSubgroupsHandler) ServeHTTP(w http.ResponseWriter, r *http. vars := mux.Vars(r) projectGroupRef, err := url.PathUnescape(vars["projectgroupref"]) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -146,7 +148,7 @@ func (h *ProjectGroupSubgroupsHandler) ServeHTTP(w http.ResponseWriter, r *http. } if projectGroup == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("project group %q doesn't exist", projectGroupRef))) return } @@ -183,7 +185,7 @@ func (h *CreateProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Req var req types.ProjectGroup d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } diff --git a/internal/services/configstore/api/remotesource.go b/internal/services/configstore/api/remotesource.go index 5835b82..092ff43 100644 --- a/internal/services/configstore/api/remotesource.go +++ b/internal/services/configstore/api/remotesource.go @@ -19,10 +19,12 @@ import ( "net/http" "strconv" + "github.com/pkg/errors" "github.com/sorintlab/agola/internal/db" "github.com/sorintlab/agola/internal/services/configstore/command" "github.com/sorintlab/agola/internal/services/configstore/readdb" "github.com/sorintlab/agola/internal/services/types" + "github.com/sorintlab/agola/internal/util" "github.com/gorilla/mux" "go.uber.org/zap" @@ -54,7 +56,7 @@ func (h *RemoteSourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) } if remoteSource == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("remote source %q doesn't exist", remoteSourceID))) return } @@ -89,7 +91,7 @@ func (h *RemoteSourceByNameHandler) ServeHTTP(w http.ResponseWriter, r *http.Req } if remoteSource == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("remote source %q doesn't exist", remoteSourceName))) return } @@ -114,7 +116,7 @@ func (h *CreateRemoteSourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Req var req types.RemoteSource d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -176,12 +178,12 @@ func (h *RemoteSourcesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) var err error limit, err = strconv.Atoi(limitS) if err != nil { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Wrapf(err, "cannot parse limit"))) return } } if limit < 0 { - http.Error(w, "limit must be greater or equal than 0", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("limit must be greater or equal than 0"))) return } if limit > MaxRemoteSourcesLimit { diff --git a/internal/services/configstore/api/secret.go b/internal/services/configstore/api/secret.go index ed33008..beccad0 100644 --- a/internal/services/configstore/api/secret.go +++ b/internal/services/configstore/api/secret.go @@ -18,10 +18,12 @@ import ( "encoding/json" "net/http" + "github.com/pkg/errors" "github.com/sorintlab/agola/internal/db" "github.com/sorintlab/agola/internal/services/configstore/command" "github.com/sorintlab/agola/internal/services/configstore/readdb" "github.com/sorintlab/agola/internal/services/types" + "github.com/sorintlab/agola/internal/util" "github.com/gorilla/mux" "go.uber.org/zap" @@ -53,7 +55,7 @@ func (h *SecretHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } if secret == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("secret %q doesn't exist", secretID))) return } @@ -135,7 +137,7 @@ func (h *CreateSecretHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) var secret *types.Secret d := json.NewDecoder(r.Body) if err := d.Decode(&secret); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } diff --git a/internal/services/configstore/api/user.go b/internal/services/configstore/api/user.go index 6907ca5..f8ebffd 100644 --- a/internal/services/configstore/api/user.go +++ b/internal/services/configstore/api/user.go @@ -19,6 +19,7 @@ import ( "net/http" "strconv" + "github.com/pkg/errors" "github.com/sorintlab/agola/internal/db" "github.com/sorintlab/agola/internal/services/configstore/command" "github.com/sorintlab/agola/internal/services/configstore/readdb" @@ -55,7 +56,7 @@ func (h *UserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } if user == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("user %q doesn't exist", userID))) return } @@ -90,7 +91,7 @@ func (h *UserByNameHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } if user == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("user %q doesn't exist", userName))) return } @@ -120,7 +121,7 @@ func (h *CreateUserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var req *CreateUserRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -196,12 +197,12 @@ func (h *UsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var err error limit, err = strconv.Atoi(limitS) if err != nil { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Wrapf(err, "cannot parse limit"))) return } } if limit < 0 { - http.Error(w, "limit must be greater or equal than 0", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("limit must be greater or equal than 0"))) return } if limit > MaxUsersLimit { @@ -235,7 +236,7 @@ func (h *UsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } if user == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("user with required token doesn't exist"))) return } users = []*types.User{user} @@ -254,7 +255,7 @@ func (h *UsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } if user == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("user with linked account %q token doesn't exist", linkedAccountID))) return } users = []*types.User{user} @@ -274,7 +275,7 @@ func (h *UsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } if user == nil { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("user with remote user %q for remote source %q token doesn't exist", remoteUserID, remoteSourceID))) return } users = []*types.User{user} @@ -323,7 +324,7 @@ func (h *CreateUserLAHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) var req CreateUserLARequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -397,7 +398,7 @@ func (h *UpdateUserLAHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) var req UpdateUserLARequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -446,7 +447,7 @@ func (h *CreateUserTokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques var req CreateUserTokenRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } diff --git a/internal/services/configstore/api/variable.go b/internal/services/configstore/api/variable.go index 8c0b4a4..b886272 100644 --- a/internal/services/configstore/api/variable.go +++ b/internal/services/configstore/api/variable.go @@ -101,7 +101,7 @@ func (h *CreateVariableHandler) ServeHTTP(w http.ResponseWriter, r *http.Request var variable *types.Variable d := json.NewDecoder(r.Body) if err := d.Decode(&variable); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } diff --git a/internal/services/configstore/configstore_test.go b/internal/services/configstore/configstore_test.go index 069c173..4fa203a 100644 --- a/internal/services/configstore/configstore_test.go +++ b/internal/services/configstore/configstore_test.go @@ -323,7 +323,7 @@ func TestUser(t *testing.T) { time.Sleep(2 * time.Second) t.Run("create duplicated user", func(t *testing.T) { - expectedErr := fmt.Sprintf("bad request: user with name %q already exists", "user01") + expectedErr := fmt.Sprintf("user with name %q already exists", "user01") _, err := cs.ch.CreateUser(ctx, &command.CreateUserRequest{UserName: "user01"}) if err == nil { t.Fatalf("expected error %v, got nil err", expectedErr) @@ -432,7 +432,7 @@ func TestProjectGroupsAndProjects(t *testing.T) { t.Run("create duplicated project in user root project group", func(t *testing.T) { projectName := "project01" - expectedErr := fmt.Sprintf("bad request: project with name %q, path %q already exists", projectName, path.Join("user", user.UserName, projectName)) + expectedErr := fmt.Sprintf("project with name %q, path %q already exists", projectName, path.Join("user", user.UserName, projectName)) _, err := cs.ch.CreateProject(ctx, &types.Project{Name: projectName, Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.UserName)}}) if err.Error() != expectedErr { t.Fatalf("expected err %v, got err: %v", expectedErr, err) @@ -440,7 +440,7 @@ func TestProjectGroupsAndProjects(t *testing.T) { }) t.Run("create duplicated project in org root project group", func(t *testing.T) { projectName := "project01" - expectedErr := fmt.Sprintf("bad request: project with name %q, path %q already exists", projectName, path.Join("org", org.Name, projectName)) + expectedErr := fmt.Sprintf("project with name %q, path %q already exists", projectName, path.Join("org", org.Name, projectName)) _, err := cs.ch.CreateProject(ctx, &types.Project{Name: projectName, Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("org", org.Name)}}) if err.Error() != expectedErr { t.Fatalf("expected err %v, got err: %v", expectedErr, err) @@ -449,7 +449,7 @@ func TestProjectGroupsAndProjects(t *testing.T) { t.Run("create duplicated project in user non root project group", func(t *testing.T) { projectName := "project01" - expectedErr := fmt.Sprintf("bad request: project with name %q, path %q already exists", projectName, path.Join("user", user.UserName, "projectgroup01", projectName)) + expectedErr := fmt.Sprintf("project with name %q, path %q already exists", projectName, path.Join("user", user.UserName, "projectgroup01", projectName)) _, err := cs.ch.CreateProject(ctx, &types.Project{Name: projectName, Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.UserName, "projectgroup01")}}) if err.Error() != expectedErr { t.Fatalf("expected err %v, got err: %v", expectedErr, err) @@ -457,7 +457,7 @@ func TestProjectGroupsAndProjects(t *testing.T) { }) t.Run("create duplicated project in org non root project group", func(t *testing.T) { projectName := "project01" - expectedErr := fmt.Sprintf("bad request: project with name %q, path %q already exists", projectName, path.Join("org", org.Name, "projectgroup01", projectName)) + expectedErr := fmt.Sprintf("project with name %q, path %q already exists", projectName, path.Join("org", org.Name, "projectgroup01", projectName)) _, err := cs.ch.CreateProject(ctx, &types.Project{Name: projectName, Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("org", org.Name, "projectgroup01")}}) if err.Error() != expectedErr { t.Fatalf("expected err %v, got err: %v", expectedErr, err) @@ -465,14 +465,14 @@ func TestProjectGroupsAndProjects(t *testing.T) { }) t.Run("create project in unexistent project group", func(t *testing.T) { - expectedErr := `bad request: project group with id "unexistentid" doesn't exist` + expectedErr := `project group with id "unexistentid" doesn't exist` _, err := cs.ch.CreateProject(ctx, &types.Project{Name: "project01", Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: "unexistentid"}}) if err.Error() != expectedErr { t.Fatalf("expected err %v, got err: %v", expectedErr, err) } }) t.Run("create project without parent id specified", func(t *testing.T) { - expectedErr := "bad request: project parent id required" + expectedErr := "project parent id required" _, err := cs.ch.CreateProject(ctx, &types.Project{Name: "project01"}) if err.Error() != expectedErr { t.Fatalf("expected err %v, got err: %v", expectedErr, err) diff --git a/internal/services/gateway/api/api.go b/internal/services/gateway/api/api.go index 97378c9..d2d4711 100644 --- a/internal/services/gateway/api/api.go +++ b/internal/services/gateway/api/api.go @@ -30,7 +30,10 @@ type ErrorResponse struct { } func ErrorResponseFromError(err error) *ErrorResponse { - if util.IsErrBadRequest(err) { + switch { + case util.IsErrBadRequest(err): + fallthrough + case util.IsErrNotFound(err): return &ErrorResponse{Message: err.Error()} } @@ -39,24 +42,28 @@ func ErrorResponseFromError(err error) *ErrorResponse { } func httpError(w http.ResponseWriter, err error) bool { - if err != nil { - response := ErrorResponseFromError(err) - resj, merr := json.Marshal(response) - if merr != nil { - w.WriteHeader(http.StatusInternalServerError) - return true - } - if util.IsErrBadRequest(err) { - w.WriteHeader(http.StatusBadRequest) - w.Write(resj) - } else { - w.WriteHeader(http.StatusInternalServerError) - w.Write(resj) - } - return true + if err == nil { + return false } - return false + response := ErrorResponseFromError(err) + resj, merr := json.Marshal(response) + if merr != nil { + w.WriteHeader(http.StatusInternalServerError) + return true + } + switch { + case util.IsErrBadRequest(err): + w.WriteHeader(http.StatusBadRequest) + w.Write(resj) + case util.IsErrNotFound(err): + w.WriteHeader(http.StatusNotFound) + w.Write(resj) + default: + w.WriteHeader(http.StatusInternalServerError) + w.Write(resj) + } + return true } func httpResponse(w http.ResponseWriter, code int, res interface{}) error { @@ -77,6 +84,29 @@ func httpResponse(w http.ResponseWriter, code int, res interface{}) error { return nil } +func httpErrorFromRemote(w http.ResponseWriter, resp *http.Response, err error) bool { + if err != nil { + // on generic error return an generic message to not leak the real error + response := &ErrorResponse{Message: "internal server error"} + if resp != nil { + response = &ErrorResponse{Message: err.Error()} + } + resj, merr := json.Marshal(response) + if merr != nil { + w.WriteHeader(http.StatusInternalServerError) + return true + } + if resp != nil { + w.WriteHeader(resp.StatusCode) + } else { + w.WriteHeader(http.StatusInternalServerError) + } + w.Write(resj) + return true + } + return false +} + func GetConfigTypeRef(r *http.Request) (types.ConfigType, string, error) { vars := mux.Vars(r) projectRef, err := url.PathUnescape(vars["projectref"]) diff --git a/internal/services/gateway/api/client.go b/internal/services/gateway/api/client.go index 6551d72..88da85c 100644 --- a/internal/services/gateway/api/client.go +++ b/internal/services/gateway/api/client.go @@ -84,16 +84,14 @@ func (c *Client) getResponse(ctx context.Context, method, path string, query url defer resp.Body.Close() data, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, err + return resp, err } - if len(data) <= 1 { - return resp, errors.New(resp.Status) + errMap := make(map[string]interface{}) + if err = json.Unmarshal(data, &errMap); err != nil { + return resp, fmt.Errorf("unknown api error (code: %d): %s", resp.StatusCode, string(data)) } - - // TODO(sgotti) use a json error response - - return resp, errors.New(string(data)) + return resp, errors.New(errMap["message"].(string)) } return resp, nil diff --git a/internal/services/gateway/api/oauth2.go b/internal/services/gateway/api/oauth2.go index 1532462..49fba7c 100644 --- a/internal/services/gateway/api/oauth2.go +++ b/internal/services/gateway/api/oauth2.go @@ -19,6 +19,7 @@ import ( csapi "github.com/sorintlab/agola/internal/services/configstore/api" "github.com/sorintlab/agola/internal/services/gateway/command" + "github.com/sorintlab/agola/internal/util" "go.uber.org/zap" ) @@ -47,7 +48,7 @@ func (h *OAuth2CallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request cresp, err := h.ch.HandleOauth2Callback(ctx, code, state) if err != nil { h.log.Errorf("err: %+v", err) - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } diff --git a/internal/services/gateway/api/org.go b/internal/services/gateway/api/org.go index 20afa16..61affc0 100644 --- a/internal/services/gateway/api/org.go +++ b/internal/services/gateway/api/org.go @@ -19,9 +19,11 @@ import ( "net/http" "strconv" + "github.com/pkg/errors" csapi "github.com/sorintlab/agola/internal/services/configstore/api" "github.com/sorintlab/agola/internal/services/gateway/command" "github.com/sorintlab/agola/internal/services/types" + "github.com/sorintlab/agola/internal/util" "go.uber.org/zap" "github.com/gorilla/mux" @@ -46,7 +48,7 @@ func (h *CreateOrgHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var req CreateOrgRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -81,13 +83,8 @@ func (h *DeleteOrgHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { orgName := vars["orgname"] resp, err := h.configstoreClient.DeleteOrg(ctx, orgName) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -111,13 +108,8 @@ func (h *OrgHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { orgID := vars["orgid"] org, resp, err := h.configstoreClient.GetOrg(ctx, orgID) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -142,13 +134,8 @@ func (h *OrgByNameHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { orgName := vars["orgname"] org, resp, err := h.configstoreClient.GetOrgByName(ctx, orgName) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -191,12 +178,12 @@ func (h *OrgsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var err error limit, err = strconv.Atoi(limitS) if err != nil { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Wrapf(err, "cannot parse limit"))) return } } if limit < 0 { - http.Error(w, "limit must be greater or equal than 0", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("limit must be greater or equal than 0"))) return } if limit > MaxRunsLimit { @@ -210,13 +197,8 @@ func (h *OrgsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { start := query.Get("start") csorgs, resp, err := h.configstoreClient.GetOrgs(ctx, start, limit, asc) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } diff --git a/internal/services/gateway/api/project.go b/internal/services/gateway/api/project.go index dfc26fa..9c5fc87 100644 --- a/internal/services/gateway/api/project.go +++ b/internal/services/gateway/api/project.go @@ -16,13 +16,14 @@ package api import ( "encoding/json" - "fmt" "net/http" "net/url" + "github.com/pkg/errors" csapi "github.com/sorintlab/agola/internal/services/configstore/api" "github.com/sorintlab/agola/internal/services/gateway/command" "github.com/sorintlab/agola/internal/services/types" + "github.com/sorintlab/agola/internal/util" "github.com/gorilla/mux" "go.uber.org/zap" @@ -53,16 +54,16 @@ func (h *CreateProjectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) var req CreateProjectRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } - ctxUserID := ctx.Value("userid") - if ctxUserID == nil { - http.Error(w, "no authenticated user", http.StatusBadRequest) + userIDVal := ctx.Value("userid") + if userIDVal == nil { + httpError(w, util.NewErrBadRequest(errors.Errorf("user not authenticated"))) return } - userID := ctxUserID.(string) + userID := userIDVal.(string) h.log.Infof("userID: %q", userID) creq := &command.CreateProjectRequest{ @@ -102,7 +103,7 @@ func (h *ProjectReconfigHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques vars := mux.Vars(r) projectRef, err := url.PathUnescape(vars["projectref"]) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -129,29 +130,19 @@ func (h *DeleteProjectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) vars := mux.Vars(r) projectRef, err := url.PathUnescape(vars["projectref"]) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } project, resp, err := h.configstoreClient.GetProject(ctx, projectRef) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, fmt.Sprintf("project with ref %q doesn't exist", projectRef), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } resp, err = h.configstoreClient.DeleteProject(ctx, project.ID) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -174,18 +165,13 @@ func (h *ProjectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) projectRef, err := url.PathUnescape(vars["projectref"]) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } project, resp, err := h.configstoreClient.GetProject(ctx, projectRef) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } diff --git a/internal/services/gateway/api/projectgroup.go b/internal/services/gateway/api/projectgroup.go index 086f508..a21c953 100644 --- a/internal/services/gateway/api/projectgroup.go +++ b/internal/services/gateway/api/projectgroup.go @@ -19,9 +19,11 @@ import ( "net/http" "net/url" + "github.com/pkg/errors" csapi "github.com/sorintlab/agola/internal/services/configstore/api" "github.com/sorintlab/agola/internal/services/gateway/command" "github.com/sorintlab/agola/internal/services/types" + "github.com/sorintlab/agola/internal/util" "github.com/gorilla/mux" "go.uber.org/zap" @@ -49,16 +51,16 @@ func (h *CreateProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Req var req CreateProjectGroupRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } - ctxUserID := ctx.Value("userid") - if ctxUserID == nil { - http.Error(w, "no authenticated user", http.StatusBadRequest) + userIDVal := ctx.Value("userid") + if userIDVal == nil { + httpError(w, util.NewErrBadRequest(errors.Errorf("user not authenticated"))) return } - userID := ctxUserID.(string) + userID := userIDVal.(string) h.log.Infof("userID: %q", userID) creq := &command.CreateProjectGroupRequest{ @@ -68,9 +70,8 @@ func (h *CreateProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Req } projectGroup, err := h.ch.CreateProjectGroup(ctx, creq) - if err != nil { + if httpError(w, err) { h.log.Errorf("err: %+v", err) - http.Error(w, err.Error(), http.StatusBadRequest) return } @@ -94,18 +95,13 @@ func (h *ProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) vars := mux.Vars(r) projectGroupRef, err := url.PathUnescape(vars["projectgroupref"]) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } projectGroup, resp, err := h.configstoreClient.GetProjectGroup(ctx, projectGroupRef) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -129,18 +125,13 @@ func (h *ProjectGroupProjectsHandler) ServeHTTP(w http.ResponseWriter, r *http.R vars := mux.Vars(r) projectGroupRef, err := url.PathUnescape(vars["projectgroupref"]) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } csprojects, resp, err := h.configstoreClient.GetProjectGroupProjects(ctx, projectGroupRef) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -168,18 +159,13 @@ func (h *ProjectGroupSubgroupsHandler) ServeHTTP(w http.ResponseWriter, r *http. vars := mux.Vars(r) projectGroupRef, err := url.PathUnescape(vars["projectgroupref"]) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } cssubgroups, resp, err := h.configstoreClient.GetProjectGroupSubgroups(ctx, projectGroupRef) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } diff --git a/internal/services/gateway/api/remotesource.go b/internal/services/gateway/api/remotesource.go index a8c061f..ca7b948 100644 --- a/internal/services/gateway/api/remotesource.go +++ b/internal/services/gateway/api/remotesource.go @@ -15,19 +15,18 @@ package api import ( - "context" "encoding/json" "net/http" "strconv" + "github.com/pkg/errors" csapi "github.com/sorintlab/agola/internal/services/configstore/api" - "github.com/sorintlab/agola/internal/services/gateway/common" + "github.com/sorintlab/agola/internal/services/gateway/command" "github.com/sorintlab/agola/internal/services/types" "github.com/sorintlab/agola/internal/util" "go.uber.org/zap" "github.com/gorilla/mux" - "github.com/pkg/errors" ) type CreateRemoteSourceRequest struct { @@ -40,12 +39,12 @@ type CreateRemoteSourceRequest struct { } type CreateRemoteSourceHandler struct { - log *zap.SugaredLogger - configstoreClient *csapi.Client + log *zap.SugaredLogger + ch *command.CommandHandler } -func NewCreateRemoteSourceHandler(logger *zap.Logger, configstoreClient *csapi.Client) *CreateRemoteSourceHandler { - return &CreateRemoteSourceHandler{log: logger.Sugar(), configstoreClient: configstoreClient} +func NewCreateRemoteSourceHandler(logger *zap.Logger, ch *command.CommandHandler) *CreateRemoteSourceHandler { + return &CreateRemoteSourceHandler{log: logger.Sugar(), ch: ch} } func (h *CreateRemoteSourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -54,13 +53,21 @@ func (h *CreateRemoteSourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Req var req CreateRemoteSourceRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } - rs, err := h.createRemoteSource(ctx, &req) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + creq := &command.CreateRemoteSourceRequest{ + Name: req.Name, + APIURL: req.APIURL, + Type: req.Type, + AuthType: req.AuthType, + Oauth2ClientID: req.Oauth2ClientID, + Oauth2ClientSecret: req.Oauth2ClientSecret, + } + rs, err := h.ch.CreateRemoteSource(ctx, creq) + if httpError(w, err) { + h.log.Errorf("err: %+v", err) return } @@ -70,57 +77,6 @@ func (h *CreateRemoteSourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Req } } -func (h *CreateRemoteSourceHandler) createRemoteSource(ctx context.Context, req *CreateRemoteSourceRequest) (*types.RemoteSource, error) { - if !util.ValidateName(req.Name) { - return nil, errors.Errorf("invalid remotesource name %q", req.Name) - } - - if req.Name == "" { - return nil, errors.Errorf("remotesource name required") - } - if req.APIURL == "" { - return nil, errors.Errorf("remotesource api url required") - } - if req.Type == "" { - return nil, errors.Errorf("remotesource type required") - } - if req.AuthType == "" { - return nil, errors.Errorf("remotesource auth type required") - } - - // validate if the remote source type supports the required auth type - if !common.SourceSupportsAuthType(types.RemoteSourceType(req.Type), types.RemoteSourceAuthType(req.AuthType)) { - return nil, errors.Errorf("remotesource type %q doesn't support auth type %q", req.Type, req.AuthType) - } - - if req.AuthType == string(types.RemoteSourceAuthTypeOauth2) { - if req.Oauth2ClientID == "" { - return nil, errors.Errorf("remotesource oauth2 clientid required") - } - if req.Oauth2ClientSecret == "" { - return nil, errors.Errorf("remotesource oauth2 client secret required") - } - } - - rs := &types.RemoteSource{ - Name: req.Name, - Type: types.RemoteSourceType(req.Type), - AuthType: types.RemoteSourceAuthType(req.AuthType), - APIURL: req.APIURL, - Oauth2ClientID: req.Oauth2ClientID, - Oauth2ClientSecret: req.Oauth2ClientSecret, - } - - h.log.Infof("creating remotesource") - rs, _, err := h.configstoreClient.CreateRemoteSource(ctx, rs) - if err != nil { - return nil, errors.Wrapf(err, "failed to create remotesource") - } - h.log.Infof("remotesource %s created, ID: %s", rs.Name, rs.ID) - - return rs, nil -} - type RemoteSourceResponse struct { ID string `json:"id"` Name string `json:"name"` @@ -151,13 +107,8 @@ func (h *RemoteSourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) rsID := vars["id"] rs, resp, err := h.configstoreClient.GetRemoteSource(ctx, rsID) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -187,12 +138,12 @@ func (h *RemoteSourcesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) var err error limit, err = strconv.Atoi(limitS) if err != nil { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Wrapf(err, "cannot parse limit"))) return } } if limit < 0 { - http.Error(w, "limit must be greater or equal than 0", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("limit must be greater or equal than 0"))) return } if limit > MaxRunsLimit { @@ -206,13 +157,8 @@ func (h *RemoteSourcesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) start := query.Get("start") csRemoteSources, resp, err := h.configstoreClient.GetRemoteSources(ctx, start, limit, asc) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } diff --git a/internal/services/gateway/api/repos.go b/internal/services/gateway/api/repos.go index 2574df6..da219e2 100644 --- a/internal/services/gateway/api/repos.go +++ b/internal/services/gateway/api/repos.go @@ -57,7 +57,8 @@ func (h *ReposHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { req, err := http.NewRequest(r.Method, u.String(), r.Body) req = req.WithContext(ctx) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + h.log.Errorf("err: %+v", err) + httpError(w, err) return } diff --git a/internal/services/gateway/api/run.go b/internal/services/gateway/api/run.go index 5aa783f..f9a67f7 100644 --- a/internal/services/gateway/api/run.go +++ b/internal/services/gateway/api/run.go @@ -22,8 +22,10 @@ import ( "strconv" "time" + "github.com/pkg/errors" rsapi "github.com/sorintlab/agola/internal/services/runservice/scheduler/api" rstypes "github.com/sorintlab/agola/internal/services/runservice/types" + "github.com/sorintlab/agola/internal/util" "go.uber.org/zap" "github.com/gorilla/mux" @@ -219,13 +221,8 @@ func (h *RunHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { runID := vars["runid"] runResp, resp, err := h.runserviceClient.GetRun(ctx, runID) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -251,13 +248,8 @@ func (h *RuntaskHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { taskID := vars["taskid"] runResp, resp, err := h.runserviceClient.GetRun(ctx, runID) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -266,7 +258,7 @@ func (h *RuntaskHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { rt, ok := run.RunTasks[taskID] if !ok { - http.Error(w, "", http.StatusNotFound) + httpError(w, util.NewErrNotFound(errors.Errorf("run %q task %q not found", runID, taskID))) return } rct := rc.Tasks[rt.ID] @@ -318,7 +310,7 @@ func (h *RunsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { groups := q["group"] // we require that groups are specified to not return all runs if len(groups) == 0 { - http.Error(w, "not groups specified", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("no groups specified"))) return } @@ -332,12 +324,12 @@ func (h *RunsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var err error limit, err = strconv.Atoi(limitS) if err != nil { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Wrapf(err, "cannot parse limit"))) return } } if limit < 0 { - http.Error(w, "limit must be greater or equal than 0", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("limit must be greater or equal than 0"))) return } if limit > MaxRunsLimit { @@ -351,13 +343,8 @@ func (h *RunsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { start := q.Get("start") runsResp, resp, err := h.runserviceClient.GetRuns(ctx, phaseFilter, groups, lastRun, changeGroups, start, limit, asc) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -401,7 +388,7 @@ func (h *RunActionsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var req RunActionsRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -413,13 +400,8 @@ func (h *RunActionsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } resp, err := h.runserviceClient.CreateRun(ctx, rsreq) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -429,13 +411,8 @@ func (h *RunActionsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } resp, err := h.runserviceClient.RunActions(ctx, runID, rsreq) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } } @@ -470,7 +447,7 @@ func (h *RunTaskActionsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request var req RunTaskActionsRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -482,17 +459,13 @@ func (h *RunTaskActionsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request } resp, err := h.runserviceClient.RunTaskActions(ctx, runID, taskID, rsreq) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } + default: - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("wrong action type %q", req.ActionType))) return } } @@ -513,23 +486,23 @@ func (h *LogsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { runID := q.Get("runID") if runID == "" { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("empty run id"))) return } taskID := q.Get("taskID") if taskID == "" { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("empty task id"))) return } _, setup := q["setup"] stepStr := q.Get("step") if !setup && stepStr == "" { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("no setup or step number provided"))) return } if setup && stepStr != "" { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("both setup and step number provided"))) return } @@ -538,7 +511,7 @@ func (h *LogsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var err error step, err = strconv.Atoi(stepStr) if err != nil { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Wrapf(err, "cannot parse step number"))) return } } @@ -556,13 +529,8 @@ func (h *LogsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } resp, err := h.runserviceClient.GetLogs(ctx, runID, taskID, setup, step, follow, stream) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } diff --git a/internal/services/gateway/api/secret.go b/internal/services/gateway/api/secret.go index bf22cd7..d33efd9 100644 --- a/internal/services/gateway/api/secret.go +++ b/internal/services/gateway/api/secret.go @@ -62,14 +62,15 @@ func (h *SecretHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } var cssecrets []*types.Secret + var resp *http.Response switch parentType { case types.ConfigTypeProjectGroup: - cssecrets, _, err = h.configstoreClient.GetProjectGroupSecrets(ctx, parentRef, tree) + cssecrets, resp, err = h.configstoreClient.GetProjectGroupSecrets(ctx, parentRef, tree) case types.ConfigTypeProject: - cssecrets, _, err = h.configstoreClient.GetProjectSecrets(ctx, parentRef, tree) + cssecrets, resp, err = h.configstoreClient.GetProjectSecrets(ctx, parentRef, tree) } - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + if httpErrorFromRemote(w, resp, err) { + h.log.Errorf("err: %+v", err) return } @@ -115,14 +116,13 @@ func (h *CreateSecretHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) var req CreateSecretRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } if !util.ValidateName(req.Name) { - err := errors.Errorf("invalid secret name %q", req.Name) - h.log.Errorf("err: %+v", err) - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("invalid secret name %q", req.Name))) + return } s := &types.Secret{ @@ -131,16 +131,17 @@ func (h *CreateSecretHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) Data: req.Data, } + var resp *http.Response switch parentType { case types.ConfigTypeProjectGroup: h.log.Infof("creating project group secret") - s, _, err = h.configstoreClient.CreateProjectGroupSecret(ctx, parentRef, s) + s, resp, err = h.configstoreClient.CreateProjectGroupSecret(ctx, parentRef, s) case types.ConfigTypeProject: h.log.Infof("creating project secret") - s, _, err = h.configstoreClient.CreateProjectSecret(ctx, parentRef, s) + s, resp, err = h.configstoreClient.CreateProjectSecret(ctx, parentRef, s) } - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + if httpErrorFromRemote(w, resp, err) { + h.log.Errorf("err: %+v", err) return } h.log.Infof("secret %s created, ID: %s", s.Name, s.ID) @@ -179,15 +180,11 @@ func (h *DeleteSecretHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) h.log.Infof("deleting project secret") resp, err = h.configstoreClient.DeleteProjectSecret(ctx, parentRef, secretName) } - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } + if err := httpResponse(w, http.StatusNoContent, nil); err != nil { h.log.Errorf("err: %+v", err) } diff --git a/internal/services/gateway/api/user.go b/internal/services/gateway/api/user.go index b2dc26d..961bc6e 100644 --- a/internal/services/gateway/api/user.go +++ b/internal/services/gateway/api/user.go @@ -21,10 +21,12 @@ import ( "sort" "strconv" + "github.com/pkg/errors" gitsource "github.com/sorintlab/agola/internal/gitsources" csapi "github.com/sorintlab/agola/internal/services/configstore/api" "github.com/sorintlab/agola/internal/services/gateway/command" "github.com/sorintlab/agola/internal/services/types" + "github.com/sorintlab/agola/internal/util" "go.uber.org/zap" "github.com/gorilla/mux" @@ -49,7 +51,7 @@ func (h *CreateUserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var req CreateUserRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -84,15 +86,14 @@ func (h *DeleteUserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { userName := vars["username"] resp, err := h.configstoreClient.DeleteUser(ctx, userName) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } + + if err := httpResponse(w, http.StatusNoContent, nil); err != nil { + h.log.Errorf("err: %+v", err) + } } type CurrentUserHandler struct { @@ -109,19 +110,14 @@ func (h *CurrentUserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { userIDVal := ctx.Value("userid") if userIDVal == nil { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("user not authenticated"))) return } userID := userIDVal.(string) user, resp, err := h.configstoreClient.GetUser(ctx, userID) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -146,13 +142,8 @@ func (h *UserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { userID := vars["userid"] user, resp, err := h.configstoreClient.GetUser(ctx, userID) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -177,13 +168,8 @@ func (h *UserByNameHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { userName := vars["username"] user, resp, err := h.configstoreClient.GetUserByName(ctx, userName) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -233,12 +219,12 @@ func (h *UsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var err error limit, err = strconv.Atoi(limitS) if err != nil { - http.Error(w, "", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Wrapf(err, "cannot parse limit"))) return } } if limit < 0 { - http.Error(w, "limit must be greater or equal than 0", http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("limit must be greater or equal than 0"))) return } if limit > MaxRunsLimit { @@ -252,13 +238,8 @@ func (h *UsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { start := query.Get("start") csusers, resp, err := h.configstoreClient.GetUsers(ctx, start, limit, asc) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -300,14 +281,13 @@ func (h *CreateUserLAHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) var req *CreateUserLARequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } res, err := h.createUserLA(ctx, userName, req) - if err != nil { + if httpError(w, err) { h.log.Errorf("err: %+v", err) - http.Error(w, err.Error(), http.StatusBadRequest) return } @@ -356,10 +336,9 @@ func (h *DeleteUserLAHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) userName := vars["username"] laID := vars["laid"] - _, err := h.configstoreClient.DeleteUserLA(ctx, userName, laID) - if err != nil { + resp, err := h.configstoreClient.DeleteUserLA(ctx, userName, laID) + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -393,7 +372,7 @@ func (h *CreateUserTokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques var req CreateUserTokenRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } @@ -433,10 +412,9 @@ func (h *DeleteUserTokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques tokenName := vars["tokenname"] h.log.Infof("deleting user %q token %q", userName, tokenName) - _, err := h.configstoreClient.DeleteUserToken(ctx, userName, tokenName) - if err != nil { + resp, err := h.configstoreClient.DeleteUserToken(ctx, userName, tokenName) + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } @@ -469,14 +447,13 @@ func (h *RegisterUserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) var req *RegisterUserRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } res, err := h.registerUser(ctx, req) - if err != nil { + if httpError(w, err) { h.log.Errorf("err: %+v", err) - http.Error(w, err.Error(), http.StatusBadRequest) return } @@ -527,14 +504,13 @@ func (h *AuthorizeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var req *LoginUserRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } res, err := h.authorize(ctx, req) - if err != nil { + if httpError(w, err) { h.log.Errorf("err: %+v", err) - http.Error(w, err.Error(), http.StatusBadRequest) return } @@ -593,14 +569,13 @@ func (h *LoginUserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var req *LoginUserRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } res, err := h.loginUser(ctx, req) - 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/gateway/api/variable.go b/internal/services/gateway/api/variable.go index d797b9c..6d19544 100644 --- a/internal/services/gateway/api/variable.go +++ b/internal/services/gateway/api/variable.go @@ -94,33 +94,31 @@ func (h *VariableHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { switch parentType { case types.ConfigTypeProjectGroup: var err error - csvars, _, err = h.configstoreClient.GetProjectGroupVariables(ctx, parentRef, tree) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + var resp *http.Response + csvars, resp, err = h.configstoreClient.GetProjectGroupVariables(ctx, parentRef, tree) + if httpErrorFromRemote(w, resp, err) { + h.log.Errorf("err: %+v", err) return } - cssecrets, _, err = h.configstoreClient.GetProjectGroupSecrets(ctx, parentRef, true) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + cssecrets, resp, err = h.configstoreClient.GetProjectGroupSecrets(ctx, parentRef, true) + if httpErrorFromRemote(w, resp, err) { + h.log.Errorf("err: %+v", err) return } case types.ConfigTypeProject: var err error - csvars, _, err = h.configstoreClient.GetProjectVariables(ctx, parentRef, tree) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + var resp *http.Response + csvars, resp, err = h.configstoreClient.GetProjectVariables(ctx, parentRef, tree) + if httpErrorFromRemote(w, resp, err) { + h.log.Errorf("err: %+v", err) return } - cssecrets, _, err = h.configstoreClient.GetProjectSecrets(ctx, parentRef, true) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + cssecrets, resp, err = h.configstoreClient.GetProjectSecrets(ctx, parentRef, true) + if httpErrorFromRemote(w, resp, err) { + h.log.Errorf("err: %+v", err) return } } - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } if removeoverriden { // remove overriden variables @@ -163,20 +161,18 @@ func (h *CreateVariableHandler) ServeHTTP(w http.ResponseWriter, r *http.Request var req CreateVariableRequest d := json.NewDecoder(r.Body) if err := d.Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(err)) return } if !util.ValidateName(req.Name) { - err := errors.Errorf("invalid variable name %q", req.Name) - h.log.Errorf("err: %+v", err) - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("invalid secret name %q", req.Name))) + return } if len(req.Values) == 0 { - err := errors.Errorf("empty variable values") - h.log.Errorf("err: %+v", err) - http.Error(w, err.Error(), http.StatusBadRequest) + httpError(w, util.NewErrBadRequest(errors.Errorf("empty variable values"))) + return } v := &types.Variable{ @@ -193,25 +189,34 @@ func (h *CreateVariableHandler) ServeHTTP(w http.ResponseWriter, r *http.Request switch parentType { case types.ConfigTypeProjectGroup: var err error - cssecrets, _, err = h.configstoreClient.GetProjectGroupSecrets(ctx, parentRef, true) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + var resp *http.Response + cssecrets, resp, err = h.configstoreClient.GetProjectGroupSecrets(ctx, parentRef, true) + if httpErrorFromRemote(w, resp, err) { + h.log.Errorf("err: %+v", err) return } + h.log.Infof("creating project group variable") - v, _, err = h.configstoreClient.CreateProjectGroupVariable(ctx, parentRef, v) - case types.ConfigTypeProject: - cssecrets, _, err = h.configstoreClient.GetProjectSecrets(ctx, parentRef, true) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + v, resp, err = h.configstoreClient.CreateProjectGroupVariable(ctx, parentRef, v) + if httpErrorFromRemote(w, resp, err) { + h.log.Errorf("err: %+v", err) return } + case types.ConfigTypeProject: + var err error + var resp *http.Response + cssecrets, resp, err = h.configstoreClient.GetProjectSecrets(ctx, parentRef, true) + if httpErrorFromRemote(w, resp, err) { + h.log.Errorf("err: %+v", err) + return + } + h.log.Infof("creating project variable") - v, _, err = h.configstoreClient.CreateProjectVariable(ctx, parentRef, v) - } - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return + v, resp, err = h.configstoreClient.CreateProjectVariable(ctx, parentRef, v) + if httpErrorFromRemote(w, resp, err) { + h.log.Errorf("err: %+v", err) + return + } } h.log.Infof("variable %s created, ID: %s", v.Name, v.ID) @@ -250,15 +255,11 @@ func (h *DeleteVariableHandler) ServeHTTP(w http.ResponseWriter, r *http.Request h.log.Infof("deleting project variable") resp, err = h.configstoreClient.DeleteProjectVariable(ctx, parentRef, variableName) } - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - http.Error(w, err.Error(), http.StatusNotFound) - return - } + if httpErrorFromRemote(w, resp, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) return } + if err := httpResponse(w, http.StatusNoContent, nil); err != nil { h.log.Errorf("err: %+v", err) } diff --git a/internal/services/gateway/command/command.go b/internal/services/gateway/command/command.go index 52b8839..e3f8b06 100644 --- a/internal/services/gateway/command/command.go +++ b/internal/services/gateway/command/command.go @@ -15,8 +15,12 @@ package command import ( + "net/http" + + "github.com/pkg/errors" csapi "github.com/sorintlab/agola/internal/services/configstore/api" "github.com/sorintlab/agola/internal/services/gateway/common" + "github.com/sorintlab/agola/internal/util" "go.uber.org/zap" ) @@ -38,3 +42,21 @@ func NewCommandHandler(logger *zap.Logger, sd *common.TokenSigningData, configst webExposedURL: webExposedURL, } } + +func ErrFromRemote(resp *http.Response, err error) error { + if err == nil { + return nil + } + + if resp != nil { + switch resp.StatusCode { + // remove wrapping from errors sent to client + case http.StatusBadRequest: + return util.NewErrBadRequest(errors.Cause(err)) + case http.StatusNotFound: + return util.NewErrNotFound(errors.Cause(err)) + } + } + + return err +} diff --git a/internal/services/gateway/command/org.go b/internal/services/gateway/command/org.go index 5662d0a..0ae647d 100644 --- a/internal/services/gateway/command/org.go +++ b/internal/services/gateway/command/org.go @@ -40,9 +40,9 @@ func (c *CommandHandler) CreateOrg(ctx context.Context, req *CreateOrgRequest) ( } c.log.Infof("creating organization") - org, _, err := c.configstoreClient.CreateOrg(ctx, org) + org, resp, err := c.configstoreClient.CreateOrg(ctx, org) if err != nil { - return nil, errors.Wrapf(err, "failed to create organization") + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to create organization")) } c.log.Infof("organization %s created, ID: %s", org.Name, org.ID) diff --git a/internal/services/gateway/command/project.go b/internal/services/gateway/command/project.go index f1abc4f..a4e946c 100644 --- a/internal/services/gateway/command/project.go +++ b/internal/services/gateway/command/project.go @@ -40,13 +40,15 @@ func (c *CommandHandler) CreateProject(ctx context.Context, req *CreateProjectRe return nil, util.NewErrBadRequest(errors.Errorf("invalid project name %q", req.Name)) } - user, _, err := c.configstoreClient.GetUser(ctx, req.CurrentUserID) + user, resp, err := c.configstoreClient.GetUser(ctx, req.CurrentUserID) if err != nil { - return nil, errors.Wrapf(err, "failed to get user %q", req.CurrentUserID) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get user %q", req.CurrentUserID)) } - rs, _, err := c.configstoreClient.GetRemoteSourceByName(ctx, req.RemoteSourceName) + + rs, resp, err := c.configstoreClient.GetRemoteSourceByName(ctx, req.RemoteSourceName) if err != nil { - return nil, errors.Wrapf(err, "failed to get remote source %q", req.RemoteSourceName) + c.log.Errorf("err: %+v", err) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get remote source %q", req.RemoteSourceName)) } c.log.Infof("rs: %s", util.Dump(rs)) var la *types.LinkedAccount @@ -101,9 +103,9 @@ func (c *CommandHandler) CreateProject(ctx context.Context, req *CreateProjectRe } c.log.Infof("creating project") - p, _, err = c.configstoreClient.CreateProject(ctx, p) + p, resp, err = c.configstoreClient.CreateProject(ctx, p) if err != nil { - return nil, errors.Wrapf(err, "failed to create project") + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to create project")) } c.log.Infof("project %s created, ID: %s", p.Name, p.ID) @@ -152,14 +154,14 @@ func (c *CommandHandler) SetupProject(ctx context.Context, rs *types.RemoteSourc } func (c *CommandHandler) ReconfigProject(ctx context.Context, projectRef string) error { - p, _, err := c.configstoreClient.GetProject(ctx, projectRef) + p, resp, err := c.configstoreClient.GetProject(ctx, projectRef) if err != nil { - return err + return ErrFromRemote(resp, errors.Wrapf(err, "failed to get project %q", projectRef)) } - user, _, err := c.configstoreClient.GetUserByLinkedAccount(ctx, p.LinkedAccountID) + user, resp, err := c.configstoreClient.GetUserByLinkedAccount(ctx, p.LinkedAccountID) if err != nil { - return errors.Wrapf(err, "failed to get user with linked account id %q", p.LinkedAccountID) + return ErrFromRemote(resp, errors.Wrapf(err, "failed to get user with linked account id %q", p.LinkedAccountID)) } la := user.LinkedAccounts[p.LinkedAccountID] @@ -168,9 +170,9 @@ func (c *CommandHandler) ReconfigProject(ctx context.Context, projectRef string) return errors.Errorf("linked account %q in user %q doesn't exist", p.LinkedAccountID, user.UserName) } - rs, _, err := c.configstoreClient.GetRemoteSource(ctx, la.RemoteSourceID) + rs, resp, err := c.configstoreClient.GetRemoteSource(ctx, la.RemoteSourceID) if err != nil { - return errors.Wrapf(err, "failed to get remote source %q", la.RemoteSourceID) + return ErrFromRemote(resp, errors.Wrapf(err, "failed to get remote source %q", la.RemoteSourceID)) } return c.SetupProject(ctx, rs, la, &SetupProjectRequest{ diff --git a/internal/services/gateway/command/projectgroup.go b/internal/services/gateway/command/projectgroup.go index c48f5ac..158f3e3 100644 --- a/internal/services/gateway/command/projectgroup.go +++ b/internal/services/gateway/command/projectgroup.go @@ -35,9 +35,9 @@ func (c *CommandHandler) CreateProjectGroup(ctx context.Context, req *CreateProj return nil, util.NewErrBadRequest(errors.Errorf("invalid projectGroup name %q", req.Name)) } - user, _, err := c.configstoreClient.GetUser(ctx, req.CurrentUserID) + user, resp, err := c.configstoreClient.GetUser(ctx, req.CurrentUserID) if err != nil { - return nil, errors.Wrapf(err, "failed to get user %q", req.CurrentUserID) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get user %q", req.CurrentUserID)) } parentID := req.ParentID @@ -55,9 +55,9 @@ func (c *CommandHandler) CreateProjectGroup(ctx context.Context, req *CreateProj } c.log.Infof("creating projectGroup") - p, _, err = c.configstoreClient.CreateProjectGroup(ctx, p) + p, resp, err = c.configstoreClient.CreateProjectGroup(ctx, p) if err != nil { - return nil, errors.Wrapf(err, "failed to create projectGroup") + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to create projectGroup")) } c.log.Infof("projectGroup %s created, ID: %s", p.Name, p.ID) diff --git a/internal/services/gateway/command/remotesource.go b/internal/services/gateway/command/remotesource.go new file mode 100644 index 0000000..f30f4f3 --- /dev/null +++ b/internal/services/gateway/command/remotesource.go @@ -0,0 +1,85 @@ +// Copyright 2019 Sorint.lab +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied +// See the License for the specific language governing permissions and +// limitations under the License. + +package command + +import ( + "context" + + "github.com/sorintlab/agola/internal/services/gateway/common" + "github.com/sorintlab/agola/internal/services/types" + "github.com/sorintlab/agola/internal/util" + + "github.com/pkg/errors" +) + +type CreateRemoteSourceRequest struct { + Name string + APIURL string + Type string + AuthType string + Oauth2ClientID string + Oauth2ClientSecret string +} + +func (c *CommandHandler) CreateRemoteSource(ctx context.Context, req *CreateRemoteSourceRequest) (*types.RemoteSource, error) { + if !util.ValidateName(req.Name) { + return nil, util.NewErrBadRequest(errors.Errorf("invalid remotesource name %q", req.Name)) + } + + if req.Name == "" { + return nil, util.NewErrBadRequest(errors.Errorf("remotesource name required")) + } + if req.APIURL == "" { + return nil, util.NewErrBadRequest(errors.Errorf("remotesource api url required")) + } + if req.Type == "" { + return nil, util.NewErrBadRequest(errors.Errorf("remotesource type required")) + } + if req.AuthType == "" { + return nil, util.NewErrBadRequest(errors.Errorf("remotesource auth type required")) + } + + // validate if the remote source type supports the required auth type + if !common.SourceSupportsAuthType(types.RemoteSourceType(req.Type), types.RemoteSourceAuthType(req.AuthType)) { + return nil, util.NewErrBadRequest(errors.Errorf("remotesource type %q doesn't support auth type %q", req.Type, req.AuthType)) + } + + if req.AuthType == string(types.RemoteSourceAuthTypeOauth2) { + if req.Oauth2ClientID == "" { + return nil, util.NewErrBadRequest(errors.Errorf("remotesource oauth2 clientid required")) + } + if req.Oauth2ClientSecret == "" { + return nil, util.NewErrBadRequest(errors.Errorf("remotesource oauth2 client secret required")) + } + } + + rs := &types.RemoteSource{ + Name: req.Name, + Type: types.RemoteSourceType(req.Type), + AuthType: types.RemoteSourceAuthType(req.AuthType), + APIURL: req.APIURL, + Oauth2ClientID: req.Oauth2ClientID, + Oauth2ClientSecret: req.Oauth2ClientSecret, + } + + c.log.Infof("creating remotesource") + rs, resp, err := c.configstoreClient.CreateRemoteSource(ctx, rs) + if err != nil { + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to create remotesource")) + } + c.log.Infof("remotesource %s created, ID: %s", rs.Name, rs.ID) + + return rs, nil +} diff --git a/internal/services/gateway/command/user.go b/internal/services/gateway/command/user.go index 6414130..cc081aa 100644 --- a/internal/services/gateway/command/user.go +++ b/internal/services/gateway/command/user.go @@ -17,7 +17,6 @@ package command import ( "context" "encoding/json" - "net/http" gitsource "github.com/sorintlab/agola/internal/gitsources" csapi "github.com/sorintlab/agola/internal/services/configstore/api" @@ -46,9 +45,9 @@ func (c *CommandHandler) CreateUser(ctx context.Context, req *CreateUserRequest) } c.log.Infof("creating user") - u, _, err := c.configstoreClient.CreateUser(ctx, creq) + u, resp, err := c.configstoreClient.CreateUser(ctx, creq) if err != nil { - return nil, errors.Wrapf(err, "failed to create user") + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to create user")) } c.log.Infof("user %s created, ID: %s", u.UserName, u.ID) @@ -76,10 +75,7 @@ func (c *CommandHandler) CreateUserToken(ctx context.Context, req *CreateUserTok userName := req.UserName user, resp, err := c.configstoreClient.GetUserByName(ctx, userName) if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - return "", util.NewErrBadRequest(errors.Errorf("user %q doesn't exist", userID)) - } - return "", errors.Wrapf(err, "failed to get user %q", userID) + return "", ErrFromRemote(resp, errors.Wrapf(err, "failed to get user")) } // only admin or the same logged user can create a token @@ -94,9 +90,9 @@ func (c *CommandHandler) CreateUserToken(ctx context.Context, req *CreateUserTok creq := &csapi.CreateUserTokenRequest{ TokenName: req.TokenName, } - res, _, err := c.configstoreClient.CreateUserToken(ctx, userName, creq) + res, resp, err := c.configstoreClient.CreateUserToken(ctx, userName, creq) if err != nil { - return "", errors.Wrapf(err, "failed to create user token") + return "", ErrFromRemote(resp, errors.Wrapf(err, "failed to create user token")) } c.log.Infof("token %q for user %q created", req.TokenName, userName) @@ -115,14 +111,11 @@ func (c *CommandHandler) CreateUserLA(ctx context.Context, req *CreateUserLARequ userName := req.UserName user, resp, err := c.configstoreClient.GetUserByName(ctx, userName) if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - return nil, util.NewErrBadRequest(errors.Errorf("user %q doesn't exist", userName)) - } - return nil, errors.Wrapf(err, "failed to get user %q", userName) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get user %q", userName)) } - rs, _, err := c.configstoreClient.GetRemoteSourceByName(ctx, req.RemoteSourceName) + rs, resp, err := c.configstoreClient.GetRemoteSourceByName(ctx, req.RemoteSourceName) if err != nil { - return nil, errors.Wrapf(err, "failed to get remote source %q", req.RemoteSourceName) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get remote source %q", req.RemoteSourceName)) } c.log.Infof("rs: %s", util.Dump(rs)) var la *types.LinkedAccount @@ -164,9 +157,9 @@ func (c *CommandHandler) CreateUserLA(ctx context.Context, req *CreateUserLARequ } c.log.Infof("creating linked account") - la, _, err = c.configstoreClient.CreateUserLA(ctx, userName, creq) + la, resp, err = c.configstoreClient.CreateUserLA(ctx, userName, creq) if err != nil { - return nil, errors.Wrapf(err, "failed to create linked account") + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to create linked account")) } c.log.Infof("linked account %q for user %q created", la.ID, userName) @@ -189,9 +182,9 @@ func (c *CommandHandler) RegisterUser(ctx context.Context, req *RegisterUserRequ return nil, util.NewErrBadRequest(errors.Errorf("invalid user name %q", req.UserName)) } - rs, _, err := c.configstoreClient.GetRemoteSourceByName(ctx, req.RemoteSourceName) + rs, resp, err := c.configstoreClient.GetRemoteSourceByName(ctx, req.RemoteSourceName) if err != nil { - return nil, errors.Wrapf(err, "failed to get remote source %q", req.RemoteSourceName) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get remote source %q", req.RemoteSourceName)) } c.log.Infof("rs: %s", util.Dump(rs)) @@ -225,9 +218,9 @@ func (c *CommandHandler) RegisterUser(ctx context.Context, req *RegisterUserRequ } c.log.Infof("creating user account") - u, _, err := c.configstoreClient.CreateUser(ctx, creq) + u, resp, err := c.configstoreClient.CreateUser(ctx, creq) if err != nil { - return nil, errors.Wrapf(err, "failed to create linked account") + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to create linked account")) } c.log.Infof("user %q created", req.UserName) @@ -247,9 +240,9 @@ type LoginUserResponse struct { } func (c *CommandHandler) LoginUser(ctx context.Context, req *LoginUserRequest) (*LoginUserResponse, error) { - rs, _, err := c.configstoreClient.GetRemoteSourceByName(ctx, req.RemoteSourceName) + rs, resp, err := c.configstoreClient.GetRemoteSourceByName(ctx, req.RemoteSourceName) if err != nil { - return nil, errors.Wrapf(err, "failed to get remote source %q", req.RemoteSourceName) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get remote source %q", req.RemoteSourceName)) } c.log.Infof("rs: %s", util.Dump(rs)) @@ -270,9 +263,9 @@ func (c *CommandHandler) LoginUser(ctx context.Context, req *LoginUserRequest) ( return nil, errors.Errorf("empty remote user id for remote source %q", rs.ID) } - user, _, err := c.configstoreClient.GetUserByLinkedAccountRemoteUserAndSource(ctx, remoteUserInfo.ID, rs.ID) + user, resp, err := c.configstoreClient.GetUserByLinkedAccountRemoteUserAndSource(ctx, remoteUserInfo.ID, rs.ID) if err != nil { - return nil, errors.Wrapf(err, "failed to get user for remote user id %q and remote source %q", remoteUserInfo.ID, rs.ID) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get user for remote user id %q and remote source %q", remoteUserInfo.ID, rs.ID)) } var la *types.LinkedAccount @@ -305,9 +298,9 @@ func (c *CommandHandler) LoginUser(ctx context.Context, req *LoginUserRequest) ( } c.log.Infof("updating user %q linked account", user.UserName) - la, _, err = c.configstoreClient.UpdateUserLA(ctx, user.UserName, la.ID, creq) + la, resp, err = c.configstoreClient.UpdateUserLA(ctx, user.UserName, la.ID, creq) if err != nil { - return nil, errors.Wrapf(err, "failed to update user") + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to update user")) } c.log.Infof("linked account %q for user %q updated", la.ID, user.UserName) } @@ -336,9 +329,9 @@ type AuthorizeResponse struct { } func (c *CommandHandler) Authorize(ctx context.Context, req *AuthorizeRequest) (*AuthorizeResponse, error) { - rs, _, err := c.configstoreClient.GetRemoteSourceByName(ctx, req.RemoteSourceName) + rs, resp, err := c.configstoreClient.GetRemoteSourceByName(ctx, req.RemoteSourceName) if err != nil { - return nil, errors.Wrapf(err, "failed to get remote source %q", req.RemoteSourceName) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get remote source %q", req.RemoteSourceName)) } c.log.Infof("rs: %s", util.Dump(rs)) @@ -371,18 +364,18 @@ type RemoteSourceAuthResponse struct { } func (c *CommandHandler) HandleRemoteSourceAuth(ctx context.Context, remoteSourceName, loginName, loginPassword string, requestType RemoteSourceRequestType, req interface{}) (*RemoteSourceAuthResponse, error) { - rs, _, err := c.configstoreClient.GetRemoteSourceByName(ctx, remoteSourceName) + rs, resp, err := c.configstoreClient.GetRemoteSourceByName(ctx, remoteSourceName) if err != nil { - return nil, errors.Wrapf(err, "failed to get remote source %q", remoteSourceName) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get remote source %q", remoteSourceName)) } c.log.Infof("rs: %s", util.Dump(rs)) switch requestType { case RemoteSourceRequestTypeCreateUserLA: req := req.(*CreateUserLARequest) - user, _, err := c.configstoreClient.GetUserByName(ctx, req.UserName) + user, resp, err := c.configstoreClient.GetUserByName(ctx, req.UserName) if err != nil { - return nil, errors.Wrapf(err, "failed to get user %q", req.UserName) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get user %q", req.UserName)) } var la *types.LinkedAccount for _, v := range user.LinkedAccounts { @@ -393,7 +386,7 @@ func (c *CommandHandler) HandleRemoteSourceAuth(ctx context.Context, remoteSourc } c.log.Infof("la: %s", util.Dump(la)) if la != nil { - return nil, errors.Errorf("user %q already have a linked account for remote source %q", req.UserName, rs.Name) + return nil, util.NewErrBadRequest(errors.Errorf("user %q already have a linked account for remote source %q", req.UserName, rs.Name)) } case RemoteSourceRequestTypeLoginUser: @@ -594,9 +587,9 @@ func (c *CommandHandler) HandleOauth2Callback(ctx context.Context, code, state s requestType := RemoteSourceRequestType(claims["request_type"].(string)) requestString := claims["request"].(string) - rs, _, err := c.configstoreClient.GetRemoteSourceByName(ctx, remoteSourceName) + rs, resp, err := c.configstoreClient.GetRemoteSourceByName(ctx, remoteSourceName) if err != nil { - return nil, errors.Wrapf(err, "failed to get remote source %q", remoteSourceName) + return nil, ErrFromRemote(resp, errors.Wrapf(err, "failed to get remote source %q", remoteSourceName)) } c.log.Infof("rs: %s", util.Dump(rs)) diff --git a/internal/services/gateway/gateway.go b/internal/services/gateway/gateway.go index 0da9627..bbc21c4 100644 --- a/internal/services/gateway/gateway.go +++ b/internal/services/gateway/gateway.go @@ -178,7 +178,7 @@ func (g *Gateway) Run(ctx context.Context) error { deleteUserTokenHandler := api.NewDeleteUserTokenHandler(logger, g.configstoreClient) remoteSourceHandler := api.NewRemoteSourceHandler(logger, g.configstoreClient) - createRemoteSourceHandler := api.NewCreateRemoteSourceHandler(logger, g.configstoreClient) + createRemoteSourceHandler := api.NewCreateRemoteSourceHandler(logger, g.ch) remoteSourcesHandler := api.NewRemoteSourcesHandler(logger, g.configstoreClient) orgHandler := api.NewOrgHandler(logger, g.configstoreClient) diff --git a/internal/services/runservice/scheduler/api/api.go b/internal/services/runservice/scheduler/api/api.go index 11c8b17..24b6221 100644 --- a/internal/services/runservice/scheduler/api/api.go +++ b/internal/services/runservice/scheduler/api/api.go @@ -44,7 +44,10 @@ type ErrorResponse struct { } func ErrorResponseFromError(err error) *ErrorResponse { - if util.IsErrBadRequest(err) { + switch { + case util.IsErrBadRequest(err): + fallthrough + case util.IsErrNotFound(err): return &ErrorResponse{Message: err.Error()} } @@ -53,24 +56,28 @@ func ErrorResponseFromError(err error) *ErrorResponse { } func httpError(w http.ResponseWriter, err error) bool { - if err != nil { - response := ErrorResponseFromError(err) - resj, merr := json.Marshal(response) - if merr != nil { - w.WriteHeader(http.StatusInternalServerError) - return true - } - if util.IsErrBadRequest(err) { - w.WriteHeader(http.StatusBadRequest) - w.Write(resj) - } else { - w.WriteHeader(http.StatusInternalServerError) - w.Write(resj) - } - return true + if err == nil { + return false } - return false + response := ErrorResponseFromError(err) + resj, merr := json.Marshal(response) + if merr != nil { + w.WriteHeader(http.StatusInternalServerError) + return true + } + switch { + case util.IsErrBadRequest(err): + w.WriteHeader(http.StatusBadRequest) + w.Write(resj) + case util.IsErrNotFound(err): + w.WriteHeader(http.StatusNotFound) + w.Write(resj) + default: + w.WriteHeader(http.StatusInternalServerError) + w.Write(resj) + } + return true } func httpResponse(w http.ResponseWriter, code int, res interface{}) error { diff --git a/internal/services/runservice/scheduler/api/client.go b/internal/services/runservice/scheduler/api/client.go index 7c55b3b..7c41581 100644 --- a/internal/services/runservice/scheduler/api/client.go +++ b/internal/services/runservice/scheduler/api/client.go @@ -80,16 +80,14 @@ func (c *Client) getResponse(ctx context.Context, method, path string, query url defer resp.Body.Close() data, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, err + return resp, err } - if len(data) <= 1 { - return resp, errors.New(resp.Status) + errMap := make(map[string]interface{}) + if err = json.Unmarshal(data, &errMap); err != nil { + return resp, fmt.Errorf("unknown api error (code: %d): %s", resp.StatusCode, string(data)) } - - // TODO(sgotti) use a json error response - - return resp, errors.New(string(data)) + return resp, errors.New(errMap["message"].(string)) } return resp, nil diff --git a/internal/util/errors.go b/internal/util/errors.go index 3d5cd3d..7800a7e 100644 --- a/internal/util/errors.go +++ b/internal/util/errors.go @@ -15,7 +15,6 @@ package util import ( - "fmt" "strings" ) @@ -64,7 +63,7 @@ type ErrBadRequest struct { } func (e *ErrBadRequest) Error() string { - return fmt.Sprintf("bad request: %s", e.Err.Error()) + return e.Err.Error() } func NewErrBadRequest(err error) *ErrBadRequest { @@ -75,3 +74,22 @@ func IsErrBadRequest(err error) bool { _, ok := err.(*ErrBadRequest) return ok } + +// ErrNotFound represent a not found error +// it's used to differentiate an internal error from an user error +type ErrNotFound struct { + Err error +} + +func (e *ErrNotFound) Error() string { + return e.Err.Error() +} + +func NewErrNotFound(err error) *ErrNotFound { + return &ErrNotFound{Err: err} +} + +func IsErrNotFound(err error) bool { + _, ok := err.(*ErrNotFound) + return ok +} diff --git a/internal/util/git.go b/internal/util/git.go index 0d31844..284e352 100644 --- a/internal/util/git.go +++ b/internal/util/git.go @@ -142,11 +142,11 @@ func (g *Git) Pipe(ctx context.Context, w io.Writer, r io.Reader, args ...string return nil } -type ErrNotFound struct { +type ErrGitKeyNotFound struct { Key string } -func (e *ErrNotFound) Error() string { +func (e *ErrGitKeyNotFound) Error() string { return fmt.Sprintf("key `%q` was not found", e.Key) } @@ -157,7 +157,7 @@ func (g *Git) ConfigGet(ctx context.Context, args ...string) (string, error) { if exitError, ok := err.(*exec.ExitError); ok { if waitStatus, ok := exitError.Sys().(syscall.WaitStatus); ok { if waitStatus.ExitStatus() == 1 { - return "", &ErrNotFound{Key: args[len(args)-1]} + return "", &ErrGitKeyNotFound{Key: args[len(args)-1]} } } return "", err @@ -173,7 +173,7 @@ func (g *Git) ConfigSet(ctx context.Context, args ...string) (string, error) { if exitError, ok := err.(*exec.ExitError); ok { if waitStatus, ok := exitError.Sys().(syscall.WaitStatus); ok { if waitStatus.ExitStatus() == 1 { - return "", &ErrNotFound{Key: args[len(args)-1]} + return "", &ErrGitKeyNotFound{Key: args[len(args)-1]} } } return "", err