Update messages to want/got for unit tests

This is the style used in the openfaas project.

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>

cmd/install.go

@@ -109,7 +109,16 @@ func binExists(folder, name string) error {
 	}
 	return nil
 }
+func ensureSecretsDir(folder string) error {
+	if _, err := os.Stat(folder); err != nil {
+		err = os.MkdirAll(folder, secretDirPermission)
+		if err != nil {
+			return err
+		}
+	}
 
+	return nil
+}
 func ensureWorkingDir(folder string) error {
 	if _, err := os.Stat(folder); err != nil {
 		err = os.MkdirAll(folder, workingDirectoryPermission)

cmd/provider.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"net/http"
@@ -21,6 +22,8 @@ import (
 	"github.com/spf13/cobra"
 )
 
+const secretDirPermission = 0755
+
 func makeProviderCmd() *cobra.Command {
 	var command = &cobra.Command{
 		Use:   "provider",
@@ -82,25 +85,25 @@ func makeProviderCmd() *cobra.Command {
 
 		invokeResolver := handlers.NewInvokeResolver(client)
 
-		userSecretPath := path.Join(wd, "secrets")
-
-		err = moveSecretsToDefaultNamespaceSecrets(userSecretPath, faasd.FunctionNamespace)
-		if err != nil {
+		baseUserSecretsPath := path.Join(wd, "secrets")
+		if err := moveSecretsToDefaultNamespaceSecrets(
+			baseUserSecretsPath,
+			faasd.DefaultFunctionNamespace); err != nil {
 			return err
 		}
 
 		bootstrapHandlers := types.FaaSHandlers{
 			FunctionProxy:        proxy.NewHandlerFunc(*config, invokeResolver),
 			DeleteHandler:        handlers.MakeDeleteHandler(client, cni),
-			DeployHandler:        handlers.MakeDeployHandler(client, cni, userSecretPath, alwaysPull),
+			DeployHandler:        handlers.MakeDeployHandler(client, cni, baseUserSecretsPath, alwaysPull),
 			FunctionReader:       handlers.MakeReadHandler(client),
 			ReplicaReader:        handlers.MakeReplicaReaderHandler(client),
 			ReplicaUpdater:       handlers.MakeReplicaUpdateHandler(client, cni),
-			UpdateHandler:        handlers.MakeUpdateHandler(client, cni, userSecretPath, alwaysPull),
+			UpdateHandler:        handlers.MakeUpdateHandler(client, cni, baseUserSecretsPath, alwaysPull),
 			HealthHandler:        func(w http.ResponseWriter, r *http.Request) {},
 			InfoHandler:          handlers.MakeInfoHandler(Version, GitCommit),
 			ListNamespaceHandler: handlers.MakeNamespacesLister(client),
-			SecretHandler:        handlers.MakeSecretHandler(client, userSecretPath),
+			SecretHandler:        handlers.MakeSecretHandler(client, baseUserSecretsPath),
 			LogHandler:           logs.NewLogHandlerFunc(faasdlogs.New(), config.ReadTimeout),
 		}
 
@@ -116,29 +119,58 @@ func makeProviderCmd() *cobra.Command {
 * Mutiple namespace support was added after release 0.13.0
 * Function will help users to migrate on multiple namespace support of faasd
  */
-func moveSecretsToDefaultNamespaceSecrets(secretPath string, namespace string) error {
-	newSecretPath := path.Join(secretPath, namespace)
+func moveSecretsToDefaultNamespaceSecrets(baseSecretPath string, defaultNamespace string) error {
+	newSecretPath := path.Join(baseSecretPath, defaultNamespace)
 
-	err := ensureWorkingDir(newSecretPath)
+	err := ensureSecretsDir(newSecretPath)
 	if err != nil {
 		return err
 	}
 
-	files, err := ioutil.ReadDir(secretPath)
+	files, err := ioutil.ReadDir(baseSecretPath)
 	if err != nil {
 		return err
 	}
 
 	for _, f := range files {
 		if !f.IsDir() {
-			oldPath := path.Join(secretPath, f.Name())
+
 			newPath := path.Join(newSecretPath, f.Name())
-			err = os.Rename(oldPath, newPath)
-			if err != nil {
-				return err
+
+			// A non-nil error means the file wasn't found in the
+			// destination path
+			if _, err := os.Stat(newPath); err != nil {
+				oldPath := path.Join(baseSecretPath, f.Name())
+
+				if err := copyFile(oldPath, newPath); err != nil {
+					return err
+				}
+
+				log.Printf("[Migration] Copied %s to %s", oldPath, newPath)
 			}
 		}
 	}
 
 	return nil
 }
+
+func copyFile(src, dst string) error {
+	inputFile, err := os.Open(src)
+	if err != nil {
+		return fmt.Errorf("opening %s failed %w", src, err)
+	}
+	defer inputFile.Close()
+
+	outputFile, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_APPEND, secretDirPermission)
+	if err != nil {
+		return fmt.Errorf("opening %s failed %w", dst, err)
+	}
+	defer outputFile.Close()
+
+	// Changed from os.Rename due to issue in #201
+	if _, err := io.Copy(outputFile, inputFile); err != nil {
+		return fmt.Errorf("writing into %s failed %w", outputFile.Name(), err)
+	}
+
+	return nil
+}

pkg/constants.go

@@ -1,8 +1,11 @@
 package pkg
 
 const (
-	// FunctionNamespace is the default containerd namespace functions are created
-	FunctionNamespace = "openfaas-fn"
+	// DefaultFunctionNamespace is the default containerd namespace functions are created
+	DefaultFunctionNamespace = "openfaas-fn"
+
+	// NamespaceLabel indicates that a namespace is managed by faasd
+	NamespaceLabel = "openfaas"
 
 	// FaasdNamespace is the containerd namespace services are created
 	FaasdNamespace = "openfaas"

pkg/logs/requestor.go

@@ -71,7 +71,7 @@ func buildCmd(ctx context.Context, req logs.Request) *exec.Cmd {
 
 	namespace := req.Namespace
 	if namespace == "" {
-		namespace = faasd.FunctionNamespace
+		namespace = faasd.DefaultFunctionNamespace
 	}
 
 	// find the description of the fields here

pkg/provider/handlers/delete.go

@@ -42,6 +42,18 @@ func MakeDeleteHandler(client *containerd.Client, cni gocni.CNI) func(w http.Res
 
 		lookupNamespace := getRequestNamespace(readNamespaceFromQuery(r))
 
+		// Check if namespace exists, and it has the openfaas label
+		valid, err := validNamespace(client, lookupNamespace)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		if !valid {
+			http.Error(w, "namespace not valid", http.StatusBadRequest)
+			return
+		}
+
 		name := req.FunctionName
 
 		function, err := GetFunction(client, name, lookupNamespace)

pkg/provider/handlers/deploy.go

@@ -52,10 +52,25 @@ func MakeDeployHandler(client *containerd.Client, cni gocni.CNI, secretMountPath
 		}
 
 		namespace := getRequestNamespace(req.Namespace)
+
+		// Check if namespace exists, and it has the openfaas label
+		valid, err := validNamespace(client, namespace)
+
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		if !valid {
+			http.Error(w, "namespace not valid", http.StatusBadRequest)
+			return
+		}
+
 		namespaceSecretMountPath := getNamespaceSecretMountPath(secretMountPath, namespace)
 		err = validateSecrets(namespaceSecretMountPath, req.Secrets)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
 		}
 
 		name := req.Service
@@ -111,7 +126,7 @@ func deploy(ctx context.Context, req types.FunctionDeployment, client *container
 	}
 
 	envs := prepareEnv(req.EnvProcess, req.EnvVars)
-	mounts := getMounts()
+	mounts := getOSMounts()
 
 	for _, secret := range req.Secrets {
 		mounts = append(mounts, specs.Mount{
@@ -126,7 +141,7 @@ func deploy(ctx context.Context, req types.FunctionDeployment, client *container
 
 	labels, err := buildLabels(&req)
 	if err != nil {
-		return fmt.Errorf("Unable to apply labels to conatiner: %s, error: %s", name, err)
+		return fmt.Errorf("unable to apply labels to container: %s, error: %w", name, err)
 	}
 
 	var memory *specs.LinuxMemory
@@ -157,7 +172,7 @@ func deploy(ctx context.Context, req types.FunctionDeployment, client *container
 	)
 
 	if err != nil {
-		return fmt.Errorf("unable to create container: %s, error: %s", name, err)
+		return fmt.Errorf("unable to create container: %s, error: %w", name, err)
 	}
 
 	return createTask(ctx, client, container, cni)
@@ -195,7 +210,7 @@ func createTask(ctx context.Context, client *containerd.Client, container contai
 	task, taskErr := container.NewTask(ctx, cio.BinaryIO("/usr/local/bin/faasd", nil))
 
 	if taskErr != nil {
-		return fmt.Errorf("unable to start task: %s, error: %s", name, taskErr)
+		return fmt.Errorf("unable to start task: %s, error: %w", name, taskErr)
 	}
 
 	log.Printf("Container ID: %s\tTask ID %s:\tTask PID: %d\t\n", name, task.ID(), task.Pid())
@@ -247,7 +262,9 @@ func prepareEnv(envProcess string, reqEnvVars map[string]string) []string {
 	return envs
 }
 
-func getMounts() []specs.Mount {
+// getOSMounts provides a mount for os-specific files such
+// as the hosts file and resolv.conf
+func getOSMounts() []specs.Mount {
 	// Prior to hosts_dir env-var, this value was set to
 	// os.Getwd()
 	hostsDir := "/var/lib/faasd"

pkg/provider/handlers/deploy_test.go

@@ -53,7 +53,7 @@ func Test_BuildLabels_WithAnnotations(t *testing.T) {
 			}
 
 			if !reflect.DeepEqual(val, tc.result) {
-				t.Errorf("Got: %s, expected %s", val, tc.result)
+				t.Errorf("Want: %s, got: %s", val, tc.result)
 			}
 		})
 	}

pkg/provider/handlers/functions.go

@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"strings"
@@ -11,6 +12,7 @@ import (
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/namespaces"
+	"github.com/openfaas/faasd/pkg"
 	faasd "github.com/openfaas/faasd/pkg"
 	"github.com/openfaas/faasd/pkg/cninetwork"
 )
@@ -32,6 +34,17 @@ type Function struct {
 
 // ListFunctions returns a map of all functions with running tasks on namespace
 func ListFunctions(client *containerd.Client, namespace string) (map[string]*Function, error) {
+
+	// Check if namespace exists, and it has the openfaas label
+	valid, err := validNamespace(client, namespace)
+	if err != nil {
+		return nil, err
+	}
+
+	if !valid {
+		return nil, errors.New("namespace not valid")
+	}
+
 	ctx := namespaces.WithNamespace(context.Background(), namespace)
 	functions := make(map[string]*Function)
 
@@ -60,7 +73,7 @@ func GetFunction(client *containerd.Client, name string, namespace string) (Func
 
 	c, err := client.LoadContainer(ctx, name)
 	if err != nil {
-		return Function{}, fmt.Errorf("unable to find function: %s, error %s", name, err)
+		return Function{}, fmt.Errorf("unable to find function: %s, error %w", name, err)
 	}
 
 	image, err := c.Image(ctx)
@@ -72,19 +85,19 @@ func GetFunction(client *containerd.Client, name string, namespace string) (Func
 	allLabels, labelErr := c.Labels(ctx)
 
 	if labelErr != nil {
-		log.Printf("cannot list container %s labels: %s", containerName, labelErr.Error())
+		log.Printf("cannot list container %s labels: %s", containerName, labelErr)
 	}
 
 	labels, annotations := buildLabelsAndAnnotations(allLabels)
 
 	spec, err := c.Spec(ctx)
 	if err != nil {
-		return Function{}, fmt.Errorf("unable to load function spec for reading secrets: %s, error %s", name, err)
+		return Function{}, fmt.Errorf("unable to load function spec for reading secrets: %s, error %w", name, err)
 	}
 
 	info, err := c.Info(ctx)
 	if err != nil {
-		return Function{}, fmt.Errorf("can't load info for: %s, error %s", name, err)
+		return Function{}, fmt.Errorf("can't load info for: %s, error %w", name, err)
 	}
 
 	envVars, envProcess := readEnvFromProcessEnv(spec.Process.Env)
@@ -106,7 +119,7 @@ func GetFunction(client *containerd.Client, name string, namespace string) (Func
 		// Task for container exists
 		svc, err := task.Status(ctx)
 		if err != nil {
-			return Function{}, fmt.Errorf("unable to get task status for container: %s %s", name, err)
+			return Function{}, fmt.Errorf("unable to get task status for container: %s %w", name, err)
 		}
 
 		if svc.Status == "running" {
@@ -187,7 +200,7 @@ func ListNamespaces(client *containerd.Client) []string {
 	namespaces, err := store.List(context.Background())
 	if err != nil {
 		log.Printf("Error listing namespaces: %s", err.Error())
-		set = append(set, faasd.FunctionNamespace)
+		set = append(set, faasd.DefaultFunctionNamespace)
 		return set
 	}
 
@@ -198,12 +211,12 @@ func ListNamespaces(client *containerd.Client) []string {
 			continue
 		}
 
-		if _, found := labels["openfaas"]; found {
+		if _, found := labels[pkg.NamespaceLabel]; found {
 			set = append(set, namespace)
 		}
 
-		if !findNamespace(faasd.FunctionNamespace, set) {
-			set = append(set, faasd.FunctionNamespace)
+		if !findNamespace(faasd.DefaultFunctionNamespace, set) {
+			set = append(set, faasd.DefaultFunctionNamespace)
 		}
 	}
 

pkg/provider/handlers/invoke_resolver.go

@@ -24,7 +24,7 @@ func (i *InvokeResolver) Resolve(functionName string) (url.URL, error) {
 	actualFunctionName := functionName
 	log.Printf("Resolve: %q\n", actualFunctionName)
 
-	namespace := getNamespace(functionName, faasd.FunctionNamespace)
+	namespace := getNamespace(functionName, faasd.DefaultFunctionNamespace)
 
 	if strings.Contains(functionName, ".") {
 		actualFunctionName = strings.TrimSuffix(functionName, "."+namespace)

pkg/provider/handlers/read.go

@@ -14,6 +14,17 @@ func MakeReadHandler(client *containerd.Client) func(w http.ResponseWriter, r *h
 	return func(w http.ResponseWriter, r *http.Request) {
 
 		lookupNamespace := getRequestNamespace(readNamespaceFromQuery(r))
+		// Check if namespace exists, and it has the openfaas label
+		valid, err := validNamespace(client, lookupNamespace)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		if !valid {
+			http.Error(w, "namespace not valid", http.StatusBadRequest)
+			return
+		}
 
 		res := []types.FunctionStatus{}
 		fns, err := ListFunctions(client, lookupNamespace)

pkg/provider/handlers/replicas.go

@@ -16,6 +16,18 @@ func MakeReplicaReaderHandler(client *containerd.Client) func(w http.ResponseWri
 		functionName := vars["name"]
 		lookupNamespace := getRequestNamespace(readNamespaceFromQuery(r))
 
+		// Check if namespace exists, and it has the openfaas label
+		valid, err := validNamespace(client, lookupNamespace)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		if !valid {
+			http.Error(w, "namespace not valid", http.StatusBadRequest)
+			return
+		}
+
 		if f, err := GetFunction(client, functionName, lookupNamespace); err == nil {
 			found := types.FunctionStatus{
 				Name:              functionName,

pkg/provider/handlers/scale.go

@@ -41,6 +41,18 @@ func MakeReplicaUpdateHandler(client *containerd.Client, cni gocni.CNI) func(w h
 
 		namespace := getRequestNamespace(readNamespaceFromQuery(r))
 
+		// Check if namespace exists, and it has the openfaas label
+		valid, err := validNamespace(client, namespace)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		if !valid {
+			http.Error(w, "namespace not valid", http.StatusBadRequest)
+			return
+		}
+
 		name := req.ServiceName
 
 		if _, err := GetFunction(client, name, namespace); err != nil {

pkg/provider/handlers/secret.go

@@ -49,6 +49,18 @@ func MakeSecretHandler(c *containerd.Client, mountPath string) func(w http.Respo
 func listSecrets(c *containerd.Client, w http.ResponseWriter, r *http.Request, mountPath string) {
 
 	lookupNamespace := getRequestNamespace(readNamespaceFromQuery(r))
+	// Check if namespace exists, and it has the openfaas label
+	valid, err := validNamespace(c, lookupNamespace)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+
+	if !valid {
+		http.Error(w, "namespace not valid", http.StatusBadRequest)
+		return
+	}
+
 	mountPath = getNamespaceSecretMountPath(mountPath, lookupNamespace)
 
 	files, err := ioutil.ReadDir(mountPath)

pkg/provider/handlers/update.go

@@ -41,6 +41,19 @@ func MakeUpdateHandler(client *containerd.Client, cni gocni.CNI, secretMountPath
 		}
 		name := req.Service
 		namespace := getRequestNamespace(req.Namespace)
+
+		// Check if namespace exists, and it has the openfaas label
+		valid, err := validNamespace(client, namespace)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		if !valid {
+			http.Error(w, "namespace not valid", http.StatusBadRequest)
+			return
+		}
+
 		namespaceSecretMountPath := getNamespaceSecretMountPath(secretMountPath, namespace)
 
 		function, err := GetFunction(client, name, namespace)

pkg/provider/handlers/utils.go

@@ -1,9 +1,13 @@
 package handlers
 
 import (
+	"context"
 	"net/http"
 	"path"
 
+	"github.com/containerd/containerd"
+
+	"github.com/openfaas/faasd/pkg"
 	faasd "github.com/openfaas/faasd/pkg"
 )
 
@@ -12,7 +16,7 @@ func getRequestNamespace(namespace string) string {
 	if len(namespace) > 0 {
 		return namespace
 	}
-	return faasd.FunctionNamespace
+	return faasd.DefaultFunctionNamespace
 }
 
 func readNamespaceFromQuery(r *http.Request) string {
@@ -23,3 +27,23 @@ func readNamespaceFromQuery(r *http.Request) string {
 func getNamespaceSecretMountPath(userSecretPath string, namespace string) string {
 	return path.Join(userSecretPath, namespace)
 }
+
+// validNamespace indicates whether the namespace is eligable to be
+// used for OpenFaaS functions.
+func validNamespace(client *containerd.Client, namespace string) (bool, error) {
+	if namespace == faasd.DefaultFunctionNamespace {
+		return true, nil
+	}
+
+	store := client.NamespaceService()
+	labels, err := store.Labels(context.Background(), namespace)
+	if err != nil {
+		return false, err
+	}
+
+	if value, found := labels[pkg.NamespaceLabel]; found && value == "true" {
+		return true, nil
+	}
+
+	return false, nil
+}

pkg/provider/handlers/utils_test.go

@@ -15,7 +15,7 @@ func Test_getRequestNamespace(t *testing.T) {
 		requestNamespace  string
 		expectedNamespace string
 	}{
-		{name: "RequestNamespace is not provided", requestNamespace: "", expectedNamespace: faasd.FunctionNamespace},
+		{name: "RequestNamespace is not provided", requestNamespace: "", expectedNamespace: faasd.DefaultFunctionNamespace},
 		{name: "RequestNamespace is provided", requestNamespace: "user-namespace", expectedNamespace: "user-namespace"},
 	}
 
@@ -23,7 +23,7 @@ func Test_getRequestNamespace(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			actualNamespace := getRequestNamespace(tc.requestNamespace)
 			if actualNamespace != tc.expectedNamespace {
-				t.Errorf("Got: %s, expected %s", actualNamespace, tc.expectedNamespace)
+				t.Errorf("Want: %s, got: %s", actualNamespace, tc.expectedNamespace)
 			}
 		})
 	}
@@ -36,7 +36,7 @@ func Test_getNamespaceSecretMountPath(t *testing.T) {
 		requestNamespace   string
 		expectedSecretPath string
 	}{
-		{name: "Default Namespace is provided", requestNamespace: faasd.FunctionNamespace, expectedSecretPath: "/var/openfaas/secrets/" + faasd.FunctionNamespace},
+		{name: "Default Namespace is provided", requestNamespace: faasd.DefaultFunctionNamespace, expectedSecretPath: "/var/openfaas/secrets/" + faasd.DefaultFunctionNamespace},
 		{name: "User Namespace is provided", requestNamespace: "user-namespace", expectedSecretPath: "/var/openfaas/secrets/user-namespace"},
 	}
 
@@ -44,7 +44,7 @@ func Test_getNamespaceSecretMountPath(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			actualNamespace := getNamespaceSecretMountPath(userSecretPath, tc.requestNamespace)
 			if actualNamespace != tc.expectedSecretPath {
-				t.Errorf("Got: %s, expected %s", actualNamespace, tc.expectedSecretPath)
+				t.Errorf("Want: %s, got: %s", actualNamespace, tc.expectedSecretPath)
 			}
 		})
 	}
@@ -68,7 +68,7 @@ func Test_readNamespaceFromQuery(t *testing.T) {
 
 			actualNamespace := readNamespaceFromQuery(r)
 			if actualNamespace != tc.expectedNamespace {
-				t.Errorf("Got: %s, expected %s", actualNamespace, tc.expectedNamespace)
+				t.Errorf("Want: %s, got: %s", actualNamespace, tc.expectedNamespace)
 			}
 		})
 	}

pkg/service/service.go

@@ -33,7 +33,7 @@ func Remove(ctx context.Context, client *containerd.Client, name string) error {
 			if errdefs.IsNotFound(err) {
 				taskFound = false
 			} else {
-				return fmt.Errorf("unable to get task %s: ", err)
+				return fmt.Errorf("unable to get task %w: ", err)
 			}
 		}
 
@@ -47,12 +47,12 @@ func Remove(ctx context.Context, client *containerd.Client, name string) error {
 
 			log.Printf("Need to kill task: %s\n", name)
 			if err = killTask(ctx, t); err != nil {
-				return fmt.Errorf("error killing task %s, %s, %s", container.ID(), name, err)
+				return fmt.Errorf("error killing task %s, %s, %w", container.ID(), name, err)
 			}
 		}
 
 		if err := container.Delete(ctx, containerd.WithSnapshotCleanup); err != nil {
-			return fmt.Errorf("error deleting container %s, %s, %s", container.ID(), name, err)
+			return fmt.Errorf("error deleting container %s, %s, %w", container.ID(), name, err)
 		}
 
 	} else {
@@ -79,9 +79,10 @@ func killTask(ctx context.Context, task containerd.Task) error {
 		if task != nil {
 			wait, err := task.Wait(ctx)
 			if err != nil {
-				err = fmt.Errorf("error waiting on task: %s", err)
+				log.Printf("error waiting on task: %s", err)
 				return
 			}
+
 			if err := task.Kill(ctx, unix.SIGTERM, containerd.WithKillAll); err != nil {
 				log.Printf("error killing container task: %s", err)
 			}