configstore api: improve response handling

* Always return a json message also on error. For internal errors return a
generic "internal server error" message to not leak the real internal error to
clients
* Return 201 Created on resource creation
* Return 204 No Content on resource deletion and other action with no json
output

internal/services/configstore/api/api.go

@@ -15,6 +15,7 @@
 package api
 
 import (
+	"encoding/json"
 	"net/http"
 	"net/url"
 
@@ -24,12 +25,33 @@ import (
 	"github.com/sorintlab/agola/internal/util"
 )
 
+type ErrorResponse struct {
+	Message string `json:"message"`
+}
+
+func ErrorResponseFromError(err error) *ErrorResponse {
+	if util.IsErrBadRequest(err) {
+		return &ErrorResponse{Message: err.Error()}
+	}
+
+	// on generic error return an generic message to not leak the real error
+	return &ErrorResponse{Message: "internal server error"}
+}
+
 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) {
-			http.Error(w, err.Error(), http.StatusBadRequest)
+			w.WriteHeader(http.StatusBadRequest)
+			w.Write(resj)
 		} else {
-			http.Error(w, "", http.StatusInternalServerError)
+			w.WriteHeader(http.StatusInternalServerError)
+			w.Write(resj)
 		}
 		return true
 	}
@@ -37,6 +59,24 @@ func httpError(w http.ResponseWriter, err error) bool {
 	return false
 }
 
