diff --git a/cmd/api/config/config.go b/cmd/api/config/config.go index 366fb496..6324ee66 100644 --- a/cmd/api/config/config.go +++ b/cmd/api/config/config.go @@ -146,11 +146,20 @@ type ImagesConfig struct { // BuildConfig holds source-to-image build system settings. type BuildConfig struct { - MaxConcurrentSourceBuilds int `koanf:"max_concurrent_source_builds"` - BuilderImage string `koanf:"builder_image"` - Timeout int `koanf:"timeout"` - SecretsDir string `koanf:"secrets_dir"` - DockerSocket string `koanf:"docker_socket"` + MaxConcurrentSourceBuilds int `koanf:"max_concurrent_source_builds"` + BuilderImage string `koanf:"builder_image"` + Timeout int `koanf:"timeout"` + SecretsDir string `koanf:"secrets_dir"` + DockerSocket string `koanf:"docker_socket"` + DiskRoot BuildDiskRootConfig `koanf:"disk_root"` +} + +// BuildDiskRootConfig holds settings for the ephemeral per-build BuildKit root +// disk. When enabled, each builder VM gets a dedicated ext4 volume mounted at +// /var/lib/buildkit instead of a tmpfs, and the volume is deleted with the VM. +type BuildDiskRootConfig struct { + Enabled bool `koanf:"enabled"` + SizeGB int `koanf:"size_gb"` } // InstancesConfig holds instance-manager internal settings. @@ -629,6 +638,9 @@ func (c *Config) Validate() error { if c.Build.Timeout <= 0 { return fmt.Errorf("build.timeout must be positive, got %d", c.Build.Timeout) } + if c.Build.DiskRoot.SizeGB < 0 { + return fmt.Errorf("build.disk_root.size_gb must be >= 0, got %d", c.Build.DiskRoot.SizeGB) + } if c.Hypervisor.FirecrackerMaxConcurrentRestores < 0 { return fmt.Errorf("hypervisor.firecracker_max_concurrent_restores must be >= 0, got %d", c.Hypervisor.FirecrackerMaxConcurrentRestores) } diff --git a/config.example.yaml b/config.example.yaml index 410fd8b2..a25dc005 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -152,6 +152,10 @@ data_dir: /var/lib/hypeman # docker_socket: /var/run/docker.sock # max_concurrent_source_builds: 2 # timeout: 600 +# disk_root: +# enabled: false # attach a dedicated ext4 volume at /var/lib/buildkit +# # in each builder VM instead of a tmpfs +# size_gb: 20 # size of the per-build BuildKit root volume # ============================================================================= # Resource Limits diff --git a/lib/builds/disk_root_test.go b/lib/builds/disk_root_test.go new file mode 100644 index 00000000..c9c80f98 --- /dev/null +++ b/lib/builds/disk_root_test.go @@ -0,0 +1,371 @@ +package builds + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + "time" + + "github.com/kernel/hypeman/lib/instances" + "github.com/kernel/hypeman/lib/volumes" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSetupDiskRootVolume_Disabled(t *testing.T) { + mgr, _, volumeMgr, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + + volID, err := mgr.setupDiskRootVolume(context.Background(), "build-1") + + require.NoError(t, err) + assert.Empty(t, volID) + assert.Equal(t, 0, volumeMgr.createCallCount) +} + +func TestSetupDiskRootVolume_DefaultSize(t *testing.T) { + mgr, _, volumeMgr, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + mgr.config.DiskRootEnabled = true + + var gotReq volumes.CreateVolumeRequest + volumeMgr.createFunc = func(ctx context.Context, req volumes.CreateVolumeRequest) (*volumes.Volume, error) { + gotReq = req + return &volumes.Volume{Id: *req.Id, Name: req.Name, SizeGb: req.SizeGb}, nil + } + + volID, err := mgr.setupDiskRootVolume(context.Background(), "build-1") + + require.NoError(t, err) + assert.Equal(t, "build-disk-build-1", volID) + require.NotNil(t, gotReq.Id) + assert.Equal(t, "build-disk-build-1", *gotReq.Id) + assert.Equal(t, DefaultDiskRootSizeGB, gotReq.SizeGb) +} + +func TestSetupDiskRootVolume_ConfiguredSize(t *testing.T) { + mgr, _, volumeMgr, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + mgr.config.DiskRootEnabled = true + mgr.config.DiskRootSizeGB = 42 + + var gotReq volumes.CreateVolumeRequest + volumeMgr.createFunc = func(ctx context.Context, req volumes.CreateVolumeRequest) (*volumes.Volume, error) { + gotReq = req + return &volumes.Volume{Id: *req.Id, Name: req.Name, SizeGb: req.SizeGb}, nil + } + + _, err := mgr.setupDiskRootVolume(context.Background(), "build-1") + + require.NoError(t, err) + assert.Equal(t, 42, gotReq.SizeGb) +} + +func TestSetupDiskRootVolume_CreateError(t *testing.T) { + mgr, _, volumeMgr, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + mgr.config.DiskRootEnabled = true + + volumeMgr.createFunc = func(ctx context.Context, req volumes.CreateVolumeRequest) (*volumes.Volume, error) { + return nil, errors.New("disk full") + } + + volID, err := mgr.setupDiskRootVolume(context.Background(), "build-1") + + require.Error(t, err) + assert.Empty(t, volID) +} + +func TestSetupDiskRootVolume_LeftoverFromCrash(t *testing.T) { + mgr, _, volumeMgr, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + mgr.config.DiskRootEnabled = true + + // Simulate a volume left behind by a crashed build: the first create + // fails with ErrAlreadyExists, the leftover is deleted, and the retry + // succeeds. + created := false + volumeMgr.createFunc = func(ctx context.Context, req volumes.CreateVolumeRequest) (*volumes.Volume, error) { + if !created { + created = true + return nil, volumes.ErrAlreadyExists + } + return &volumes.Volume{Id: *req.Id, Name: req.Name, SizeGb: req.SizeGb}, nil + } + + volID, err := mgr.setupDiskRootVolume(context.Background(), "build-1") + + require.NoError(t, err) + assert.Equal(t, "build-disk-build-1", volID) + assert.Equal(t, 1, volumeMgr.deleteCallCount) + assert.Equal(t, 2, volumeMgr.createCallCount) +} + +func TestSetupDiskRootVolume_LeftoverDeleteError(t *testing.T) { + mgr, _, volumeMgr, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + mgr.config.DiskRootEnabled = true + + volumeMgr.createFunc = func(ctx context.Context, req volumes.CreateVolumeRequest) (*volumes.Volume, error) { + return nil, volumes.ErrAlreadyExists + } + volumeMgr.deleteFunc = func(ctx context.Context, id string) error { + return errors.New("volume attached") + } + + volID, err := mgr.setupDiskRootVolume(context.Background(), "build-1") + + require.Error(t, err) + assert.Empty(t, volID) +} + +// TestSetupDiskRootVolume_LeftoverAttachedToStaleBuilder verifies recovery +// when a crash left the volume attached to a surviving builder VM: the first +// delete fails with ErrInUse, the stale builder is deleted (detaching the +// volume), and the retry succeeds. +func TestSetupDiskRootVolume_LeftoverAttachedToStaleBuilder(t *testing.T) { + mgr, instanceMgr, volumeMgr, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + mgr.config.DiskRootEnabled = true + + created := false + volumeMgr.createFunc = func(ctx context.Context, req volumes.CreateVolumeRequest) (*volumes.Volume, error) { + if !created { + created = true + return nil, volumes.ErrAlreadyExists + } + return &volumes.Volume{Id: *req.Id, Name: req.Name, SizeGb: req.SizeGb}, nil + } + builderDeleted := false + volumeMgr.deleteFunc = func(ctx context.Context, id string) error { + if !builderDeleted { + return volumes.ErrInUse + } + return nil + } + instanceMgr.getFunc = func(ctx context.Context, id string) (*instances.Instance, error) { + if id == "builder-build-1" { + return &instances.Instance{ + StoredMetadata: instances.StoredMetadata{Id: "inst-builder-build-1", Name: "builder-build-1"}, + }, nil + } + return nil, instances.ErrNotFound + } + instanceMgr.deleteFunc = func(ctx context.Context, id string) error { + assert.Equal(t, "inst-builder-build-1", id) + builderDeleted = true + return nil + } + + volID, err := mgr.setupDiskRootVolume(context.Background(), "build-1") + + require.NoError(t, err) + assert.Equal(t, "build-disk-build-1", volID) + assert.True(t, builderDeleted, "stale builder holding the volume must be deleted") + assert.Equal(t, 2, volumeMgr.deleteCallCount) + assert.Equal(t, 2, volumeMgr.createCallCount) +} + +// TestSetupDiskRootVolume_LeftoverInUseWithoutBuilder verifies recovery fails +// loudly when the leftover volume is attached but no stale builder exists to +// clean up. +func TestSetupDiskRootVolume_LeftoverInUseWithoutBuilder(t *testing.T) { + mgr, _, volumeMgr, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + mgr.config.DiskRootEnabled = true + + volumeMgr.createFunc = func(ctx context.Context, req volumes.CreateVolumeRequest) (*volumes.Volume, error) { + return nil, volumes.ErrAlreadyExists + } + volumeMgr.deleteFunc = func(ctx context.Context, id string) error { + return volumes.ErrInUse + } + + volID, err := mgr.setupDiskRootVolume(context.Background(), "build-1") + + require.Error(t, err) + assert.Contains(t, err.Error(), "stale builder") + assert.ErrorIs(t, err, instances.ErrNotFound, "error must wrap the builder lookup failure, not the volume delete error") + assert.Empty(t, volID) +} + +// TestCreateBuildConfigVolume_UniqueTempPath verifies the config disk is +// written to a unique per-call path, so concurrent builds reusing a build ID +// (as parallel test runs sharing TMPDIR do) never collide on a fixed file. +func TestCreateBuildConfigVolume_UniqueTempPath(t *testing.T) { + mgr, _, _, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + + req := CreateBuildRequest{Dockerfile: "FROM alpine"} + prepareBuildOnDisk(t, mgr, "build-1", req) + + path1, err := mgr.createBuildConfigVolume("build-1", "build-config-build-1") + require.NoError(t, err) + defer os.RemoveAll(filepath.Dir(path1)) + path2, err := mgr.createBuildConfigVolume("build-1", "build-config-build-1") + require.NoError(t, err) + defer os.RemoveAll(filepath.Dir(path2)) + + assert.NotEqual(t, path1, path2) + for _, p := range []string{path1, path2} { + _, err := os.Stat(p) + require.NoError(t, err) + } +} + +func TestBuilderVolumeAttachments(t *testing.T) { + attachments := builderVolumeAttachments("src-vol", "cfg-vol", "") + require.Len(t, attachments, 2) + assert.Equal(t, "/src", attachments[0].MountPath) + assert.False(t, attachments[0].Readonly) + assert.Equal(t, "/config", attachments[1].MountPath) + assert.True(t, attachments[1].Readonly) + + attachments = builderVolumeAttachments("src-vol", "cfg-vol", "disk-vol") + require.Len(t, attachments, 3) + assert.Equal(t, "disk-vol", attachments[2].VolumeID) + assert.Equal(t, "/var/lib/buildkit", attachments[2].MountPath) + assert.False(t, attachments[2].Readonly) +} + +// prepareBuildOnDisk writes the metadata, source, and config files executeBuild +// expects, without going through CreateBuild (which would start a background +// build goroutine via the queue). +func prepareBuildOnDisk(t *testing.T, mgr *manager, id string, req CreateBuildRequest) { + t.Helper() + + meta := &buildMetadata{ + ID: id, + Status: StatusQueued, + Request: &req, + CreatedAt: time.Now(), + } + require.NoError(t, writeMetadata(mgr.paths, meta)) + require.NoError(t, mgr.storeSource(id, []byte("fake-tarball-data"))) + + config := &BuildConfig{ + JobID: id, + RegistryURL: mgr.config.RegistryURL, + SourcePath: "/src", + Dockerfile: req.Dockerfile, + TimeoutSeconds: 600, + NetworkMode: "isolated", + } + require.NoError(t, writeBuildConfig(mgr.paths, id, config)) +} + +// stoppedBuilderInstance returns a CreateInstance hook that records the +// request and reports the instance as already stopped, so waitForResult +// returns quickly. +func stoppedBuilderInstance(instanceMgr *mockInstanceManager, gotReq *instances.CreateInstanceRequest) { + instanceMgr.createFunc = func(ctx context.Context, req instances.CreateInstanceRequest) (*instances.Instance, error) { + if gotReq != nil { + *gotReq = req + } + inst := &instances.Instance{ + StoredMetadata: instances.StoredMetadata{ + Id: "inst-" + req.Name, + Name: req.Name, + }, + State: instances.StateStopped, + } + instanceMgr.instances[inst.Id] = inst + return inst, nil + } +} + +// TestExecuteBuild_DiskRootLifecycle runs executeBuild with the disk root +// feature enabled and verifies the volume is created, attached at +// /var/lib/buildkit, and deleted when the build finishes. It uses mock +// managers only — no privileged mounts. +func TestExecuteBuild_DiskRootLifecycle(t *testing.T) { + mgr, instanceMgr, volumeMgr, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + mgr.config.DiskRootEnabled = true + + ctx := context.Background() + req := CreateBuildRequest{ + Dockerfile: "FROM alpine\nRUN echo hello", + } + prepareBuildOnDisk(t, mgr, "build-1", req) + + var createReq instances.CreateInstanceRequest + stoppedBuilderInstance(instanceMgr, &createReq) + + policy := DefaultBuildPolicy() + result, err := mgr.executeBuild(ctx, "build-1", req, &policy) + require.NoError(t, err) + require.NotNil(t, result) + assert.False(t, result.Success) + + // Attached at /var/lib/buildkit in the builder instance request. + require.Len(t, createReq.Volumes, 3) + assert.Equal(t, "build-disk-build-1", createReq.Volumes[2].VolumeID) + assert.Equal(t, "/var/lib/buildkit", createReq.Volumes[2].MountPath) + assert.False(t, createReq.Volumes[2].Readonly) + + // Deleted with the builder VM. + _, err = volumeMgr.GetVolume(ctx, "build-disk-build-1") + assert.ErrorIs(t, err, volumes.ErrNotFound) + assert.GreaterOrEqual(t, volumeMgr.deleteCallCount, 1) +} + +// TestExecuteBuild_DiskRootDisabledLeavesDefaultPath verifies that with the +// feature disabled no disk root volume is created and the builder instance +// only gets the source and config attachments. +func TestExecuteBuild_DiskRootDisabledLeavesDefaultPath(t *testing.T) { + mgr, instanceMgr, volumeMgr, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + + ctx := context.Background() + req := CreateBuildRequest{ + Dockerfile: "FROM alpine\nRUN echo hello", + } + prepareBuildOnDisk(t, mgr, "build-1", req) + + var createReq instances.CreateInstanceRequest + stoppedBuilderInstance(instanceMgr, &createReq) + + policy := DefaultBuildPolicy() + _, err := mgr.executeBuild(ctx, "build-1", req, &policy) + require.NoError(t, err) + + require.Len(t, createReq.Volumes, 2) + assert.Equal(t, "/src", createReq.Volumes[0].MountPath) + assert.Equal(t, "/config", createReq.Volumes[1].MountPath) + for _, v := range volumeMgr.volumes { + assert.NotContains(t, v.Name, "build-disk-") + } +} + +// TestExecuteBuild_DiskRootCreateErrorFailsBuild verifies a volume creation +// failure fails the build before the builder instance is created. +func TestExecuteBuild_DiskRootCreateErrorFailsBuild(t *testing.T) { + mgr, instanceMgr, volumeMgr, tempDir := setupTestManager(t) + defer os.RemoveAll(tempDir) + mgr.config.DiskRootEnabled = true + + ctx := context.Background() + req := CreateBuildRequest{ + Dockerfile: "FROM alpine\nRUN echo hello", + } + prepareBuildOnDisk(t, mgr, "build-1", req) + + volumeMgr.createFunc = func(ctx context.Context, req volumes.CreateVolumeRequest) (*volumes.Volume, error) { + if req.Name == "build-disk-build-1" { + return nil, errors.New("disk full") + } + vol := &volumes.Volume{Id: "vol-" + req.Name, Name: req.Name} + volumeMgr.volumes[vol.Id] = vol + return vol, nil + } + + policy := DefaultBuildPolicy() + _, err := mgr.executeBuild(ctx, "build-1", req, &policy) + + require.Error(t, err) + assert.Contains(t, err.Error(), "buildkit root volume") + assert.Equal(t, 0, instanceMgr.createCallCount) +} diff --git a/lib/builds/manager.go b/lib/builds/manager.go index 87f21dfe..d41a8c24 100644 --- a/lib/builds/manager.go +++ b/lib/builds/manager.go @@ -5,6 +5,7 @@ import ( "context" _ "embed" "encoding/json" + "errors" "fmt" "log/slog" "net" @@ -89,8 +90,23 @@ type Config struct { // DockerSocket is the path to the Docker socket for building the builder image DockerSocket string + + // DiskRootEnabled attaches a dedicated ext4 volume at /var/lib/buildkit in + // each builder VM instead of relying on the tmpfs fallback in the guest. + DiskRootEnabled bool + + // DiskRootSizeGB is the size of the per-build BuildKit root volume. + // Values <= 0 use DefaultDiskRootSizeGB. + DiskRootSizeGB int } +// buildkitRootMountPath is where the BuildKit root volume is mounted in the +// builder guest. The builder agent detects the mount and uses it directly. +const buildkitRootMountPath = "/var/lib/buildkit" + +// DefaultDiskRootSizeGB is the default size of the per-build BuildKit root volume. +const DefaultDiskRootSizeGB = 20 + // DefaultConfig returns the default build manager configuration func DefaultConfig() Config { return Config{ @@ -694,7 +710,7 @@ func (m *manager) executeBuild(ctx context.Context, id string, req CreateBuildRe if err != nil { return nil, fmt.Errorf("create config volume: %w", err) } - defer os.Remove(configVolPath) // Clean up the config disk file + defer os.RemoveAll(filepath.Dir(configVolPath)) // Clean up the config disk file // Register the config volume with the volume manager _, err = m.volumeManager.CreateVolume(ctx, volumes.CreateVolumeRequest{ @@ -719,28 +735,28 @@ func (m *manager) executeBuild(ctx context.Context, id string, req CreateBuildRe } defer m.volumeManager.DeleteVolume(context.Background(), configVolID) + // Optionally create a dedicated BuildKit root volume for this build. + // It is attached at /var/lib/buildkit and deleted with the builder VM. + diskRootVolID, err := m.setupDiskRootVolume(ctx, id) + if err != nil { + return nil, fmt.Errorf("create buildkit root volume: %w", err) + } + if diskRootVolID != "" { + defer m.volumeManager.DeleteVolume(context.Background(), diskRootVolID) + } + // Create builder instance builderName := fmt.Sprintf("builder-%s", id) networkEnabled := policy.NetworkMode == "egress" inst, err := m.instanceManager.CreateInstance(ctx, instances.CreateInstanceRequest{ - Name: builderName, - Image: m.config.BuilderImage, - Size: int64(policy.MemoryMB) * 1024 * 1024, - Vcpus: policy.CPUs, - NetworkEnabled: networkEnabled, - Volumes: []instances.VolumeAttachment{ - { - VolumeID: sourceVolID, - MountPath: "/src", - Readonly: false, // Builder needs to write generated Dockerfile - }, - { - VolumeID: configVolID, - MountPath: "/config", - Readonly: true, - }, - }, + Name: builderName, + Image: m.config.BuilderImage, + Size: int64(policy.MemoryMB) * 1024 * 1024, + Vcpus: policy.CPUs, + NetworkEnabled: networkEnabled, + Volumes: builderVolumeAttachments(sourceVolID, configVolID, diskRootVolID), + AllowSystemVolumeMounts: true, // builder owns the BuildKit root mount }) if err != nil { return nil, fmt.Errorf("create builder instance: %w", err) @@ -767,6 +783,94 @@ func (m *manager) executeBuild(ctx context.Context, id string, req CreateBuildRe return result, nil } +// setupDiskRootVolume creates the ephemeral BuildKit root volume for a build +// when the disk root feature is enabled. Returns "" when disabled. +func (m *manager) setupDiskRootVolume(ctx context.Context, buildID string) (string, error) { + if !m.config.DiskRootEnabled { + return "", nil + } + + sizeGB := m.config.DiskRootSizeGB + if sizeGB <= 0 { + sizeGB = DefaultDiskRootSizeGB + } + + volID := fmt.Sprintf("build-disk-%s", buildID) + _, err := m.volumeManager.CreateVolume(ctx, volumes.CreateVolumeRequest{ + Id: &volID, + Name: volID, + SizeGb: sizeGB, + }) + if errors.Is(err, volumes.ErrAlreadyExists) { + // A previous attempt at this build crashed before cleanup. Delete + // the leftover volume and start fresh. + if delErr := m.deleteLeftoverDiskRootVolume(ctx, buildID, volID); delErr != nil { + return "", delErr + } + _, err = m.volumeManager.CreateVolume(ctx, volumes.CreateVolumeRequest{ + Id: &volID, + Name: volID, + SizeGb: sizeGB, + }) + } + if err != nil { + return "", err + } + return volID, nil +} + +// deleteLeftoverDiskRootVolume deletes the buildkit root volume left behind +// by a crashed build attempt. When the crash left the volume attached to a +// surviving builder VM the delete fails with ErrInUse; delete the stale +// builder (which detaches its volumes) and retry. +func (m *manager) deleteLeftoverDiskRootVolume(ctx context.Context, buildID, volID string) error { + err := m.volumeManager.DeleteVolume(ctx, volID) + if !errors.Is(err, volumes.ErrInUse) { + if err != nil { + return fmt.Errorf("delete leftover buildkit root volume: %w", err) + } + return nil + } + + builderName := fmt.Sprintf("builder-%s", buildID) + inst, getErr := m.instanceManager.GetInstance(ctx, builderName) + if getErr != nil { + return fmt.Errorf("leftover buildkit root volume still attached and stale builder %q not found: %w", builderName, getErr) + } + if delErr := m.instanceManager.DeleteInstance(ctx, inst.Id); delErr != nil { + return fmt.Errorf("delete stale builder %s holding leftover buildkit root volume: %w", inst.Id, delErr) + } + if err := m.volumeManager.DeleteVolume(ctx, volID); err != nil { + return fmt.Errorf("delete leftover buildkit root volume after stale builder removal: %w", err) + } + return nil +} + +// builderVolumeAttachments returns the volume attachments for a builder VM. +// diskRootVolID is empty when the disk root feature is disabled. +func builderVolumeAttachments(sourceVolID, configVolID, diskRootVolID string) []instances.VolumeAttachment { + attachments := []instances.VolumeAttachment{ + { + VolumeID: sourceVolID, + MountPath: "/src", + Readonly: false, // Builder needs to write generated Dockerfile + }, + { + VolumeID: configVolID, + MountPath: "/config", + Readonly: true, + }, + } + if diskRootVolID != "" { + attachments = append(attachments, instances.VolumeAttachment{ + VolumeID: diskRootVolID, + MountPath: buildkitRootMountPath, + Readonly: false, + }) + } + return attachments +} + // waitForResult waits for the build result from the builder agent via vsock func (m *manager) waitForResult(ctx context.Context, buildID string, inst *instances.Instance) (*BuildResult, error) { // Wait a bit for the VM to start and the builder agent to listen on vsock @@ -1421,8 +1525,8 @@ func readFile(path string) ([]byte, error) { return os.ReadFile(path) } -// createBuildConfigVolume creates an ext4 disk containing the build.json config file -// Returns the path to the disk file +// createBuildConfigVolume creates an ext4 disk containing the build.json config file. +// Returns the path to the disk file; the caller removes its parent directory. func (m *manager) createBuildConfigVolume(buildID, volID string) (string, error) { // Read the build config configPath := m.paths.BuildConfig(buildID) @@ -1453,10 +1557,17 @@ func (m *manager) createBuildConfigVolume(buildID, volID string) (string, error) metadataPath := filepath.Join(tmpDir, "metadata.json") os.WriteFile(metadataPath, metadataData, 0644) - // Create ext4 disk from the directory - diskPath := filepath.Join(os.TempDir(), fmt.Sprintf("build-config-%s.ext4", buildID)) + // Create the ext4 disk in a dedicated temp directory so concurrent + // builds and parallel test runs sharing TMPDIR never collide on a + // fixed path. The caller removes the directory. + diskDir, err := os.MkdirTemp("", "hypeman-build-config-disk-*") + if err != nil { + return "", fmt.Errorf("create config disk dir: %w", err) + } + diskPath := filepath.Join(diskDir, "build-config.ext4") _, err = images.ExportRootfs(tmpDir, diskPath, images.FormatExt4) if err != nil { + os.RemoveAll(diskDir) return "", fmt.Errorf("create config disk: %w", err) } diff --git a/lib/instances/create.go b/lib/instances/create.go index 1bc6c867..25f34d1c 100644 --- a/lib/instances/create.go +++ b/lib/instances/create.go @@ -32,6 +32,14 @@ const ( MaxVolumesPerInstance = 23 ) +// allowedSystemMountPaths are exact paths exempt from the system directory +// check for instances created with AllowSystemVolumeMounts: internal +// services that own the path and require a volume mounted at a fixed +// location under it. +var allowedSystemMountPaths = []string{ + "/var/lib/buildkit", // BuildKit root volume in builder VMs +} + // systemDirectories are paths that cannot be used as volume mount points var systemDirectories = []string{ "/", @@ -633,15 +641,17 @@ func validateCreateRequest(req *CreateInstanceRequest) error { req.RestartPolicy = normalizedRestartPolicy // Validate volume attachments - if err := validateVolumeAttachments(req.Volumes); err != nil { + if err := validateVolumeAttachments(req.Volumes, req.AllowSystemVolumeMounts); err != nil { return err } return nil } -// validateVolumeAttachments validates volume attachment requests -func validateVolumeAttachments(volumes []VolumeAttachment) error { +// validateVolumeAttachments validates volume attachment requests. +// allowSystemPaths permits mounts at the reserved paths in +// allowedSystemMountPaths and is only set for internal instances. +func validateVolumeAttachments(volumes []VolumeAttachment, allowSystemPaths bool) error { // Count total devices needed (each overlay volume needs 2 devices: base + overlay) totalDevices := 0 for _, vol := range volumes { @@ -665,7 +675,7 @@ func validateVolumeAttachments(volumes []VolumeAttachment) error { cleanPath := filepath.Clean(vol.MountPath) // Check for system directories - if isSystemDirectory(cleanPath) { + if isSystemDirectory(cleanPath) && !(allowSystemPaths && isAllowedSystemMountPath(cleanPath)) { return fmt.Errorf("volume %s: cannot mount to system directory %q", vol.VolumeID, cleanPath) } @@ -689,6 +699,17 @@ func validateVolumeAttachments(volumes []VolumeAttachment) error { return nil } +// isAllowedSystemMountPath reports whether path is an exact match for a +// system path that an internal instance may mount a volume at. +func isAllowedSystemMountPath(path string) bool { + for _, allowed := range allowedSystemMountPaths { + if path == allowed { + return true + } + } + return false +} + // isSystemDirectory checks if a path is or is under a system directory func isSystemDirectory(path string) bool { cleanPath := filepath.Clean(path) diff --git a/lib/instances/resource_limits_test.go b/lib/instances/resource_limits_test.go index 32cad66c..f1d50a98 100644 --- a/lib/instances/resource_limits_test.go +++ b/lib/instances/resource_limits_test.go @@ -27,7 +27,7 @@ func TestValidateVolumeAttachments_MaxVolumes(t *testing.T) { } } - err := validateVolumeAttachments(volumes) + err := validateVolumeAttachments(volumes, false) assert.Error(t, err) assert.Contains(t, err.Error(), "cannot attach more than 23") } @@ -39,7 +39,7 @@ func TestValidateVolumeAttachments_SystemDirectory(t *testing.T) { MountPath: "/etc/secrets", // system directory }} - err := validateVolumeAttachments(volumes) + err := validateVolumeAttachments(volumes, false) assert.Error(t, err) assert.Contains(t, err.Error(), "system directory") } @@ -51,7 +51,7 @@ func TestValidateVolumeAttachments_DuplicatePaths(t *testing.T) { {VolumeID: "vol-2", MountPath: "/mnt/data"}, // duplicate } - err := validateVolumeAttachments(volumes) + err := validateVolumeAttachments(volumes, false) assert.Error(t, err) assert.Contains(t, err.Error(), "duplicate mount path") } @@ -63,7 +63,7 @@ func TestValidateVolumeAttachments_RelativePath(t *testing.T) { MountPath: "relative/path", // not absolute }} - err := validateVolumeAttachments(volumes) + err := validateVolumeAttachments(volumes, false) assert.Error(t, err) assert.Contains(t, err.Error(), "must be absolute") } @@ -75,19 +75,44 @@ func TestValidateVolumeAttachments_Valid(t *testing.T) { {VolumeID: "vol-2", MountPath: "/mnt/logs"}, } - err := validateVolumeAttachments(volumes) + err := validateVolumeAttachments(volumes, false) assert.NoError(t, err) } func TestValidateVolumeAttachments_Empty(t *testing.T) { t.Parallel() - err := validateVolumeAttachments(nil) + err := validateVolumeAttachments(nil, false) assert.NoError(t, err) - err = validateVolumeAttachments([]VolumeAttachment{}) + err = validateVolumeAttachments([]VolumeAttachment{}, false) assert.NoError(t, err) } +func TestValidateVolumeAttachments_AllowedSystemMountPath(t *testing.T) { + t.Parallel() + // The BuildKit root volume in builder VMs mounts at a fixed path under + // /var and is allowed only for internal instances that opt in. + buildkitRoot := []VolumeAttachment{{ + VolumeID: "vol-1", + MountPath: "/var/lib/buildkit", + }} + err := validateVolumeAttachments(buildkitRoot, true) + assert.NoError(t, err) + + err = validateVolumeAttachments(buildkitRoot, false) + assert.Error(t, err, "system mount path rejected without AllowSystemVolumeMounts") + + // Parent paths and other /var subdirectories remain rejected even for + // internal instances. + for _, path := range []string{"/var", "/var/lib", "/var/lib/buildkit/nested", "/var/log"} { + err := validateVolumeAttachments([]VolumeAttachment{{ + VolumeID: "vol-1", + MountPath: path, + }}, true) + assert.Error(t, err, "expected %q to be rejected", path) + } +} + func TestValidateVolumeAttachments_OverlayRequiresReadonly(t *testing.T) { t.Parallel() // Overlay=true with Readonly=false should fail @@ -99,7 +124,7 @@ func TestValidateVolumeAttachments_OverlayRequiresReadonly(t *testing.T) { OverlaySize: 100 * 1024 * 1024, }} - err := validateVolumeAttachments(volumes) + err := validateVolumeAttachments(volumes, false) assert.Error(t, err) assert.Contains(t, err.Error(), "overlay mode requires readonly=true") } @@ -115,7 +140,7 @@ func TestValidateVolumeAttachments_OverlayRequiresSize(t *testing.T) { OverlaySize: 0, // Invalid: overlay requires size }} - err := validateVolumeAttachments(volumes) + err := validateVolumeAttachments(volumes, false) assert.Error(t, err) assert.Contains(t, err.Error(), "overlay_size is required") } @@ -131,7 +156,7 @@ func TestValidateVolumeAttachments_OverlayValid(t *testing.T) { OverlaySize: 100 * 1024 * 1024, // 100MB }} - err := validateVolumeAttachments(volumes) + err := validateVolumeAttachments(volumes, false) assert.NoError(t, err) } @@ -151,7 +176,7 @@ func TestValidateVolumeAttachments_OverlayCountsAsTwoDevices(t *testing.T) { } } - err := validateVolumeAttachments(volumes) + err := validateVolumeAttachments(volumes, false) assert.Error(t, err) assert.Contains(t, err.Error(), "cannot attach more than 23") } diff --git a/lib/instances/types.go b/lib/instances/types.go index 1ff97575..f7227185 100644 --- a/lib/instances/types.go +++ b/lib/instances/types.go @@ -274,6 +274,12 @@ type CreateInstanceRequest struct { AutoStandby *autostandby.Policy // Optional automatic standby policy HealthCheck *healthcheck.Policy // Optional workload health check policy RestartPolicy *restartpolicy.Policy // Optional whole-instance restart policy + + // AllowSystemVolumeMounts permits volume mounts at the reserved system + // paths in allowedSystemMountPaths. Only set by internal services that + // own those paths (e.g. builder VMs mounting the BuildKit root); it is + // never populated from API requests. + AllowSystemVolumeMounts bool } // StartInstanceRequest is the domain request for starting a stopped instance diff --git a/lib/providers/providers.go b/lib/providers/providers.go index 090d2ccf..99361fbb 100644 --- a/lib/providers/providers.go +++ b/lib/providers/providers.go @@ -421,6 +421,8 @@ func ProvideBuildManager(p *paths.Paths, cfg *config.Config, instanceManager ins DefaultTimeout: cfg.Build.Timeout, RegistrySecret: cfg.JwtSecret, // Use same secret for registry tokens DockerSocket: cfg.Build.DockerSocket, + DiskRootEnabled: cfg.Build.DiskRoot.Enabled, + DiskRootSizeGB: cfg.Build.DiskRoot.SizeGB, } // Configure secret provider (use NoOpSecretProvider as fallback to avoid nil panics)