feat: temporarily remount read-only mounts before cleaning container fs

envbuilder.go

@@ -36,6 +36,7 @@ import (
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/envbuilder/devcontainer"
+	"github.com/coder/envbuilder/internal/ebutil"
 	"github.com/containerd/containerd/platforms"
 	"github.com/distribution/distribution/v3/configuration"
 	"github.com/distribution/distribution/v3/registry/handlers"
@@ -401,6 +402,19 @@ func Run(ctx context.Context, options Options) error {
 		})
 	}
 
+	// temp move of all ro mounts
+	tempRemountDest := filepath.Join("/", MagicDir, "mnt")
+	ignorePrefixes := []string{tempRemountDest, "/proc", "/sys"}
+	restoreMounts, err := ebutil.TempRemount(options.Logger, tempRemountDest, ignorePrefixes...)
+	defer func() { // restoreMounts should never be nil
+		if err := restoreMounts(); err != nil {
+			options.Logger(codersdk.LogLevelError, "restore mounts: %s", err.Error())
+		}
+	}()
+	if err != nil {
+		return fmt.Errorf("temp remount: %w", err)
+	}
+
 	skippedRebuild := false
 	build := func() (v1.Image, error) {
 		_, err := options.Filesystem.Stat(MagicFile)
@@ -547,6 +561,10 @@ func Run(ctx context.Context, options Options) error {
 		closeAfterBuild()
 	}
 
+	if err := restoreMounts(); err != nil {
+		return fmt.Errorf("restore mounts: %w", err)
+	}
+
 	// Create the magic file to indicate that this build
 	// has already been ran before!
 	file, err := options.Filesystem.Create(MagicFile)

go.mod

@@ -30,14 +30,17 @@ require (
 	github.com/go-git/go-billy/v5 v5.5.0
 	github.com/go-git/go-git/v5 v5.12.0
 	github.com/google/go-containerregistry v0.15.2
+	github.com/hashicorp/go-multierror v1.1.1
 	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
 	github.com/mattn/go-isatty v0.0.20
 	github.com/moby/buildkit v0.11.6
 	github.com/otiai10/copy v1.14.0
+	github.com/prometheus/procfs v0.12.0
 	github.com/sirupsen/logrus v1.9.3
 	github.com/skeema/knownhosts v1.2.2
 	github.com/stretchr/testify v1.9.0
 	github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a
+	go.uber.org/mock v0.4.0
 	golang.org/x/crypto v0.22.0
 	golang.org/x/sync v0.7.0
 	golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028
@@ -155,7 +158,6 @@ require (
 	github.com/hashicorp/go-hclog v1.5.0 // indirect
 	github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
 	github.com/hashicorp/go-memdb v1.3.2 // indirect
-	github.com/hashicorp/go-multierror v1.1.1 // indirect
 	github.com/hashicorp/go-uuid v1.0.3 // indirect
 	github.com/hashicorp/go-version v1.6.0 // indirect
 	github.com/hashicorp/golang-lru v1.0.2 // indirect
@@ -221,7 +223,6 @@ require (
 	github.com/prometheus/client_golang v1.18.0 // indirect
 	github.com/prometheus/client_model v0.5.0 // indirect
 	github.com/prometheus/common v0.46.0 // indirect
-	github.com/prometheus/procfs v0.12.0 // indirect
 	github.com/richardartoul/molecule v1.0.1-0.20221107223329-32cfee06a052 // indirect
 	github.com/rivo/uniseg v0.4.4 // indirect
 	github.com/robfig/cron/v3 v3.0.1 // 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/ebutil/remount.go

@@ -0,0 +1,134 @@
+package ebutil
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+	"syscall"
+
+	"github.com/coder/coder/v2/codersdk"
+	"github.com/hashicorp/go-multierror"
+	"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.
+//
+// Some container runtimes such as sysbox-runc will mount in `/lib/modules` read-only.
+// See https://github.com/nestybox/sysbox/issues/564
+// This trips us up because:
+//  1. We call a Kaniko library function `util.DeleteFilesystem` that does exactly what it says
+//     on the tin. If this hits a read-only volume mounted in, unhappiness is the result.
+//  2. After deleting the filesystem and building the image, we extract it to the filesystem.
+//     If some paths mounted in via volume are present at that time, unhappiness is also likely
+//     to result -- especially in case of read-only mounts.
+//
+// To work around this we move the mounts out of the way temporarily by bind-mounting them
+// while we do our thing, and move them back when we're done.
+//
+// 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 function will make a best-effort attempt to restore
+// the original state.
+func TempRemount(logf func(codersdk.LogLevel, string, ...any), dest string, ignorePrefixes ...string) (restore func() error, err error,
+) {
+	return tempRemount(&realMounter{}, logf, dest, ignorePrefixes...)
+}
+
+func tempRemount(m mounter, logf func(codersdk.LogLevel, string, ...any), base string, ignorePrefixes ...string) (restore 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
+	var restoreOnce sync.Once
+	restore = func() error {
+		var merr error
+		restoreOnce.Do(func() {
+			for orig, moved := range mounts {
+				if err := remount(m, moved, orig); err != nil {
+					merr = multierror.Append(merr, fmt.Errorf("restore mount: %w", err))
+				}
+			}
+		})
+		return merr
+	}
+
+outer:
+	for _, mountInfo := range mountInfos {
+		// TODO: do this for all mounts
+		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
+		dest := filepath.Join("/", base, src)
+		if err := remount(m, src, dest); err != nil {
+			return restore, fmt.Errorf("temp remount: %w", err)
+		}
+
+		mounts[src] = dest
+	}
+
+	return restore, nil
+}
+
+func remount(m mounter, src, dest string) error {
+	if err := m.MkdirAll(dest, 0o750); err != nil {
+		return fmt.Errorf("ensure path: %w", err)
+	}
+	if err := m.Mount(src, dest, "bind", syscall.MS_BIND, ""); err != nil {
+		return fmt.Errorf("bind mount %s => %s: %w", src, dest, err)
+	}
+	if err := m.Unmount(src, 0); err != nil {
+		return fmt.Errorf("unmount orig src %s: %w", src, err)
+	}
+	return 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)
+}