refactor temp remount logic into util package, add unit tests

Author: johnstcn

envbuilder.go

@@ -26,6 +26,8 @@ import (
 	"syscall"
 	"time"
 
+	ebutil "github.com/coder/envbuilder/internal/util"
+
 	dcontext "github.com/distribution/distribution/v3/context"
 	"github.com/kballard/go-shellquote"
 	"github.com/mattn/go-isatty"
@@ -47,7 +49,6 @@ import (
 	"github.com/go-git/go-git/v5/plumbing/transport"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
-	"github.com/prometheus/procfs"
 	"github.com/sirupsen/logrus"
 	"github.com/tailscale/hujson"
 	"golang.org/x/xerrors"
@@ -396,58 +397,11 @@ func Run(ctx context.Context, options Options) error {
 	}
 
 	// temp move of all ro mounts
-	tempRemounts := map[string]string{}
-
-	mountInfos, err := procfs.GetMounts()
+	tempRemountDest := filepath.Join("/", MagicDir)
+	ignorePrefixes := []string{tempRemountDest, "/proc", "/sys"}
+	remount, err := ebutil.TempRemount(options.Logger, tempRemountDest, ignorePrefixes...)
 	if err != nil {
-		options.Logger(codersdk.LogLevelError, "Failed to read mountinfo: %s", err)
-	}
-
-	prefixes := []string{filepath.Join("/", MagicDir), "/proc", "/sys"}
-	for _, mountInfo := range mountInfos {
-		options.Logger(codersdk.LogLevelTrace, "Found mountpoint %s", mountInfo.MountPoint)
-		if _, ok := mountInfo.Options["ro"]; !ok {
-			continue
-		}
-		options.Logger(codersdk.LogLevelTrace, "Found ro mountpoint %s ", mountInfo.MountPoint)
-
-		relevantMountpoint := true
-		for _, prefix := range prefixes {
-			if strings.HasPrefix(mountInfo.MountPoint, prefix) {
-				options.Logger(codersdk.LogLevelTrace, "Mountpoint %s is NOT relevant (prefix: %s)", mountInfo.MountPoint, prefix)
-				relevantMountpoint = false
-				break
-			}
-		}
-		if !relevantMountpoint {
-			continue
-		}
-
-		src := mountInfo.MountPoint
-		tgt := filepath.Join("/", MagicDir, mountInfo.MountPoint)
-
-		options.Logger(codersdk.LogLevelTrace, "Mountpoint %s is relevant", src)
-
-		options.Logger(codersdk.LogLevelTrace, "Remount mountpoint %s to %s", src, tgt)
-
-		err := os.MkdirAll(tgt, 0750)
-		if err != nil {
-			options.Logger(codersdk.LogLevelError, "Could not create temp mountpoint %s for %s: %s", tgt, src, err.Error())
-			continue
-		}
-
-		err = syscall.Mount(src, tgt, "bind", syscall.MS_BIND, "")
-		if err != nil {
-			options.Logger(codersdk.LogLevelError, "Could not bindmount %s to %s: %s", src, tgt, err.Error())
-			continue
-		}
-		err = syscall.Unmount(src, 0)
-		if err != nil {
-			options.Logger(codersdk.LogLevelError, "Could not unmount %s: %s", src, err.Error())
-			continue
-		}
-
-		tempRemounts[src] = tgt
+		return fmt.Errorf("temp remount: %w", err)
 	}
 
 	skippedRebuild := false
@@ -596,25 +550,9 @@ func Run(ctx context.Context, options Options) error {
 		closeAfterBuild()
 	}
 
-	for src, tgt := range tempRemounts {
-		options.Logger(codersdk.LogLevelTrace, "Cleanup mountpoint %s", tgt)
-		err := os.MkdirAll(src, 0750)
-		if err != nil {
-			options.Logger(codersdk.LogLevelError, "Could not create mountpoint %s for %s: %w", src, tgt, err.Error())
-			continue
-		}
-
-		err = syscall.Mount(tgt, src, "bind", syscall.MS_BIND, "")
-		if err != nil {
-			options.Logger(codersdk.LogLevelError, "Could not bindmount %s to %s: %s", tgt, src, err.Error())
-			continue
-		}
-
-		err = syscall.Unmount(tgt, 0)
-		if err != nil {
-			options.Logger(codersdk.LogLevelError, "Could not unmount %s: %s", tgt, err.Error())
-			continue
-		}
+	if err := remount(); err != nil {
+		// Don't fail at this point as it may be useful to be able to inspect the build.
+		options.Logger(codersdk.LogLevelError, "recreate mountpoint: %w", err)
 	}
 
 	// Create the magic file to indicate that this build

go.mod

@@ -262,6 +262,7 @@ require (
 	go.opentelemetry.io/otel/trace v1.19.0 // indirect
 	go.opentelemetry.io/proto/otlp v1.0.0 // indirect
 	go.uber.org/atomic v1.11.0 // indirect
+	go.uber.org/mock v0.4.0 // indirect
 	go4.org/intern v0.0.0-20230525184215-6c62f75575cb // indirect
 	go4.org/mem v0.0.0-20220726221520-4f986261bf13 // indirect
 	go4.org/netipx v0.0.0-20230728180743-ad4cb58a6516 // indirect

go.sum

@@ -910,6 +910,8 @@ go.uber.org/atomic v1.11.0 h1:ZvwS0R+56ePWxUNi+Atn9dWONBPp/AUETXlHW0DxSjE=
 go.uber.org/atomic v1.11.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0=
 go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
 go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
+go.uber.org/mock v0.4.0 h1:VcM4ZOtdbR4f6VXfiOpwpVJDL6lCReaZ6mw31wqh7KU=
+go.uber.org/mock v0.4.0/go.mod h1:a6FSlNadKUHUa9IP5Vyt1zh4fC7uAwxMutEAscFbkZc=
 go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
 go.uber.org/zap v1.17.0/go.mod h1:MXVU+bhUf/A7Xi2HNOnopQOrmycQ5Ih87HtOu4q5SSo=
 go4.org/intern v0.0.0-20211027215823-ae77deb06f29/go.mod h1:cS2ma+47FKrLPdXFpr7CuxiTW3eyJbWew4qx0qtQWDA=

internal/util/mock_mounter_test.go

@@ -0,0 +1,120 @@
+package ebutil
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"syscall"
+
+	"github.com/coder/coder/v2/codersdk"
+	"github.com/prometheus/procfs"
+)
+
+// TempRemount iterates through all read-only mounted filesystems, bind-mounts them at dest,
+// and unmounts them from their original source. All mount points underneath ignorePrefixes
+// will not be touched.
+//
+// It is the responsibility of the caller to call the returned function
+// to restore the original mount points. If an error is encountered while attempting to perform
+// the operation, calling the returned remount function will make a best-effort attempt to
+// restore the original state.
+func TempRemount(logf func(codersdk.LogLevel, string, ...any), dest string, ignorePrefixes ...string) (remount func() error, err error,
+) {
+	return tempRemount(&realMounter{}, logf, dest, ignorePrefixes...)
+}
+
+func tempRemount(m mounter, logf func(codersdk.LogLevel, string, ...any), dest string, ignorePrefixes ...string) (remount func() error, err error) {
+	mountInfos, err := m.GetMounts()
+	if err != nil {
+		return func() error { return nil }, fmt.Errorf("get mounts: %w", err)
+	}
+
+	// temp move of all ro mounts
+	mounts := map[string]string{}
+	// closer to attempt to restore original mount points
+	remount = func() error {
+		for src, tgt := range mounts {
+			err := m.MkdirAll(src, 0750)
+			if err != nil {
+				return fmt.Errorf("recreate original mountpoint %s: %w", src, err)
+			}
+
+			err = m.Mount(tgt, src, "bind", syscall.MS_BIND, "")
+			if err != nil {
+				return fmt.Errorf("bind mount %s => %s: %w", tgt, src, err)
+			}
+
+			err = m.Unmount(tgt, 0)
+			if err != nil {
+				return fmt.Errorf("unmount temporary dest %s: %w", tgt, err)
+			}
+		}
+		return nil
+	}
+
+outer:
+	for _, mountInfo := range mountInfos {
+		if _, ok := mountInfo.Options["ro"]; !ok {
+			logf(codersdk.LogLevelTrace, "skip rw mount %s", mountInfo.MountPoint)
+			continue
+		}
+
+		for _, prefix := range ignorePrefixes {
+			if strings.HasPrefix(mountInfo.MountPoint, prefix) {
+				logf(codersdk.LogLevelTrace, "skip mount %s under ignored prefix %s", mountInfo.MountPoint, prefix)
+				continue outer
+			}
+		}
+
+		src := mountInfo.MountPoint
+		tgt := filepath.Join("/", dest, src)
+		err := m.MkdirAll(tgt, 0750)
+		if err != nil {
+			return remount, fmt.Errorf("create temp mountpoint %s: %w", dest, err)
+		}
+
+		err = m.Mount(src, tgt, "bind", syscall.MS_BIND, "")
+		if err != nil {
+			return remount, fmt.Errorf("bind mount %s => %s: %s", src, dest, err.Error())
+		}
+		err = m.Unmount(src, 0)
+		if err != nil {
+			return remount, fmt.Errorf("temp unmount src %s: %s", src, err.Error())
+		}
+
+		mounts[src] = tgt
+	}
+
+	return remount, nil
+}
+
+// mounter is an interface to system-level calls used by TempRemount.
+type mounter interface {
+	// GetMounts wraps procfs.GetMounts
+	GetMounts() ([]*procfs.MountInfo, error)
+	// MkdirAll wraps os.MkdirAll
+	MkdirAll(string, os.FileMode) error
+	// Mount wraps syscall.Mount
+	Mount(string, string, string, uintptr, string) error
+	// Unmount wraps syscall.Unmount
+	Unmount(string, int) error
+}
+
+// realMounter implements mounter and actually does the thing.
+type realMounter struct{}
+
+var _ mounter = &realMounter{}
+
+func (m *realMounter) Mount(src string, dest string, fstype string, flags uintptr, data string) error {
+	return syscall.Mount(src, dest, fstype, flags, data)
+}
+func (m *realMounter) Unmount(tgt string, flags int) error {
+	return syscall.Unmount(tgt, flags)
+}
+func (m *realMounter) GetMounts() ([]*procfs.MountInfo, error) {
+	return procfs.GetMounts()
+}
+func (m *realMounter) MkdirAll(path string, perm os.FileMode) error {
+	return os.MkdirAll(path, perm)
+}