Move HTTP/handler utils to a separate package (#1311)

* Move HTTP functions to pkg/handlerutil Moving forward, they will be needed to implement Helm 3 support (see #1309), and they were in an internal package, so they couldn't be imported outside the `cmd/tiller-proxy` directory. Because they were all pure (or, in the case of ServeHTTP, at least very simple), I felt confident moving them to another file and reusing them. * Move error code test to pkg/handlerutil #1311 (comment)
vmware-tanzu · Nov 26, 2019 · db3fc03 · db3fc03
1 parent 10b1cfa
commit db3fc03
Show file tree

Hide file tree

Showing 5 changed files with 128 additions and 113 deletions.
diff --git a/cmd/tiller-proxy/internal/handler/handler.go b/cmd/tiller-proxy/internal/handler/handler.go
@@ -21,14 +21,13 @@ import (
 	"encoding/json"
 	"io/ioutil"
 	"net/http"
-	"regexp"
 	"strconv"
 	"strings"
 
-	"github.com/gorilla/mux"
 	"github.com/kubeapps/common/response"
 	"github.com/kubeapps/kubeapps/pkg/auth"
 	chartUtils "github.com/kubeapps/kubeapps/pkg/chart"
+	"github.com/kubeapps/kubeapps/pkg/handlerutil"
 	proxy "github.com/kubeapps/kubeapps/pkg/proxy"
 	log "github.com/sirupsen/logrus"
 	"github.com/urfave/negroni"
@@ -64,59 +63,6 @@ func AuthGate() negroni.HandlerFunc {
 	}
 }
 
-// Params a key-value map of path params
-type Params map[string]string
-
-// WithParams can be used to wrap handlers to take an extra arg for path params
-type WithParams func(http.ResponseWriter, *http.Request, Params)
-
-func (h WithParams) ServeHTTP(w http.ResponseWriter, req *http.Request) {
-	vars := mux.Vars(req)
-	h(w, req, vars)
-}
-
-// WithoutParams can be used to wrap handlers that doesn't take params
-type WithoutParams func(http.ResponseWriter, *http.Request)
-
-func (h WithoutParams) ServeHTTP(w http.ResponseWriter, req *http.Request) {
-	h(w, req)
-}
-
-func isNotFound(err error) bool {
-	return strings.Contains(err.Error(), "not found")
-}
-
-func isAlreadyExists(err error) bool {
-	return strings.Contains(err.Error(), "is still in use") || strings.Contains(err.Error(), "already exists")
-}
-
-func isForbidden(err error) bool {
-	return strings.Contains(err.Error(), "Unauthorized")
-}
-
-func isUnprocessable(err error) bool {
-	re := regexp.MustCompile(`release.*failed`)
-	return re.MatchString(err.Error())
-}
-
-func errorCode(err error) int {
-	return errorCodeWithDefault(err, http.StatusInternalServerError)
-}
-
-func errorCodeWithDefault(err error, defaultCode int) int {
-	errCode := defaultCode
-	if isAlreadyExists(err) {
-		errCode = http.StatusConflict
-	} else if isNotFound(err) {
-		errCode = http.StatusNotFound
-	} else if isForbidden(err) {
-		errCode = http.StatusForbidden
-	} else if isUnprocessable(err) {
-		errCode = http.StatusUnprocessableEntity
-	}
-	return errCode
-}
-
 func getChart(req *http.Request, cu chartUtils.Resolver) (*chartUtils.Details, *chart.Chart, error) {
 	defer req.Body.Close()
 	body, err := ioutil.ReadAll(req.Body)
@@ -142,7 +88,7 @@ func returnForbiddenActions(forbiddenActions []auth.Action, w http.ResponseWrite
 	w.Header().Set("Content-Type", "application/json")
 	body, err := json.Marshal(forbiddenActions)
 	if err != nil {
-		response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
 	response.NewErrorResponse(http.StatusForbidden, string(body)).Write(w)
@@ -166,23 +112,23 @@ func (h *TillerProxy) logStatus(name string) {
 }
 
 // CreateRelease creates a new release in the namespace given as Param
-func (h *TillerProxy) CreateRelease(w http.ResponseWriter, req *http.Request, params Params) {
+func (h *TillerProxy) CreateRelease(w http.ResponseWriter, req *http.Request, params handlerutil.Params) {
 	log.Printf("Creating Helm Release")
 	chartDetails, ch, err := getChart(req, h.ChartClient)
 	if err != nil {
-		response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
 	if !h.DisableAuth {
 		manifest, err := h.ProxyClient.ResolveManifest(params["namespace"], chartDetails.Values, ch)
 		if err != nil {
-			response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 			return
 		}
 		userAuth := req.Context().Value(userKey).(auth.Checker)
 		forbiddenActions, err := userAuth.GetForbiddenActions(params["namespace"], "create", manifest)
 		if err != nil {
-			response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 			return
 		}
 		if len(forbiddenActions) > 0 {
@@ -192,7 +138,7 @@ func (h *TillerProxy) CreateRelease(w http.ResponseWriter, req *http.Request, pa
 	}
 	rel, err := h.ProxyClient.CreateRelease(chartDetails.ReleaseName, params["namespace"], chartDetails.Values, ch)
 	if err != nil {
-		response.NewErrorResponse(errorCodeWithDefault(err, http.StatusUnprocessableEntity), err.Error()).Write(w)
+		response.NewErrorResponse(handlerutil.ErrorCodeWithDefault(err, http.StatusUnprocessableEntity), err.Error()).Write(w)
 		return
 	}
 	log.Printf("Installed release %s", rel.Name)
@@ -201,7 +147,7 @@ func (h *TillerProxy) CreateRelease(w http.ResponseWriter, req *http.Request, pa
 }
 
 // OperateRelease decides which method to call depending in the "action" query param
-func (h *TillerProxy) OperateRelease(w http.ResponseWriter, req *http.Request, params Params) {
+func (h *TillerProxy) OperateRelease(w http.ResponseWriter, req *http.Request, params handlerutil.Params) {
 	switch req.FormValue("action") {
 	case "upgrade":
 		h.UpgradeRelease(w, req, params)
@@ -214,7 +160,7 @@ func (h *TillerProxy) OperateRelease(w http.ResponseWriter, req *http.Request, p
 }
 
 // RollbackRelease performs an action over a release
-func (h *TillerProxy) RollbackRelease(w http.ResponseWriter, req *http.Request, params Params) {
+func (h *TillerProxy) RollbackRelease(w http.ResponseWriter, req *http.Request, params handlerutil.Params) {
 	log.Printf("Rolling back %s", params["releaseName"])
 	revision := req.FormValue("revision")
 	if revision == "" {
@@ -223,20 +169,20 @@ func (h *TillerProxy) RollbackRelease(w http.ResponseWriter, req *http.Request,
 	}
 	revisionInt, err := strconv.ParseInt(revision, 10, 64)
 	if err != nil {
-		response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
 	if !h.DisableAuth {
 		manifest, err := h.ProxyClient.ResolveManifestFromRelease(params["releaseName"], int32(revisionInt))
 		if err != nil {
-			response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 			return
 		}
 		userAuth := req.Context().Value(userKey).(auth.Checker)
 		// Using "upgrade" action since the concept is the same
 		forbiddenActions, err := userAuth.GetForbiddenActions(params["namespace"], "upgrade", manifest)
 		if err != nil {
-			response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 			return
 		}
 		if len(forbiddenActions) > 0 {
@@ -246,7 +192,7 @@ func (h *TillerProxy) RollbackRelease(w http.ResponseWriter, req *http.Request,
 	}
 	rel, err := h.ProxyClient.RollbackRelease(params["releaseName"], params["namespace"], int32(revisionInt))
 	if err != nil {
-		response.NewErrorResponse(errorCodeWithDefault(err, http.StatusUnprocessableEntity), err.Error()).Write(w)
+		response.NewErrorResponse(handlerutil.ErrorCodeWithDefault(err, http.StatusUnprocessableEntity), err.Error()).Write(w)
 		return
 	}
 	log.Printf("Rollback release for %s to %d", rel.Name, revisionInt)
@@ -255,23 +201,23 @@ func (h *TillerProxy) RollbackRelease(w http.ResponseWriter, req *http.Request,
 }
 
 // UpgradeRelease upgrades a release in the namespace given as Param
-func (h *TillerProxy) UpgradeRelease(w http.ResponseWriter, req *http.Request, params Params) {
+func (h *TillerProxy) UpgradeRelease(w http.ResponseWriter, req *http.Request, params handlerutil.Params) {
 	log.Printf("Upgrading Helm Release")
 	chartDetails, ch, err := getChart(req, h.ChartClient)
 	if err != nil {
-		response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
 	if !h.DisableAuth {
 		manifest, err := h.ProxyClient.ResolveManifest(params["namespace"], chartDetails.Values, ch)
 		if err != nil {
-			response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 			return
 		}
 		userAuth := req.Context().Value(userKey).(auth.Checker)
 		forbiddenActions, err := userAuth.GetForbiddenActions(params["namespace"], "upgrade", manifest)
 		if err != nil {
-			response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 			return
 		}
 		if len(forbiddenActions) > 0 {
@@ -281,7 +227,7 @@ func (h *TillerProxy) UpgradeRelease(w http.ResponseWriter, req *http.Request, p
 	}
 	rel, err := h.ProxyClient.UpdateRelease(params["releaseName"], params["namespace"], chartDetails.Values, ch)
 	if err != nil {
-		response.NewErrorResponse(errorCodeWithDefault(err, http.StatusUnprocessableEntity), err.Error()).Write(w)
+		response.NewErrorResponse(handlerutil.ErrorCodeWithDefault(err, http.StatusUnprocessableEntity), err.Error()).Write(w)
 		return
 	}
 	log.Printf("Upgraded release %s", rel.Name)
@@ -293,39 +239,39 @@ func (h *TillerProxy) UpgradeRelease(w http.ResponseWriter, req *http.Request, p
 func (h *TillerProxy) ListAllReleases(w http.ResponseWriter, req *http.Request) {
 	apps, err := h.ProxyClient.ListReleases("", h.ListLimit, req.URL.Query().Get("statuses"))
 	if err != nil {
-		response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
 	response.NewDataResponse(apps).Write(w)
 }
 
 // ListReleases in the namespace given as Param
-func (h *TillerProxy) ListReleases(w http.ResponseWriter, req *http.Request, params Params) {
+func (h *TillerProxy) ListReleases(w http.ResponseWriter, req *http.Request, params handlerutil.Params) {
 	apps, err := h.ProxyClient.ListReleases(params["namespace"], h.ListLimit, req.URL.Query().Get("statuses"))
 	if err != nil {
-		response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
 	response.NewDataResponse(apps).Write(w)
 }
 
 // GetRelease returns the release info
-func (h *TillerProxy) GetRelease(w http.ResponseWriter, req *http.Request, params Params) {
+func (h *TillerProxy) GetRelease(w http.ResponseWriter, req *http.Request, params handlerutil.Params) {
 	rel, err := h.ProxyClient.GetRelease(params["releaseName"], params["namespace"])
 	if err != nil {
-		response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
 	if !h.DisableAuth {
 		manifest, err := h.ProxyClient.ResolveManifest(params["namespace"], rel.Config.Raw, rel.Chart)
 		if err != nil {
-			response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 			return
 		}
 		userAuth := req.Context().Value(userKey).(auth.Checker)
 		forbiddenActions, err := userAuth.GetForbiddenActions(params["namespace"], "get", manifest)
 		if err != nil {
-			response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 			return
 		}
 		if len(forbiddenActions) > 0 {
@@ -337,22 +283,22 @@ func (h *TillerProxy) GetRelease(w http.ResponseWriter, req *http.Request, param
 }
 
 // DeleteRelease removes a release from a namespace
-func (h *TillerProxy) DeleteRelease(w http.ResponseWriter, req *http.Request, params Params) {
+func (h *TillerProxy) DeleteRelease(w http.ResponseWriter, req *http.Request, params handlerutil.Params) {
 	if !h.DisableAuth {
 		rel, err := h.ProxyClient.GetRelease(params["releaseName"], params["namespace"])
 		if err != nil {
-			response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 			return
 		}
 		manifest, err := h.ProxyClient.ResolveManifest(params["namespace"], rel.Config.Raw, rel.Chart)
 		if err != nil {
-			response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 			return
 		}
 		userAuth := req.Context().Value(userKey).(auth.Checker)
 		forbiddenActions, err := userAuth.GetForbiddenActions(params["namespace"], "delete", manifest)
 		if err != nil {
-			response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 			return
 		}
 		if len(forbiddenActions) > 0 {
@@ -366,7 +312,7 @@ func (h *TillerProxy) DeleteRelease(w http.ResponseWriter, req *http.Request, pa
 	}
 	err := h.ProxyClient.DeleteRelease(params["releaseName"], params["namespace"], purge)
 	if err != nil {
-		response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
+		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
 	w.Header().Set("Status-Code", "200")

diff --git a/cmd/tiller-proxy/internal/handler/handler_test.go b/cmd/tiller-proxy/internal/handler/handler_test.go
@@ -19,7 +19,6 @@ package handler
 import (
 	"context"
 	"fmt"
-	"net/http"
 	"net/http/httptest"
 	"reflect"
 	"strings"
@@ -34,28 +33,6 @@ import (
 	proxyFake "github.com/kubeapps/kubeapps/pkg/proxy/fake"
 )
 
-func TestErrorCodeWithDefault(t *testing.T) {
-	type test struct {
-		err          error
-		defaultCode  int
-		expectedCode int
-	}
-	tests := []test{
-		{fmt.Errorf("a release named foo already exists"), http.StatusInternalServerError, http.StatusConflict},
-		{fmt.Errorf("release foo not found"), http.StatusInternalServerError, http.StatusNotFound},
-		{fmt.Errorf("Unauthorized to get release foo"), http.StatusInternalServerError, http.StatusForbidden},
-		{fmt.Errorf("release \"Foo \" failed"), http.StatusInternalServerError, http.StatusUnprocessableEntity},
-		{fmt.Errorf("This is an unexpected error"), http.StatusInternalServerError, http.StatusInternalServerError},
-		{fmt.Errorf("This is an unexpected error"), http.StatusUnprocessableEntity, http.StatusUnprocessableEntity},
-	}
-	for _, s := range tests {
-		code := errorCodeWithDefault(s.err, s.defaultCode)
-		if code != s.expectedCode {
-			t.Errorf("Expected '%v' to return code %v got %v", s.err, s.expectedCode, code)
-		}
-	}
-}
-
 func TestActions(t *testing.T) {
 	type testScenario struct {
 		// Scenario params

diff --git a/cmd/tiller-proxy/main.go b/cmd/tiller-proxy/main.go
@@ -32,6 +32,7 @@ import (
 	appRepo "github.com/kubeapps/kubeapps/cmd/apprepository-controller/pkg/client/clientset/versioned"
 	"github.com/kubeapps/kubeapps/cmd/tiller-proxy/internal/handler"
 	chartUtils "github.com/kubeapps/kubeapps/pkg/chart"
+	"github.com/kubeapps/kubeapps/pkg/handlerutil"
 	tillerProxy "github.com/kubeapps/kubeapps/pkg/proxy"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
@@ -156,27 +157,27 @@ func main() {
 	apiv1 := r.PathPrefix("/v1").Subrouter()
 	apiv1.Methods("GET").Path("/releases").Handler(negroni.New(
 		authGate,
-		negroni.Wrap(handler.WithoutParams(h.ListAllReleases)),
+		negroni.Wrap(handlerutil.WithoutParams(h.ListAllReleases)),
 	))
 	apiv1.Methods("GET").Path("/namespaces/{namespace}/releases").Handler(negroni.New(
 		authGate,
-		negroni.Wrap(handler.WithParams(h.ListReleases)),
+		negroni.Wrap(handlerutil.WithParams(h.ListReleases)),
 	))
 	apiv1.Methods("POST").Path("/namespaces/{namespace}/releases").Handler(negroni.New(
 		authGate,
-		negroni.Wrap(handler.WithParams(h.CreateRelease)),
+		negroni.Wrap(handlerutil.WithParams(h.CreateRelease)),
 	))
 	apiv1.Methods("GET").Path("/namespaces/{namespace}/releases/{releaseName}").Handler(negroni.New(
 		authGate,
-		negroni.Wrap(handler.WithParams(h.GetRelease)),
+		negroni.Wrap(handlerutil.WithParams(h.GetRelease)),
 	))
 	apiv1.Methods("PUT").Path("/namespaces/{namespace}/releases/{releaseName}").Handler(negroni.New(
 		authGate,
-		negroni.Wrap(handler.WithParams(h.OperateRelease)),
+		negroni.Wrap(handlerutil.WithParams(h.OperateRelease)),
 	))
 	apiv1.Methods("DELETE").Path("/namespaces/{namespace}/releases/{releaseName}").Handler(negroni.New(
 		authGate,
-		negroni.Wrap(handler.WithParams(h.DeleteRelease)),
+		negroni.Wrap(handlerutil.WithParams(h.DeleteRelease)),
 	))
 
 	// Chartsvc reverse proxy