Deduplicate VMConfig, strengthen tests for /tmp size

Remove duplicate VMConfig struct and path constant from hooks package
by importing guest/vmconfig directly. Extract essentialMounts() from
Essential() so mount tests exercise real production code instead of
hand-constructed literals. Add table-driven tests for vmconfig.Read()
and hooks.InjectVMConfig. Improve WithTmpSize godoc.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: JAORMX

guest/mount/export_test.go

@@ -0,0 +1,9 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+//go:build linux
+
+package mount
+
+// EssentialMountsForTest exposes essentialMounts for testing.
+var EssentialMountsForTest = essentialMounts

guest/mount/mount.go

@@ -24,20 +24,26 @@ type mountEntry struct {
 	data   string
 }
 
-// Essential mounts the core filesystems required for a minimal Linux userspace.
-// tmpSizeMiB sets the size of the /tmp tmpfs in MiB; 0 uses the default (256 MiB).
-func Essential(logger *slog.Logger, tmpSizeMiB uint32) error {
+// essentialMounts returns the mount table for Essential, applying the default
+// tmp size when tmpSizeMiB is zero.
+func essentialMounts(tmpSizeMiB uint32) []mountEntry {
 	if tmpSizeMiB == 0 {
 		tmpSizeMiB = defaultTmpSizeMiB
 	}
-	mounts := []mountEntry{
+	return []mountEntry{
 		{"proc", "/proc", "proc", syscall.MS_NOSUID | syscall.MS_NODEV | syscall.MS_NOEXEC, ""},
 		{"sysfs", "/sys", "sysfs", syscall.MS_NOSUID | syscall.MS_NODEV | syscall.MS_NOEXEC, ""},
 		{"devtmpfs", "/dev", "devtmpfs", syscall.MS_NOSUID | syscall.MS_NOEXEC, ""},
 		{"devpts", "/dev/pts", "devpts", syscall.MS_NOSUID | syscall.MS_NOEXEC, "newinstance,ptmxmode=0666,mode=0620,gid=5"},
 		{"tmpfs", "/tmp", "tmpfs", syscall.MS_NOSUID | syscall.MS_NODEV, fmt.Sprintf("size=%dm", tmpSizeMiB)},
 		{"tmpfs", "/run", "tmpfs", syscall.MS_NOSUID | syscall.MS_NODEV | syscall.MS_NOEXEC, "size=64m"},
 	}
+}
+
+// Essential mounts the core filesystems required for a minimal Linux userspace.
+// tmpSizeMiB sets the size of the /tmp tmpfs in MiB; 0 uses the default (256 MiB).
+func Essential(logger *slog.Logger, tmpSizeMiB uint32) error {
+	mounts := essentialMounts(tmpSizeMiB)
 
 	for _, m := range mounts {
 		if err := os.MkdirAll(m.target, 0o755); err != nil {

guest/mount/mount_test.go

@@ -6,7 +6,6 @@
 package mount
 
 import (
-	"fmt"
 	"log/slog"
 	"os"
 	"testing"
@@ -32,31 +31,30 @@ func TestWorkspaceReturnsErrorForInvalidMount(t *testing.T) {
 	assert.Error(t, err)
 }
 
-func TestEssentialMountPoints(t *testing.T) {
+func TestEssentialMounts(t *testing.T) {
 	t.Parallel()
 
-	// Verify the mount table contains the expected entries.
-	expected := []string{"/proc", "/sys", "/dev", "/dev/pts", "/tmp", "/run"}
-	mounts := []mountEntry{
-		{"proc", "/proc", "proc", 0, ""},
-		{"sysfs", "/sys", "sysfs", 0, ""},
-		{"devtmpfs", "/dev", "devtmpfs", 0, ""},
-		{"devpts", "/dev/pts", "devpts", 0, "newinstance,ptmxmode=0666,mode=0620,gid=5"},
-		{"tmpfs", "/tmp", "tmpfs", 0, fmt.Sprintf("size=%dm", defaultTmpSizeMiB)},
-		{"tmpfs", "/run", "tmpfs", 0, "size=64m"},
-	}
-	for i, m := range mounts {
-		assert.Equal(t, expected[i], m.target)
-	}
-}
-
-func TestEssentialTmpSizeDefault(t *testing.T) {
-	t.Parallel()
-	if os.Getuid() == 0 {
-		t.Skip("test must run as non-root")
-	}
-	// Zero should be treated identically to the default size (no panic, same error path).
-	err0 := Essential(slog.Default(), 0)
-	errDef := Essential(slog.Default(), defaultTmpSizeMiB)
-	assert.Equal(t, err0 != nil, errDef != nil)
+	t.Run("default targets", func(t *testing.T) {
+		t.Parallel()
+		mounts := EssentialMountsForTest(0)
+		expected := []string{"/proc", "/sys", "/dev", "/dev/pts", "/tmp", "/run"}
+		targets := make([]string, len(mounts))
+		for i, m := range mounts {
+			targets[i] = m.target
+		}
+		assert.Equal(t, expected, targets)
+	})
+
+	t.Run("zero uses default tmp size", func(t *testing.T) {
+		t.Parallel()
+		mounts := EssentialMountsForTest(0)
+		// /tmp is the 5th entry.
+		assert.Equal(t, "size=256m", mounts[4].data)
+	})
+
+	t.Run("custom tmp size flows through", func(t *testing.T) {
+		t.Parallel()
+		mounts := EssentialMountsForTest(512)
+		assert.Equal(t, "size=512m", mounts[4].data)
+	})
 }

guest/vmconfig/export_test.go

@@ -0,0 +1,7 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package vmconfig
+
+// ReadFromForTest exposes readFrom for testing.
+var ReadFromForTest = readFrom

guest/vmconfig/vmconfig.go

@@ -10,8 +10,8 @@ import (
 	"os"
 )
 
-// configPath is the guest path where the host writes the VM config.
-const configPath = "/etc/propolis-vm.json"
+// GuestPath is the guest path where the host writes the VM config.
+const GuestPath = "/etc/propolis-vm.json"
 
 // Config holds settings written by the host and read by the guest init.
 // Zero values mean "use the built-in default" for each field.
@@ -25,7 +25,12 @@ type Config struct {
 // Returns a zero-value Config (all defaults) if the file does not exist,
 // ensuring backward compatibility with hosts that do not write the file.
 func Read() (Config, error) {
-	data, err := os.ReadFile(configPath)
+	return readFrom(GuestPath)
+}
+
+// readFrom loads the VM config from the given path.
+func readFrom(path string) (Config, error) {
+	data, err := os.ReadFile(path)
 	if err != nil {
 		if errors.Is(err, os.ErrNotExist) {
 			return Config{}, nil

guest/vmconfig/vmconfig_test.go

@@ -0,0 +1,68 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package vmconfig
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestReadFrom(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name    string
+		content *string // nil = file does not exist
+		want    Config
+		wantErr bool
+	}{
+		{
+			name:    "file does not exist",
+			content: nil,
+			want:    Config{},
+		},
+		{
+			name:    "valid JSON with TmpSizeMiB",
+			content: strPtr(`{"tmp_size_mib":512}`),
+			want:    Config{TmpSizeMiB: 512},
+		},
+		{
+			name:    "empty JSON object",
+			content: strPtr(`{}`),
+			want:    Config{},
+		},
+		{
+			name:    "malformed JSON",
+			content: strPtr(`{not json`),
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			dir := t.TempDir()
+			path := filepath.Join(dir, "vm.json")
+
+			if tt.content != nil {
+				require.NoError(t, os.WriteFile(path, []byte(*tt.content), 0o644))
+			}
+
+			got, err := ReadFromForTest(path)
+			if tt.wantErr {
+				assert.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func strPtr(s string) *string { return &s }

hooks/hooks.go

@@ -13,31 +13,22 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/stacklok/propolis/guest/vmconfig"
 	"github.com/stacklok/propolis/image"
 	"github.com/stacklok/propolis/internal/pathutil"
 	"github.com/stacklok/propolis/internal/xattr"
 )
 
-// vmConfigGuestPath is the guest path for the VM config file written by InjectVMConfig.
-const vmConfigGuestPath = "/etc/propolis-vm.json"
-
-// VMConfig holds settings written by the host into the rootfs and read by the
-// guest init before mounting filesystems. Fields use omitempty so the file is
-// only written when a non-default value is present.
-type VMConfig struct {
-	TmpSizeMiB uint32 `json:"tmp_size_mib,omitempty"`
-}
-
 // InjectVMConfig returns a RootFSHook that writes the given VM config as JSON
 // to /etc/propolis-vm.json inside the rootfs. The guest init reads this file
 // to configure mounts before the SSH server starts.
-func InjectVMConfig(cfg VMConfig) func(string, *image.OCIConfig) error {
+func InjectVMConfig(cfg vmconfig.Config) func(string, *image.OCIConfig) error {
 	return func(rootfsPath string, _ *image.OCIConfig) error {
 		data, err := json.Marshal(cfg)
 		if err != nil {
 			return fmt.Errorf("marshal vm config: %w", err)
 		}
-		return InjectFile(vmConfigGuestPath, data, 0o644)(rootfsPath, nil)
+		return InjectFile(vmconfig.GuestPath, data, 0o644)(rootfsPath, nil)
 	}
 }
 

hooks/hooks_test.go

@@ -4,6 +4,7 @@
 package hooks
 
 import (
+	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -12,6 +13,8 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/stacklok/propolis/guest/vmconfig"
 )
 
 // chownCall records a single chown invocation.
@@ -41,6 +44,47 @@ func recordingChown() (ChownFunc, func() []chownCall) {
 	return fn, get
 }
 
+func TestInjectVMConfig(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		cfg  vmconfig.Config
+	}{
+		{
+			name: "non-zero config",
+			cfg:  vmconfig.Config{TmpSizeMiB: 512},
+		},
+		{
+			name: "zero-value config",
+			cfg:  vmconfig.Config{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			rootfs := t.TempDir()
+			hook := InjectVMConfig(tt.cfg)
+
+			err := hook(rootfs, nil)
+			require.NoError(t, err)
+
+			data, err := os.ReadFile(filepath.Join(rootfs, "etc", "propolis-vm.json"))
+			require.NoError(t, err)
+
+			var got vmconfig.Config
+			require.NoError(t, json.Unmarshal(data, &got))
+			assert.Equal(t, tt.cfg, got)
+
+			info, err := os.Stat(filepath.Join(rootfs, "etc", "propolis-vm.json"))
+			require.NoError(t, err)
+			assert.Equal(t, os.FileMode(0o644), info.Mode().Perm())
+		})
+	}
+}
+
 func TestInjectFile_WritesContent(t *testing.T) {
 	t.Parallel()
 

options.go

@@ -300,7 +300,9 @@ func WithLogLevel(level uint32) Option {
 }
 
 // WithTmpSize sets the size of the /tmp tmpfs inside the guest VM in MiB.
-// Defaults to 256 MiB when 0 or not set.
+// Defaults to 256 MiB when 0 or not set. The kernel enforces available
+// memory as the upper bound; unreasonable values will cause a mount failure
+// inside the guest.
 // The value is written to /etc/propolis-vm.json in the rootfs and read by
 // the guest init before mounting filesystems.
 func WithTmpSize(mib uint32) Option {

propolis.go

@@ -27,6 +27,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/stacklok/propolis/guest/vmconfig"
 	"github.com/stacklok/propolis/hooks"
 	"github.com/stacklok/propolis/hypervisor"
 	"github.com/stacklok/propolis/hypervisor/libkrun"
@@ -128,7 +129,7 @@ func Run(ctx context.Context, imageRef string, opts ...Option) (*VM, error) {
 	// Only written when a non-default value is configured, keeping the
 	// file absent for callers that rely on the built-in 256 MiB default.
 	if cfg.tmpSizeMiB > 0 {
-		vmCfgHook := hooks.InjectVMConfig(hooks.VMConfig{TmpSizeMiB: cfg.tmpSizeMiB})
+		vmCfgHook := hooks.InjectVMConfig(vmconfig.Config{TmpSizeMiB: cfg.tmpSizeMiB})
 		if err := vmCfgHook(rootfs.Path, rootfs.Config); err != nil {
 			return nil, fmt.Errorf("inject vm config: %w", err)
 		}