Fix for #201

Old secrets are now copied, rather than moved, so that any
existing functions do not need to be redeployed by the user.

As a maintenance task, users should remove the older secrets.

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.FunctionNamespace); 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/provider/handlers/deploy.go

@@ -111,7 +111,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 +126,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 +157,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 +195,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 +247,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/functions.go

@@ -60,7 +60,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 +72,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 +106,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" {

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)
 			}