+func httpResponse(w http.ResponseWriter, code int, res interface{}) error {
+	w.Header().Set("Content-Type", "application/json")
+
+	if res != nil {
+		resj, err := json.Marshal(res)
+		if err != nil {
+			httpError(w, err)
+			return err
+		}
+		w.WriteHeader(code)
+		_, err = w.Write(resj)
+		return err
+	}
+
+	w.WriteHeader(code)
+	return nil
+}
+
 func GetConfigTypeRef(r *http.Request) (types.ConfigType, string, error) {
 	vars := mux.Vars(r)
 	projectRef, err := url.PathUnescape(vars["projectref"])

internal/services/configstore/api/org.go

@@ -58,10 +58,8 @@ func (h *OrgHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(org); err != nil {
+	if err := httpResponse(w, http.StatusOK, org); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -95,10 +93,8 @@ func (h *OrgByNameHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(org); err != nil {
+	if err := httpResponse(w, http.StatusOK, org); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -127,10 +123,8 @@ func (h *CreateOrgHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(org); err != nil {
+	if err := httpResponse(w, http.StatusCreated, org); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -154,6 +148,9 @@ func (h *DeleteOrgHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if httpError(w, err) {
 		h.log.Errorf("err: %+v", err)
 	}
+	if err := httpResponse(w, http.StatusNoContent, nil); err != nil {
+		h.log.Errorf("err: %+v", err)
+	}
 }
 
 const (
@@ -209,9 +206,7 @@ func (h *OrgsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(orgs); err != nil {
+	if err := httpResponse(w, http.StatusOK, orgs); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }

internal/services/configstore/api/project.go

@@ -74,10 +74,8 @@ func (h *ProjectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(project); err != nil {
+	if err := httpResponse(w, http.StatusOK, project); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -107,10 +105,8 @@ func (h *CreateProjectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(project); err != nil {
+	if err := httpResponse(w, http.StatusCreated, project); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -137,6 +133,9 @@ func (h *DeleteProjectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 	if httpError(w, err) {
 		h.log.Errorf("err: %+v", err)
 	}
+	if err := httpResponse(w, http.StatusNoContent, nil); err != nil {
+		h.log.Errorf("err: %+v", err)
+	}
 }
 
 const (

internal/services/configstore/api/projectgroup.go

@@ -62,10 +62,8 @@ func (h *ProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(projectGroup); err != nil {
+	if err := httpResponse(w, http.StatusOK, projectGroup); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -114,10 +112,8 @@ func (h *ProjectGroupProjectsHandler) ServeHTTP(w http.ResponseWriter, r *http.R
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(projects); err != nil {
+	if err := httpResponse(w, http.StatusOK, projects); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -166,10 +162,8 @@ func (h *ProjectGroupSubgroupsHandler) ServeHTTP(w http.ResponseWriter, r *http.
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(projectGroups); err != nil {
+	if err := httpResponse(w, http.StatusOK, projectGroups); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -199,9 +193,7 @@ func (h *CreateProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Req
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(projectGroup); err != nil {
+	if err := httpResponse(w, http.StatusCreated, projectGroup); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }

internal/services/configstore/api/remotesource.go

@@ -58,10 +58,8 @@ func (h *RemoteSourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(remoteSource); err != nil {
+	if err := httpResponse(w, http.StatusOK, remoteSource); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -95,10 +93,8 @@ func (h *RemoteSourceByNameHandler) ServeHTTP(w http.ResponseWriter, r *http.Req
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(remoteSource); err != nil {
+	if err := httpResponse(w, http.StatusOK, remoteSource); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -128,10 +124,8 @@ func (h *CreateRemoteSourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Req
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(remoteSource); err != nil {
+	if err := httpResponse(w, http.StatusCreated, remoteSource); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -154,6 +148,9 @@ func (h *DeleteRemoteSourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Req
 	if httpError(w, err) {
 		h.log.Errorf("err: %+v", err)
 	}
+	if err := httpResponse(w, http.StatusNoContent, nil); err != nil {
+		h.log.Errorf("err: %+v", err)
+	}
 }
 
 const (
@@ -204,9 +201,7 @@ func (h *RemoteSourcesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(remoteSources); err != nil {
+	if err := httpResponse(w, http.StatusOK, remoteSources); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }

internal/services/configstore/api/secret.go

@@ -57,10 +57,8 @@ func (h *SecretHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(secret); err != nil {
+	if err := httpResponse(w, http.StatusOK, secret); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -111,10 +109,8 @@ func (h *SecretsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(secrets); err != nil {
+	if err := httpResponse(w, http.StatusOK, secrets); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -152,10 +148,8 @@ func (h *CreateSecretHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(secret); err != nil {
+	if err := httpResponse(w, http.StatusCreated, secret); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -183,4 +177,7 @@ func (h *DeleteSecretHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 	if httpError(w, err) {
 		h.log.Errorf("err: %+v", err)
 	}
+	if err := httpResponse(w, http.StatusNoContent, nil); err != nil {
+		h.log.Errorf("err: %+v", err)
+	}
 }

internal/services/configstore/api/user.go

@@ -59,10 +59,8 @@ func (h *UserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(user); err != nil {
+	if err := httpResponse(w, http.StatusOK, user); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -96,10 +94,8 @@ func (h *UserByNameHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(user); err != nil {
+	if err := httpResponse(w, http.StatusOK, user); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -148,10 +144,8 @@ func (h *CreateUserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(user); err != nil {
+	if err := httpResponse(w, http.StatusCreated, user); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -174,6 +168,9 @@ func (h *DeleteUserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if httpError(w, err) {
 		h.log.Errorf("err: %+v", err)
 	}
+	if err := httpResponse(w, http.StatusNoContent, nil); err != nil {
+		h.log.Errorf("err: %+v", err)
+	}
 }
 
 const (
@@ -295,10 +292,8 @@ func (h *UsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	if err := json.NewEncoder(w).Encode(users); err != nil {
+	if err := httpResponse(w, http.StatusOK, users); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -347,10 +342,8 @@ func (h *CreateUserLAHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(user); err != nil {
+	if err := httpResponse(w, http.StatusCreated, user); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -373,6 +366,9 @@ func (h *DeleteUserLAHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 	if httpError(w, err) {
 		h.log.Errorf("err: %+v", err)
 	}
+	if err := httpResponse(w, http.StatusNoContent, nil); err != nil {
+		h.log.Errorf("err: %+v", err)
+	}
 }
 
 type UpdateUserLARequest struct {
@@ -420,10 +416,8 @@ func (h *UpdateUserLAHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(user); err != nil {
+	if err := httpResponse(w, http.StatusOK, user); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -465,10 +459,8 @@ func (h *CreateUserTokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques
 	resp := &CreateUserTokenResponse{
 		Token: token,
 	}
-	if err := json.NewEncoder(w).Encode(resp); err != nil {
+	if err := httpResponse(w, http.StatusCreated, resp); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -491,4 +483,7 @@ func (h *DeleteUserTokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques
 	if httpError(w, err) {
 		h.log.Errorf("err: %+v", err)
 	}
+	if err := httpResponse(w, http.StatusNoContent, nil); err != nil {
+		h.log.Errorf("err: %+v", err)
+	}
 }

internal/services/configstore/api/variable.go

@@ -75,10 +75,8 @@ func (h *VariablesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(variables); err != nil {
+	if err := httpResponse(w, http.StatusOK, variables); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -118,10 +116,8 @@ func (h *CreateVariableHandler) ServeHTTP(w http.ResponseWriter, r *http.Request
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(variable); err != nil {
+	if err := httpResponse(w, http.StatusCreated, variable); err != nil {
 		h.log.Errorf("err: %+v", err)
-		httpError(w, err)
-		return
 	}
 }
 
@@ -149,4 +145,7 @@ func (h *DeleteVariableHandler) ServeHTTP(w http.ResponseWriter, r *http.Request
 	if httpError(w, err) {
 		h.log.Errorf("err: %+v", err)
 	}
+	if err := httpResponse(w, http.StatusNoContent, nil); err != nil {
+		h.log.Errorf("err: %+v", err)
+	}
 }