Fix issues with namespace usage

The update handler was trying to read the namespace from a
query-string, but clients pass this in the request body. The
handler was changed to match the approach of the create handler.

Tested e2e with Kubernetes 1.14.1

Additionally the delete handler was writing HTTP headers more than
once in some conditions. This has also been updated.

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <[email protected]>

Author: alexellis

handlers/delete.go

@@ -5,6 +5,7 @@ package handlers
 
 import (
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"net/http"
 
@@ -31,6 +32,7 @@ func MakeDeleteHandler(defaultNamespace string, clientset *kubernetes.Clientset)
 
 		if lookupNamespace == "kube-system" {
 			http.Error(w, "unable to list within the kube-system namespace", http.StatusUnauthorized)
+			return
 		}
 
 		body, _ := ioutil.ReadAll(r.Body)
@@ -66,7 +68,10 @@ func MakeDeleteHandler(defaultNamespace string, clientset *kubernetes.Clientset)
 		}
 
 		if isFunction(deployment) {
-			deleteFunction(lookupNamespace, clientset, request, w)
+			err := deleteFunction(lookupNamespace, clientset, request, w)
+			if err != nil {
+				return
+			}
 		} else {
 			w.WriteHeader(http.StatusBadRequest)
 
@@ -88,7 +93,7 @@ func isFunction(deployment *appsv1.Deployment) bool {
 	return false
 }
 
-func deleteFunction(functionNamespace string, clientset *kubernetes.Clientset, request requests.DeleteFunctionRequest, w http.ResponseWriter) {
+func deleteFunction(functionNamespace string, clientset *kubernetes.Clientset, request requests.DeleteFunctionRequest, w http.ResponseWriter) error {
 	foregroundPolicy := metav1.DeletePropagationForeground
 	opts := &metav1.DeleteOptions{PropagationPolicy: &foregroundPolicy}
 
@@ -101,7 +106,7 @@ func deleteFunction(functionNamespace string, clientset *kubernetes.Clientset, r
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 		w.Write([]byte(deployErr.Error()))
-		return
+		return fmt.Errorf("error deleting function's deployment")
 	}
 
 	if svcErr := clientset.CoreV1().
@@ -115,6 +120,7 @@ func deleteFunction(functionNamespace string, clientset *kubernetes.Clientset, r
 		}
 
 		w.Write([]byte(svcErr.Error()))
-		return
+		return fmt.Errorf("error deleting function's service")
 	}
+	return nil
 }

handlers/deploy.go

@@ -30,7 +30,10 @@ const initialReplicasCount = 1
 // MakeDeployHandler creates a handler to create new functions in the cluster
 func MakeDeployHandler(functionNamespace string, factory k8s.FunctionFactory) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		defer r.Body.Close()
+
+		if r.Body != nil {
+			defer r.Body.Close()
+		}
 
 		body, _ := ioutil.ReadAll(r.Body)
 

handlers/reader.go

@@ -19,6 +19,7 @@ import (
 // MakeFunctionReader handler for reading functions deployed in the cluster as deployments.
 func MakeFunctionReader(defaultNamespace string, clientset *kubernetes.Clientset) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
+
 		q := r.URL.Query()
 		namespace := q.Get("namespace")
 
@@ -30,6 +31,7 @@ func MakeFunctionReader(defaultNamespace string, clientset *kubernetes.Clientset
 
 		if lookupNamespace == "kube-system" {
 			http.Error(w, "unable to list within the kube-system namespace", http.StatusUnauthorized)
+			return
 		}
 
 		functions, err := getServiceList(lookupNamespace, clientset)

handlers/update.go

@@ -20,20 +20,8 @@ import (
 // MakeUpdateHandler update specified function
 func MakeUpdateHandler(defaultNamespace string, factory k8s.FunctionFactory) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		defer r.Body.Close()
-
-		q := r.URL.Query()
-		namespace := q.Get("namespace")
-
-		lookupNamespace := defaultNamespace
-
-		if len(namespace) > 0 {
-			lookupNamespace = namespace
-		}
-
-		if lookupNamespace == "kube-system" {
-			http.Error(w, "unable to list within the kube-system namespace", http.StatusUnauthorized)
-			return
+		if r.Body != nil {
+			defer r.Body.Close()
 		}
 
 		body, _ := ioutil.ReadAll(r.Body)
@@ -46,25 +34,35 @@ func MakeUpdateHandler(defaultNamespace string, factory k8s.FunctionFactory) htt
 			return
 		}
 
+		lookupNamespace := defaultNamespace
+		if len(request.Namespace) > 0 {
+			lookupNamespace = request.Namespace
+		}
+
+		if lookupNamespace == "kube-system" {
+			http.Error(w, "unable to list within the kube-system namespace", http.StatusUnauthorized)
+			return
+		}
+
 		annotations := buildAnnotations(request)
 		if err, status := updateDeploymentSpec(lookupNamespace, factory, request, annotations); err != nil {
 			if !isNotFound(err) {
-				log.Printf("error updating deployment: %s\n", err)
+				log.Printf("error updating deployment: %s.%s, error: %s\n", request.Service, lookupNamespace, err)
 
 				return
 			}
 
-			wrappedErr := fmt.Errorf("unable update Deployment: %s", err.Error())
+			wrappedErr := fmt.Errorf("unable update Deployment: %s.%s, error: %s", request.Service, lookupNamespace, err.Error())
 			http.Error(w, wrappedErr.Error(), status)
 			return
 		}
 
 		if err, status := updateService(lookupNamespace, factory, request, annotations); err != nil {
 			if !isNotFound(err) {
-				log.Printf("error updating service: %s\n", err)
+				log.Printf("error updating service: %s.%s, error: %s\n", request.Service, lookupNamespace, err)
 			}
 
-			wrappedErr := fmt.Errorf("unable update Service: %s", err.Error())
+			wrappedErr := fmt.Errorf("unable update Service: %s.%s, error: %s", request.Service, request.Namespace, err.Error())
 			http.Error(w, wrappedErr.Error(), status)
 			return
 		}