Tests for pkg/http-handler (#1516)

* Tests for http-handler

* Remove HTTP references from pkg/apprepo (#1517)

go.sum

@@ -569,6 +569,7 @@ k8s.io/apiextensions-apiserver v0.0.0-20191016113550-5357c4baaf65/go.mod h1:5BIN
 k8s.io/apimachinery v0.0.0-20191004115801-a2eda9f80ab8 h1:Iieh/ZEgT3BWwbLD5qEKcY06jKuPEl6zC7gPSehoLw4=
 k8s.io/apimachinery v0.0.0-20191004115801-a2eda9f80ab8/go.mod h1:llRdnznGEAqC3DcNm6yEj472xaFVfLM7hnYofMb12tQ=
 k8s.io/apimachinery v0.17.2 h1:hwDQQFbdRlpnnsR64Asdi55GyCaIP/3WQpMmbNBeWr4=
+k8s.io/apimachinery v0.17.3 h1:f+uZV6rm4/tHE7xXgLyToprg6xWairaClGVkm2t8omg=
 k8s.io/apiserver v0.0.0-20191016112112-5190913f932d/go.mod h1:7OqfAolfWxUM/jJ/HBLyE+cdaWFBUoo5Q5pHgJVj2ws=
 k8s.io/cli-runtime v0.0.0-20191016114015-74ad18325ed5 h1:8ZfMjkMBzcXEawLsYHg9lDM7aLEVso3NiVKfUTnN56A=
 k8s.io/cli-runtime v0.0.0-20191016114015-74ad18325ed5/go.mod h1:sDl6WKSQkDM6zS1u9F49a0VooQ3ycYFBFLqd2jf2Xfo=

pkg/apprepo/apprepos_handler.go

@@ -19,7 +19,7 @@ package apprepo
 import (
 	"encoding/json"
 	"fmt"
-	"net/http"
+	"io"
 
 	log "github.com/sirupsen/logrus"
 	authorizationapi "k8s.io/api/authorization/v1"
@@ -34,7 +34,6 @@ import (
 	"github.com/kubeapps/kubeapps/cmd/apprepository-controller/pkg/apis/apprepository/v1alpha1"
 	apprepoclientset "github.com/kubeapps/kubeapps/cmd/apprepository-controller/pkg/client/clientset/versioned"
 	v1alpha1typed "github.com/kubeapps/kubeapps/cmd/apprepository-controller/pkg/client/clientset/versioned/typed/apprepository/v1alpha1"
-	"github.com/kubeapps/kubeapps/pkg/auth"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -77,9 +76,9 @@ type AppRepositoriesHandler struct {
 
 // Handler exposes the handler method for testing purposes
 type Handler interface {
-	CreateAppRepository(req *http.Request, namespace string) (*v1alpha1.AppRepository, error)
-	DeleteAppRepository(req *http.Request, name, namespace string) error
-	GetNamespaces(req *http.Request) ([]corev1.Namespace, error)
+	CreateAppRepository(appRepoBody io.ReadCloser, requestNamespace, token string) (*v1alpha1.AppRepository, error)
+	DeleteAppRepository(name, namespace, token string) error
+	GetNamespaces(token string) ([]corev1.Namespace, error)
 }
 
 // appRepositoryRequest is used to parse the JSON request
@@ -156,8 +155,7 @@ func (a *AppRepositoriesHandler) ConfigForToken(token string) *rest.Config {
 	return &configCopy
 }
 
-func (a *AppRepositoriesHandler) clientsetForRequest(req *http.Request) (combinedClientsetInterface, error) {
-	token := auth.ExtractToken(req.Header.Get("Authorization"))
+func (a *AppRepositoriesHandler) clientsetForRequest(token string) (combinedClientsetInterface, error) {
 	clientset, err := a.clientsetForConfig(a.ConfigForToken(token))
 	if err != nil {
 		log.Errorf("unable to create clientset: %v", err)
@@ -166,20 +164,20 @@ func (a *AppRepositoriesHandler) clientsetForRequest(req *http.Request) (combine
 }
 
 // CreateAppRepository creates an AppRepository resource based on the request data
-func (a *AppRepositoriesHandler) CreateAppRepository(req *http.Request, requestNamespace string) (*v1alpha1.AppRepository, error) {
+func (a *AppRepositoriesHandler) CreateAppRepository(appRepoBody io.ReadCloser, requestNamespace, token string) (*v1alpha1.AppRepository, error) {
 	if a.kubeappsNamespace == "" {
 		log.Errorf("attempt to use app repositories handler without kubeappsNamespace configured")
 		return nil, fmt.Errorf("kubeappsNamespace must be configured to enable app repository handler")
 	}
 
-	clientset, err := a.clientsetForRequest(req)
+	clientset, err := a.clientsetForRequest(token)
 	if err != nil {
 		log.Errorf("unable to create clientset: %v", err)
 		return nil, err
 	}
 
 	var appRepoRequest appRepositoryRequest
-	err = json.NewDecoder(req.Body).Decode(&appRepoRequest)
+	err = json.NewDecoder(appRepoBody).Decode(&appRepoRequest)
 	if err != nil {
 		log.Infof("unable to decode: %v", err)
 		return nil, err
@@ -223,8 +221,8 @@ func (a *AppRepositoriesHandler) CreateAppRepository(req *http.Request, requestN
 }
 
 // DeleteAppRepository deletes an AppRepository resource from a namespace.
-func (a *AppRepositoriesHandler) DeleteAppRepository(req *http.Request, repoName, repoNamespace string) error {
-	clientset, err := a.clientsetForRequest(req)
+func (a *AppRepositoriesHandler) DeleteAppRepository(repoName, repoNamespace, token string) error {
+	clientset, err := a.clientsetForRequest(token)
 	if err != nil {
 		return err
 	}
@@ -362,8 +360,8 @@ func filterAllowedNamespaces(userClientset combinedClientsetInterface, namespace
 // TODO(andresmgot): I am adding this method in this package for simplicity
 // (since it already allows to impersonate the user)
 // We should refactor this code to make it more generic (not apprepository-specific)
-func (a *AppRepositoriesHandler) GetNamespaces(req *http.Request) ([]corev1.Namespace, error) {
-	userClientset, err := a.clientsetForRequest(req)
+func (a *AppRepositoriesHandler) GetNamespaces(token string) ([]corev1.Namespace, error) {
+	userClientset, err := a.clientsetForRequest(token)
 	if err != nil {
 		return nil, err
 	}

pkg/apprepo/apprepos_handler_test.go

@@ -19,7 +19,7 @@ package apprepo
 import (
 	"encoding/json"
 	"fmt"
-	"net/http/httptest"
+	"io/ioutil"
 	"strings"
 	"testing"
 
@@ -188,9 +188,7 @@ func TestAppRepositoryCreate(t *testing.T) {
 				svcKubeClient:      fakecoreclientset.NewSimpleClientset(),
 			}
 
-			req := httptest.NewRequest("POST", "https://foo.bar/backend/v1/namespaces/kubeapps/apprepositories", strings.NewReader(tc.requestData))
-
-			apprepo, err := handler.CreateAppRepository(req, tc.requestNamespace)
+			apprepo, err := handler.CreateAppRepository(ioutil.NopCloser(strings.NewReader(tc.requestData)), tc.requestNamespace, "token")
 
 			if err == nil && tc.expectedError != nil {
 				t.Errorf("got: nil, want: %+v", tc.expectedError)
@@ -317,11 +315,7 @@ func TestDeleteAppRepository(t *testing.T) {
 				svcKubeClient:      fakecoreclientset.NewSimpleClientset(),
 			}
 
-			// TODO: Currently the request is only used to create the clientset with user creds.
-			// Remove the need for http at all in this module.
-			req := httptest.NewRequest("DELETE", "https://foo.bar/backend/v1/namespaces/kubeapps/apprepositories", strings.NewReader(""))
-
-			err := handler.DeleteAppRepository(req, tc.repoName, tc.requestNamespace)
+			err := handler.DeleteAppRepository(tc.repoName, tc.requestNamespace, "token")
 
 			if got, want := errorCodeForK8sError(t, err), tc.expectedErrorCode; got != want {
 				t.Errorf("got: %d, want: %d", got, want)
@@ -353,9 +347,8 @@ func errorCodeForK8sError(t *testing.T, err error) int {
 	}
 	if statusErr, ok := err.(*errors.StatusError); ok {
 		return int(statusErr.ErrStatus.Code)
-	} else {
-		t.Fatalf("unable to convert error to status error")
 	}
+	t.Fatalf("unable to convert error to status error")
 	return 0
 }
 
@@ -652,9 +645,7 @@ func TestGetNamespaces(t *testing.T) {
 				kubeappsNamespace:  "kubeapps",
 			}
 
-			req := httptest.NewRequest("GET", "https://foo.bar/backend/v1/namespaces", nil)
-
-			namespaces, err := handler.GetNamespaces(req)
+			namespaces, err := handler.GetNamespaces("token")
 			if err != nil {
 				t.Errorf("Unexpected error %v", err)
 			}

pkg/http-handler/http-handler.go

@@ -24,6 +24,7 @@ import (
 	"github.com/gorilla/mux"
 	"github.com/kubeapps/kubeapps/cmd/apprepository-controller/pkg/apis/apprepository/v1alpha1"
 	"github.com/kubeapps/kubeapps/pkg/apprepo"
+	"github.com/kubeapps/kubeapps/pkg/auth"
 	log "github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
@@ -39,20 +40,25 @@ type appRepositoryResponse struct {
 	AppRepository v1alpha1.AppRepository `json:"appRepository"`
 }
 
+func returnK8sError(err error, w http.ResponseWriter) {
+	if statusErr, ok := err.(*k8sErrors.StatusError); ok {
+		status := statusErr.ErrStatus
+		log.Infof("unable to create app repo: %v", status.Reason)
+		http.Error(w, status.Message, int(status.Code))
+	} else {
+		log.Errorf("unable to create app repo: %v", err)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+	}
+}
+
 // CreateAppRepository creates App Repository
 func CreateAppRepository(appRepo apprepo.Handler) func(w http.ResponseWriter, req *http.Request) {
 	return func(w http.ResponseWriter, req *http.Request) {
 		requestNamespace := mux.Vars(req)["namespace"]
-		appRepo, err := appRepo.CreateAppRepository(req, requestNamespace)
+		token := auth.ExtractToken(req.Header.Get("Authorization"))
+		appRepo, err := appRepo.CreateAppRepository(req.Body, requestNamespace, token)
 		if err != nil {
-			if statusErr, ok := err.(*k8sErrors.StatusError); ok {
-				status := statusErr.ErrStatus
-				log.Infof("unable to create app repo: %v", status.Reason)
-				http.Error(w, status.Message, int(status.Code))
-			} else {
-				log.Errorf("unable to create app repo: %v", err)
-				http.Error(w, err.Error(), http.StatusInternalServerError)
-			}
+			returnK8sError(err, w)
 			return
 		}
 		w.WriteHeader(http.StatusCreated)
@@ -73,32 +79,23 @@ func DeleteAppRepository(appRepo apprepo.Handler) func(w http.ResponseWriter, re
 	return func(w http.ResponseWriter, req *http.Request) {
 		repoNamespace := mux.Vars(req)["namespace"]
 		repoName := mux.Vars(req)["name"]
+		token := auth.ExtractToken(req.Header.Get("Authorization"))
 
-		err := appRepo.DeleteAppRepository(req, repoName, repoNamespace)
+		err := appRepo.DeleteAppRepository(repoName, repoNamespace, token)
 
 		if err != nil {
-			if statusErr, ok := err.(*k8sErrors.StatusError); ok {
-				status := statusErr.ErrStatus
-				log.Infof("unable to create app repo: %v", status.Reason)
-				http.Error(w, status.Message, int(status.Code))
-			} else {
-				log.Errorf("unable to create app repo: %v", err)
-				http.Error(w, err.Error(), http.StatusInternalServerError)
-			}
+			returnK8sError(err, w)
 		}
 	}
 }
 
 // GetNamespaces return the list of namespaces
 func GetNamespaces(appRepo apprepo.Handler) func(w http.ResponseWriter, req *http.Request) {
 	return func(w http.ResponseWriter, req *http.Request) {
-		namespaces, err := appRepo.GetNamespaces(req)
+		token := auth.ExtractToken(req.Header.Get("Authorization"))
+		namespaces, err := appRepo.GetNamespaces(token)
 		if err != nil {
-			code := http.StatusInternalServerError
-			if k8sErrors.IsForbidden(err) {
-				code = http.StatusForbidden
-			}
-			http.Error(w, err.Error(), code)
+			returnK8sError(err, w)
 		}
 		response := namespacesResponse{
 			Namespaces: namespaces,