bug: fix moving files across different filesystems (#444)

go rename's function does not support moving files across different
filesystems. implement our own 'move' function.

Author: ricardomaraschini

cmd/embedded-cluster/install.go

@@ -57,7 +57,7 @@ func runCommand(bin string, args ...string) (string, error) {
 func installAndEnableLocalArtifactMirror() error {
 	ourbin := defaults.PathToEmbeddedClusterBinary("local-artifact-mirror")
 	hstbin := defaults.LocalArtifactMirrorPath()
-	if err := os.Rename(ourbin, hstbin); err != nil {
+	if err := helpers.MoveFile(ourbin, hstbin); err != nil {
 		return fmt.Errorf("unable to move local artifact mirror binary: %w", err)
 	}
 	if err := goods.MaterializeLocalArtifactMirrorUnitFile(); err != nil {
@@ -258,7 +258,7 @@ func applyUnsupportedOverrides(c *cli.Context, cfg *k0sconfig.ClusterConfig) (*k
 func installK0s() error {
 	ourbin := defaults.PathToEmbeddedClusterBinary("k0s")
 	hstbin := defaults.K0sBinaryPath()
-	if err := os.Rename(ourbin, hstbin); err != nil {
+	if err := helpers.MoveFile(ourbin, hstbin); err != nil {
 		return fmt.Errorf("unable to move k0s binary: %w", err)
 	}
 	if _, err := runCommand(hstbin, config.InstallFlags()...); err != nil {

cmd/embedded-cluster/join.go

@@ -22,6 +22,7 @@ import (
 	"github.com/replicatedhq/embedded-cluster/pkg/config"
 	"github.com/replicatedhq/embedded-cluster/pkg/defaults"
 	"github.com/replicatedhq/embedded-cluster/pkg/goods"
+	"github.com/replicatedhq/embedded-cluster/pkg/helpers"
 	"github.com/replicatedhq/embedded-cluster/pkg/metrics"
 )
 
@@ -283,7 +284,7 @@ func saveTokenToDisk(token string) error {
 func installK0sBinary() error {
 	ourbin := defaults.PathToEmbeddedClusterBinary("k0s")
 	hstbin := defaults.K0sBinaryPath()
-	if err := os.Rename(ourbin, hstbin); err != nil {
+	if err := helpers.MoveFile(ourbin, hstbin); err != nil {
 		return fmt.Errorf("unable to move k0s binary: %w", err)
 	}
 	return nil

pkg/helpers/fs.go

@@ -0,0 +1,47 @@
+package helpers
+
+import (
+	"fmt"
+	"io"
+	"os"
+)
+
+// MoveFile moves a file from one location to another, overwriting the destination if it
+// exists. File mode is preserved.
+func MoveFile(src, dst string) error {
+	srcinfo, err := os.Stat(src)
+	if err != nil {
+		return fmt.Errorf("unable to stat %s: %s", src, err)
+	}
+
+	if srcinfo.IsDir() {
+		return fmt.Errorf("unable to move directory %s", src)
+	}
+
+	srcfp, err := os.Open(src)
+	if err != nil {
+		return fmt.Errorf("unable to open source file: %s", err)
+	}
+	defer srcfp.Close()
+
+	opts := os.O_CREATE | os.O_WRONLY | os.O_TRUNC
+	dstfp, err := os.OpenFile(dst, opts, srcinfo.Mode())
+	if err != nil {
+		return fmt.Errorf("unable to open destination file: %s", err)
+	}
+	defer dstfp.Close()
+
+	if _, err := io.Copy(dstfp, srcfp); err != nil {
+		return fmt.Errorf("unable to copy file: %s", err)
+	}
+
+	if err := dstfp.Sync(); err != nil {
+		return fmt.Errorf("unable to sync file: %s", err)
+	}
+
+	if err := os.Remove(src); err != nil {
+		return fmt.Errorf("unable to remove source file: %s", err)
+	}
+
+	return nil
+}

pkg/helpers/fs_test.go

@@ -0,0 +1,87 @@
+package helpers
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMoveFile(t *testing.T) {
+	srcContent := []byte("test")
+	srcFile, err := os.CreateTemp("", "source-*")
+	assert.NoError(t, err)
+	defer os.Remove(srcFile.Name())
+	defer srcFile.Close()
+
+	_, err = srcFile.Write(srcContent)
+	assert.NoError(t, err)
+
+	dstFile, err := os.CreateTemp("", "destination-*")
+	assert.NoError(t, err)
+	defer os.Remove(dstFile.Name())
+	defer dstFile.Close()
+
+	err = MoveFile(srcFile.Name(), dstFile.Name())
+	assert.NoError(t, err)
+
+	_, err = os.Stat(dstFile.Name())
+	assert.NoError(t, err)
+
+	content, err := os.ReadFile(dstFile.Name())
+	assert.NoError(t, err)
+	assert.Equal(t, srcContent, content)
+}
+
+func TestMoveFile_PreserveMode(t *testing.T) {
+	srcContent := []byte("test")
+	srcFile, err := os.CreateTemp("", "source-*")
+	assert.NoError(t, err)
+	defer os.Remove(srcFile.Name())
+	defer srcFile.Close()
+
+	_, err = srcFile.Write(srcContent)
+	assert.NoError(t, err)
+
+	err = os.Chmod(srcFile.Name(), 0755)
+	assert.NoError(t, err)
+
+	defer os.Remove("/tmp/testbin")
+	err = MoveFile(srcFile.Name(), "/tmp/testbin")
+	assert.NoError(t, err)
+
+	info, err := os.Stat("/tmp/testbin")
+	assert.NoError(t, err)
+	assert.Equal(t, os.FileMode(0755), info.Mode())
+}
+
+func TestMoveFile_Directory(t *testing.T) {
+	srcDir, err := os.MkdirTemp("", "sourcedir-*")
+	assert.NoError(t, err)
+	defer os.RemoveAll(srcDir)
+	err = MoveFile(srcDir, "destination")
+	assert.Error(t, err)
+}
+
+func TestMoveFile_Symlink(t *testing.T) {
+	srcFile, err := os.CreateTemp("", "source-*")
+	assert.NoError(t, err)
+	_, err = srcFile.Write([]byte("test"))
+	assert.NoError(t, err)
+	defer os.Remove(srcFile.Name())
+	defer srcFile.Close()
+
+	symlinkPath := fmt.Sprintf("%s-symlink", srcFile.Name())
+	err = os.Symlink(srcFile.Name(), symlinkPath)
+	assert.NoError(t, err)
+	defer os.Remove(symlinkPath)
+
+	err = MoveFile(symlinkPath, "/tmp/destination")
+	assert.NoError(t, err)
+	defer os.RemoveAll("/tmp/destination")
+
+	target, err := os.Readlink(symlinkPath)
+	assert.Error(t, err)
+	assert.Empty(t, target)
+}