From 33d515898e9938e6d885008d5af5cb7896dcf0da Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 5 Oct 2026 17:16:25 -0600 Subject: [PATCH 01/10] fix(microsandbox): synchronize workspace permissions across lifecycle --- e2e/tests/up/provider_microsandbox.go | 112 +++++-- .../testdata/microsandbox/.devcontainer.json | 2 +- pkg/devcontainer/capabilities_test.go | 4 +- pkg/devcontainer/single.go | 41 ++- pkg/devcontainer/single_test.go | 102 +++++++ pkg/driver/microsandbox/cliclient.go | 90 ++++-- pkg/driver/microsandbox/cliclient_test.go | 70 +++++ pkg/driver/microsandbox/client.go | 5 +- pkg/driver/microsandbox/image_user.go | 288 ++++++++++++++++++ pkg/driver/microsandbox/image_user_test.go | 196 ++++++++++++ pkg/driver/microsandbox/microsandbox.go | 90 +++++- pkg/driver/microsandbox/microsandbox_test.go | 43 ++- pkg/driver/microsandbox/mounts.go | 16 +- .../microsandbox/workspace_owner_test.go | 149 +++++++++ pkg/driver/types.go | 17 ++ pkg/image/image.go | 1 + .../docs/developing-providers/driver.mdx | 31 +- 17 files changed, 1159 insertions(+), 98 deletions(-) create mode 100644 pkg/driver/microsandbox/image_user.go create mode 100644 pkg/driver/microsandbox/image_user_test.go create mode 100644 pkg/driver/microsandbox/workspace_owner_test.go diff --git a/e2e/tests/up/provider_microsandbox.go b/e2e/tests/up/provider_microsandbox.go index af6d31feed..5f10021e9b 100644 --- a/e2e/tests/up/provider_microsandbox.go +++ b/e2e/tests/up/provider_microsandbox.go @@ -2,13 +2,16 @@ package up import ( "context" + "encoding/json" "fmt" "os" "os/exec" "path/filepath" "runtime" + "strings" "github.com/devsy-org/devsy/e2e/framework" + "github.com/devsy-org/devsy/pkg/devcontainer" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" ) @@ -73,42 +76,105 @@ var _ = ginkgo.Describe( framework.ExpectNoError(err) workspacePath := filepath.Join("/workspaces", filepath.Base(tempDir)) - guestFile := filepath.Join(tempDir, "guest-created.txt") _, err = f.DevsySSHOnce(ctx, tempDir, fmt.Sprintf( - "touch %s/guest-created.txt && chmod 0644 %s/guest-created.txt", + "umask 022; : > %s/guest-created.txt && mkdir %s/guest-created-dir", workspacePath, workspacePath, )) framework.ExpectNoError(err) - guestInfo, err := os.Stat(guestFile) - framework.ExpectNoError(err) - gomega.Expect(guestInfo.Mode().Perm()).To(gomega.Equal(os.FileMode(0o644))) + for name, mode := range map[string]os.FileMode{"guest-created.txt": 0o644, "guest-created-dir": 0o755} { + info, err := os.Stat(filepath.Join(tempDir, name)) + framework.ExpectNoError(err) + gomega.Expect(info.Mode().Perm()).To(gomega.Equal(mode)) + } - hostFile := filepath.Join(tempDir, "host-created.txt") - //nolint:gosec // permission mirroring is the behavior under test - err = os.WriteFile( - hostFile, - []byte("host"), - 0o644, - ) - framework.ExpectNoError(err) - //nolint:gosec // permission mirroring is the behavior under test - err = os.Chmod(hostFile, 0o644) + workspace, err := f.FindWorkspace(ctx, tempDir) framework.ExpectNoError(err) - owner, err := f.DevsySSHOnce(ctx, tempDir, fmt.Sprintf( - "stat -c '%%U %%a' %s/host-created.txt", workspacePath, - )) + sandbox := "devsy-" + devcontainer.GetRunnerIDFromWorkspace(workspace) + createdAt := microsandboxCreationTime(ctx, sandbox) + // No --user override: verify the runtime's default execution identity. + // #nosec G204 -- fixed command and Devsy-generated sandbox name + workloadUser, err := exec.CommandContext( + ctx, + "msb", + "exec", + "--stream", + sandbox, + "--", + "id", + "-u", + ).Output() framework.ExpectNoError(err) - gomega.Expect(owner).To(gomega.ContainSubstring("vscode 644")) + gomega.Expect(strings.TrimSpace(string(workloadUser))).To(gomega.Equal("0")) - // stop, then bring it back up and confirm it is reachable again + assertMicrosandboxHostEntries(ctx, f, tempDir, "fresh") err = f.DevsyWorkspaceStop(ctx, tempDir) framework.ExpectNoError(err) - err = f.DevsyUp(ctx, tempDir) framework.ExpectNoError(err) - - err = f.DevsySSHEchoTestString(ctx, tempDir) + gomega.Expect(microsandboxCreationTime(ctx, sandbox)).To(gomega.Equal(createdAt)) + assertMicrosandboxHostEntries(ctx, f, tempDir, "restarted") + err = f.DevsyUpRecreate(ctx, tempDir) framework.ExpectNoError(err) + gomega.Expect(microsandboxCreationTime(ctx, sandbox)).NotTo(gomega.Equal(createdAt)) + assertMicrosandboxHostEntries(ctx, f, tempDir, "recreated") }, ginkgo.SpecTimeout(framework.TimeoutModerate())) }, ) + +// Entries are created after setup so recursive workspace chown cannot mask fallback ownership. +func assertMicrosandboxHostEntries( + ctx context.Context, + f *framework.Framework, + hostPath, prefix string, +) { + guestPath := filepath.Join("/workspaces", filepath.Base(hostPath)) + identity, err := f.DevsySSHOnce(ctx, hostPath, `printf '%s:%s' "$(id -u)" "$(id -g)"`) + framework.ExpectNoError(err) + identity = strings.TrimSpace(identity) + gomega.Expect(identity).NotTo(gomega.Equal("0:0")) + for name, mode := range map[string]os.FileMode{prefix + "-file": 0o644, prefix + "-dir": 0o755} { + hostEntry := filepath.Join(hostPath, name) + if mode == 0o644 { + //nolint:gosec // permission mirroring is the behavior under test + err = os.WriteFile(hostEntry, []byte("host"), mode) + } else { + err = os.Mkdir(hostEntry, mode) + } + framework.ExpectNoError(err) + //nolint:gosec // normalize host umask before checking guest-visible modes + framework.ExpectNoError(os.Chmod(hostEntry, mode)) + ownerBefore := microsandboxHostOwner(ctx, hostEntry) + result, err := f.DevsySSHOnce( + ctx, + hostPath, + fmt.Sprintf("stat -c '%%u:%%g %%a' %s/%s", guestPath, name), + ) + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(result)). + To(gomega.Equal(fmt.Sprintf("%s %o", identity, mode))) + gomega.Expect(microsandboxHostOwner(ctx, hostEntry)).To(gomega.Equal(ownerBefore)) + } +} + +func microsandboxHostOwner(ctx context.Context, entry string) string { + args := []string{"-c", "%u:%g", entry} + if runtime.GOOS == "darwin" { + args = []string{"-f", "%u:%g", entry} + } + // #nosec G204 -- fixed stat executable and test-created path + output, err := exec.CommandContext(ctx, "stat", args...).Output() + framework.ExpectNoError(err) + return strings.TrimSpace(string(output)) +} + +func microsandboxCreationTime(ctx context.Context, sandbox string) string { + // #nosec G204 -- fixed inspect command and Devsy-generated sandbox name + output, err := exec.CommandContext(ctx, "msb", "inspect", sandbox, "--format", "json").Output() + framework.ExpectNoError(err) + var info struct { + CreatedAt string `json:"created_at"` + } + framework.ExpectNoError(json.Unmarshal(output, &info)) + gomega.Expect(info.CreatedAt).NotTo(gomega.BeEmpty()) + return info.CreatedAt +} diff --git a/e2e/tests/up/testdata/microsandbox/.devcontainer.json b/e2e/tests/up/testdata/microsandbox/.devcontainer.json index c8d7c4041c..3517343eed 100644 --- a/e2e/tests/up/testdata/microsandbox/.devcontainer.json +++ b/e2e/tests/up/testdata/microsandbox/.devcontainer.json @@ -1,6 +1,6 @@ { "name": "microsandbox", "image": "mcr.microsoft.com/devcontainers/base:ubuntu", - "containerUser": "vscode", + "containerUser": "root", "remoteUser": "vscode" } diff --git a/pkg/devcontainer/capabilities_test.go b/pkg/devcontainer/capabilities_test.go index 7821abe297..0a21dbcf2d 100644 --- a/pkg/devcontainer/capabilities_test.go +++ b/pkg/devcontainer/capabilities_test.go @@ -135,7 +135,9 @@ type recreateDriver struct { func (d *recreateDriver) RecreateMode() driver.RecreateMode { return d.mode } func TestRecreatePolicy(t *testing.T) { - for _, mode := range []driver.RecreateMode{driver.RecreateDelete, driver.RecreateStop, "invalid"} { + for _, mode := range []driver.RecreateMode{ + driver.RecreateDelete, driver.RecreateStop, driver.RecreateOnRun, "invalid", + } { t.Run(string(mode), func(t *testing.T) { d := &recreateDriver{ mode: mode, diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index 9455d24109..99705ea330 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -138,6 +138,9 @@ func (r *runner) resolveContainer( params *resolveParams, containerDetails *config.ContainerDetails, ) (*resolvedContainer, error) { + if err := r.applyDriverRecreateRequirement(containerDetails, params); err != nil { + return nil, err + } options := params.options if containerDetails != nil && !options.Recreate && r.needsTerminalSecretEnvironmentMigration(containerDetails) { @@ -425,9 +428,8 @@ func (r *runner) lingerWarning(ctx context.Context) string { return helper.LingerWarning(ctx) } -// buildNewContainerConfig builds the image (deleting the existing container -// first when recreating) and produces the merged devcontainer config from the -// build's image metadata. +// buildNewContainerConfig builds the image and merges its metadata. Drivers +// with runner-managed recreation are torn down after the build succeeds. func (r *runner) buildNewContainerConfig( ctx context.Context, p *resolveParams, @@ -561,6 +563,8 @@ func (r *runner) deleteForRecreate(ctx context.Context) error { } return nil + case driver.RecreateOnRun: + return nil case driver.RecreateStop: if err := r.driver.StopDevContainer(ctx, r.id); err != nil { return fmt.Errorf("stop devcontainer: %w", err) @@ -856,6 +860,7 @@ func (r *runner) getDockerlessRunOptions( UID: r.workspaceUID(), Image: image, User: containerRootUser, + RemoteUser: effectiveRemoteUser(mergedConfig, buildInfo.Dockerless.User), Entrypoint: "/.dockerless/dockerless", Cmd: []string{ "start", @@ -932,6 +937,7 @@ func (r *runner) getRunOptions( Image: buildInfo.ImageName, ImageBuilt: buildInfo.BuiltLocally, User: user, + RemoteUser: effectiveRemoteUser(mergedConfig, user), Entrypoint: entrypoint, Cmd: cmd, Env: mergedConfig.ContainerEnv, @@ -1097,3 +1103,32 @@ func GetContainerEntrypointAndArgs( } return shShellPath, cmd } + +func effectiveRemoteUser(merged *config.MergedDevContainerConfig, containerUser string) string { + if merged.RemoteUser != "" { + return merged.RemoteUser + } + if containerUser != "" { + return containerUser + } + return containerRootUser +} + +func (r *runner) applyDriverRecreateRequirement( + details *config.ContainerDetails, + p *resolveParams, +) error { + if details == nil || p.options.Recreate { + return nil + } + required, reason := driver.DriverRequiresRecreate(r.driver, details) + if !required { + return nil + } + if p.parsedConfig.Config.ContainerID != "" { + return fmt.Errorf("cannot migrate externally managed container: %s", reason) + } + log.Infof("recreating workspace because %s", reason) + p.options.Recreate = true + return nil +} diff --git a/pkg/devcontainer/single_test.go b/pkg/devcontainer/single_test.go index 9672023c58..a4f65b745c 100644 --- a/pkg/devcontainer/single_test.go +++ b/pkg/devcontainer/single_test.go @@ -425,3 +425,105 @@ func TestIsRecoveryContainer(t *testing.T) { t.Error("container with the recovery label must be flagged") } } + +type migrationMockDriver struct { + *provisioningPreflightMockDriver + required bool + migrationCalls int +} + +func (d *migrationMockDriver) RequiresRecreate(*config.ContainerDetails) (bool, string) { + d.migrationCalls++ + return d.required, "workspace mount contract changed" +} + +func TestDriverMountContractMigration(t *testing.T) { + for _, tt := range []struct { + name string + required, explicit, external, wantRecreate, wantError bool + calls int + }{ + {"incompatible", true, false, false, true, false, 1}, + {"compatible", false, false, false, false, false, 1}, + {"explicit", true, true, false, true, false, 0}, + {"external", true, false, true, false, true, 1}, + } { + t.Run(tt.name, func(t *testing.T) { + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + required: tt.required, + } + r := newTestRunner(d) + params := recreateResolveParams() + params.options.Recreate = tt.explicit + if tt.external { + params.parsedConfig.Config.ContainerID = testContainerID + } + err := r.applyDriverRecreateRequirement(runningContainerDetails(), params) + if tt.wantError { + require.ErrorContains(t, err, "cannot migrate externally managed container") + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantRecreate, params.options.Recreate) + require.Equal(t, tt.calls, d.migrationCalls) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) + }) + } +} + +func TestAutomaticMigrationPreflightFailurePreservesExisting(t *testing.T) { + sentinel := errors.New("unsupported provisioning runtime") + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + provisioningErr: sentinel, + }, + required: true, + } + params := recreateResolveParams() + params.options.Recreate = false + _, err := newTestRunner( + d, + ).resolveContainer(context.Background(), params, runningContainerDetails()) + require.ErrorIs(t, err, sentinel) + require.True(t, d.provisioningCalled) + require.True(t, params.options.Recreate) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) +} + +func TestEffectiveRemoteUser(t *testing.T) { + for _, tt := range []struct{ remote, container, want string }{ + {"vscode", containerRootUser, "vscode"}, {"", "node", "node"}, {"", "", containerRootUser}, + } { + cfg := &config.MergedDevContainerConfig{} + cfg.RemoteUser = tt.remote + require.Equal(t, tt.want, effectiveRemoteUser(cfg, tt.container)) + } +} + +func TestRunOptionsSeparateDeveloperIdentity(t *testing.T) { + r := newTestRunner(&mockDriver{}) + cfg := &config.MergedDevContainerConfig{} + cfg.ContainerUser = containerRootUser + cfg.RemoteUser = "vscode" + info := &config.BuildInfo{ + ImageName: "final-image", + ImageMetadata: &config.ImageMetadataConfig{}, + ImageDetails: &config.ImageDetails{}, + } + info.ImageDetails.Config.User = "node" + options, err := r.getRunOptions(cfg, &config.SubstitutionContext{}, info, false) + require.NoError(t, err) + require.Equal(t, containerRootUser, options.User) + require.Equal(t, "vscode", options.RemoteUser) + info.Dockerless = &config.BuildInfoDockerless{User: "node"} + options, err = r.getDockerlessRunOptions(cfg, &config.SubstitutionContext{}, info, false) + require.NoError(t, err) + require.Equal(t, containerRootUser, options.User) + require.Equal(t, "vscode", options.RemoteUser) +} diff --git a/pkg/driver/microsandbox/cliclient.go b/pkg/driver/microsandbox/cliclient.go index 25f9d7f24c..aa0f7002c8 100644 --- a/pkg/driver/microsandbox/cliclient.go +++ b/pkg/driver/microsandbox/cliclient.go @@ -14,13 +14,15 @@ import ( "strings" "time" + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/driver" "github.com/devsy-org/devsy/pkg/flags/names" "github.com/devsy-org/devsy/pkg/image" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/tarball" ) -type cliClient struct{} +type cliClient struct{ dockerPath string } var _ sandboxClient = cliClient{} @@ -50,9 +52,9 @@ func (cliClient) Version(ctx context.Context) (string, error) { return strings.TrimSpace(string(out)), nil } -func (cliClient) EnsureImage(ctx context.Context, imageRef string) error { - if dockerImageExists(ctx, imageRef) { - return loadFromDocker(ctx, imageRef) +func (c cliClient) EnsureImage(ctx context.Context, imageRef string, builtLocally bool) error { + if builtLocally { + return loadFromDocker(ctx, imageRef, c.dockerPath) } // #nosec G204 -- args are a resolved binary path and a validated image ref out, err := exec.CommandContext(ctx, msbBinary(), "pull", imageRef).CombinedOutput() @@ -89,24 +91,41 @@ func (cliClient) Find(ctx context.Context, sandbox string) (*sandboxInfo, error) } return nil, fmt.Errorf("inspect microsandbox VM %q: %w", sandbox, err) } + return parseSandboxInfo(out) +} + +func parseSandboxInfo(out []byte) (*sandboxInfo, error) { type activeConfig struct { Labels map[string]string `json:"labels"` + Mounts []inspectedMount `json:"mounts"` } var raw struct { - Name string `json:"name"` - Status string `json:"status"` - CreatedAt string `json:"created_at"` - ActiveConfig activeConfig `json:"active_config"` + Name string `json:"name"` + Status string `json:"status"` + CreatedAt string `json:"created_at"` + ActiveConfig *activeConfig `json:"active_config"` + Config *activeConfig `json:"config"` } if err := json.Unmarshal(out, &raw); err != nil { return nil, fmt.Errorf("parse microsandbox inspect output: %w", err) } + current := raw.ActiveConfig + if current == nil { + current = raw.Config + } + var labels map[string]string + var mounts []config.ContainerMount + if current != nil { + labels = current.Labels + mounts = inspectedMountDetails(current.Mounts) + } created, _ := time.Parse(time.RFC3339Nano, raw.CreatedAt) return &sandboxInfo{ Name: raw.Name, Running: strings.EqualFold(raw.Status, "running"), CreatedAt: created, - Labels: raw.ActiveConfig.Labels, + Labels: labels, + Mounts: mounts, }, nil } @@ -290,17 +309,11 @@ func redactArgs(args []string) string { return strings.Join(out, " ") } -func dockerImageExists(ctx context.Context, image string) bool { - docker, err := exec.LookPath("docker") - if err != nil { - return false +func loadFromDocker(ctx context.Context, image, dockerPath string) error { + if dockerPath == "" { + dockerPath = "docker" } - // #nosec G204 -- docker path is resolved and the image ref is validated - return exec.CommandContext(ctx, docker, "image", "inspect", image).Run() == nil -} - -func loadFromDocker(ctx context.Context, image string) error { - docker, err := exec.LookPath("docker") + docker, err := exec.LookPath(dockerPath) if err != nil { return fmt.Errorf("docker not found to load built image %q: %w", image, err) } @@ -308,22 +321,29 @@ func loadFromDocker(ctx context.Context, image string) error { save := exec.CommandContext(ctx, docker, "save", image) // #nosec G204 -- docker/msb paths are resolved and the image ref is validated load := exec.CommandContext(ctx, msbBinary(), "load", "-t", image) - pipe, err := save.StdoutPipe() + reader, writer, err := os.Pipe() if err != nil { return fmt.Errorf("pipe docker save: %w", err) } - load.Stdin = pipe + defer func() { _ = reader.Close() }() + defer func() { _ = writer.Close() }() + save.Stdout = writer + load.Stdin = reader + var saveErr strings.Builder + save.Stderr = &saveErr var loadErr strings.Builder load.Stderr = &loadErr if err := load.Start(); err != nil { return fmt.Errorf("start msb load: %w", err) } + _ = reader.Close() if err := save.Run(); err != nil { // load is still running on the broken pipe; kill and reap it. _ = load.Process.Kill() _ = load.Wait() - return fmt.Errorf("docker save %q: %w", image, err) + return fmt.Errorf("docker save %q: %s: %w", image, saveErr.String(), err) } + _ = writer.Close() if err := load.Wait(); err != nil { return fmt.Errorf("msb load %q: %s: %w", image, loadErr.String(), err) } @@ -376,3 +396,29 @@ func msbBinary() string { } return "msb" } + +type inspectedMount struct { + Type string `json:"type"` + Host string `json:"host"` + Guest string `json:"guest"` +} + +func inspectedMountDetails(mounts []inspectedMount) []config.ContainerMount { + var details []config.ContainerMount + for _, mount := range mounts { + var mountType string + switch mount.Type { + case "Bind": + mountType = driver.MountTypeBind + case "Tmpfs": + mountType = driver.MountTypeTmpfs + default: + continue + } + details = append( + details, + config.ContainerMount{Type: mountType, Source: mount.Host, Destination: mount.Guest}, + ) + } + return details +} diff --git a/pkg/driver/microsandbox/cliclient_test.go b/pkg/driver/microsandbox/cliclient_test.go index 0ff80ff49e..de6ef8ddac 100644 --- a/pkg/driver/microsandbox/cliclient_test.go +++ b/pkg/driver/microsandbox/cliclient_test.go @@ -7,7 +7,10 @@ import ( "testing" "time" + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/driver" "github.com/devsy-org/devsy/pkg/flags/names" + "github.com/stretchr/testify/require" ) func runPrefix() []string { @@ -228,3 +231,70 @@ func hasFlagValue(args []string, flag, value string) bool { } return false } + +func TestInspectStoppedSandboxRetainsMountContract(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + output := fmt.Sprintf( + `{"name":"%s","status":"Stopped","active_config":null,"config":{"labels":{"%s":"%s","%s":"root"}}}`, + wsName, + workspaceMountContractLabel, + d.workspaceMountContract(), + userLabel, + ) + info, err := parseSandboxInfo([]byte(output)) + if err != nil { + t.Fatal(err) + } + details := toContainerDetails(info) + required, _ := d.RequiresRecreate(details) + if required { + t.Fatal("stopped sandbox lost persisted contract") + } + if details.Config.User != "root" { + t.Fatal("stopped sandbox lost container identity") + } +} + +func TestInspectUsesActiveContractWhenConfigHasPendingChanges(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + output := fmt.Sprintf( + `{"name":"%s","status":"Running","active_config":{"labels":{"%s":"%s"}},"config":{"labels":{"%s":"pending"}}}`, + wsName, + workspaceMountContractLabel, + d.workspaceMountContract(), + workspaceMountContractLabel, + ) + info, err := parseSandboxInfo([]byte(output)) + if err != nil { + t.Fatal(err) + } + required, _ := d.RequiresRecreate(toContainerDetails(info)) + if required { + t.Fatal("pending configuration replaced active mount contract") + } +} + +func TestInspectReportsPersistedMounts(t *testing.T) { + for _, field := range []string{"config", "active_config"} { + t.Run(field, func(t *testing.T) { + output := fmt.Sprintf( + `{"name":"%s","%s":{"mounts":[`+ + `{"type":"Bind","host":"/host/project","guest":"/workspaces/project"},`+ + `{"type":"Tmpfs","guest":"%s"}]}}`, + wsName, + field, + config.SecretsEnvDir, + ) + info, err := parseSandboxInfo([]byte(output)) + require.NoError(t, err) + require.Equal(t, []config.ContainerMount{ + { + Type: driver.MountTypeBind, + Source: "/host/project", + Destination: "/workspaces/project", + }, + {Type: driver.MountTypeTmpfs, Destination: config.SecretsEnvDir}, + }, toContainerDetails(info).Mounts) + }) + } +} diff --git a/pkg/driver/microsandbox/client.go b/pkg/driver/microsandbox/client.go index ac0a325a61..f95f241b2c 100644 --- a/pkg/driver/microsandbox/client.go +++ b/pkg/driver/microsandbox/client.go @@ -4,6 +4,8 @@ import ( "context" "io" "time" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" ) type sandboxSpec struct { @@ -38,6 +40,7 @@ type sandboxInfo struct { Running bool CreatedAt time.Time Labels map[string]string + Mounts []config.ContainerMount } type execRequest struct { @@ -55,7 +58,7 @@ type execRequest struct { type sandboxClient interface { EnsureInstalled(ctx context.Context) error Version(ctx context.Context) (string, error) - EnsureImage(ctx context.Context, image string) error + EnsureImage(ctx context.Context, image string, builtLocally bool) error Create(ctx context.Context, name string, spec sandboxSpec) error Find(ctx context.Context, name string) (*sandboxInfo, error) Start(ctx context.Context, name string) error diff --git a/pkg/driver/microsandbox/image_user.go b/pkg/driver/microsandbox/image_user.go new file mode 100644 index 0000000000..f902552276 --- /dev/null +++ b/pkg/driver/microsandbox/image_user.go @@ -0,0 +1,288 @@ +package microsandbox + +import ( + "archive/tar" + "context" + "fmt" + "io" + "os" + "os/exec" + "path" + "runtime" + "strconv" + "strings" + + "github.com/devsy-org/devsy/pkg/image" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/tarball" +) + +const ( + maxAccountFileSize = 4 << 20 + rootUser = "root" + passwdPath = "etc/passwd" + groupPath = "etc/group" +) + +type imageUserIdentity struct { + name, group string + uid, gid uint32 + numericUID, numericGID bool +} + +type imageUserResolver interface { + Resolve(ctx context.Context, image string, builtLocally bool, user string) (*mountOwner, error) +} + +type filesystemUserResolver struct{ dockerPath string } + +func (r filesystemUserResolver) Resolve( + ctx context.Context, + ref string, + builtLocally bool, + user string, +) (*mountOwner, error) { + if owner, complete, err := explicitOwner(user); complete || err != nil { + return owner, err + } + var img v1.Image + var err error + if builtLocally { + var cleanup func() + img, cleanup, err = r.localImage(ctx, ref) + if err != nil { + return nil, err + } + defer cleanup() + } else { + img, err = image.GetImageForArch(ctx, ref, runtime.GOARCH) + if err != nil { + return nil, err + } + } + return ownerFromImage(ctx, img, user) +} + +// The archive must remain available until lazy layer reads finish. +func (r filesystemUserResolver) localImage( + ctx context.Context, + ref string, +) (v1.Image, func(), error) { + archive, err := os.CreateTemp("", "devsy-msb-owner-*.tar") + if err != nil { + return nil, nil, err + } + cleanup := func() { _ = os.Remove(archive.Name()) } + docker := r.dockerPath + if docker == "" { + docker = "docker" + } + // #nosec G204 -- configured Docker-compatible executable, fixed subcommand, image argument + cmd := exec.CommandContext(ctx, docker, "save", ref) + cmd.Stdout = archive + var stderr strings.Builder + cmd.Stderr = &stderr + runErr := cmd.Run() + closeErr := archive.Close() + if runErr != nil { + cleanup() + return nil, nil, fmt.Errorf( + "save final local image %q: %s: %w", + ref, + strings.TrimSpace(stderr.String()), + runErr, + ) + } + if closeErr != nil { + cleanup() + return nil, nil, closeErr + } + img, err := tarball.ImageFromPath(archive.Name(), nil) + if err != nil { + cleanup() + return nil, nil, fmt.Errorf("read final local image %q: %w", ref, err) + } + return img, cleanup, nil +} + +func numericID(value string) (uint32, bool, error) { + if value == "" { + return 0, false, nil + } + for _, c := range value { + if c < '0' || c > '9' { + return 0, false, nil + } + } + n, err := strconv.ParseUint(value, 10, 32) + if err != nil { + return 0, true, fmt.Errorf("invalid numeric account ID %q: %w", value, err) + } + return uint32(n), true, nil +} + +func parseImageUserIdentity(user string) (imageUserIdentity, error) { + name, group, hasGroup := strings.Cut(user, ":") + if name == "" || (hasGroup && (group == "" || strings.Contains(group, ":"))) { + return imageUserIdentity{}, fmt.Errorf("invalid user identity %q", user) + } + uid, numericUID, err := numericID(name) + if err != nil { + return imageUserIdentity{}, err + } + gid, numericGID, err := numericID(group) + if err != nil { + return imageUserIdentity{}, err + } + return imageUserIdentity{ + name: name, + group: group, + uid: uid, + gid: gid, + numericUID: numericUID, + numericGID: numericGID, + }, nil +} + +func explicitOwner(user string) (*mountOwner, bool, error) { + identity, err := parseImageUserIdentity(user) + if err != nil { + return nil, false, err + } + if identity.group == "" { + if identity.name == rootUser || (identity.numericUID && identity.uid == 0) { + return &mountOwner{}, true, nil + } + } + if identity.numericUID && identity.numericGID { + return &mountOwner{UID: identity.uid, GID: identity.gid}, true, nil + } + return nil, false, nil +} + +func ownerFromImage(ctx context.Context, img v1.Image, user string) (*mountOwner, error) { + if owner, complete, err := explicitOwner(user); complete || err != nil { + return owner, err + } + accounts, err := imageAccountFiles(ctx, img) + if err != nil { + return nil, err + } + return ownerFromAccounts(user, accounts[passwdPath], accounts[groupPath]) +} + +func imageAccountFiles(ctx context.Context, img v1.Image) (map[string]string, error) { + stream := mutate.Extract(img) + defer func() { _ = stream.Close() }() + accounts := map[string]string{} + reader := tar.NewReader(stream) + for len(accounts) < 2 { + if err := ctx.Err(); err != nil { + return nil, err + } + header, err := reader.Next() + if err == io.EOF { + break + } + if err != nil { + return nil, fmt.Errorf("read image accounts: %w", err) + } + file := path.Clean(strings.TrimPrefix(header.Name, "/")) + if file != passwdPath && file != groupPath { + continue + } + accounts[file], err = readAccountFile(reader, header) + if err != nil { + return nil, err + } + } + return accounts, nil +} + +func readAccountFile(reader io.Reader, header *tar.Header) (string, error) { + if header.Typeflag != tar.TypeReg { + return "", fmt.Errorf("final image /%s is not a regular account file", header.Name) + } + data, err := io.ReadAll(io.LimitReader(reader, maxAccountFileSize+1)) + if err != nil { + return "", fmt.Errorf("read image /%s: %w", header.Name, err) + } + if len(data) > maxAccountFileSize { + return "", fmt.Errorf("image /%s exceeds account file size limit", header.Name) + } + return string(data), nil +} + +func ownerFromAccounts(user, passwd, groups string) (*mountOwner, error) { + name, group, hasGroup := strings.Cut(user, ":") + uid, numeric, err := numericID(name) + if err != nil { + return nil, err + } + owner := &mountOwner{UID: uid} + // Explicit numeric IDs need no passwd entry when a group is supplied. + if !numeric || !hasGroup { + owner, err = passwdOwner(passwd, name, uid, numeric) + if err != nil { + return nil, err + } + } + if hasGroup { + owner.GID, err = groupID(groups, group) + if err != nil { + return nil, err + } + } + return owner, nil +} + +func passwdOwner(passwd, name string, uid uint32, numeric bool) (*mountOwner, error) { + for line := range strings.SplitSeq(passwd, "\n") { + fields := strings.Split(line, ":") + if len(fields) != 7 { + continue + } + if !matchesPasswdRow(fields, name, uid, numeric) { + continue + } + return passwdRowOwner(fields) + } + return nil, fmt.Errorf("user %q not found in final image /etc/passwd", name) +} + +func matchesPasswdRow(fields []string, name string, uid uint32, numeric bool) bool { + if !numeric { + return fields[0] == name + } + entryUID, valid, err := numericID(fields[2]) + return valid && err == nil && entryUID == uid +} + +func passwdRowOwner(fields []string) (*mountOwner, error) { + uid, validUID, uidErr := numericID(fields[2]) + gid, validGID, gidErr := numericID(fields[3]) + if uidErr != nil || gidErr != nil || !validUID || !validGID { + return nil, fmt.Errorf("invalid account IDs for %q in final image /etc/passwd", fields[0]) + } + return &mountOwner{UID: uid, GID: gid}, nil +} + +func groupID(groups, group string) (uint32, error) { + gid, numeric, err := numericID(group) + if err != nil || numeric { + return gid, err + } + for line := range strings.SplitSeq(groups, "\n") { + fields := strings.Split(line, ":") + if len(fields) != 4 || fields[0] != group { + continue + } + gid, valid, err := numericID(fields[2]) + if err != nil || !valid { + return 0, fmt.Errorf("invalid group ID for %q in final image /etc/group", group) + } + return gid, nil + } + return 0, fmt.Errorf("group %q not found in final image /etc/group", group) +} diff --git a/pkg/driver/microsandbox/image_user_test.go b/pkg/driver/microsandbox/image_user_test.go new file mode 100644 index 0000000000..54a13593da --- /dev/null +++ b/pkg/driver/microsandbox/image_user_test.go @@ -0,0 +1,196 @@ +package microsandbox + +import ( + "archive/tar" + "bytes" + "context" + "io" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/tarball" + "github.com/stretchr/testify/suite" +) + +type imageUserSuite struct{ suite.Suite } + +func TestImageUser(t *testing.T) { suite.Run(t, new(imageUserSuite)) } + +func (s *imageUserSuite) TestResolution() { + img, err := mutate.AppendLayers(empty.Image, s.layer(map[string]string{ + passwdPath: "invalid-row\nroot:x:0:0:root:/root:/bin/sh\nvscode:x:1000:1002:dev:/home/vscode:/bin/sh\n", + groupPath: "developers:x:1001:vscode\n", + })) + s.Require().NoError(err) + for _, tt := range []struct { + user string + want mountOwner + }{ + {rootUser, mountOwner{}}, + {"0", mountOwner{}}, + {testUser, mountOwner{1000, 1002}}, + {"1000", mountOwner{1000, 1002}}, + {"1000:1001", mountOwner{1000, 1001}}, + {"vscode:developers", mountOwner{1000, 1001}}, + {"vscode:1003", mountOwner{1000, 1003}}, + {"1000:developers", mountOwner{1000, 1001}}, + } { + s.Run(tt.user, func() { + owner, err := ownerFromImage(context.Background(), img, tt.user) + s.Require().NoError(err) + s.Equal(tt.want, *owner) + }) + } + for _, user := range []string{ + "missing", "1009", "vscode:missing", "4294967296:1", "vscode:4294967296", "", "vscode:", "1:2:3", + } { + s.Run( + user, + func() { _, err := ownerFromImage(context.Background(), img, user); s.Error(err) }, + ) + } +} + +func (s *imageUserSuite) TestMissingAccounts() { + owner, err := ownerFromImage(context.Background(), empty.Image, "1000:1001") + s.Require().NoError(err) + s.Equal(mountOwner{1000, 1001}, *owner) + _, err = ownerFromImage(context.Background(), empty.Image, testUser) + s.ErrorContains(err, "not found in final image /etc/passwd") +} + +//nolint:gosec // Synthetic account rows contain no credentials. +func (s *imageUserSuite) TestFinalLayerOverridesAndWhiteouts() { + base := s.layer( + map[string]string{ + passwdPath: "vscode:x:10:11:dev:/home/vscode:/bin/sh\n", + groupPath: "old:x:99:\n", + }, + ) + final := s.layer( + map[string]string{ + passwdPath: "vscode:x:2000:2001:dev:/home/vscode:/bin/sh\n", + "etc/.wh.group": "", + }, + ) + img, err := mutate.AppendLayers(empty.Image, base, final) + s.Require().NoError(err) + owner, err := ownerFromImage(context.Background(), img, testUser) + s.Require().NoError(err) + s.Equal(mountOwner{2000, 2001}, *owner) + _, err = ownerFromImage(context.Background(), img, "vscode:old") + s.ErrorContains(err, "not found") +} + +func (s *imageUserSuite) TestMalformedAccount() { + for _, passwd := range []string{ + "vscode:x:no:1000:dev:/home/vscode:/bin/sh\n", + "vscode:x:4294967296:1000:dev:/home/vscode:/bin/sh\n", + } { + _, err := ownerFromAccounts(testUser, passwd, "") + s.ErrorContains(err, "invalid account IDs") + } +} + +func (s *imageUserSuite) TestBoundedAccountFile() { + img, err := mutate.AppendLayers( + empty.Image, + s.layer(map[string]string{passwdPath: strings.Repeat("x", maxAccountFileSize+1)}), + ) + s.Require().NoError(err) + _, err = ownerFromImage(context.Background(), img, testUser) + s.ErrorContains(err, "size limit") +} + +func (s *imageUserSuite) TestCanceledExtraction() { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := ownerFromImage(ctx, empty.Image, testUser) + s.ErrorIs(err, context.Canceled) +} + +//nolint:gosec // Synthetic account archive and executable fixture under TempDir. +func (s *imageUserSuite) TestLocalFinalImageUsesConfiguredCLI() { + if runtime.GOOS == "windows" { + s.T().Skip("fixture executable requires a POSIX shell") + } + img, err := mutate.AppendLayers( + empty.Image, + s.layer(map[string]string{passwdPath: "vscode:x:2000:2001:dev:/home/vscode:/bin/sh\n"}), + ) + s.Require().NoError(err) + dir := s.T().TempDir() + archive := filepath.Join(dir, "image.tar") + tag, err := name.NewTag("final-image:latest") + s.Require().NoError(err) + s.Require().NoError(tarball.WriteToFile(archive, tag, img)) + argsFile := filepath.Join(dir, "args") + s.T().Setenv("DEVSY_TEST_ARCHIVE", archive) + s.T().Setenv("DEVSY_TEST_ARGS", argsFile) + executable := filepath.Join(dir, "configured-cli") + script := `#!/bin/sh +printf '%s\n' "$@" > "$DEVSY_TEST_ARGS" +cat "$DEVSY_TEST_ARCHIVE" +` + s.Require().NoError(os.WriteFile(executable, []byte(script), 0o700)) + owner, err := (filesystemUserResolver{dockerPath: executable}).Resolve( + context.Background(), + "final-image:latest", + true, + testUser, + ) + s.Require().NoError(err) + s.Equal(mountOwner{2000, 2001}, *owner) + args, err := os.ReadFile(argsFile) + s.Require().NoError(err) + s.Equal("save\nfinal-image:latest\n", string(args)) +} + +//nolint:gosec // Test-created executable and archive paths under TempDir. +func (s *imageUserSuite) TestLocalImportUsesConfiguredCLIAndDrainsArchive() { + if runtime.GOOS == "windows" { + s.T().Skip("fixture executables require a POSIX shell") + } + dir := s.T().TempDir() + source := filepath.Join(dir, "source") + target := filepath.Join(dir, "target") + content := strings.Repeat("archive data", 100000) + s.Require().NoError(os.WriteFile(source, []byte(content), 0o600)) + s.T().Setenv("DEVSY_TEST_ARCHIVE", source) + s.T().Setenv("DEVSY_TEST_TARGET", target) + s.T().Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + saveCLI := filepath.Join(dir, "save-cli") + s.Require(). + NoError(os.WriteFile(saveCLI, []byte("#!/bin/sh\ncat \"$DEVSY_TEST_ARCHIVE\"\n"), 0o700)) + s.Require(). + NoError(os.WriteFile(filepath.Join(dir, "msb"), []byte("#!/bin/sh\ncat > \"$DEVSY_TEST_TARGET\"\n"), 0o700)) + s.Require(). + NoError((cliClient{dockerPath: saveCLI}).EnsureImage(context.Background(), "final-image:latest", true)) + data, err := os.ReadFile(target) + s.Require().NoError(err) + s.Equal(content, string(data)) +} + +func (s *imageUserSuite) layer(files map[string]string) v1.Layer { + var buf bytes.Buffer + writer := tar.NewWriter(&buf) + for name, data := range files { + s.Require(). + NoError(writer.WriteHeader(&tar.Header{Name: name, Mode: 0o644, Size: int64(len(data))})) + _, err := writer.Write([]byte(data)) + s.Require().NoError(err) + } + s.Require().NoError(writer.Close()) + layer, err := tarball.LayerFromOpener( + func() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(buf.Bytes())), nil }, + ) + s.Require().NoError(err) + return layer +} diff --git a/pkg/driver/microsandbox/microsandbox.go b/pkg/driver/microsandbox/microsandbox.go index c925ca2aba..49ceb12d26 100644 --- a/pkg/driver/microsandbox/microsandbox.go +++ b/pkg/driver/microsandbox/microsandbox.go @@ -43,13 +43,15 @@ type microsandboxDriver struct { idLabels []string defaults specDefaults workspaceMountPolicy workspaceMountPolicy + userResolver imageUserResolver } var ( - _ driver.MountDeliveryDriver = (*microsandboxDriver)(nil) - _ driver.RecreatePolicyDriver = (*microsandboxDriver)(nil) - _ driver.WorkspaceChowner = (*microsandboxDriver)(nil) - _ driver.ArgvExecDriver = (*microsandboxDriver)(nil) + _ driver.MountDeliveryDriver = (*microsandboxDriver)(nil) + _ driver.RecreatePolicyDriver = (*microsandboxDriver)(nil) + _ driver.RecreateRequiredDriver = (*microsandboxDriver)(nil) + _ driver.WorkspaceChowner = (*microsandboxDriver)(nil) + _ driver.ArgvExecDriver = (*microsandboxDriver)(nil) _ driver.RunOptionsDriver = (*microsandboxDriver)(nil) _ driver.ReprovisioningDriver = (*microsandboxDriver)(nil) @@ -98,7 +100,7 @@ func NewMicrosandboxDriver( _ context.Context, workspaceInfo *provider.AgentWorkspaceInfo, ) (driver.Driver, error) { - client := cliClient{} + client := cliClient{dockerPath: workspaceInfo.Agent.Docker.Path} cfg := workspaceInfo.Agent.Microsandbox defaults := specDefaults{ @@ -122,12 +124,14 @@ func NewMicrosandboxDriver( ) d := newDriver(client, workspaceInfo.CLIOptions.IDLabels, defaults) d.workspaceMountPolicy = workspacePolicy + d.userResolver = filesystemUserResolver{dockerPath: workspaceInfo.Agent.Docker.Path} return d, nil } func newDriver(client sandboxClient, idLabels []string, defaults specDefaults) *microsandboxDriver { return µsandboxDriver{ client: client, idLabels: idLabels, defaults: defaults, + userResolver: filesystemUserResolver{}, workspaceMountPolicy: workspaceMountPolicy{ StatVirtualization: statVirtStrict, HostPermissions: hostPermissionsMirror, @@ -270,8 +274,8 @@ func (d *microsandboxDriver) UpdateContainerUserUID( return nil } -// RequiresWorkspaceChown reports that the virtiofs workspace share is -// root-owned in the guest, so the agent must chown it to the remote user. +// RequiresWorkspaceChown repairs existing per-file guest ownership overrides, +// which take precedence over the workspace mount fallback owner. func (d *microsandboxDriver) RequiresWorkspaceChown() bool { return true } @@ -364,17 +368,21 @@ func (d *microsandboxDriver) runFromOptions( if err := d.ProvisioningPreflight(ctx); err != nil { return err } + owner, err := d.resolveWorkspaceOwner(ctx, options) + if err != nil { + return err + } warnUnsupportedOptions(options) + if err := d.client.EnsureImage(ctx, options.Image, options.ImageBuilt); err != nil { + return fmt.Errorf("ensure microsandbox final image: %w", err) + } if err := d.DeleteDevContainer(ctx, workspaceID); err != nil { return fmt.Errorf("clean stale microsandbox VM before create: %w", err) } - if err := d.client.EnsureImage(ctx, options.Image); err != nil { - log.Debugf("microsandbox ensure image %q failed (continuing): %v", options.Image, err) - } if err := d.client.Create( ctx, sandboxName(workspaceID), - d.buildSpec(workspaceID, options, hostReqs), + d.buildSpec(workspaceID, options, hostReqs, owner), ); err != nil { return fmt.Errorf("create microsandbox VM: %w", err) } @@ -385,12 +393,16 @@ func (d *microsandboxDriver) runFromOptions( // MICROSANDBOX_* defaults, then the devcontainer's hostRequirements, falling // back to the microsandbox runtime default (zero) when neither is set. func (d *microsandboxDriver) buildSpec( - workspaceID string, options *driver.RunOptions, hostReqs *config.HostRequirements, + workspaceID string, + options *driver.RunOptions, + hostReqs *config.HostRequirements, + owner *mountOwner, ) sandboxSpec { labels := config.ListToObject(config.GetIDLabels(workspaceID, d.idLabels)) if labels == nil { labels = map[string]string{} } + labels[workspaceMountContractLabel] = d.workspaceMountContract() if options.User != "" { labels[userLabel] = options.User } @@ -417,7 +429,7 @@ func (d *microsandboxDriver) buildSpec( Labels: labels, Ephemeral: d.defaults.ephemeral, IdleTimeout: d.defaults.idleTimeout, - Mounts: d.volumeMounts(options), + Mounts: d.volumeMounts(options, owner), MaxMemory: d.defaults.maxMemory, MaxCPUs: d.defaults.maxCPUs, BlockEgress: d.defaults.blockEgress, @@ -463,6 +475,7 @@ func toContainerDetails(info *sandboxInfo) *config.ContainerDetails { return &config.ContainerDetails{ ID: info.Name, Created: info.CreatedAt.Format(createdAtLayout), + Mounts: info.Mounts, State: config.ContainerDetailsState{Status: status}, Config: config.ContainerDetailsConfig{ Labels: info.Labels, @@ -590,4 +603,53 @@ func ceilBytesToUint32(bytes, unit uint64) uint32 { func (d *microsandboxDriver) RequiresMountStreaming() bool { return false } -func (d *microsandboxDriver) RecreateMode() driver.RecreateMode { return driver.RecreateDelete } +func (d *microsandboxDriver) RecreateMode() driver.RecreateMode { return driver.RecreateOnRun } + +const workspaceMountContractLabel = "devsy.sh/microsandbox-workspace-mount" + +func (d *microsandboxDriver) RequiresRecreate(details *config.ContainerDetails) (bool, string) { + if details == nil || + details.Config.Labels[workspaceMountContractLabel] == d.workspaceMountContract() { + return false, "" + } + return true, "microsandbox workspace mount contract changed" +} + +func (d *microsandboxDriver) workspaceMountContract() string { + owner := "none" + if d.workspaceMountPolicy.StatVirtualization != statVirtOff { + owner = "remote-user" + } + return fmt.Sprintf( + "v2;stat=%s;host=%s;owner=%s", + d.workspaceMountPolicy.StatVirtualization, + d.workspaceMountPolicy.HostPermissions, + owner, + ) +} + +func (d *microsandboxDriver) resolveWorkspaceOwner( + ctx context.Context, + options *driver.RunOptions, +) (*mountOwner, error) { + if d.workspaceMountPolicy.StatVirtualization == statVirtOff || + bindMount(options.WorkspaceMount) == nil { + return nil, nil + } + user := options.RemoteUser + if user == "" { + user = options.User + } + if user == "" { + user = rootUser + } + owner, err := d.userResolver.Resolve(ctx, options.Image, options.ImageBuilt, user) + if err != nil { + return nil, fmt.Errorf( + "resolve microsandbox workspace owner for remoteUser %q: %w", + user, + err, + ) + } + return owner, nil +} diff --git a/pkg/driver/microsandbox/microsandbox_test.go b/pkg/driver/microsandbox/microsandbox_test.go index 8c205dce6f..2b22bb9340 100644 --- a/pkg/driver/microsandbox/microsandbox_test.go +++ b/pkg/driver/microsandbox/microsandbox_test.go @@ -24,6 +24,7 @@ const ( testVersion = "0.7.2" oldVersion = "microsandbox 0.7.1" shPath = "/bin/sh" + callEnsure = "ensure:x:1" callFind = "find:" + wsName callRemove = "remove:" + wsName testBindSrc = "/host/proj" @@ -66,7 +67,7 @@ func (f *fakeClient) Version(context.Context) (string, error) { return testVersion, nil } -func (f *fakeClient) EnsureImage(_ context.Context, image string) error { +func (f *fakeClient) EnsureImage(_ context.Context, image string, _ bool) error { f.calls = append(f.calls, "ensure:"+image) return f.failEnsure } @@ -180,10 +181,10 @@ func TestRunDevContainerReplacesStaleSandbox(t *testing.T) { t.Fatalf("RunDevContainer: %v", err) } want := []string{ + callEnsure, callFind, "stop:devsy-ws1", callRemove, - "ensure:x:1", "create:devsy-ws1", } if !slices.Equal(f.calls, want) { @@ -218,7 +219,7 @@ func TestRunDevContainerAcceptsProvisioningRuntimeVersions(t *testing.T) { if err != nil { t.Fatalf("RunDevContainer: %v", err) } - want := []string{callFind, "ensure:x:1", "create:" + wsName} + want := []string{callEnsure, callFind, "create:" + wsName} if !slices.Equal(f.calls, want) { t.Fatalf("calls = %v, want %v", f.calls, want) } @@ -293,21 +294,17 @@ func assertProvisioningUnchanged(t *testing.T, f *fakeClient) { } } -func TestRunDevContainerContinuesWhenPrePullFails(t *testing.T) { +func TestRunDevContainerImageFailurePreservesExisting(t *testing.T) { f := newFakeClient() - f.failEnsure = errors.New("registry hiccup") + f.failEnsure = errors.New("image unavailable") + f.info[wsName] = &sandboxInfo{Name: wsName, Running: true} d := newDriver(f, nil, specDefaults{}) - - // Pre-pull failure must not abort the run; create should still be attempted. - if err := d.RunDevContainer( - context.Background(), - wsID, - &driver.RunOptions{Image: imgX}, - ); err != nil { - t.Fatalf("RunDevContainer should proceed despite pull failure: %v", err) + err := d.RunDevContainer(context.Background(), wsID, &driver.RunOptions{Image: imgX}) + if err == nil || !strings.Contains(err.Error(), "image unavailable") { + t.Fatalf("unexpected error: %v", err) } - if _, ok := f.created[wsName]; !ok { - t.Error("expected create to run even though pre-pull failed") + if !slices.Equal(f.calls, []string{callEnsure}) || !f.info[wsName].Running { + t.Fatalf("image failure changed sandbox: %v", f.calls) } } @@ -322,7 +319,7 @@ func TestRunImageDevContainerRunsFromParams(t *testing.T) { if err != nil { t.Fatalf("RunImageDevContainer: %v", err) } - want := []string{callFind, "ensure:built-local:latest", "create:devsy-ws1"} + want := []string{"ensure:built-local:latest", callFind, "create:devsy-ws1"} if !slices.Equal(f.calls, want) { t.Errorf("call order = %v, want %v", f.calls, want) } @@ -600,7 +597,7 @@ func TestBuildSpecMapsAllMountTypes(t *testing.T) { {Type: driver.MountTypeBind, Source: testBindSrc, Target: "/mnt"}, nil, }, - }, nil) + }, nil, nil) want := []volumeMount{ {Target: "/data", Volume: "vol1"}, {Target: "/scratch", Tmpfs: true}, @@ -620,7 +617,7 @@ func TestBuildSpecMapsWorkspaceMount(t *testing.T) { Source: testBindSrc, Target: testBindDst, }, - }, nil) + }, nil, nil) want := []volumeMount{ { Target: testBindDst, @@ -638,7 +635,7 @@ func TestBuildSpecMapsWorkspaceMount(t *testing.T) { func TestBuildSpecCarriesIdleTimeout(t *testing.T) { d := newDriver(newFakeClient(), nil, specDefaults{idleTimeout: 90 * time.Second}) - spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil) + spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil, nil) if spec.IdleTimeout != 90*time.Second { t.Errorf("idle timeout = %s, want 90s", spec.IdleTimeout) } @@ -650,7 +647,7 @@ func TestBuildSpecCarriesCeilingsAndEgress(t *testing.T) { nil, specDefaults{maxMemory: 4096, maxCPUs: 4, blockEgress: true}, ) - spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil) + spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil, nil) if spec.MaxMemory != 4096 || spec.MaxCPUs != 4 || !spec.BlockEgress { t.Errorf("unexpected spec ceilings/egress: %+v", spec) } @@ -659,7 +656,7 @@ func TestBuildSpecCarriesCeilingsAndEgress(t *testing.T) { func TestBuildSpecUsesHostRequirementsWhenDefaultsUnset(t *testing.T) { d := newDriver(newFakeClient(), nil, specDefaults{}) hostReqs := &config.HostRequirements{CPUs: 4, Memory: "8gb", Storage: size32GB} - spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, hostReqs) + spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, hostReqs, nil) if spec.CPUs != 4 { t.Errorf("CPUs = %d, want 4", spec.CPUs) } @@ -678,7 +675,7 @@ func TestBuildSpecPrefersConfiguredDefaultsOverHostRequirements(t *testing.T) { specDefaults{memory: 2048, cpus: 2, rootDiskGB: 16}, ) hostReqs := &config.HostRequirements{CPUs: 8, Memory: size32GB, Storage: "64gb"} - spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, hostReqs) + spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, hostReqs, nil) if spec.CPUs != 2 || spec.Memory != 2048 || spec.RootDiskGB != 16 { t.Errorf("configured defaults should win, got %+v", spec) } @@ -686,7 +683,7 @@ func TestBuildSpecPrefersConfiguredDefaultsOverHostRequirements(t *testing.T) { func TestBuildSpecIgnoresNilHostRequirements(t *testing.T) { d := newDriver(newFakeClient(), nil, specDefaults{}) - spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil) + spec := d.buildSpec(wsID, &driver.RunOptions{Image: imgX}, nil, nil) if spec.CPUs != 0 || spec.Memory != 0 || spec.RootDiskGB != 0 { t.Errorf("nil hostRequirements with no defaults should leave sizing zero, got %+v", spec) } diff --git a/pkg/driver/microsandbox/mounts.go b/pkg/driver/microsandbox/mounts.go index 49742eb285..b3710a21be 100644 --- a/pkg/driver/microsandbox/mounts.go +++ b/pkg/driver/microsandbox/mounts.go @@ -85,9 +85,12 @@ func parseHostPermissions(value string) (hostPermissions, error) { } } -func (d *microsandboxDriver) volumeMounts(options *driver.RunOptions) []volumeMount { +func (d *microsandboxDriver) volumeMounts( + options *driver.RunOptions, + owner *mountOwner, +) []volumeMount { var out []volumeMount - if m := d.workspaceMount(options.WorkspaceMount); m != nil { + if m := d.workspaceMount(options.WorkspaceMount, owner); m != nil { out = append(out, *m) } for _, m := range options.Mounts { @@ -98,14 +101,21 @@ func (d *microsandboxDriver) volumeMounts(options *driver.RunOptions) []volumeMo return out } -func (d *microsandboxDriver) workspaceMount(m *devcontainerconfig.Mount) *volumeMount { +func (d *microsandboxDriver) workspaceMount( + m *devcontainerconfig.Mount, + owner *mountOwner, +) *volumeMount { b := bindMount(m) if b == nil { return nil } + if d.workspaceMountPolicy.StatVirtualization == statVirtOff { + owner = nil + } b.Policy = mountPolicy{ StatVirtualization: d.workspaceMountPolicy.StatVirtualization, HostPermissions: d.workspaceMountPolicy.HostPermissions, + Owner: owner, } return b } diff --git a/pkg/driver/microsandbox/workspace_owner_test.go b/pkg/driver/microsandbox/workspace_owner_test.go new file mode 100644 index 0000000000..d9283e40b4 --- /dev/null +++ b/pkg/driver/microsandbox/workspace_owner_test.go @@ -0,0 +1,149 @@ +package microsandbox + +import ( + "context" + "errors" + "testing" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/driver" + "github.com/stretchr/testify/require" +) + +type recordingUserResolver struct { + image string + built bool + user string + owner *mountOwner + err error + calls int +} + +func (r *recordingUserResolver) Resolve( + _ context.Context, + image string, + built bool, + user string, +) (*mountOwner, error) { + r.image, r.built, r.user = image, built, user + r.calls++ + return r.owner, r.err +} + +func TestWorkspaceOwnerKeepsContainerIdentity(t *testing.T) { + f := newFakeClient() + d := newDriver(f, nil, specDefaults{}) + resolver := &recordingUserResolver{owner: &mountOwner{UID: 1000, GID: 1001}} + d.userResolver = resolver + options := &driver.RunOptions{ + Image: testImage, + ImageBuilt: true, + User: rootUser, + RemoteUser: testUser, + WorkspaceMount: &config.Mount{ + Type: driver.MountTypeBind, + Source: testBindSrc, + Target: testBindDst, + }, + Mounts: []*config.Mount{ + {Type: driver.MountTypeBind, Source: "/extra", Target: "/extra"}, + }, + } + require.NoError( + t, + d.RunImageDevContainer( + context.Background(), + &driver.RunImageDevContainerParams{WorkspaceID: wsID, Options: options}, + ), + ) + require.Equal(t, testUser, resolver.user) + require.Equal(t, testImage, resolver.image) + require.True(t, resolver.built) + spec := f.created[wsName] + require.Equal(t, rootUser, spec.User) + require.Equal( + t, + testBindSrc+":"+testBindDst+":stat-virt=strict,host-perms=mirror,uid=1000,gid=1001", + bindMountSpec(spec.Mounts[0]), + ) + require.Nil(t, spec.Mounts[1].Policy.Owner) + require.Equal(t, d.workspaceMountContract(), spec.Labels[workspaceMountContractLabel]) +} + +func TestWorkspaceOwnerFailurePreservesExistingSandbox(t *testing.T) { + f := newFakeClient() + f.info[wsName] = &sandboxInfo{Name: wsName, Running: true} + d := newDriver(f, nil, specDefaults{}) + d.userResolver = &recordingUserResolver{err: errors.New("user not found")} + err := d.RunDevContainer( + context.Background(), + wsID, + &driver.RunOptions{ + Image: imgX, + RemoteUser: "missing", + WorkspaceMount: &config.Mount{Source: testBindSrc, Target: testBindDst}, + }, + ) + require.ErrorContains(t, err, `remoteUser "missing"`) + require.Empty(t, f.calls) + require.True(t, f.info[wsName].Running) +} + +func TestWorkspaceOwnerPolicyOff(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + d.workspaceMountPolicy = workspaceMountPolicy{ + StatVirtualization: statVirtOff, + HostPermissions: hostPermissionsPrivate, + } + resolver := &recordingUserResolver{err: errors.New("must not resolve")} + d.userResolver = resolver + options := &driver.RunOptions{ + WorkspaceMount: &config.Mount{Source: testBindSrc, Target: testBindDst}, + RemoteUser: testUser, + } + owner, err := d.resolveWorkspaceOwner(context.Background(), options) + require.NoError(t, err) + require.Nil(t, owner) + require.Zero(t, resolver.calls) + mount := d.workspaceMount(options.WorkspaceMount, &mountOwner{UID: 1000, GID: 1000}) + require.Equal( + t, + testBindSrc+":"+testBindDst+":stat-virt=off,host-perms=private", + bindMountSpec(*mount), + ) +} + +func TestWorkspaceMountContractRecreation(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + for _, label := range []string{"", "v1", d.workspaceMountContract(), "v2;stat=relaxed;host=mirror;owner=remote-user"} { + details := &config.ContainerDetails{ + Config: config.ContainerDetailsConfig{ + Labels: map[string]string{workspaceMountContractLabel: label}, + }, + } + required, _ := driver.DriverRequiresRecreate(d, details) + require.Equal(t, label != d.workspaceMountContract(), required) + } + required, _ := d.RequiresRecreate(nil) + require.False(t, required) +} + +func TestWorkspaceOwnerFallbackIdentity(t *testing.T) { + for _, tt := range []struct{ user, remote, want string }{ + {"node", "", "node"}, {"", "", rootUser}, {rootUser, testUser, testUser}, + } { + d := newDriver(newFakeClient(), nil, specDefaults{}) + resolver := &recordingUserResolver{owner: &mountOwner{}} + d.userResolver = resolver + _, err := d.resolveWorkspaceOwner( + context.Background(), + &driver.RunOptions{ + User: tt.user, + RemoteUser: tt.remote, + WorkspaceMount: &config.Mount{Source: testBindSrc, Target: testBindDst}, + }, + ) + require.NoError(t, err) + require.Equal(t, tt.want, resolver.user) + } +} diff --git a/pkg/driver/types.go b/pkg/driver/types.go index b70465a469..736074f018 100644 --- a/pkg/driver/types.go +++ b/pkg/driver/types.go @@ -92,6 +92,8 @@ type RecreateMode string const ( RecreateDelete RecreateMode = "delete" RecreateStop RecreateMode = "stop" + // RecreateOnRun lets the driver validate final run options before replacing the container. + RecreateOnRun RecreateMode = "on-run" ) type RecreatePolicyDriver interface { @@ -170,6 +172,9 @@ type RunOptions struct { // User is the user to run the container as User string `json:"user,omitempty"` + // RemoteUser is the developer identity for remote tooling and workspace ownership. + RemoteUser string `json:"remoteUser,omitempty"` + // Entrypoint is the entrypoint of the container Entrypoint string `json:"entrypoint,omitempty"` @@ -219,3 +224,15 @@ type RunOptions struct { // pullable from a registry. False for a devcontainer.json "image:" reference. ImageBuilt bool `json:"imageBuilt,omitempty"` } + +// RecreateRequiredDriver detects incompatible creation-time container contracts. +type RecreateRequiredDriver interface { + RequiresRecreate(details *config.ContainerDetails) (bool, string) +} + +func DriverRequiresRecreate(d Driver, details *config.ContainerDetails) (bool, string) { + if migration, ok := d.(RecreateRequiredDriver); ok { + return migration.RequiresRecreate(details) + } + return false, "" +} diff --git a/pkg/image/image.go b/pkg/image/image.go index edc32cd337..86caae7094 100644 --- a/pkg/image/image.go +++ b/pkg/image/image.go @@ -54,6 +54,7 @@ func GetImageForArch(ctx context.Context, image, arch string) (v1.Image, error) } remoteOptions := []remote.Option{ + remote.WithContext(ctx), remote.WithAuthFromKeychain(keychain), remote.WithPlatform(v1.Platform{Architecture: arch, OS: osLinux}), } diff --git a/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx b/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx index 778ded7797..142a7f19bb 100644 --- a/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx +++ b/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx @@ -204,13 +204,30 @@ Available options: support; `relaxed` tolerates filesystems without it; `off` exposes literal host ownership and modes. `off` cannot be combined with `mirror`. -The workspace mount uses `stat-virt=strict` and `host-perms=mirror` by default, -while additional bind mounts retain MicroSandbox's safer defaults. Devsy passes -the resolved devcontainer `containerUser` to the MicroSandbox runtime so files -created on the host appear as the effective sandbox user in the guest. Devsy -requires MicroSandbox v0.7.2 or newer when creating or recreating workspaces -that use workspace ownership synchronization. This provisioning version check -does not block inspection, logs, stop, or deletion of an existing sandbox. +The primary workspace bind mount uses `stat-virt=strict` and `host-perms=mirror` +by default, while additional bind mounts retain MicroSandbox's defaults. +`containerUser` controls the workload execution user; `remoteUser` controls the +workspace developer identity, falling back to `containerUser`, the image user, +and then root. Devsy resolves the developer's numeric UID and primary GID from +the final image's account files and sets them as the mount's guest-visible +fallback owner. These IDs apply to host-created entries without per-file guest +metadata; they do not change host inode ownership. Account resolution does not +execute image code. An unresolvable user fails provisioning before replacement +of the existing VM. With `workspaceStatVirtualization: "off"`, Devsy omits the +owner override. + +Devsy-managed workspaces with an incompatible persisted workspace mount contract +are automatically recreated once on `devsy up`, including workspaces created +before the contract label existed. Changing either workspace permission policy +also requires recreation. The host workspace and named volumes persist, but +changes stored only on the VM's root disk are lost, as with `--recreate`. +Externally managed containers are never automatically replaced. + +Devsy requires MicroSandbox v0.7.2 or newer when creating or recreating workspaces +that use workspace ownership synchronization. Runtime compatibility, account +resolution, and final image import are checked before removing an existing VM. +The provisioning version check does not block inspection, logs, stop, or deletion +of an existing sandbox. Apple Silicon macOS is the canonical supported integration-test host; use `relaxed` when a host filesystem cannot provide strict metadata virtualization. From ab029771ba6a783ddb2f99eca50a12df3271bf2a Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 5 Oct 2026 18:22:01 -0600 Subject: [PATCH 02/10] fix(ci): route rootful podman through remote service --- e2e/tests/up/podman_rootful.go | 14 +++- e2e/tests/up/podman_rootful_test.go | 86 ++++++++++++++++++++ pkg/driver/drivercreate/capabilities_test.go | 5 +- 3 files changed, 103 insertions(+), 2 deletions(-) diff --git a/e2e/tests/up/podman_rootful.go b/e2e/tests/up/podman_rootful.go index 3732ba1b94..a72b0b7cd4 100644 --- a/e2e/tests/up/podman_rootful.go +++ b/e2e/tests/up/podman_rootful.go @@ -88,6 +88,18 @@ func shouldAttemptPodmanRecovery(class podmanHealthClass) bool { return class == podmanHealthTimeout || class == podmanHealthUnavailable } +func rootfulPodmanWrapperScript() string { + return `#!/bin/sh +set -eu + +if [ -n "${DOCKER_HOST:-}" ]; then + exec sudo podman --remote --url "$DOCKER_HOST" "$@" +fi + +exec sudo podman "$@" +` +} + // podmanDaemonGate records the first unrecoverable daemon failure so later // specs in the shard skip instead of cascading into identical infrastructure // failures that would bury the actionable one. Scoped to the test process, @@ -311,7 +323,7 @@ func setupRootfulPodman(ctx context.Context, initialDir string) *framework.Frame wrapper, err := os.Create(wrapperPath) //nolint:gosec // G304: test-controlled path framework.ExpectNoError(err) - _, err = wrapper.WriteString("#!/bin/sh\nsudo podman \"$@\"\n") + _, err = wrapper.WriteString(rootfulPodmanWrapperScript()) if err != nil { _ = wrapper.Close() } diff --git a/e2e/tests/up/podman_rootful_test.go b/e2e/tests/up/podman_rootful_test.go index 6b45c46de1..9304a0ab27 100644 --- a/e2e/tests/up/podman_rootful_test.go +++ b/e2e/tests/up/podman_rootful_test.go @@ -1,11 +1,16 @@ package up import ( + "bytes" "context" + "os" + "os/exec" + "path/filepath" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func expiredContext(t *testing.T) context.Context { @@ -106,3 +111,84 @@ func TestRunPodmanDiagnosticsStopsAtBudget(t *testing.T) { }, func(_, _ string) {}) assert.Equal(t, []string{"ps"}, commands) } + +func TestRootfulPodmanWrapper(t *testing.T) { + endpoint := "unix:///run/podman/podman.sock" + emptyEndpoint := "" + cases := []struct { + name string + endpoint *string + wantPrefix []string + args []string + }{ + { + name: "configured endpoint", + endpoint: &endpoint, + wantPrefix: []string{"podman", "--remote", "--url", endpoint}, + args: []string{"ps", "-a"}, + }, + { + name: "preserves arguments", + endpoint: &endpoint, + wantPrefix: []string{"podman", "--remote", "--url", endpoint}, + args: []string{"run", "value with spaces", "$(printf unsafe)"}, + }, + { + name: "unset endpoint uses local mode", + wantPrefix: []string{"podman"}, + args: []string{"ps", "-a"}, + }, + { + name: "empty endpoint uses local mode", + endpoint: &emptyEndpoint, + wantPrefix: []string{"podman"}, + args: []string{"ps", "-a"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := runRootfulPodmanWrapper(t, tc.endpoint, tc.args...) + require.Equal(t, append(tc.wantPrefix, tc.args...), got) + require.NotContains(t, got, "-E") + }) + } +} + +func runRootfulPodmanWrapper(t *testing.T, endpoint *string, args ...string) []string { + t.Helper() + + dir := t.TempDir() + capturePath := filepath.Join(dir, "argv") + sudoPath := filepath.Join(dir, "sudo") + sudoScript := `#!/bin/sh +printf '%s\0' "$@" > "$CAPTURE_FILE" + ` + require.NoError(t, os.WriteFile(sudoPath, []byte(sudoScript), 0o600)) + //nolint:gosec // G302: test executable needs owner execute permission. + require.NoError(t, os.Chmod(sudoPath, 0o700)) + + wrapperPath := filepath.Join(dir, "podman-rootful") + require.NoError(t, os.WriteFile(wrapperPath, []byte(rootfulPodmanWrapperScript()), 0o600)) + //nolint:gosec // G302: generated wrapper needs owner execute permission. + require.NoError(t, os.Chmod(wrapperPath, 0o700)) + + //nolint:gosec // G204: generated wrapper path is test-controlled. + cmd := exec.Command(wrapperPath, args...) + cmd.Env = []string{"PATH=" + dir, "CAPTURE_FILE=" + capturePath} + if endpoint != nil { + cmd.Env = append(cmd.Env, "DOCKER_HOST="+*endpoint) + } + output, err := cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + argv, err := os.ReadFile(capturePath) //nolint:gosec // G304: test-controlled capture path. + require.NoError(t, err) + encodedArgs := bytes.Split(argv, []byte{0}) + encodedArgs = encodedArgs[:len(encodedArgs)-1] + got := make([]string, len(encodedArgs)) + for i, arg := range encodedArgs { + got[i] = string(arg) + } + return got +} diff --git a/pkg/driver/drivercreate/capabilities_test.go b/pkg/driver/drivercreate/capabilities_test.go index 298f3d7b6d..d606dcc3b0 100644 --- a/pkg/driver/drivercreate/capabilities_test.go +++ b/pkg/driver/drivercreate/capabilities_test.go @@ -19,6 +19,7 @@ func TestBuiltInCapabilities(t *testing.T) { name string runtime string images, imageRunner, streaming, chown, argv bool + recreateRequired bool recreate driver.RecreateMode }{ {name: provider.DockerDriver, images: true, imageRunner: true, recreate: driver.RecreateDelete}, @@ -27,7 +28,7 @@ func TestBuiltInCapabilities(t *testing.T) { {name: provider.CustomDriver, streaming: true, chown: true, recreate: driver.RecreateStop}, { name: provider.MicrosandboxDriver, images: true, imageRunner: true, - chown: true, argv: true, recreate: driver.RecreateDelete, + chown: true, argv: true, recreateRequired: true, recreate: driver.RecreateOnRun, }, } { t.Run(tc.name+tc.runtime, func(t *testing.T) { @@ -55,6 +56,8 @@ func TestBuiltInCapabilities(t *testing.T) { assert.Equal(t, tc.streaming, driver.DriverRequiresMountStreaming(bundle.Runtime)) assert.Equal(t, tc.chown, driver.DriverRequiresWorkspaceChown(bundle.Runtime)) assert.Equal(t, tc.recreate, driver.DriverRecreateMode(bundle.Runtime)) + _, recreateRequired := bundle.Runtime.(driver.RecreateRequiredDriver) + assert.Equal(t, tc.recreateRequired, recreateRequired) if tc.name == provider.MicrosandboxDriver { assert.NotSame(t, bundle.Runtime, bundle.Images) _, runtimeImages := bundle.Runtime.(driver.ImageBackend) From 9e330decadee3677e1b8fb32a96a76f11728ce2e Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 5 Oct 2026 18:59:18 -0600 Subject: [PATCH 03/10] refactor(devcontainer): separate container recreation preparation --- pkg/devcontainer/single.go | 119 +++++++++++++-------- pkg/devcontainer/single_recreate_test.go | 130 +++++++++++++++++++++++ 2 files changed, 202 insertions(+), 47 deletions(-) create mode 100644 pkg/devcontainer/single_recreate_test.go diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index 99705ea330..745d5ed131 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -138,55 +138,10 @@ func (r *runner) resolveContainer( params *resolveParams, containerDetails *config.ContainerDetails, ) (*resolvedContainer, error) { - if err := r.applyDriverRecreateRequirement(containerDetails, params); err != nil { + if err := r.prepareContainerRecreation(ctx, containerDetails, params); err != nil { return nil, err } - options := params.options - if containerDetails != nil && !options.Recreate && - r.needsTerminalSecretEnvironmentMigration(containerDetails) { - if err := r.validateTerminalSecretEnvironmentSupport(); err != nil { - return nil, err - } - if params.parsedConfig.Config.ContainerID != "" { - return nil, fmt.Errorf( - "cannot inject attached terminal secrets into externally managed container: "+ - "the container does not have the required %s tmpfs mount", - config.SecretsEnvDir, - ) - } - log.Info( - "recreating workspace because attached terminal secrets require the secure runtime mount", - ) - options.Recreate = true - params.options.Recreate = true - } - if containerDetails != nil && !options.Recreate && - needsSecretFileMountMigration(containerDetails, options.SecretsMount) { - if !driver.DriverSupportsMountType(r.driver, driver.MountTypeTmpfs) { - return nil, fmt.Errorf( - "the current provider does not support securely mounting workspace file secrets", - ) - } - if params.parsedConfig.Config.ContainerID != "" { - return nil, fmt.Errorf( - "cannot inject file secrets into externally managed container: "+ - "the container does not have the required %s tmpfs mount", - config.SecretsMountDir, - ) - } - log.Info("recreating workspace because file secrets require the secure runtime mount") - options.Recreate = true - params.options.Recreate = true - } - - if options.Recreate && params.parsedConfig.Config.ContainerID != "" { - return nil, fmt.Errorf("cannot recreate container not created by Devsy") - } - if err := r.provisioningPreflightForRecreate(ctx, options); err != nil { - return nil, err - } - - if options.Recreate || containerDetails == nil { + if params.options.Recreate || containerDetails == nil { return r.resolveNewContainer(ctx, params) } @@ -202,6 +157,76 @@ func (r *runner) resolveContainer( return r.resolveExistingContainer(ctx, containerDetails, params) } +func (r *runner) prepareContainerRecreation( + ctx context.Context, + details *config.ContainerDetails, + params *resolveParams, +) error { + if err := r.applyDriverRecreateRequirement(details, params); err != nil { + return err + } + if err := r.applyTerminalSecretRecreateRequirement(details, params); err != nil { + return err + } + if err := r.applyFileSecretRecreateRequirement(details, params); err != nil { + return err + } + if params.options.Recreate && params.parsedConfig.Config.ContainerID != "" { + return fmt.Errorf("cannot recreate container not created by Devsy") + } + return r.provisioningPreflightForRecreate(ctx, params.options) +} + +func (r *runner) applyTerminalSecretRecreateRequirement( + details *config.ContainerDetails, + params *resolveParams, +) error { + if details == nil || params.options.Recreate || + !r.needsTerminalSecretEnvironmentMigration(details) { + return nil + } + if err := r.validateTerminalSecretEnvironmentSupport(); err != nil { + return err + } + if params.parsedConfig.Config.ContainerID != "" { + return fmt.Errorf( + "cannot inject attached terminal secrets into externally managed container: "+ + "the container does not have the required %s tmpfs mount", + config.SecretsEnvDir, + ) + } + log.Info( + "recreating workspace because attached terminal secrets require the secure runtime mount", + ) + params.options.Recreate = true + return nil +} + +func (r *runner) applyFileSecretRecreateRequirement( + details *config.ContainerDetails, + params *resolveParams, +) error { + if details == nil || params.options.Recreate || + !needsSecretFileMountMigration(details, params.options.SecretsMount) { + return nil + } + if !driver.DriverSupportsMountType(r.driver, driver.MountTypeTmpfs) { + return fmt.Errorf( + "the current provider does not support securely mounting workspace file secrets", + ) + } + if params.parsedConfig.Config.ContainerID != "" { + return fmt.Errorf( + "cannot inject file secrets into externally managed container: "+ + "the container does not have the required %s tmpfs mount", + config.SecretsMountDir, + ) + } + log.Info("recreating workspace because file secrets require the secure runtime mount") + params.options.Recreate = true + return nil +} + func (r *runner) provisioningPreflightForRecreate(ctx context.Context, options UpOptions) error { if !options.Recreate { return nil diff --git a/pkg/devcontainer/single_recreate_test.go b/pkg/devcontainer/single_recreate_test.go new file mode 100644 index 0000000000..8378556112 --- /dev/null +++ b/pkg/devcontainer/single_recreate_test.go @@ -0,0 +1,130 @@ +package devcontainer + +import ( + "errors" + "testing" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/driver" + "github.com/stretchr/testify/require" +) + +type secretMigrationDriver struct { + *migrationMockDriver + tmpfsSupported bool +} + +func (d *secretMigrationDriver) SupportsMountType(mountType string) bool { + return mountType == driver.MountTypeTmpfs && d.tmpfsSupported +} + +func newSecretMigrationRunner() (*runner, *resolveParams, *secretMigrationDriver) { + d := &secretMigrationDriver{ + migrationMockDriver: &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + }, + } + params := recreateResolveParams() + params.options.Recreate = false + return newTestRunner(d), params, d +} + +func TestSecretMigrationPreflightFailurePreservesExistingContainer(t *testing.T) { + for _, tc := range []struct { + name string + terminal, file, driver bool + explicit, tmpfsSupported bool + }{ + {name: "terminal", terminal: true, tmpfsSupported: true}, + {name: "file", file: true, tmpfsSupported: true}, + {name: "both", terminal: true, file: true, tmpfsSupported: true}, + {name: "driver migration precedes secrets", terminal: true, file: true, driver: true}, + {name: "explicit recreation skips migration", terminal: true, file: true, explicit: true}, + } { + t.Run(tc.name, func(t *testing.T) { + r, params, d := newSecretMigrationRunner() + d.required, d.tmpfsSupported = tc.driver, tc.tmpfsSupported + d.provisioningErr = errors.New("provisioning unavailable") + params.options.Recreate = tc.explicit + if tc.terminal { + r.workspaceConfig.CLIOptions.TerminalSecretEnvNames = []string{ + terminalSecretSentinelName, + } + } + if tc.file { + params.options.SecretsMount = []string{secretFileMountRequestSentinel} + } + _, err := r.resolveContainer(t.Context(), params, runningContainerDetails()) + require.ErrorIs(t, err, d.provisioningErr) + require.True(t, params.options.Recreate) + require.True(t, d.provisioningCalled) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) + }) + } +} + +func TestSecretMigrationRejectsBeforeProvisioning(t *testing.T) { + for _, tc := range []struct { + name string + terminal, file, external bool + tmpfsSupported bool + want string + }{ + {name: "terminal unsupported", terminal: true, want: "securely injecting workspace secrets"}, + {name: "file unsupported", file: true, want: "securely mounting workspace file secrets"}, + { + name: "terminal external", terminal: true, external: true, tmpfsSupported: true, + want: "cannot inject attached terminal secrets into externally managed container", + }, + { + name: "file external", file: true, external: true, tmpfsSupported: true, + want: "cannot inject file secrets into externally managed container", + }, + { + name: "terminal rejection precedes file", terminal: true, file: true, + external: true, tmpfsSupported: true, want: "cannot inject attached terminal secrets", + }, + } { + t.Run(tc.name, func(t *testing.T) { + r, params, d := newSecretMigrationRunner() + d.tmpfsSupported = tc.tmpfsSupported + if tc.terminal { + r.workspaceConfig.CLIOptions.TerminalSecretEnvNames = []string{ + terminalSecretSentinelName, + } + } + if tc.file { + params.options.SecretsMount = []string{secretFileMountRequestSentinel} + } + if tc.external { + params.parsedConfig.Config.ContainerID = testContainerID + } + _, err := r.resolveContainer(t.Context(), params, runningContainerDetails()) + require.ErrorContains(t, err, tc.want) + require.False(t, params.options.Recreate) + require.False(t, d.provisioningCalled) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) + }) + } +} + +func TestPrepareContainerRecreationKeepsCompatibleContainer(t *testing.T) { + for _, details := range []*config.ContainerDetails{ + nil, + {Mounts: []config.ContainerMount{ + {Type: driver.MountTypeTmpfs, Destination: config.SecretsEnvDir}, + {Type: driver.MountTypeTmpfs, Destination: config.SecretsMountDir}, + }}, + } { + r, params, d := newSecretMigrationRunner() + r.workspaceConfig.CLIOptions.TerminalSecretEnvNames = []string{terminalSecretSentinelName} + params.options.SecretsMount = []string{secretFileMountRequestSentinel} + require.NoError(t, r.prepareContainerRecreation(t.Context(), details, params)) + require.False(t, params.options.Recreate) + require.False(t, d.provisioningCalled) + } +} From 88aa51ab5a295fc331507d4ccd5e61020e6692b6 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 5 Oct 2026 21:03:22 -0600 Subject: [PATCH 04/10] fix(microsandbox): preserve owner and image lifecycle consistency --- e2e/tests/up/provider_microsandbox.go | 29 ++++++- pkg/devcontainer/metadata/metadata.go | 3 + pkg/devcontainer/single.go | 52 +++++++++++- pkg/devcontainer/single_test.go | 83 ++++++++++++++++++- pkg/driver/microsandbox/cliclient.go | 4 +- pkg/driver/microsandbox/cliclient_test.go | 10 ++- pkg/driver/microsandbox/image_user.go | 50 +++++++---- pkg/driver/microsandbox/image_user_test.go | 40 +++++---- pkg/driver/microsandbox/microsandbox.go | 77 +++++++++++++---- .../microsandbox/workspace_owner_test.go | 77 ++++++++++++++++- pkg/driver/types.go | 13 ++- .../docs/developing-providers/driver.mdx | 15 +++- 12 files changed, 387 insertions(+), 66 deletions(-) diff --git a/e2e/tests/up/provider_microsandbox.go b/e2e/tests/up/provider_microsandbox.go index 5f10021e9b..3cf8a60106 100644 --- a/e2e/tests/up/provider_microsandbox.go +++ b/e2e/tests/up/provider_microsandbox.go @@ -67,7 +67,7 @@ var _ = ginkgo.Describe( }) // full up: boots the microVM, streams in the agent, opens the tunnel - err = f.DevsyUp(ctx, tempDir) + err = f.DevsyUp(ctx, tempDir, "--devcontainer", ".devcontainer.json") framework.ExpectNoError(err) ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, tempDir) @@ -106,6 +106,9 @@ var _ = ginkgo.Describe( framework.ExpectNoError(err) gomega.Expect(strings.TrimSpace(string(workloadUser))).To(gomega.Equal("0")) + developer, err := f.DevsySSHOnce(ctx, tempDir, "id -un") + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(developer)).To(gomega.Equal("vscode")) assertMicrosandboxHostEntries(ctx, f, tempDir, "fresh") err = f.DevsyWorkspaceStop(ctx, tempDir) framework.ExpectNoError(err) @@ -117,6 +120,28 @@ var _ = ginkgo.Describe( framework.ExpectNoError(err) gomega.Expect(microsandboxCreationTime(ctx, sandbox)).NotTo(gomega.Equal(createdAt)) assertMicrosandboxHostEntries(ctx, f, tempDir, "recreated") + + previousCreation := microsandboxCreationTime(ctx, sandbox) + configPath := filepath.Join(tempDir, ".devcontainer.json") + // #nosec G304 -- configuration copied into the test-owned temporary directory + data, err := os.ReadFile(configPath) + framework.ExpectNoError(err) + var devConfig map[string]any + framework.ExpectNoError(json.Unmarshal(data, &devConfig)) + devConfig["remoteUser"] = "root" + data, err = json.Marshal(devConfig) + framework.ExpectNoError(err) + framework.ExpectNoError(os.WriteFile(configPath, data, 0o600)) + err = f.DevsyWorkspaceStop(ctx, tempDir) + framework.ExpectNoError(err) + err = f.DevsyUp(ctx, tempDir, "--devcontainer", ".devcontainer.json") + framework.ExpectNoError(err) + gomega.Expect(microsandboxCreationTime(ctx, sandbox)). + NotTo(gomega.Equal(previousCreation)) + developer, err = f.DevsySSHOnce(ctx, tempDir, "id -u") + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(developer)).To(gomega.Equal("0")) + assertMicrosandboxHostEntries(ctx, f, tempDir, "identity-changed") }, ginkgo.SpecTimeout(framework.TimeoutModerate())) }, ) @@ -131,7 +156,7 @@ func assertMicrosandboxHostEntries( identity, err := f.DevsySSHOnce(ctx, hostPath, `printf '%s:%s' "$(id -u)" "$(id -g)"`) framework.ExpectNoError(err) identity = strings.TrimSpace(identity) - gomega.Expect(identity).NotTo(gomega.Equal("0:0")) + for name, mode := range map[string]os.FileMode{prefix + "-file": 0o644, prefix + "-dir": 0o755} { hostEntry := filepath.Join(hostPath, name) if mode == 0o644 { diff --git a/pkg/devcontainer/metadata/metadata.go b/pkg/devcontainer/metadata/metadata.go index 57878b5379..91b492f294 100644 --- a/pkg/devcontainer/metadata/metadata.go +++ b/pkg/devcontainer/metadata/metadata.go @@ -10,6 +10,9 @@ import ( const ImageMetadataLabel = pkgconfig.DevcontainerMetadataLabel +// CreationConfigLabel identifies a metadata chain ending with the creation-time workspace config. +const CreationConfigLabel = "devsy.sh/metadata-creation-config" + const metadataLabelSizeWarningThreshold = 100 * 1024 func MarshalImageMetadata(raw []*config.ImageMetadata) ([]byte, error) { diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index 745d5ed131..fd2f3240a5 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -162,7 +162,7 @@ func (r *runner) prepareContainerRecreation( details *config.ContainerDetails, params *resolveParams, ) error { - if err := r.applyDriverRecreateRequirement(details, params); err != nil { + if err := r.applyDriverRecreateRequirement(ctx, details, params); err != nil { return err } if err := r.applyTerminalSecretRecreateRequirement(details, params); err != nil { @@ -324,6 +324,14 @@ func (r *runner) mergeExistingContainerConfig( return nil, err } + return r.mergeContainerMetadata(ctx, imageMetadataConfig, p) +} + +func (r *runner) mergeContainerMetadata( + ctx context.Context, + imageMetadataConfig *config.ImageMetadataConfig, + p *resolveParams, +) (*config.MergedDevContainerConfig, error) { if p.options.ExtraDevContainerPath != "" { if imageMetadataConfig == nil { imageMetadataConfig = &config.ImageMetadataConfig{} @@ -763,6 +771,9 @@ func (r *runner) runContainer( } runOptions.Env = r.addExtraEnvVars(runOptions.Env) + if _, ok := r.driver.(driver.RecreateRequiredDriver); ok { + runOptions.Labels = append(runOptions.Labels, metadata.CreationConfigLabel+"="+stringTrue) + } // Image drivers (Docker, Apple) build and run a local OCI image. if imageDriver, ok := r.driver.(driver.ImageRunner); ok { @@ -886,6 +897,7 @@ func (r *runner) getDockerlessRunOptions( Image: image, User: containerRootUser, RemoteUser: effectiveRemoteUser(mergedConfig, buildInfo.Dockerless.User), + Dockerless: true, Entrypoint: "/.dockerless/dockerless", Cmd: []string{ "start", @@ -1140,13 +1152,29 @@ func effectiveRemoteUser(merged *config.MergedDevContainerConfig, containerUser } func (r *runner) applyDriverRecreateRequirement( + ctx context.Context, details *config.ContainerDetails, p *resolveParams, ) error { if details == nil || p.options.Recreate { return nil } - required, reason := driver.DriverRequiresRecreate(r.driver, details) + if _, ok := r.driver.(driver.RecreateRequiredDriver); !ok { + return nil + } + merged, err := r.currentContainerIdentity(ctx, details, p) + if err != nil { + return err + } + containerUser := merged.ContainerUser + if containerUser == "" { + containerUser = details.Config.Labels[config.UserLabel] + } + required, reason := driver.DriverRequiresRecreate( + r.driver, + details, + effectiveRemoteUser(merged, containerUser), + ) if !required { return nil } @@ -1157,3 +1185,23 @@ func (r *runner) applyDriverRecreateRequirement( p.options.Recreate = true return nil } + +func (r *runner) currentContainerIdentity( + ctx context.Context, + details *config.ContainerDetails, + p *resolveParams, +) (*config.MergedDevContainerConfig, error) { + imageMetadata, err := metadata.GetImageMetadataFromContainer(details, p.substitutionContext) + if err != nil { + return nil, err + } + if details.Config.Labels[metadata.CreationConfigLabel] == stringTrue && + len(imageMetadata.Config) > 0 { + imageMetadata.Config = imageMetadata.Config[:len(imageMetadata.Config)-1] + } + imageMetadata.Config = append( + imageMetadata.Config, + metadata.DevContainerConfigToImageMetadata(p.parsedConfig.Config), + ) + return r.mergeContainerMetadata(ctx, imageMetadata, p) +} diff --git a/pkg/devcontainer/single_test.go b/pkg/devcontainer/single_test.go index a4f65b745c..3e19d8e3df 100644 --- a/pkg/devcontainer/single_test.go +++ b/pkg/devcontainer/single_test.go @@ -8,6 +8,7 @@ import ( pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/devcontainer/metadata" "github.com/devsy-org/devsy/pkg/provider" "github.com/devsy-org/devsy/pkg/types" "github.com/stretchr/testify/require" @@ -430,9 +431,14 @@ type migrationMockDriver struct { *provisioningPreflightMockDriver required bool migrationCalls int + remoteUser string } -func (d *migrationMockDriver) RequiresRecreate(*config.ContainerDetails) (bool, string) { +func (d *migrationMockDriver) RequiresRecreate( + _ *config.ContainerDetails, + user string, +) (bool, string) { + d.remoteUser = user d.migrationCalls++ return d.required, "workspace mount contract changed" } @@ -461,7 +467,11 @@ func TestDriverMountContractMigration(t *testing.T) { if tt.external { params.parsedConfig.Config.ContainerID = testContainerID } - err := r.applyDriverRecreateRequirement(runningContainerDetails(), params) + err := r.applyDriverRecreateRequirement( + context.Background(), + runningContainerDetails(), + params, + ) if tt.wantError { require.ErrorContains(t, err, "cannot migrate externally managed container") } else { @@ -526,4 +536,73 @@ func TestRunOptionsSeparateDeveloperIdentity(t *testing.T) { require.NoError(t, err) require.Equal(t, containerRootUser, options.User) require.Equal(t, "vscode", options.RemoteUser) + require.True(t, options.Dockerless) +} + +func TestMigrationUsesCurrentDeveloperIdentity(t *testing.T) { + for _, tt := range []struct{ remote, container, image, want string }{ + {"new-user", containerRootUser, "node", "new-user"}, + {"", "node", "vscode", "node"}, + {"", "", "node", "node"}, + {"", "", "", containerRootUser}, + } { + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + } + p := recreateResolveParams() + p.options.Recreate = false + p.parsedConfig.Config.RemoteUser = tt.remote + p.parsedConfig.Config.ContainerUser = tt.container + details := runningContainerDetails() + details.Config.Labels[config.UserLabel] = tt.image + require.NoError( + t, + newTestRunner(d).applyDriverRecreateRequirement(context.Background(), details, p), + ) + require.Equal(t, tt.want, d.remoteUser) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) + } +} + +func TestMigrationUsesImageMetadataDeveloperIdentity(t *testing.T) { + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + } + p := recreateResolveParams() + p.options.Recreate = false + p.substitutionContext = &config.SubstitutionContext{} + details := runningContainerDetails() + details.Config.Labels["devcontainer.metadata"] = `[{"remoteUser":"feature-user"}]` + require.NoError( + t, + newTestRunner(d).applyDriverRecreateRequirement(context.Background(), details, p), + ) + require.Equal(t, "feature-user", d.remoteUser) +} + +func TestMigrationReplacesSavedUserSettingsWithCurrentConfiguration(t *testing.T) { + for _, tt := range []struct{ current, want string }{{"root", "root"}, {"", "feature-user"}} { + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + } + p := recreateResolveParams() + p.options.Recreate = false + p.substitutionContext = &config.SubstitutionContext{} + p.parsedConfig.Config.RemoteUser = tt.current + details := runningContainerDetails() + details.Config.Labels[metadata.CreationConfigLabel] = stringTrue + details.Config.Labels[metadata.ImageMetadataLabel] = `[{"remoteUser":"feature-user"},{"remoteUser":"old-user"}]` + require.NoError( + t, + newTestRunner(d).applyDriverRecreateRequirement(context.Background(), details, p), + ) + require.Equal(t, tt.want, d.remoteUser) + } } diff --git a/pkg/driver/microsandbox/cliclient.go b/pkg/driver/microsandbox/cliclient.go index aa0f7002c8..e6a6453ff2 100644 --- a/pkg/driver/microsandbox/cliclient.go +++ b/pkg/driver/microsandbox/cliclient.go @@ -53,7 +53,7 @@ func (cliClient) Version(ctx context.Context) (string, error) { } func (c cliClient) EnsureImage(ctx context.Context, imageRef string, builtLocally bool) error { - if builtLocally { + if builtLocally || localImageAvailable(ctx, c.dockerPath, imageRef) { return loadFromDocker(ctx, imageRef, c.dockerPath) } // #nosec G204 -- args are a resolved binary path and a validated image ref @@ -311,7 +311,7 @@ func redactArgs(args []string) string { func loadFromDocker(ctx context.Context, image, dockerPath string) error { if dockerPath == "" { - dockerPath = "docker" + dockerPath = dockerExecutable } docker, err := exec.LookPath(dockerPath) if err != nil { diff --git a/pkg/driver/microsandbox/cliclient_test.go b/pkg/driver/microsandbox/cliclient_test.go index de6ef8ddac..14f77b8e85 100644 --- a/pkg/driver/microsandbox/cliclient_test.go +++ b/pkg/driver/microsandbox/cliclient_test.go @@ -235,7 +235,8 @@ func hasFlagValue(args []string, flag, value string) bool { func TestInspectStoppedSandboxRetainsMountContract(t *testing.T) { d := newDriver(newFakeClient(), nil, specDefaults{}) output := fmt.Sprintf( - `{"name":"%s","status":"Stopped","active_config":null,"config":{"labels":{"%s":"%s","%s":"root"}}}`, + `{"name":"%s","status":"Stopped","active_config":null,"config":{"labels":{`+ + `"%s":"%s","devsy.sh/microsandbox-workspace-user":"root","%s":"root"}}}`, wsName, workspaceMountContractLabel, d.workspaceMountContract(), @@ -246,7 +247,7 @@ func TestInspectStoppedSandboxRetainsMountContract(t *testing.T) { t.Fatal(err) } details := toContainerDetails(info) - required, _ := d.RequiresRecreate(details) + required, _ := d.RequiresRecreate(details, rootUser) if required { t.Fatal("stopped sandbox lost persisted contract") } @@ -258,7 +259,8 @@ func TestInspectStoppedSandboxRetainsMountContract(t *testing.T) { func TestInspectUsesActiveContractWhenConfigHasPendingChanges(t *testing.T) { d := newDriver(newFakeClient(), nil, specDefaults{}) output := fmt.Sprintf( - `{"name":"%s","status":"Running","active_config":{"labels":{"%s":"%s"}},"config":{"labels":{"%s":"pending"}}}`, + `{"name":"%s","status":"Running","active_config":{"labels":{`+ + `"%s":"%s","devsy.sh/microsandbox-workspace-user":"root"}},"config":{"labels":{"%s":"pending"}}}`, wsName, workspaceMountContractLabel, d.workspaceMountContract(), @@ -268,7 +270,7 @@ func TestInspectUsesActiveContractWhenConfigHasPendingChanges(t *testing.T) { if err != nil { t.Fatal(err) } - required, _ := d.RequiresRecreate(toContainerDetails(info)) + required, _ := d.RequiresRecreate(toContainerDetails(info), rootUser) if required { t.Fatal("pending configuration replaced active mount contract") } diff --git a/pkg/driver/microsandbox/image_user.go b/pkg/driver/microsandbox/image_user.go index f902552276..a4e4662511 100644 --- a/pkg/driver/microsandbox/image_user.go +++ b/pkg/driver/microsandbox/image_user.go @@ -11,6 +11,7 @@ import ( "runtime" "strconv" "strings" + "time" "github.com/devsy-org/devsy/pkg/image" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -21,6 +22,7 @@ import ( const ( maxAccountFileSize = 4 << 20 rootUser = "root" + dockerExecutable = "docker" passwdPath = "etc/passwd" groupPath = "etc/group" ) @@ -46,24 +48,32 @@ func (r filesystemUserResolver) Resolve( if owner, complete, err := explicitOwner(user); complete || err != nil { return owner, err } - var img v1.Image - var err error - if builtLocally { - var cleanup func() - img, cleanup, err = r.localImage(ctx, ref) - if err != nil { - return nil, err - } - defer cleanup() - } else { - img, err = image.GetImageForArch(ctx, ref, runtime.GOARCH) - if err != nil { - return nil, err - } + img, cleanup, err := r.openImage(ctx, ref, builtLocally) + if err != nil { + return nil, err } + defer cleanup() return ownerFromImage(ctx, img, user) } +func (r filesystemUserResolver) openImage( + ctx context.Context, + ref string, + builtLocally bool, +) (v1.Image, func(), error) { + if err := ctx.Err(); err != nil { + return nil, nil, err + } + if builtLocally || localImageAvailable(ctx, r.dockerPath, ref) { + return r.localImage(ctx, ref) + } + img, err := image.GetImageForArch(ctx, ref, runtime.GOARCH) + if err != nil { + return nil, nil, err + } + return img, func() {}, nil +} + // The archive must remain available until lazy layer reads finish. func (r filesystemUserResolver) localImage( ctx context.Context, @@ -76,7 +86,7 @@ func (r filesystemUserResolver) localImage( cleanup := func() { _ = os.Remove(archive.Name()) } docker := r.dockerPath if docker == "" { - docker = "docker" + docker = dockerExecutable } // #nosec G204 -- configured Docker-compatible executable, fixed subcommand, image argument cmd := exec.CommandContext(ctx, docker, "save", ref) @@ -286,3 +296,13 @@ func groupID(groups, group string) (uint32, error) { } return 0, fmt.Errorf("group %q not found in final image /etc/group", group) } + +func localImageAvailable(ctx context.Context, dockerPath, ref string) bool { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + if dockerPath == "" { + dockerPath = dockerExecutable + } + // #nosec G204 -- configured Docker-compatible executable and fixed inspection arguments + return exec.CommandContext(ctx, dockerPath, "image", "inspect", ref).Run() == nil +} diff --git a/pkg/driver/microsandbox/image_user_test.go b/pkg/driver/microsandbox/image_user_test.go index 54a13593da..78b78d7ddc 100644 --- a/pkg/driver/microsandbox/image_user_test.go +++ b/pkg/driver/microsandbox/image_user_test.go @@ -37,7 +37,7 @@ func (s *imageUserSuite) TestResolution() { {"0", mountOwner{}}, {testUser, mountOwner{1000, 1002}}, {"1000", mountOwner{1000, 1002}}, - {"1000:1001", mountOwner{1000, 1001}}, + {testNumericIdentity, mountOwner{1000, 1001}}, {"vscode:developers", mountOwner{1000, 1001}}, {"vscode:1003", mountOwner{1000, 1003}}, {"1000:developers", mountOwner{1000, 1001}}, @@ -59,7 +59,7 @@ func (s *imageUserSuite) TestResolution() { } func (s *imageUserSuite) TestMissingAccounts() { - owner, err := ownerFromImage(context.Background(), empty.Image, "1000:1001") + owner, err := ownerFromImage(context.Background(), empty.Image, testNumericIdentity) s.Require().NoError(err) s.Equal(mountOwner{1000, 1001}, *owner) _, err = ownerFromImage(context.Background(), empty.Image, testUser) @@ -126,6 +126,11 @@ func (s *imageUserSuite) TestLocalFinalImageUsesConfiguredCLI() { s.layer(map[string]string{passwdPath: "vscode:x:2000:2001:dev:/home/vscode:/bin/sh\n"}), ) s.Require().NoError(err) + cfg, err := img.ConfigFile() + s.Require().NoError(err) + cfg.Config.User = testUser + img, err = mutate.ConfigFile(img, cfg) + s.Require().NoError(err) dir := s.T().TempDir() archive := filepath.Join(dir, "image.tar") tag, err := name.NewTag("final-image:latest") @@ -136,21 +141,26 @@ func (s *imageUserSuite) TestLocalFinalImageUsesConfiguredCLI() { s.T().Setenv("DEVSY_TEST_ARGS", argsFile) executable := filepath.Join(dir, "configured-cli") script := `#!/bin/sh +if [ "$1" = image ]; then exit 0; fi printf '%s\n' "$@" > "$DEVSY_TEST_ARGS" cat "$DEVSY_TEST_ARCHIVE" ` s.Require().NoError(os.WriteFile(executable, []byte(script), 0o700)) - owner, err := (filesystemUserResolver{dockerPath: executable}).Resolve( - context.Background(), - "final-image:latest", - true, - testUser, - ) - s.Require().NoError(err) - s.Equal(mountOwner{2000, 2001}, *owner) + for _, built := range []bool{true, false} { + owner, err := (filesystemUserResolver{dockerPath: executable}).Resolve( + context.Background(), "final-image:latest", built, testUser, + ) + s.Require().NoError(err) + s.Equal(mountOwner{2000, 2001}, *owner) + } args, err := os.ReadFile(argsFile) s.Require().NoError(err) s.Equal("save\nfinal-image:latest\n", string(args)) + d := newDriver(newFakeClient(), nil, specDefaults{}) + d.dockerPath = executable + details, err := d.InspectImage(context.Background(), "final-image:latest") + s.Require().NoError(err) + s.Equal(testUser, details.Config.User) } //nolint:gosec // Test-created executable and archive paths under TempDir. @@ -167,12 +177,14 @@ func (s *imageUserSuite) TestLocalImportUsesConfiguredCLIAndDrainsArchive() { s.T().Setenv("DEVSY_TEST_TARGET", target) s.T().Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) saveCLI := filepath.Join(dir, "save-cli") - s.Require(). - NoError(os.WriteFile(saveCLI, []byte("#!/bin/sh\ncat \"$DEVSY_TEST_ARCHIVE\"\n"), 0o700)) + saveScript := "#!/bin/sh\nif [ \"$1\" = image ]; then exit 0; fi\ncat \"$DEVSY_TEST_ARCHIVE\"\n" + s.Require().NoError(os.WriteFile(saveCLI, []byte(saveScript), 0o700)) s.Require(). NoError(os.WriteFile(filepath.Join(dir, "msb"), []byte("#!/bin/sh\ncat > \"$DEVSY_TEST_TARGET\"\n"), 0o700)) - s.Require(). - NoError((cliClient{dockerPath: saveCLI}).EnsureImage(context.Background(), "final-image:latest", true)) + for _, built := range []bool{true, false} { + s.Require(). + NoError((cliClient{dockerPath: saveCLI}).EnsureImage(context.Background(), "final-image:latest", built)) + } data, err := os.ReadFile(target) s.Require().NoError(err) s.Equal(content, string(data)) diff --git a/pkg/driver/microsandbox/microsandbox.go b/pkg/driver/microsandbox/microsandbox.go index 49ceb12d26..b90d7f41c4 100644 --- a/pkg/driver/microsandbox/microsandbox.go +++ b/pkg/driver/microsandbox/microsandbox.go @@ -7,6 +7,7 @@ import ( "encoding/hex" "fmt" "io" + "maps" "math" "regexp" "runtime" @@ -18,7 +19,6 @@ import ( pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/driver" - "github.com/devsy-org/devsy/pkg/image" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/provider" ) @@ -40,6 +40,7 @@ type specDefaults struct { type microsandboxDriver struct { client sandboxClient + dockerPath string idLabels []string defaults specDefaults workspaceMountPolicy workspaceMountPolicy @@ -124,7 +125,8 @@ func NewMicrosandboxDriver( ) d := newDriver(client, workspaceInfo.CLIOptions.IDLabels, defaults) d.workspaceMountPolicy = workspacePolicy - d.userResolver = filesystemUserResolver{dockerPath: workspaceInfo.Agent.Docker.Path} + d.dockerPath = workspaceInfo.Agent.Docker.Path + d.userResolver = filesystemUserResolver{dockerPath: d.dockerPath} return d, nil } @@ -238,11 +240,19 @@ func (d *microsandboxDriver) InspectImage( ctx context.Context, imageName string, ) (*config.ImageDetails, error) { - arch, err := d.TargetArchitecture(ctx, "") - if err != nil { + if _, err := d.TargetArchitecture(ctx, ""); err != nil { return nil, err } - cfg, _, err := image.GetImageConfigForArch(ctx, imageName, arch) + img, cleanup, err := (filesystemUserResolver{dockerPath: d.dockerPath}).openImage( + ctx, + imageName, + false, + ) + if err != nil { + return nil, fmt.Errorf("get image for %s: %w", imageName, err) + } + defer cleanup() + cfg, err := img.ConfigFile() if err != nil { return nil, fmt.Errorf("get image config for %s: %w", imageName, err) } @@ -398,11 +408,15 @@ func (d *microsandboxDriver) buildSpec( hostReqs *config.HostRequirements, owner *mountOwner, ) sandboxSpec { - labels := config.ListToObject(config.GetIDLabels(workspaceID, d.idLabels)) + labels := config.ListToObject(options.Labels) if labels == nil { labels = map[string]string{} } + maps.Copy(labels, config.ListToObject(config.GetIDLabels(workspaceID, d.idLabels))) labels[workspaceMountContractLabel] = d.workspaceMountContract() + if owner != nil { + labels[workspaceRemoteUserLabel] = workspaceRemoteUser(options) + } if options.User != "" { labels[userLabel] = options.User } @@ -605,14 +619,27 @@ func (d *microsandboxDriver) RequiresMountStreaming() bool { return false } func (d *microsandboxDriver) RecreateMode() driver.RecreateMode { return driver.RecreateOnRun } -const workspaceMountContractLabel = "devsy.sh/microsandbox-workspace-mount" +const ( + workspaceMountContractLabel = "devsy.sh/microsandbox-workspace-mount" + workspaceRemoteUserLabel = "devsy.sh/microsandbox-workspace-user" +) -func (d *microsandboxDriver) RequiresRecreate(details *config.ContainerDetails) (bool, string) { - if details == nil || - details.Config.Labels[workspaceMountContractLabel] == d.workspaceMountContract() { +func (d *microsandboxDriver) RequiresRecreate( + details *config.ContainerDetails, + remoteUser string, +) (bool, string) { + if details == nil { return false, "" } - return true, "microsandbox workspace mount contract changed" + if details.Config.Labels[workspaceMountContractLabel] != d.workspaceMountContract() { + return true, "microsandbox workspace mount contract changed" + } + if d.workspaceMountPolicy.StatVirtualization != statVirtOff && + details.Config.Labels[workspaceRemoteUserLabel] != "" && + details.Config.Labels[workspaceRemoteUserLabel] != remoteUser { + return true, "microsandbox workspace developer identity changed" + } + return false, "" } func (d *microsandboxDriver) workspaceMountContract() string { @@ -621,7 +648,7 @@ func (d *microsandboxDriver) workspaceMountContract() string { owner = "remote-user" } return fmt.Sprintf( - "v2;stat=%s;host=%s;owner=%s", + "v3;stat=%s;host=%s;owner=%s", d.workspaceMountPolicy.StatVirtualization, d.workspaceMountPolicy.HostPermissions, owner, @@ -636,12 +663,16 @@ func (d *microsandboxDriver) resolveWorkspaceOwner( bindMount(options.WorkspaceMount) == nil { return nil, nil } - user := options.RemoteUser - if user == "" { - user = options.User - } - if user == "" { - user = rootUser + user := workspaceRemoteUser(options) + if options.Dockerless { + if user != rootUser { + return nil, fmt.Errorf( + "microsandbox cannot resolve a non-root Dockerless workspace owner before VM creation; " + + "use a prebuilt developer image, remoteUser=root, or " + + "workspaceStatVirtualization=off with private host permissions", + ) + } + return &mountOwner{}, nil } owner, err := d.userResolver.Resolve(ctx, options.Image, options.ImageBuilt, user) if err != nil { @@ -653,3 +684,13 @@ func (d *microsandboxDriver) resolveWorkspaceOwner( } return owner, nil } + +func workspaceRemoteUser(options *driver.RunOptions) string { + if options.RemoteUser != "" { + return options.RemoteUser + } + if options.User != "" { + return options.User + } + return rootUser +} diff --git a/pkg/driver/microsandbox/workspace_owner_test.go b/pkg/driver/microsandbox/workspace_owner_test.go index d9283e40b4..be5e932664 100644 --- a/pkg/driver/microsandbox/workspace_owner_test.go +++ b/pkg/driver/microsandbox/workspace_owner_test.go @@ -10,6 +10,8 @@ import ( "github.com/stretchr/testify/require" ) +const testNumericIdentity = "1000:1001" + type recordingUserResolver struct { image string built bool @@ -100,6 +102,7 @@ func TestWorkspaceOwnerPolicyOff(t *testing.T) { options := &driver.RunOptions{ WorkspaceMount: &config.Mount{Source: testBindSrc, Target: testBindDst}, RemoteUser: testUser, + Dockerless: true, } owner, err := d.resolveWorkspaceOwner(context.Background(), options) require.NoError(t, err) @@ -118,13 +121,16 @@ func TestWorkspaceMountContractRecreation(t *testing.T) { for _, label := range []string{"", "v1", d.workspaceMountContract(), "v2;stat=relaxed;host=mirror;owner=remote-user"} { details := &config.ContainerDetails{ Config: config.ContainerDetailsConfig{ - Labels: map[string]string{workspaceMountContractLabel: label}, + Labels: map[string]string{ + workspaceMountContractLabel: label, + workspaceRemoteUserLabel: rootUser, + }, }, } - required, _ := driver.DriverRequiresRecreate(d, details) + required, _ := driver.DriverRequiresRecreate(d, details, rootUser) require.Equal(t, label != d.workspaceMountContract(), required) } - required, _ := d.RequiresRecreate(nil) + required, _ := d.RequiresRecreate(nil, rootUser) require.False(t, required) } @@ -147,3 +153,68 @@ func TestWorkspaceOwnerFallbackIdentity(t *testing.T) { require.Equal(t, tt.want, resolver.user) } } + +func TestWorkspaceIdentityChangeRequiresRecreation(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + spec := d.buildSpec( + wsID, + &driver.RunOptions{User: rootUser, RemoteUser: testUser}, + nil, + &mountOwner{UID: 1000, GID: 1000}, + ) + details := toContainerDetails(&sandboxInfo{Labels: spec.Labels}) + for _, user := range []string{testUser, "2000:2001", rootUser} { + required, _ := d.RequiresRecreate(details, user) + require.Equal(t, user != testUser, required) + } + d.workspaceMountPolicy.StatVirtualization = statVirtOff + details.Config.Labels[workspaceMountContractLabel] = d.workspaceMountContract() + required, _ := d.RequiresRecreate(details, rootUser) + require.False(t, required) +} + +func TestDockerlessOwnerNeverLooksUpRunnerAccounts(t *testing.T) { + for _, user := range []string{testUser, "1000", testNumericIdentity, rootUser} { + t.Run(user, func(t *testing.T) { + f := newFakeClient() + f.info[wsName] = &sandboxInfo{Name: wsName, Running: true} + d := newDriver(f, nil, specDefaults{}) + resolver := &recordingUserResolver{err: errors.New("runner accounts must not be read")} + d.userResolver = resolver + err := d.RunDevContainer(context.Background(), wsID, &driver.RunOptions{ + Image: imgX, User: rootUser, RemoteUser: user, Dockerless: true, + WorkspaceMount: &config.Mount{Source: testBindSrc, Target: testBindDst}, + }) + if user != rootUser { + require.ErrorContains(t, err, "non-root Dockerless workspace owner") + require.Empty(t, f.calls) + require.True(t, f.info[wsName].Running) + } else { + require.NoError(t, err) + require.Equal(t, &mountOwner{}, f.created[wsName].Mounts[0].Policy.Owner) + } + require.Zero(t, resolver.calls) + }) + } +} + +func TestBuildSpecPreservesImageMetadataAndImageUser(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + spec := d.buildSpec(wsID, &driver.RunOptions{ + Labels: []string{"devcontainer.metadata=[]", config.UserLabel + "=node"}, + User: rootUser, RemoteUser: testUser, + }, nil, nil) + require.Equal(t, "[]", spec.Labels["devcontainer.metadata"]) + require.Equal(t, "node", spec.Labels[config.UserLabel]) + require.Equal(t, rootUser, spec.Labels[userLabel]) +} + +func TestWorkspaceWithoutOwnerDoesNotMigrateOnIdentityChange(t *testing.T) { + d := newDriver(newFakeClient(), nil, specDefaults{}) + spec := d.buildSpec(wsID, &driver.RunOptions{RemoteUser: testUser}, nil, nil) + required, _ := d.RequiresRecreate( + toContainerDetails(&sandboxInfo{Labels: spec.Labels}), + rootUser, + ) + require.False(t, required) +} diff --git a/pkg/driver/types.go b/pkg/driver/types.go index 736074f018..1f3ccc8477 100644 --- a/pkg/driver/types.go +++ b/pkg/driver/types.go @@ -175,6 +175,9 @@ type RunOptions struct { // RemoteUser is the developer identity for remote tooling and workspace ownership. RemoteUser string `json:"remoteUser,omitempty"` + // Dockerless builds the developer filesystem inside the runner after creation. + Dockerless bool `json:"dockerless,omitempty"` + // Entrypoint is the entrypoint of the container Entrypoint string `json:"entrypoint,omitempty"` @@ -227,12 +230,16 @@ type RunOptions struct { // RecreateRequiredDriver detects incompatible creation-time container contracts. type RecreateRequiredDriver interface { - RequiresRecreate(details *config.ContainerDetails) (bool, string) + RequiresRecreate(details *config.ContainerDetails, remoteUser string) (bool, string) } -func DriverRequiresRecreate(d Driver, details *config.ContainerDetails) (bool, string) { +func DriverRequiresRecreate( + d Driver, + details *config.ContainerDetails, + remoteUser string, +) (bool, string) { if migration, ok := d.(RecreateRequiredDriver); ok { - return migration.RequiresRecreate(details) + return migration.RequiresRecreate(details, remoteUser) } return false, "" } diff --git a/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx b/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx index 142a7f19bb..c19857aa29 100644 --- a/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx +++ b/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx @@ -216,10 +216,23 @@ execute image code. An unresolvable user fails provisioning before replacement of the existing VM. With `workspaceStatVirtualization: "off"`, Devsy omits the owner override. +In Dockerless mode, the developer filesystem is built inside the VM after its +workspace mount is created. Ownership synchronization therefore supports only +`remoteUser: "root"` in this mode. Other identities fail before replacing an +existing VM. For a non-root developer, use a prebuilt image or explicitly select +`workspaceStatVirtualization: "off"` with private host permissions. Devsy never +looks up the developer account in the unrelated Dockerless runner image. + +Image inspection, account resolution, and import prefer an image cached in the +configured Docker-compatible CLI. This permits offline/private-registry startup +and keeps metadata and mount ownership aligned with the image actually imported. +Registry-only images do not require Docker initialization. + Devsy-managed workspaces with an incompatible persisted workspace mount contract are automatically recreated once on `devsy up`, including workspaces created before the contract label existed. Changing either workspace permission policy -also requires recreation. The host workspace and named volumes persist, but +or the effective developer identity also requires recreation. The host workspace +and named volumes persist, but changes stored only on the VM's root disk are lost, as with `--recreate`. Externally managed containers are never automatically replaced. From aab09e63967901a9689edb171290c533c370ff36 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 5 Oct 2026 21:18:47 -0600 Subject: [PATCH 05/10] fix(microsandbox): refresh developer identity on reuse --- pkg/devcontainer/single.go | 3 +++ pkg/devcontainer/single_test.go | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index fd2f3240a5..36b517283f 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -316,6 +316,9 @@ func (r *runner) mergeExistingContainerConfig( containerDetails *config.ContainerDetails, p *resolveParams, ) (*config.MergedDevContainerConfig, error) { + if _, ok := r.driver.(driver.RecreateRequiredDriver); ok { + return r.currentContainerIdentity(ctx, containerDetails, p) + } imageMetadataConfig, err := metadata.GetImageMetadataFromContainer( containerDetails, p.substitutionContext, diff --git a/pkg/devcontainer/single_test.go b/pkg/devcontainer/single_test.go index 3e19d8e3df..869064faf5 100644 --- a/pkg/devcontainer/single_test.go +++ b/pkg/devcontainer/single_test.go @@ -606,3 +606,23 @@ func TestMigrationReplacesSavedUserSettingsWithCurrentConfiguration(t *testing.T require.Equal(t, tt.want, d.remoteUser) } } + +func TestOwnerIndependentReuseRefreshesDeveloperIdentity(t *testing.T) { + d := &migrationMockDriver{ + provisioningPreflightMockDriver: &provisioningPreflightMockDriver{ + mockDriver: &mockDriver{}, + }, + } + p := recreateResolveParams() + p.options.Recreate = false + p.parsedConfig.Config.RemoteUser = "current-user" + p.substitutionContext = &config.SubstitutionContext{} + details := runningContainerDetails() + details.Config.Labels[metadata.CreationConfigLabel] = stringTrue + details.Config.Labels[metadata.ImageMetadataLabel] = `[{"remoteUser":"previous-user"}]` + merged, err := newTestRunner(d).mergeExistingContainerConfig(context.Background(), details, p) + require.NoError(t, err) + require.Equal(t, "current-user", merged.RemoteUser) + require.False(t, d.stopCalled) + require.False(t, d.deleteCalled) +} From 65172b17498eff9292044af6104cd34aed185424 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 5 Oct 2026 22:20:41 -0600 Subject: [PATCH 06/10] fix(e2e): isolate remote Podman failures from local recovery --- e2e/tests/up/podman_rootful.go | 21 +++++++--- e2e/tests/up/podman_rootful_test.go | 62 +++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/e2e/tests/up/podman_rootful.go b/e2e/tests/up/podman_rootful.go index a72b0b7cd4..d2f16834ef 100644 --- a/e2e/tests/up/podman_rootful.go +++ b/e2e/tests/up/podman_rootful.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net/url" "os" "os/exec" "strings" @@ -88,6 +89,14 @@ func shouldAttemptPodmanRecovery(class podmanHealthClass) bool { return class == podmanHealthTimeout || class == podmanHealthUnavailable } +func rootfulPodmanUsesLocalDaemon(endpoint string) bool { + if endpoint == "" { + return true + } + parsed, err := url.Parse(endpoint) + return err == nil && parsed.Scheme == "unix" && parsed.Host == "" && parsed.Path != "" +} + func rootfulPodmanWrapperScript() string { return `#!/bin/sh set -eu @@ -312,7 +321,8 @@ func attemptPodmanRecovery(ctx context.Context, wrapperPath string) (podmanHealt func setupRootfulPodman(ctx context.Context, initialDir string) *framework.Framework { wrapperPath := initialDir + "/bin/" + podmanRootfulWrapperName - if since := rootfulDaemonGate.unhealthy(); since != "" { + localDaemon := rootfulPodmanUsesLocalDaemon(os.Getenv("DOCKER_HOST")) + if since := rootfulDaemonGate.unhealthy(); localDaemon && since != "" { ginkgo.Skip(fmt.Sprintf( "rootful Podman daemon unhealthy since first failure in %q; "+ "skipping to avoid cascading infrastructure failures", @@ -347,7 +357,7 @@ func setupRootfulPodman(ctx context.Context, initialDir string) *framework.Frame healthErr, ) collectPodmanDiagnostics(wrapperPath) - if shouldAttemptPodmanRecovery(class) { + if localDaemon && shouldAttemptPodmanRecovery(class) { if !rootfulDaemonGate.claimRecovery() { rootfulDaemonGate.markUnhealthy(ginkgo.CurrentSpecReport().FullText()) framework.ExpectNoError( @@ -372,8 +382,8 @@ func setupRootfulPodman(ctx context.Context, initialDir string) *framework.Frame "[podman-recovery] daemon healthy again after single restart", ) } else { - // The daemon answers but errors: a restart would only hide a - // product or configuration problem, so fail without gating. + // Local service recovery cannot repair a remote endpoint or + // a responsive daemon error, so fail without gating. framework.ExpectNoError(healthErr) } } @@ -416,7 +426,8 @@ func recoverPodmanCleanup( // mirroring CleanupWorkspace. recoveryCtx := context.WithoutCancel(ctx) class, healthErr := checkPodmanHealth(recoveryCtx, wrapperPath) - if healthErr == nil || !shouldAttemptPodmanRecovery(class) { + if healthErr == nil || !rootfulPodmanUsesLocalDaemon(os.Getenv("DOCKER_HOST")) || + !shouldAttemptPodmanRecovery(class) { return cleanupErr } if !rootfulDaemonGate.claimRecovery() { diff --git a/e2e/tests/up/podman_rootful_test.go b/e2e/tests/up/podman_rootful_test.go index 9304a0ab27..8843bbfc05 100644 --- a/e2e/tests/up/podman_rootful_test.go +++ b/e2e/tests/up/podman_rootful_test.go @@ -3,6 +3,7 @@ package up import ( "bytes" "context" + "errors" "os" "os/exec" "path/filepath" @@ -192,3 +193,64 @@ printf '%s\0' "$@" > "$CAPTURE_FILE" } return got } + +func TestRootfulPodmanUsesLocalDaemon(t *testing.T) { + cases := []struct { + endpoint string + local bool + }{ + {"", true}, + {"unix:///run/podman/podman.sock", true}, + {"unix:///custom/podman.sock", true}, + {"tcp://127.0.0.1:2375", false}, + {"ssh://root@remote/run/podman/podman.sock", false}, + {"unix://remote/run/podman/podman.sock", false}, + {"unix://", false}, + {":invalid", false}, + } + for _, tc := range cases { + t.Run(tc.endpoint, func(t *testing.T) { + assert.Equal(t, tc.local, rootfulPodmanUsesLocalDaemon(tc.endpoint)) + }) + } +} + +func TestRemotePodmanCleanupPreservesLocalRecovery(t *testing.T) { + dir := t.TempDir() + binDir := filepath.Join(dir, "bin") + require.NoError(t, os.Mkdir(binDir, 0o700)) + wrapper := "#!/bin/sh\necho 'cannot connect to remote endpoint' >&2\nexit 1\n" + require.NoError( + t, + os.WriteFile(filepath.Join(binDir, podmanRootfulWrapperName), []byte(wrapper), 0o600), + ) + //nolint:gosec // G302: test wrapper needs owner execute permission. + require.NoError(t, os.Chmod(filepath.Join(binDir, podmanRootfulWrapperName), 0o700)) + capture := filepath.Join(dir, "sudo-commands") + sudo := "#!/bin/sh\nprintf '%s\\n' \"$*\" >> \"$CAPTURE_FILE\"\n" + require.NoError(t, os.WriteFile(filepath.Join(binDir, podmanSudoCommand), []byte(sudo), 0o600)) + //nolint:gosec // G302: test sudo stub needs owner execute permission. + require.NoError(t, os.Chmod(filepath.Join(binDir, podmanSudoCommand), 0o700)) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("CAPTURE_FILE", capture) + t.Setenv("DOCKER_HOST", "ssh://root@remote/run/podman/podman.sock") + previousGate := rootfulDaemonGate + rootfulDaemonGate = &podmanDaemonGate{} + t.Cleanup(func() { rootfulDaemonGate = previousGate }) + cleanupErr := errors.New("remote cleanup failed") + require.ErrorIs( + t, + recoverPodmanCleanup( + context.Background(), + nil, + podmanCleanupDirs{initialDir: dir}, + cleanupErr, + ), + cleanupErr, + ) + assert.Empty(t, rootfulDaemonGate.unhealthy()) + assert.True(t, rootfulDaemonGate.claimRecovery()) + commands, err := os.ReadFile(capture) //nolint:gosec // G304: test-controlled capture path. + require.NoError(t, err) + assert.NotContains(t, string(commands), "systemctl restart") +} From e330340207497919d24e28f497cc560295030f4c Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 5 Oct 2026 23:18:40 -0600 Subject: [PATCH 07/10] fix(e2e): recover only the managed rootful Podman socket --- e2e/tests/up/podman_rootful.go | 19 +++++++++---------- e2e/tests/up/podman_rootful_test.go | 25 ++++++++++++++++++++----- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/e2e/tests/up/podman_rootful.go b/e2e/tests/up/podman_rootful.go index d2f16834ef..926b395c6d 100644 --- a/e2e/tests/up/podman_rootful.go +++ b/e2e/tests/up/podman_rootful.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "net/url" "os" "os/exec" "strings" @@ -89,12 +88,12 @@ func shouldAttemptPodmanRecovery(class podmanHealthClass) bool { return class == podmanHealthTimeout || class == podmanHealthUnavailable } -func rootfulPodmanUsesLocalDaemon(endpoint string) bool { +func rootfulPodmanUsesManagedDaemon(endpoint string) bool { if endpoint == "" { return true } - parsed, err := url.Parse(endpoint) - return err == nil && parsed.Scheme == "unix" && parsed.Host == "" && parsed.Path != "" + return endpoint == "unix:///run/podman/podman.sock" || + endpoint == "unix:///var/run/podman/podman.sock" } func rootfulPodmanWrapperScript() string { @@ -321,8 +320,8 @@ func attemptPodmanRecovery(ctx context.Context, wrapperPath string) (podmanHealt func setupRootfulPodman(ctx context.Context, initialDir string) *framework.Framework { wrapperPath := initialDir + "/bin/" + podmanRootfulWrapperName - localDaemon := rootfulPodmanUsesLocalDaemon(os.Getenv("DOCKER_HOST")) - if since := rootfulDaemonGate.unhealthy(); localDaemon && since != "" { + managedDaemon := rootfulPodmanUsesManagedDaemon(os.Getenv("DOCKER_HOST")) + if since := rootfulDaemonGate.unhealthy(); managedDaemon && since != "" { ginkgo.Skip(fmt.Sprintf( "rootful Podman daemon unhealthy since first failure in %q; "+ "skipping to avoid cascading infrastructure failures", @@ -357,7 +356,7 @@ func setupRootfulPodman(ctx context.Context, initialDir string) *framework.Frame healthErr, ) collectPodmanDiagnostics(wrapperPath) - if localDaemon && shouldAttemptPodmanRecovery(class) { + if managedDaemon && shouldAttemptPodmanRecovery(class) { if !rootfulDaemonGate.claimRecovery() { rootfulDaemonGate.markUnhealthy(ginkgo.CurrentSpecReport().FullText()) framework.ExpectNoError( @@ -382,8 +381,8 @@ func setupRootfulPodman(ctx context.Context, initialDir string) *framework.Frame "[podman-recovery] daemon healthy again after single restart", ) } else { - // Local service recovery cannot repair a remote endpoint or - // a responsive daemon error, so fail without gating. + // The managed service cannot repair another endpoint or a + // responsive daemon error, so fail without gating. framework.ExpectNoError(healthErr) } } @@ -426,7 +425,7 @@ func recoverPodmanCleanup( // mirroring CleanupWorkspace. recoveryCtx := context.WithoutCancel(ctx) class, healthErr := checkPodmanHealth(recoveryCtx, wrapperPath) - if healthErr == nil || !rootfulPodmanUsesLocalDaemon(os.Getenv("DOCKER_HOST")) || + if healthErr == nil || !rootfulPodmanUsesManagedDaemon(os.Getenv("DOCKER_HOST")) || !shouldAttemptPodmanRecovery(class) { return cleanupErr } diff --git a/e2e/tests/up/podman_rootful_test.go b/e2e/tests/up/podman_rootful_test.go index 8843bbfc05..cdc3f1e98c 100644 --- a/e2e/tests/up/podman_rootful_test.go +++ b/e2e/tests/up/podman_rootful_test.go @@ -194,14 +194,16 @@ printf '%s\0' "$@" > "$CAPTURE_FILE" return got } -func TestRootfulPodmanUsesLocalDaemon(t *testing.T) { +func TestRootfulPodmanUsesManagedDaemon(t *testing.T) { cases := []struct { endpoint string local bool }{ {"", true}, {"unix:///run/podman/podman.sock", true}, - {"unix:///custom/podman.sock", true}, + {"unix:///var/run/podman/podman.sock", true}, + {"unix:///run/user/1000/podman/podman.sock", false}, + {"unix:///custom/podman.sock", false}, {"tcp://127.0.0.1:2375", false}, {"ssh://root@remote/run/podman/podman.sock", false}, {"unix://remote/run/podman/podman.sock", false}, @@ -210,12 +212,25 @@ func TestRootfulPodmanUsesLocalDaemon(t *testing.T) { } for _, tc := range cases { t.Run(tc.endpoint, func(t *testing.T) { - assert.Equal(t, tc.local, rootfulPodmanUsesLocalDaemon(tc.endpoint)) + assert.Equal(t, tc.local, rootfulPodmanUsesManagedDaemon(tc.endpoint)) }) } } -func TestRemotePodmanCleanupPreservesLocalRecovery(t *testing.T) { +func TestUnmanagedPodmanCleanupPreservesLocalRecovery(t *testing.T) { + for _, endpoint := range []string{ + "ssh://root@remote/run/podman/podman.sock", + "unix:///custom/podman.sock", + "unix:///run/user/1000/podman/podman.sock", + } { + t.Run(endpoint, func(t *testing.T) { + checkUnmanagedPodmanCleanup(t, endpoint) + }) + } +} + +func checkUnmanagedPodmanCleanup(t *testing.T, endpoint string) { + t.Helper() dir := t.TempDir() binDir := filepath.Join(dir, "bin") require.NoError(t, os.Mkdir(binDir, 0o700)) @@ -233,7 +248,7 @@ func TestRemotePodmanCleanupPreservesLocalRecovery(t *testing.T) { require.NoError(t, os.Chmod(filepath.Join(binDir, podmanSudoCommand), 0o700)) t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) t.Setenv("CAPTURE_FILE", capture) - t.Setenv("DOCKER_HOST", "ssh://root@remote/run/podman/podman.sock") + t.Setenv("DOCKER_HOST", endpoint) previousGate := rootfulDaemonGate rootfulDaemonGate = &podmanDaemonGate{} t.Cleanup(func() { rootfulDaemonGate = previousGate }) From 2d0c40d65da6a3a51decb49a017ce8d407c5fb1d Mon Sep 17 00:00:00 2001 From: Samuel K Date: Tue, 6 Oct 2026 00:41:14 -0600 Subject: [PATCH 08/10] fix(microsandbox): freeze images and require explicit replacement --- e2e/tests/up/provider_microsandbox.go | 31 ++++- pkg/devcontainer/single.go | 29 +++-- pkg/devcontainer/single_recreate_test.go | 46 ++++++- pkg/driver/microsandbox/cliclient.go | 89 ++++---------- pkg/driver/microsandbox/client.go | 11 +- pkg/driver/microsandbox/image_user.go | 12 +- pkg/driver/microsandbox/image_user_test.go | 115 ++++++++++++++---- pkg/driver/microsandbox/microsandbox.go | 64 +++++++--- pkg/driver/microsandbox/microsandbox_test.go | 67 +++++++++- .../microsandbox/workspace_owner_test.go | 22 ++-- pkg/driver/types.go | 3 + .../docs/developing-providers/driver.mdx | 26 ++-- 12 files changed, 362 insertions(+), 153 deletions(-) diff --git a/e2e/tests/up/provider_microsandbox.go b/e2e/tests/up/provider_microsandbox.go index 3cf8a60106..3a363864ac 100644 --- a/e2e/tests/up/provider_microsandbox.go +++ b/e2e/tests/up/provider_microsandbox.go @@ -122,6 +122,14 @@ var _ = ginkgo.Describe( assertMicrosandboxHostEntries(ctx, f, tempDir, "recreated") previousCreation := microsandboxCreationTime(ctx, sandbox) + // #nosec G204 -- fixed command and Devsy-generated sandbox name + _, err = exec.CommandContext( + ctx, "msb", "exec", "--stream", sandbox, "--", "sh", "-c", + "echo preserved > /root/recreation-preserved.txt", + ). + Output() + framework.ExpectNoError(err) + configPath := filepath.Join(tempDir, ".devcontainer.json") // #nosec G304 -- configuration copied into the test-owned temporary directory data, err := os.ReadFile(configPath) @@ -132,9 +140,28 @@ var _ = ginkgo.Describe( data, err = json.Marshal(devConfig) framework.ExpectNoError(err) framework.ExpectNoError(os.WriteFile(configPath, data, 0o600)) - err = f.DevsyWorkspaceStop(ctx, tempDir) + + stdout, stderr, err := f.DevsyUpStreams( + ctx, tempDir, "--devcontainer", ".devcontainer.json", + ) + gomega.Expect(err).To(gomega.HaveOccurred()) + gomega.Expect(stdout + stderr).To(gomega.ContainSubstring("--recreate")) + gomega.Expect(microsandboxCreationTime(ctx, sandbox)).To(gomega.Equal(previousCreation)) + // #nosec G204 -- fixed command and Devsy-generated sandbox name + preserved, err := exec.CommandContext( + ctx, + "msb", + "exec", + "--stream", + sandbox, + "--", + "cat", + "/root/recreation-preserved.txt", + ). + Output() framework.ExpectNoError(err) - err = f.DevsyUp(ctx, tempDir, "--devcontainer", ".devcontainer.json") + gomega.Expect(strings.TrimSpace(string(preserved))).To(gomega.Equal("preserved")) + err = f.DevsyUp(ctx, tempDir, "--devcontainer", ".devcontainer.json", "--recreate") framework.ExpectNoError(err) gomega.Expect(microsandboxCreationTime(ctx, sandbox)). NotTo(gomega.Equal(previousCreation)) diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index 36b517283f..8087abd84a 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -195,11 +195,10 @@ func (r *runner) applyTerminalSecretRecreateRequirement( config.SecretsEnvDir, ) } - log.Info( - "recreating workspace because attached terminal secrets require the secure runtime mount", + return r.scheduleContainerRecreation( + params, + "attached terminal secrets require the secure runtime mount", ) - params.options.Recreate = true - return nil } func (r *runner) applyFileSecretRecreateRequirement( @@ -222,9 +221,7 @@ func (r *runner) applyFileSecretRecreateRequirement( config.SecretsMountDir, ) } - log.Info("recreating workspace because file secrets require the secure runtime mount") - params.options.Recreate = true - return nil + return r.scheduleContainerRecreation(params, "file secrets require the secure runtime mount") } func (r *runner) provisioningPreflightForRecreate(ctx context.Context, options UpOptions) error { @@ -773,6 +770,7 @@ func (r *runner) runContainer( } } + runOptions.AllowRecreate = p.options.Recreate runOptions.Env = r.addExtraEnvVars(runOptions.Env) if _, ok := r.driver.(driver.RecreateRequiredDriver); ok { runOptions.Labels = append(runOptions.Labels, metadata.CreationConfigLabel+"="+stringTrue) @@ -1184,9 +1182,7 @@ func (r *runner) applyDriverRecreateRequirement( if p.parsedConfig.Config.ContainerID != "" { return fmt.Errorf("cannot migrate externally managed container: %s", reason) } - log.Infof("recreating workspace because %s", reason) - p.options.Recreate = true - return nil + return r.scheduleContainerRecreation(p, reason) } func (r *runner) currentContainerIdentity( @@ -1208,3 +1204,16 @@ func (r *runner) currentContainerIdentity( ) return r.mergeContainerMetadata(ctx, imageMetadata, p) } + +func (r *runner) scheduleContainerRecreation(params *resolveParams, reason string) error { + if driver.DriverRecreateMode(r.driver) == driver.RecreateOnRun { + return fmt.Errorf( + "workspace requires recreation because %s; back up VM-local data and rerun with --recreate: "+ + "replacement discards the VM root disk and cannot roll back after removal", + reason, + ) + } + log.Infof("recreating workspace because %s", reason) + params.options.Recreate = true + return nil +} diff --git a/pkg/devcontainer/single_recreate_test.go b/pkg/devcontainer/single_recreate_test.go index 8378556112..0e9b99d202 100644 --- a/pkg/devcontainer/single_recreate_test.go +++ b/pkg/devcontainer/single_recreate_test.go @@ -2,6 +2,7 @@ package devcontainer import ( "errors" + "fmt" "testing" "github.com/devsy-org/devsy/pkg/devcontainer/config" @@ -9,6 +10,11 @@ import ( "github.com/stretchr/testify/require" ) +const ( + migrationTerminal = "terminal" + migrationFile = "file" +) + type secretMigrationDriver struct { *migrationMockDriver tmpfsSupported bool @@ -37,8 +43,8 @@ func TestSecretMigrationPreflightFailurePreservesExistingContainer(t *testing.T) terminal, file, driver bool explicit, tmpfsSupported bool }{ - {name: "terminal", terminal: true, tmpfsSupported: true}, - {name: "file", file: true, tmpfsSupported: true}, + {name: migrationTerminal, terminal: true, tmpfsSupported: true}, + {name: migrationFile, file: true, tmpfsSupported: true}, {name: "both", terminal: true, file: true, tmpfsSupported: true}, {name: "driver migration precedes secrets", terminal: true, file: true, driver: true}, {name: "explicit recreation skips migration", terminal: true, file: true, explicit: true}, @@ -128,3 +134,39 @@ func TestPrepareContainerRecreationKeepsCompatibleContainer(t *testing.T) { require.False(t, d.provisioningCalled) } } + +type onRunMigrationDriver struct{ *secretMigrationDriver } + +func (*onRunMigrationDriver) RecreateMode() driver.RecreateMode { return driver.RecreateOnRun } + +func TestOnRunMigrationRequiresExplicitRecreation(t *testing.T) { + for _, reason := range []string{"driver", migrationTerminal, migrationFile} { + for _, explicit := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/explicit=%t", reason, explicit), func(t *testing.T) { + _, params, base := newSecretMigrationRunner() + base.tmpfsSupported = true + base.required = reason == "driver" + r := newTestRunner(&onRunMigrationDriver{base}) + params.options.Recreate = explicit + if reason == migrationTerminal { + r.workspaceConfig.CLIOptions.TerminalSecretEnvNames = []string{ + terminalSecretSentinelName, + } + } + if reason == migrationFile { + params.options.SecretsMount = []string{secretFileMountRequestSentinel} + } + err := r.prepareContainerRecreation(t.Context(), runningContainerDetails(), params) + if explicit { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, "--recreate") + } + require.Equal(t, explicit, params.options.Recreate) + require.False(t, base.stopCalled) + require.False(t, base.deleteCalled) + require.Equal(t, explicit, base.provisioningCalled) + }) + } + } +} diff --git a/pkg/driver/microsandbox/cliclient.go b/pkg/driver/microsandbox/cliclient.go index e6a6453ff2..7ce48beb22 100644 --- a/pkg/driver/microsandbox/cliclient.go +++ b/pkg/driver/microsandbox/cliclient.go @@ -9,7 +9,6 @@ import ( "os" "os/exec" "path/filepath" - "runtime" "strconv" "strings" "time" @@ -17,8 +16,8 @@ import ( "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/driver" "github.com/devsy-org/devsy/pkg/flags/names" - "github.com/devsy-org/devsy/pkg/image" "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/tarball" ) @@ -52,25 +51,36 @@ func (cliClient) Version(ctx context.Context) (string, error) { return strings.TrimSpace(string(out)), nil } -func (c cliClient) EnsureImage(ctx context.Context, imageRef string, builtLocally bool) error { - if builtLocally || localImageAvailable(ctx, c.dockerPath, imageRef) { - return loadFromDocker(ctx, imageRef, c.dockerPath) - } - // #nosec G204 -- args are a resolved binary path and a validated image ref - out, err := exec.CommandContext(ctx, msbBinary(), "pull", imageRef).CombinedOutput() - if err == nil { - return nil +func (c cliClient) PrepareImage( + ctx context.Context, + ref string, + builtLocally bool, +) (*preparedImage, error) { + img, cleanup, err := filesystemUserResolver(c).openImage(ctx, ref, builtLocally) + if err != nil { + return nil, err } - if loadErr := loadViaRegistry(ctx, imageRef); loadErr != nil { - return fmt.Errorf("msb pull %s: %s: %w; registry fallback: %v", imageRef, out, err, loadErr) + digest, err := img.Digest() + if err != nil { + cleanup() + return nil, fmt.Errorf("identify final image %q: %w", ref, err) } - return nil + return &preparedImage{ + reference: "devsy-msb-image:" + digest.Hex, + image: img, + cleanup: cleanup, + }, nil +} + +func (cliClient) EnsureImage(ctx context.Context, prepared *preparedImage) error { + return loadImageSnapshot(ctx, prepared.reference, prepared.image) +} + +func (c cliClient) PrepareVolumes(ctx context.Context, mounts []volumeMount) error { + return c.ensureVolumes(ctx, mounts) } func (c cliClient) Create(ctx context.Context, sandbox string, spec sandboxSpec) error { - if err := c.ensureVolumes(ctx, spec.Mounts); err != nil { - return err - } return msbRun(ctx, runArgs(sandbox, spec)...) } @@ -309,56 +319,11 @@ func redactArgs(args []string) string { return strings.Join(out, " ") } -func loadFromDocker(ctx context.Context, image, dockerPath string) error { - if dockerPath == "" { - dockerPath = dockerExecutable - } - docker, err := exec.LookPath(dockerPath) - if err != nil { - return fmt.Errorf("docker not found to load built image %q: %w", image, err) - } - // #nosec G204 -- docker/msb paths are resolved and the image ref is validated - save := exec.CommandContext(ctx, docker, "save", image) - // #nosec G204 -- docker/msb paths are resolved and the image ref is validated - load := exec.CommandContext(ctx, msbBinary(), "load", "-t", image) - reader, writer, err := os.Pipe() - if err != nil { - return fmt.Errorf("pipe docker save: %w", err) - } - defer func() { _ = reader.Close() }() - defer func() { _ = writer.Close() }() - save.Stdout = writer - load.Stdin = reader - var saveErr strings.Builder - save.Stderr = &saveErr - var loadErr strings.Builder - load.Stderr = &loadErr - if err := load.Start(); err != nil { - return fmt.Errorf("start msb load: %w", err) - } - _ = reader.Close() - if err := save.Run(); err != nil { - // load is still running on the broken pipe; kill and reap it. - _ = load.Process.Kill() - _ = load.Wait() - return fmt.Errorf("docker save %q: %s: %w", image, saveErr.String(), err) - } - _ = writer.Close() - if err := load.Wait(); err != nil { - return fmt.Errorf("msb load %q: %s: %w", image, loadErr.String(), err) - } - return nil -} - -func loadViaRegistry(ctx context.Context, imageRef string) error { +func loadImageSnapshot(ctx context.Context, imageRef string, img v1.Image) error { ref, err := name.ParseReference(imageRef) if err != nil { return fmt.Errorf("parse image reference %q: %w", imageRef, err) } - img, err := image.GetImageForArch(ctx, imageRef, runtime.GOARCH) - if err != nil { - return fmt.Errorf("pull image %q: %w", imageRef, err) - } tmp, err := os.CreateTemp("", "devsy-msb-*.tar") if err != nil { diff --git a/pkg/driver/microsandbox/client.go b/pkg/driver/microsandbox/client.go index f95f241b2c..6fa3be4f9b 100644 --- a/pkg/driver/microsandbox/client.go +++ b/pkg/driver/microsandbox/client.go @@ -6,8 +6,15 @@ import ( "time" "github.com/devsy-org/devsy/pkg/devcontainer/config" + v1 "github.com/google/go-containerregistry/pkg/v1" ) +type preparedImage struct { + reference string + image v1.Image + cleanup func() +} + type sandboxSpec struct { Image string User string @@ -58,7 +65,9 @@ type execRequest struct { type sandboxClient interface { EnsureInstalled(ctx context.Context) error Version(ctx context.Context) (string, error) - EnsureImage(ctx context.Context, image string, builtLocally bool) error + PrepareImage(ctx context.Context, image string, builtLocally bool) (*preparedImage, error) + EnsureImage(ctx context.Context, image *preparedImage) error + PrepareVolumes(ctx context.Context, mounts []volumeMount) error Create(ctx context.Context, name string, spec sandboxSpec) error Find(ctx context.Context, name string) (*sandboxInfo, error) Start(ctx context.Context, name string) error diff --git a/pkg/driver/microsandbox/image_user.go b/pkg/driver/microsandbox/image_user.go index a4e4662511..dc414f506f 100644 --- a/pkg/driver/microsandbox/image_user.go +++ b/pkg/driver/microsandbox/image_user.go @@ -34,25 +34,19 @@ type imageUserIdentity struct { } type imageUserResolver interface { - Resolve(ctx context.Context, image string, builtLocally bool, user string) (*mountOwner, error) + ResolveImage(ctx context.Context, img v1.Image, user string) (*mountOwner, error) } type filesystemUserResolver struct{ dockerPath string } -func (r filesystemUserResolver) Resolve( +func (filesystemUserResolver) ResolveImage( ctx context.Context, - ref string, - builtLocally bool, + img v1.Image, user string, ) (*mountOwner, error) { if owner, complete, err := explicitOwner(user); complete || err != nil { return owner, err } - img, cleanup, err := r.openImage(ctx, ref, builtLocally) - if err != nil { - return nil, err - } - defer cleanup() return ownerFromImage(ctx, img, user) } diff --git a/pkg/driver/microsandbox/image_user_test.go b/pkg/driver/microsandbox/image_user_test.go index 78b78d7ddc..27c41e493b 100644 --- a/pkg/driver/microsandbox/image_user_test.go +++ b/pkg/driver/microsandbox/image_user_test.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "io" + "net/http/httptest" "os" "path/filepath" "runtime" @@ -12,13 +13,18 @@ import ( "testing" "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/tarball" "github.com/stretchr/testify/suite" ) +//nolint:gosec // Synthetic account fixture contains no credentials. +const snapshotPasswd = "vscode:x:2000:2001:dev:/home/vscode:/bin/sh\n" + type imageUserSuite struct{ suite.Suite } func TestImageUser(t *testing.T) { suite.Run(t, new(imageUserSuite)) } @@ -76,7 +82,7 @@ func (s *imageUserSuite) TestFinalLayerOverridesAndWhiteouts() { ) final := s.layer( map[string]string{ - passwdPath: "vscode:x:2000:2001:dev:/home/vscode:/bin/sh\n", + passwdPath: snapshotPasswd, "etc/.wh.group": "", }, ) @@ -123,7 +129,7 @@ func (s *imageUserSuite) TestLocalFinalImageUsesConfiguredCLI() { } img, err := mutate.AppendLayers( empty.Image, - s.layer(map[string]string{passwdPath: "vscode:x:2000:2001:dev:/home/vscode:/bin/sh\n"}), + s.layer(map[string]string{passwdPath: snapshotPasswd}), ) s.Require().NoError(err) cfg, err := img.ConfigFile() @@ -147,10 +153,16 @@ cat "$DEVSY_TEST_ARCHIVE" ` s.Require().NoError(os.WriteFile(executable, []byte(script), 0o700)) for _, built := range []bool{true, false} { - owner, err := (filesystemUserResolver{dockerPath: executable}).Resolve( - context.Background(), "final-image:latest", built, testUser, + resolver := filesystemUserResolver{dockerPath: executable} + snapshot, cleanup, err := resolver.openImage( + context.Background(), + "final-image:latest", + built, ) s.Require().NoError(err) + defer cleanup() + owner, err := resolver.ResolveImage(context.Background(), snapshot, testUser) + s.Require().NoError(err) s.Equal(mountOwner{2000, 2001}, *owner) } args, err := os.ReadFile(argsFile) @@ -163,31 +175,84 @@ cat "$DEVSY_TEST_ARCHIVE" s.Equal(testUser, details.Config.User) } -//nolint:gosec // Test-created executable and archive paths under TempDir. -func (s *imageUserSuite) TestLocalImportUsesConfiguredCLIAndDrainsArchive() { +//nolint:gosec // Synthetic archives and executable fixtures under TempDir. +func (s *imageUserSuite) TestSnapshotSurvivesRetagBeforeImport() { if runtime.GOOS == "windows" { - s.T().Skip("fixture executables require a POSIX shell") + s.T().Skip("fixture executable requires a POSIX shell") } - dir := s.T().TempDir() - source := filepath.Join(dir, "source") - target := filepath.Join(dir, "target") - content := strings.Repeat("archive data", 100000) - s.Require().NoError(os.WriteFile(source, []byte(content), 0o600)) - s.T().Setenv("DEVSY_TEST_ARCHIVE", source) - s.T().Setenv("DEVSY_TEST_TARGET", target) - s.T().Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) - saveCLI := filepath.Join(dir, "save-cli") - saveScript := "#!/bin/sh\nif [ \"$1\" = image ]; then exit 0; fi\ncat \"$DEVSY_TEST_ARCHIVE\"\n" - s.Require().NoError(os.WriteFile(saveCLI, []byte(saveScript), 0o700)) - s.Require(). - NoError(os.WriteFile(filepath.Join(dir, "msb"), []byte("#!/bin/sh\ncat > \"$DEVSY_TEST_TARGET\"\n"), 0o700)) - for _, built := range []bool{true, false} { - s.Require(). - NoError((cliClient{dockerPath: saveCLI}).EnsureImage(context.Background(), "final-image:latest", built)) + for _, source := range []string{"local", "registry"} { + s.Run(source, func() { + ctx := context.Background() + original := s.snapshotImage(snapshotPasswd) + replacement := s.snapshotImage("vscode:x:3000:3001:dev:/home/vscode:/bin/sh\n") + dir := s.T().TempDir() + archive := filepath.Join(dir, "source.tar") + imported := filepath.Join(dir, "imported.tar") + aliasFile := filepath.Join(dir, "alias") + s.T().Setenv("DEVSY_TEST_ARCHIVE", archive) + s.T().Setenv("DEVSY_TEST_IMPORTED", imported) + s.T().Setenv("DEVSY_TEST_ALIAS", aliasFile) + s.T().Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + cliPath := filepath.Join(dir, "configured-cli") + script := "#!/bin/sh\nif [ \"$1\" = image ]; then exit 0; fi\ncat \"$DEVSY_TEST_ARCHIVE\"\n" + ref := "final-image:latest" + if source == "registry" { + server := httptest.NewServer(registry.New()) + defer server.Close() + ref = strings.TrimPrefix(server.URL, "http://") + "/final-image:latest" + script = "#!/bin/sh\nexit 1\n" + } + s.Require().NoError(os.WriteFile(cliPath, []byte(script), 0o700)) + tag, err := name.NewTag(ref) + s.Require().NoError(err) + writeImage := func(img v1.Image) { + if source == "local" { + s.Require().NoError(tarball.WriteToFile(archive, tag, img)) + } else { + s.Require().NoError(remote.Write(tag, img)) + } + } + writeImage(original) + client := cliClient{dockerPath: cliPath} + prepared, err := client.PrepareImage(ctx, ref, false) + s.Require().NoError(err) + defer prepared.cleanup() + writeImage(replacement) + owner, err := (filesystemUserResolver{}).ResolveImage(ctx, prepared.image, testUser) + s.Require().NoError(err) + s.Equal(mountOwner{2000, 2001}, *owner) + s.writeSnapshotImportCLI(dir) + s.Require().NoError(client.EnsureImage(ctx, prepared)) + importedImage, err := tarball.ImageFromPath(imported, nil) + s.Require().NoError(err) + importedOwner, err := ownerFromImage(ctx, importedImage, testUser) + s.Require().NoError(err) + s.Equal(*owner, *importedOwner) + alias, err := os.ReadFile(aliasFile) + s.Require().NoError(err) + s.Equal(prepared.reference, string(alias)) + newer, err := client.PrepareImage(ctx, ref, false) + s.Require().NoError(err) + defer newer.cleanup() + s.NotEqual(prepared.reference, newer.reference) + }) } - data, err := os.ReadFile(target) +} + +func (s *imageUserSuite) snapshotImage(accounts string) v1.Image { + img, err := mutate.AppendLayers(empty.Image, s.layer(map[string]string{passwdPath: accounts})) s.Require().NoError(err) - s.Equal(content, string(data)) + return img +} + +//nolint:gosec // Test-created executable under TempDir. +func (s *imageUserSuite) writeSnapshotImportCLI(dir string) { + msb := `#!/bin/sh +[ "$1" = load ] || exit 1 +cp "$3" "$DEVSY_TEST_IMPORTED" +printf '%s' "$5" > "$DEVSY_TEST_ALIAS" +` + s.Require().NoError(os.WriteFile(filepath.Join(dir, "msb"), []byte(msb), 0o700)) } func (s *imageUserSuite) layer(files map[string]string) v1.Layer { diff --git a/pkg/driver/microsandbox/microsandbox.go b/pkg/driver/microsandbox/microsandbox.go index b90d7f41c4..04b3a9d019 100644 --- a/pkg/driver/microsandbox/microsandbox.go +++ b/pkg/driver/microsandbox/microsandbox.go @@ -21,6 +21,7 @@ import ( "github.com/devsy-org/devsy/pkg/driver" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/provider" + v1 "github.com/google/go-containerregistry/pkg/v1" ) const userLabel = "devsy.sh/user" @@ -312,18 +313,7 @@ func (d *microsandboxDriver) DeleteDevContainer(ctx context.Context, workspaceID if err != nil { return err } - if info == nil { - return nil - } - if info.Running { - if err := d.client.Stop(ctx, name); err != nil { - return fmt.Errorf("stop microsandbox VM before delete: %w", err) - } - } - if err := d.client.Remove(ctx, name); err != nil { - return fmt.Errorf("remove microsandbox VM: %w", err) - } - return nil + return d.deleteSandbox(ctx, name, info) } func (d *microsandboxDriver) GetDevContainerLogs( @@ -378,21 +368,41 @@ func (d *microsandboxDriver) runFromOptions( if err := d.ProvisioningPreflight(ctx); err != nil { return err } - owner, err := d.resolveWorkspaceOwner(ctx, options) + prepared, err := d.client.PrepareImage(ctx, options.Image, options.ImageBuilt) + if err != nil { + return fmt.Errorf("prepare microsandbox final image: %w", err) + } + defer prepared.cleanup() + owner, err := d.resolveWorkspaceOwner(ctx, options, prepared.image) if err != nil { return err } warnUnsupportedOptions(options) - if err := d.client.EnsureImage(ctx, options.Image, options.ImageBuilt); err != nil { + if err := d.client.EnsureImage(ctx, prepared); err != nil { return fmt.Errorf("ensure microsandbox final image: %w", err) } - if err := d.DeleteDevContainer(ctx, workspaceID); err != nil { + info, err := d.client.Find(ctx, sandboxName(workspaceID)) + if err != nil { + return err + } + if info != nil && !options.AllowRecreate { + return fmt.Errorf( + "microsandbox VM already exists; back up VM-local data and rerun with --recreate: " + + "replacement discards the VM root disk and cannot roll back after removal", + ) + } + spec := d.buildSpec(workspaceID, options, hostReqs, owner) + spec.Image = prepared.reference + if err := d.client.PrepareVolumes(ctx, spec.Mounts); err != nil { + return fmt.Errorf("prepare microsandbox volumes: %w", err) + } + if err := d.deleteSandbox(ctx, sandboxName(workspaceID), info); err != nil { return fmt.Errorf("clean stale microsandbox VM before create: %w", err) } if err := d.client.Create( ctx, sandboxName(workspaceID), - d.buildSpec(workspaceID, options, hostReqs, owner), + spec, ); err != nil { return fmt.Errorf("create microsandbox VM: %w", err) } @@ -658,6 +668,7 @@ func (d *microsandboxDriver) workspaceMountContract() string { func (d *microsandboxDriver) resolveWorkspaceOwner( ctx context.Context, options *driver.RunOptions, + img v1.Image, ) (*mountOwner, error) { if d.workspaceMountPolicy.StatVirtualization == statVirtOff || bindMount(options.WorkspaceMount) == nil { @@ -674,7 +685,7 @@ func (d *microsandboxDriver) resolveWorkspaceOwner( } return &mountOwner{}, nil } - owner, err := d.userResolver.Resolve(ctx, options.Image, options.ImageBuilt, user) + owner, err := d.userResolver.ResolveImage(ctx, img, user) if err != nil { return nil, fmt.Errorf( "resolve microsandbox workspace owner for remoteUser %q: %w", @@ -694,3 +705,22 @@ func workspaceRemoteUser(options *driver.RunOptions) string { } return rootUser } + +func (d *microsandboxDriver) deleteSandbox( + ctx context.Context, + name string, + info *sandboxInfo, +) error { + if info == nil { + return nil + } + if info.Running { + if err := d.client.Stop(ctx, name); err != nil { + return fmt.Errorf("stop microsandbox VM before delete: %w", err) + } + } + if err := d.client.Remove(ctx, name); err != nil { + return fmt.Errorf("remove microsandbox VM: %w", err) + } + return nil +} diff --git a/pkg/driver/microsandbox/microsandbox_test.go b/pkg/driver/microsandbox/microsandbox_test.go index 2b22bb9340..6e9823b5bc 100644 --- a/pkg/driver/microsandbox/microsandbox_test.go +++ b/pkg/driver/microsandbox/microsandbox_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "io" "slices" "strings" @@ -12,6 +13,8 @@ import ( "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/driver" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/stretchr/testify/require" ) const ( @@ -40,6 +43,8 @@ type fakeClient struct { calls []string execReq execRequest execName string + failPrepare error + failVolumes error failFind error failStop error failCreat error @@ -67,11 +72,22 @@ func (f *fakeClient) Version(context.Context) (string, error) { return testVersion, nil } -func (f *fakeClient) EnsureImage(_ context.Context, image string, _ bool) error { - f.calls = append(f.calls, "ensure:"+image) +func (f *fakeClient) PrepareImage(_ context.Context, image string, _ bool) (*preparedImage, error) { + if f.failPrepare != nil { + return nil, f.failPrepare + } + return &preparedImage{reference: image, image: empty.Image, cleanup: func() {}}, nil +} + +func (f *fakeClient) EnsureImage(_ context.Context, image *preparedImage) error { + f.calls = append(f.calls, "ensure:"+image.reference) return f.failEnsure } +func (f *fakeClient) PrepareVolumes(_ context.Context, _ []volumeMount) error { + return f.failVolumes +} + func (f *fakeClient) Create(_ context.Context, name string, spec sandboxSpec) error { f.calls = append(f.calls, "create:"+name) if f.failCreat != nil { @@ -176,7 +192,11 @@ func TestRunDevContainerReplacesStaleSandbox(t *testing.T) { f.info[wsName] = &sandboxInfo{Name: wsName, Running: true} d := newDriver(f, nil, specDefaults{}) - err := d.RunDevContainer(context.Background(), wsID, &driver.RunOptions{Image: imgX}) + err := d.RunDevContainer( + context.Background(), + wsID, + &driver.RunOptions{Image: imgX, AllowRecreate: true}, + ) if err != nil { t.Fatalf("RunDevContainer: %v", err) } @@ -738,3 +758,44 @@ func TestParseUint8(t *testing.T) { } } } + +func TestReplacementRequiresConsentAndSuccessfulPreparation(t *testing.T) { + sentinel := errors.New("preparation failed") + for _, running := range []bool{false, true} { + for _, tc := range []struct { + name string + allow bool + imageErr, volumesErr error + }{ + {name: "consent"}, + {name: "image", allow: true, imageErr: sentinel}, + {name: "volumes", allow: true, volumesErr: sentinel}, + } { + t.Run(fmt.Sprintf("running=%t/%s", running, tc.name), func(t *testing.T) { + f := newFakeClient() + original := &sandboxInfo{Name: wsName, Running: running, CreatedAt: time.Now()} + f.info[wsName] = original + f.failPrepare, f.failVolumes = tc.imageErr, tc.volumesErr + opts := &driver.RunOptions{Image: imgX, AllowRecreate: tc.allow} + err := newDriver(f, nil, specDefaults{}).RunDevContainer(t.Context(), wsID, opts) + require.Error(t, err) + if !tc.allow { + require.ErrorContains(t, err, "--recreate") + } else { + require.ErrorIs(t, err, sentinel) + } + require.Same(t, original, f.info[wsName]) + require.Equal(t, running, original.Running) + require.Empty(t, f.created) + for _, call := range f.calls { + require.False( + t, + strings.HasPrefix(call, "stop:") || strings.HasPrefix(call, "remove:") || + strings.HasPrefix(call, "create:"), + call, + ) + } + }) + } + } +} diff --git a/pkg/driver/microsandbox/workspace_owner_test.go b/pkg/driver/microsandbox/workspace_owner_test.go index be5e932664..d4f13609fc 100644 --- a/pkg/driver/microsandbox/workspace_owner_test.go +++ b/pkg/driver/microsandbox/workspace_owner_test.go @@ -7,27 +7,25 @@ import ( "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/driver" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/stretchr/testify/require" ) const testNumericIdentity = "1000:1001" type recordingUserResolver struct { - image string - built bool user string owner *mountOwner err error calls int } -func (r *recordingUserResolver) Resolve( +func (r *recordingUserResolver) ResolveImage( _ context.Context, - image string, - built bool, + _ v1.Image, user string, ) (*mountOwner, error) { - r.image, r.built, r.user = image, built, user + r.user = user r.calls++ return r.owner, r.err } @@ -59,8 +57,6 @@ func TestWorkspaceOwnerKeepsContainerIdentity(t *testing.T) { ), ) require.Equal(t, testUser, resolver.user) - require.Equal(t, testImage, resolver.image) - require.True(t, resolver.built) spec := f.created[wsName] require.Equal(t, rootUser, spec.User) require.Equal( @@ -104,7 +100,7 @@ func TestWorkspaceOwnerPolicyOff(t *testing.T) { RemoteUser: testUser, Dockerless: true, } - owner, err := d.resolveWorkspaceOwner(context.Background(), options) + owner, err := d.resolveWorkspaceOwner(context.Background(), options, nil) require.NoError(t, err) require.Nil(t, owner) require.Zero(t, resolver.calls) @@ -147,7 +143,7 @@ func TestWorkspaceOwnerFallbackIdentity(t *testing.T) { User: tt.user, RemoteUser: tt.remote, WorkspaceMount: &config.Mount{Source: testBindSrc, Target: testBindDst}, - }, + }, nil, ) require.NoError(t, err) require.Equal(t, tt.want, resolver.user) @@ -182,7 +178,11 @@ func TestDockerlessOwnerNeverLooksUpRunnerAccounts(t *testing.T) { resolver := &recordingUserResolver{err: errors.New("runner accounts must not be read")} d.userResolver = resolver err := d.RunDevContainer(context.Background(), wsID, &driver.RunOptions{ - Image: imgX, User: rootUser, RemoteUser: user, Dockerless: true, + Image: imgX, + User: rootUser, + RemoteUser: user, + Dockerless: true, + AllowRecreate: true, WorkspaceMount: &config.Mount{Source: testBindSrc, Target: testBindDst}, }) if user != rootUser { diff --git a/pkg/driver/types.go b/pkg/driver/types.go index 1f3ccc8477..0ab7c13e9a 100644 --- a/pkg/driver/types.go +++ b/pkg/driver/types.go @@ -178,6 +178,9 @@ type RunOptions struct { // Dockerless builds the developer filesystem inside the runner after creation. Dockerless bool `json:"dockerless,omitempty"` + // AllowRecreate authorizes replacing an existing VM and discarding its root disk. + AllowRecreate bool `json:"allowRecreate,omitempty"` + // Entrypoint is the entrypoint of the container Entrypoint string `json:"entrypoint,omitempty"` diff --git a/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx b/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx index c19857aa29..52416f7f9e 100644 --- a/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx +++ b/sites/docs-devsy-sh/content/docs/developing-providers/driver.mdx @@ -223,22 +223,26 @@ existing VM. For a non-root developer, use a prebuilt image or explicitly select `workspaceStatVirtualization: "off"` with private host permissions. Devsy never looks up the developer account in the unrelated Dockerless runner image. -Image inspection, account resolution, and import prefer an image cached in the -configured Docker-compatible CLI. This permits offline/private-registry startup -and keeps metadata and mount ownership aligned with the image actually imported. -Registry-only images do not require Docker initialization. +Image inspection and preparation prefer an image cached in the configured +Docker-compatible CLI, permitting offline/private-registry startup. Preparation +freezes the final image before account resolution and import. Both use that same +snapshot, imported under a content-derived tag, so retagging the original image +cannot change the imported filesystem or its fallback owner. Registry-only +images do not require Docker initialization. Devsy-managed workspaces with an incompatible persisted workspace mount contract -are automatically recreated once on `devsy up`, including workspaces created -before the contract label existed. Changing either workspace permission policy -or the effective developer identity also requires recreation. The host workspace -and named volumes persist, but -changes stored only on the VM's root disk are lost, as with `--recreate`. -Externally managed containers are never automatically replaced. +require explicit `devsy up --recreate`, including workspaces created before the +contract label existed. Changing workspace permission policy or the effective +developer identity also requires explicit recreation. Ordinary `devsy up` reports +an actionable error and preserves the existing VM. Back up VM-local data before +recreating: the host workspace and named volumes persist, but the VM root disk +is discarded. Replacement cannot roll back failures after removal. +Externally managed containers cannot be replaced through this flow. Devsy requires MicroSandbox v0.7.2 or newer when creating or recreating workspaces that use workspace ownership synchronization. Runtime compatibility, account -resolution, and final image import are checked before removing an existing VM. +resolution, final image import, and volume preparation complete before removing +an existing VM. The provisioning version check does not block inspection, logs, stop, or deletion of an existing sandbox. Apple Silicon macOS is the canonical supported integration-test host; use From 2587907afab4740fc0788fbfa9912daf08d4402d Mon Sep 17 00:00:00 2001 From: Samuel K Date: Tue, 6 Oct 2026 01:47:19 -0600 Subject: [PATCH 09/10] fix(microsandbox): stream frozen image imports --- pkg/driver/microsandbox/cliclient.go | 40 +++++++---- pkg/driver/microsandbox/image_user_test.go | 79 ++++++++++++++++------ 2 files changed, 83 insertions(+), 36 deletions(-) diff --git a/pkg/driver/microsandbox/cliclient.go b/pkg/driver/microsandbox/cliclient.go index 7ce48beb22..9624bcff30 100644 --- a/pkg/driver/microsandbox/cliclient.go +++ b/pkg/driver/microsandbox/cliclient.go @@ -325,22 +325,32 @@ func loadImageSnapshot(ctx context.Context, imageRef string, img v1.Image) error return fmt.Errorf("parse image reference %q: %w", imageRef, err) } - tmp, err := os.CreateTemp("", "devsy-msb-*.tar") + // #nosec G204 -- resolved runtime binary and content-derived image reference + load := exec.CommandContext(ctx, msbBinary(), "load", "-t", imageRef) + var output strings.Builder + load.Stdout, load.Stderr = &output, &output + writer, err := load.StdinPipe() if err != nil { - return fmt.Errorf("create image tarball: %w", err) - } - tmpPath := tmp.Name() - _ = tmp.Close() - defer func() { _ = os.Remove(tmpPath) }() - - if err := tarball.WriteToFile(tmpPath, ref, img); err != nil { - return fmt.Errorf("write image tarball: %w", err) - } - - // #nosec G204 -- args are a resolved binary path and an internally-created file - load := exec.CommandContext(ctx, msbBinary(), "load", "-i", tmpPath, "-t", imageRef) - if out, err := load.CombinedOutput(); err != nil { - return fmt.Errorf("msb load %q: %s: %w", imageRef, out, err) + return fmt.Errorf("pipe image import: %w", err) + } + defer func() { _ = writer.Close() }() + stopClose := context.AfterFunc(ctx, func() { _ = writer.Close() }) + defer stopClose() + if err := load.Start(); err != nil { + return fmt.Errorf("start msb image import: %w", err) + } + writeErr := tarball.Write(ref, img, writer) + closeErr := writer.Close() + if err := errors.Join(writeErr, closeErr, ctx.Err()); err != nil { + _ = load.Process.Kill() + _ = load.Wait() + return fmt.Errorf("stream image %q: %s: %w", imageRef, output.String(), err) + } + if err := load.Wait(); err != nil { + return fmt.Errorf( + "msb load %q: %s: %w", + imageRef, output.String(), errors.Join(err, ctx.Err()), + ) } return nil } diff --git a/pkg/driver/microsandbox/image_user_test.go b/pkg/driver/microsandbox/image_user_test.go index 27c41e493b..34d3c74e65 100644 --- a/pkg/driver/microsandbox/image_user_test.go +++ b/pkg/driver/microsandbox/image_user_test.go @@ -4,6 +4,7 @@ import ( "archive/tar" "bytes" "context" + "crypto/rand" "io" "net/http/httptest" "os" @@ -11,6 +12,7 @@ import ( "runtime" "strings" "testing" + "time" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/registry" @@ -124,9 +126,7 @@ func (s *imageUserSuite) TestCanceledExtraction() { //nolint:gosec // Synthetic account archive and executable fixture under TempDir. func (s *imageUserSuite) TestLocalFinalImageUsesConfiguredCLI() { - if runtime.GOOS == "windows" { - s.T().Skip("fixture executable requires a POSIX shell") - } + s.requirePOSIXShell() img, err := mutate.AppendLayers( empty.Image, s.layer(map[string]string{passwdPath: snapshotPasswd}), @@ -177,9 +177,7 @@ cat "$DEVSY_TEST_ARCHIVE" //nolint:gosec // Synthetic archives and executable fixtures under TempDir. func (s *imageUserSuite) TestSnapshotSurvivesRetagBeforeImport() { - if runtime.GOOS == "windows" { - s.T().Skip("fixture executable requires a POSIX shell") - } + s.requirePOSIXShell() for _, source := range []string{"local", "registry"} { s.Run(source, func() { ctx := context.Background() @@ -218,6 +216,10 @@ func (s *imageUserSuite) TestSnapshotSurvivesRetagBeforeImport() { s.Require().NoError(err) defer prepared.cleanup() writeImage(replacement) + newer, err := client.PrepareImage(ctx, ref, false) + s.Require().NoError(err) + defer newer.cleanup() + s.NotEqual(prepared.reference, newer.reference) owner, err := (filesystemUserResolver{}).ResolveImage(ctx, prepared.image, testUser) s.Require().NoError(err) s.Equal(mountOwner{2000, 2001}, *owner) @@ -231,28 +233,35 @@ func (s *imageUserSuite) TestSnapshotSurvivesRetagBeforeImport() { alias, err := os.ReadFile(aliasFile) s.Require().NoError(err) s.Equal(prepared.reference, string(alias)) - newer, err := client.PrepareImage(ctx, ref, false) - s.Require().NoError(err) - defer newer.cleanup() - s.NotEqual(prepared.reference, newer.reference) }) } } -func (s *imageUserSuite) snapshotImage(accounts string) v1.Image { - img, err := mutate.AppendLayers(empty.Image, s.layer(map[string]string{passwdPath: accounts})) - s.Require().NoError(err) - return img +//nolint:gosec // Test-created executable under TempDir. +func (s *imageUserSuite) TestStreamImportReapsEarlyConsumerExit() { + s.requirePOSIXShell() + dir := s.T().TempDir() + s.T().Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + script := "#!/bin/sh\necho rejected >&2\nexit 1\n" + s.Require().NoError(os.WriteFile(filepath.Join(dir, "msb"), []byte(script), 0o700)) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + err := loadImageSnapshot(ctx, "devsy-msb-image:test", s.snapshotImage(snapshotPasswd)) + s.ErrorContains(err, "rejected") + s.NoError(ctx.Err(), "an early consumer exit must not leave a blocked writer") } //nolint:gosec // Test-created executable under TempDir. -func (s *imageUserSuite) writeSnapshotImportCLI(dir string) { - msb := `#!/bin/sh -[ "$1" = load ] || exit 1 -cp "$3" "$DEVSY_TEST_IMPORTED" -printf '%s' "$5" > "$DEVSY_TEST_ALIAS" -` - s.Require().NoError(os.WriteFile(filepath.Join(dir, "msb"), []byte(msb), 0o700)) +func (s *imageUserSuite) TestStreamImportHonorsCancellation() { + s.requirePOSIXShell() + dir := s.T().TempDir() + s.T().Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + script := "#!/bin/sh\nexec sleep 60\n" + s.Require().NoError(os.WriteFile(filepath.Join(dir, "msb"), []byte(script), 0o700)) + img := s.snapshotImage(snapshotPasswd) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + s.ErrorIs(loadImageSnapshot(ctx, "devsy-msb-image:test", img), context.DeadlineExceeded) } func (s *imageUserSuite) layer(files map[string]string) v1.Layer { @@ -271,3 +280,31 @@ func (s *imageUserSuite) layer(files map[string]string) v1.Layer { s.Require().NoError(err) return layer } + +func (s *imageUserSuite) snapshotImage(accounts string) v1.Image { + padding := make([]byte, 1024*1024) + _, err := rand.Read(padding) + s.Require().NoError(err) + img, err := mutate.AppendLayers(empty.Image, s.layer(map[string]string{ + passwdPath: accounts, "padding.bin": string(padding), + })) + s.Require().NoError(err) + return img +} + +//nolint:gosec // Test-created executable under TempDir. +func (s *imageUserSuite) writeSnapshotImportCLI(dir string) { + msb := `#!/bin/sh +[ "$1" = load ] || exit 1 +cat > "$DEVSY_TEST_IMPORTED" +printf '%s' "$3" > "$DEVSY_TEST_ALIAS" +` + s.T().Setenv("TMPDIR", filepath.Join(dir, "unavailable-temp")) + s.Require().NoError(os.WriteFile(filepath.Join(dir, "msb"), []byte(msb), 0o700)) +} + +func (s *imageUserSuite) requirePOSIXShell() { + if runtime.GOOS == "windows" { + s.T().Skip("fixture executable requires a POSIX shell") + } +} From ff8ac5dc91871ec8d2ae4a141c2ab9437c43299c Mon Sep 17 00:00:00 2001 From: Samuel K Date: Tue, 6 Oct 2026 03:32:10 -0600 Subject: [PATCH 10/10] fix(microsandbox): inspect metadata without exporting image layers Read cached image configuration directly while preserving registry fallback. Redact rootful Podman endpoint credentials from bounded diagnostics and retain only the command error in the diagnostic error chain. --- e2e/tests/up/podman_rootful.go | 61 +++++++++++++++-- e2e/tests/up/podman_rootful_test.go | 54 ++++++++++++++- pkg/driver/microsandbox/image_user.go | 32 +++++++++ pkg/driver/microsandbox/image_user_test.go | 76 ++++++++++++++++++++-- pkg/driver/microsandbox/microsandbox.go | 21 ++---- 5 files changed, 216 insertions(+), 28 deletions(-) diff --git a/e2e/tests/up/podman_rootful.go b/e2e/tests/up/podman_rootful.go index 926b395c6d..bc7a4632e1 100644 --- a/e2e/tests/up/podman_rootful.go +++ b/e2e/tests/up/podman_rootful.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net/url" "os" "os/exec" "strings" @@ -12,6 +13,7 @@ import ( "github.com/devsy-org/devsy/e2e/framework" "github.com/devsy-org/devsy/pkg/docker" + "github.com/devsy-org/devsy/pkg/secrets" "github.com/onsi/ginkgo/v2" ) @@ -159,9 +161,9 @@ func checkPodmanHealth(ctx context.Context, wrapperPath string) (podmanHealthCla return podmanHealthOK, nil } class := classifyPodmanHealthFailure(healthCtx, string(out)) - return class, fmt.Errorf( + return class, redactPodmanError(err, fmt.Sprintf( "rootful Podman readiness check failed (class: %s) or exceeded %s\n"+ - "command: %s ps\nDOCKER_HOST: %s\ncontext err: %v\noutput:\n%s\nerror: %w", + "command: %s ps\nDOCKER_HOST: %s\ncontext err: %v\noutput:\n%s\nerror: %v", class, podmanHealthCheckTimeout, wrapperPath, @@ -169,7 +171,7 @@ func checkPodmanHealth(ctx context.Context, wrapperPath string) (podmanHealthCla healthCtx.Err(), string(out), err, - ) + )) } // runDiagCommand never fails the caller: diagnostics are best-effort so a @@ -188,6 +190,7 @@ func runDiagCommand(ctx context.Context, name string, args ...string) string { if err != nil { text += fmt.Sprintf("\n(command failed: %v; context err: %v)", err, diagCtx.Err()) } + text = redactPodmanDiagnostics(text) limit := podmanDiagMaxOutput if name == "ps" || name == "lslocks" || name == "sh" || strings.Contains(strings.Join(args, " "), "lslocks") { @@ -294,12 +297,12 @@ func attemptPodmanRecovery(ctx context.Context, wrapperPath string) (podmanHealt docker.PrepareForGroupCancellation(cmd) started := time.Now() if out, err := cmd.CombinedOutput(); err != nil { - return podmanHealthPoisoned, fmt.Errorf( - "PODMAN_ROOTFUL_RECOVERY_FAILED: systemctl restart failed after %s: %w\noutput:\n%s", + return podmanHealthPoisoned, redactPodmanError(err, fmt.Sprintf( + "PODMAN_ROOTFUL_RECOVERY_FAILED: systemctl restart failed after %s: %v\noutput:\n%s", time.Since(started), err, string(out), - ) + )) } ginkgo.GinkgoWriter.Printf("[podman-recovery] restart elapsed=%s\n", time.Since(started)) class, err := checkPodmanHealth(ctx, wrapperPath) @@ -468,3 +471,49 @@ func recoverPodmanCleanup( ) return nil } + +// Redact before truncation so a clipped URL cannot expose partial credentials. +func redactPodmanDiagnostics(text string) string { + return secrets.Combine(secrets.NewRedactor(podmanEndpointSecrets(os.Getenv("DOCKER_HOST"))), + secrets.NewEnvironmentRedactor(os.Environ())).Redact(text) +} + +func podmanEndpointSecrets(value string) []string { + endpoint, err := url.Parse(value) + if err != nil { + return []string{"endpoint=" + value} + } + if endpoint.User == nil { + return nil + } + entries := podmanUserinfoSecrets(endpoint.User.String()) + entries = append(entries, "username="+endpoint.User.Username()) + if password, ok := endpoint.User.Password(); ok { + entries = append(entries, "password="+password) + } + start, end := strings.Index(value, "://"), strings.LastIndex(value, "@") + if start >= 0 && end > start+3 { + entries = append(entries, podmanUserinfoSecrets(value[start+3:end])...) + } + return entries +} + +func podmanUserinfoSecrets(userinfo string) []string { + entries := []string{"userinfo=" + userinfo} + if _, encoded, ok := strings.Cut(userinfo, ":"); ok { + entries = append(entries, "encoded="+encoded) + } + return entries +} + +type podmanDiagnosticError struct { + cause error + message string +} + +func (e podmanDiagnosticError) Error() string { return e.message } +func (e podmanDiagnosticError) Unwrap() error { return e.cause } + +func redactPodmanError(cause error, message string) error { + return podmanDiagnosticError{cause: cause, message: redactPodmanDiagnostics(message)} +} diff --git a/e2e/tests/up/podman_rootful_test.go b/e2e/tests/up/podman_rootful_test.go index cdc3f1e98c..1a740335ee 100644 --- a/e2e/tests/up/podman_rootful_test.go +++ b/e2e/tests/up/podman_rootful_test.go @@ -113,9 +113,15 @@ func TestRunPodmanDiagnosticsStopsAtBudget(t *testing.T) { assert.Equal(t, []string{"ps"}, commands) } +//nolint:gosec // Synthetic endpoint credentials verify unchanged routing. func TestRootfulPodmanWrapper(t *testing.T) { + const ( + remoteFlag = "--remote" + urlFlag = "--url" + ) endpoint := "unix:///run/podman/podman.sock" emptyEndpoint := "" + credentialEndpoint := "ssh://fixture-user:fixture%40password@podman.example/run/podman.sock" cases := []struct { name string endpoint *string @@ -125,13 +131,19 @@ func TestRootfulPodmanWrapper(t *testing.T) { { name: "configured endpoint", endpoint: &endpoint, - wantPrefix: []string{"podman", "--remote", "--url", endpoint}, + wantPrefix: []string{"podman", remoteFlag, urlFlag, endpoint}, args: []string{"ps", "-a"}, }, + { + name: "credential endpoint remains unchanged", + endpoint: &credentialEndpoint, + wantPrefix: []string{"podman", remoteFlag, urlFlag, credentialEndpoint}, + args: []string{"ps"}, + }, { name: "preserves arguments", endpoint: &endpoint, - wantPrefix: []string{"podman", "--remote", "--url", endpoint}, + wantPrefix: []string{"podman", remoteFlag, urlFlag, endpoint}, args: []string{"run", "value with spaces", "$(printf unsafe)"}, }, { @@ -269,3 +281,41 @@ func checkUnmanagedPodmanCleanup(t *testing.T, endpoint string) { require.NoError(t, err) assert.NotContains(t, string(commands), "systemctl restart") } + +//nolint:gosec // Synthetic credentials exercise diagnostic redaction. +func TestPodmanDiagnosticsRedactEndpointCredentials(t *testing.T) { + const endpoint = "ssh://fixture-user:fixture%40password@podman.example/run/podman.sock" + t.Setenv("DOCKER_HOST", endpoint) + dir := t.TempDir() + wrapper := filepath.Join(dir, "wrapper") + script := `#!/bin/sh +printf 'cannot connect to %s; password=fixture@password; encoded=fixture%%40password\n' "$DOCKER_HOST" +exit 1 +` + require.NoError(t, os.WriteFile(wrapper, []byte(script), 0o700)) + class, err := checkPodmanHealth(context.Background(), wrapper) + require.Error(t, err) + assert.Equal(t, podmanHealthUnavailable, class) + var exitError *exec.ExitError + assert.ErrorAs(t, err, &exitError) + assert.Contains(t, err.Error(), "ssh://***@podman.example/run/podman.sock") + assert.Contains(t, err.Error(), "cannot connect") + diagnostics := runDiagCommand(context.Background(), wrapper) + // The error must remain safe even after the originating environment changes. + t.Setenv("DOCKER_HOST", "") + for _, text := range []string{err.Error(), errors.Unwrap(err).Error(), diagnostics} { + assert.NotContains(t, text, "fixture-user") + assert.NotContains(t, text, "fixture@password") + assert.NotContains(t, text, "fixture%40password") + } + assert.Contains(t, diagnostics, "podman.example") +} + +//nolint:gosec // Synthetic credentials cover noncanonical URL escaping. +func TestPodmanDiagnosticsRedactOriginalURLEscaping(t *testing.T) { + t.Setenv("DOCKER_HOST", "ssh://fixture-user:fixture%4apassword@podman.example/run/podman.sock") + text := redactPodmanDiagnostics( + os.Getenv("DOCKER_HOST") + " fixture%4apassword fixtureJpassword", + ) + assert.Equal(t, "ssh://***@podman.example/run/podman.sock *** ***", text) +} diff --git a/pkg/driver/microsandbox/image_user.go b/pkg/driver/microsandbox/image_user.go index dc414f506f..a6b0916770 100644 --- a/pkg/driver/microsandbox/image_user.go +++ b/pkg/driver/microsandbox/image_user.go @@ -3,6 +3,7 @@ package microsandbox import ( "archive/tar" "context" + "encoding/json" "fmt" "io" "os" @@ -300,3 +301,34 @@ func localImageAvailable(ctx context.Context, dockerPath, ref string) bool { // #nosec G204 -- configured Docker-compatible executable and fixed inspection arguments return exec.CommandContext(ctx, dockerPath, "image", "inspect", ref).Run() == nil } + +func inspectImageConfig(ctx context.Context, dockerPath, ref string) (*v1.Config, error) { + if dockerPath == "" { + dockerPath = dockerExecutable + } + inspectCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + // #nosec G204 -- configured Docker-compatible executable and fixed inspection arguments + out, err := exec.CommandContext(inspectCtx, dockerPath, + "image", "inspect", "--format", "{{json .Config}}", ref).Output() + if err == nil { + var cfg v1.Config + if err := json.Unmarshal(out, &cfg); err != nil { + return nil, fmt.Errorf("parse cached image config for %s: %w", ref, err) + } + return &cfg, nil + } + if err := ctx.Err(); err != nil { + return nil, err + } + // A failed cache probe must not make Docker a requirement for registry images. + img, err := image.GetImageForArch(ctx, ref, runtime.GOARCH) + if err != nil { + return nil, err + } + cfg, err := img.ConfigFile() + if err != nil { + return nil, err + } + return &cfg.Config, nil +} diff --git a/pkg/driver/microsandbox/image_user_test.go b/pkg/driver/microsandbox/image_user_test.go index 34d3c74e65..3862dbf387 100644 --- a/pkg/driver/microsandbox/image_user_test.go +++ b/pkg/driver/microsandbox/image_user_test.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "crypto/rand" + "encoding/json" "io" "net/http/httptest" "os" @@ -168,11 +169,6 @@ cat "$DEVSY_TEST_ARCHIVE" args, err := os.ReadFile(argsFile) s.Require().NoError(err) s.Equal("save\nfinal-image:latest\n", string(args)) - d := newDriver(newFakeClient(), nil, specDefaults{}) - d.dockerPath = executable - details, err := d.InspectImage(context.Background(), "final-image:latest") - s.Require().NoError(err) - s.Equal(testUser, details.Config.User) } //nolint:gosec // Synthetic archives and executable fixtures under TempDir. @@ -264,6 +260,76 @@ func (s *imageUserSuite) TestStreamImportHonorsCancellation() { s.ErrorIs(loadImageSnapshot(ctx, "devsy-msb-image:test", img), context.DeadlineExceeded) } +func (s *imageUserSuite) TestRegistryMetadataWithoutCLI() { + s.requirePOSIXShell() + server := httptest.NewServer(registry.New()) + defer server.Close() + ref, err := name.NewTag(strings.TrimPrefix(server.URL, "http://") + "/metadata:latest") + s.Require().NoError(err) + img := s.snapshotImage(snapshotPasswd) + img, err = mutate.Config(img, v1.Config{User: testUser}) + s.Require().NoError(err) + s.Require().NoError(remote.Write(ref, img)) + d := newDriver(newFakeClient(), nil, specDefaults{}) + dir := s.T().TempDir() + failingCLI := filepath.Join(dir, "failing-cli") + // #nosec G306 -- executable fixture under TempDir. + s.Require().NoError(os.WriteFile(failingCLI, []byte("#!/bin/sh\nexit 1\n"), 0o700)) + for _, cli := range []string{filepath.Join(dir, "missing-cli"), failingCLI} { + d.dockerPath = cli + details, err := d.InspectImage(context.Background(), ref.Name()) + s.Require().NoError(err) + s.Equal(testUser, details.Config.User) + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err = d.InspectImage(ctx, ref.Name()) + s.ErrorIs(err, context.Canceled) +} + +//nolint:gosec // Test-created executable under TempDir. +func (s *imageUserSuite) TestInspectCachedImageConfig() { + s.requirePOSIXShell() + cfg := v1.Config{ + User: testUser, + Env: []string{"EXAMPLE=value"}, + Labels: map[string]string{ + "example": "value", + }, + Entrypoint: []string{"/bin/sh"}, + Cmd: []string{"-l"}, + } + dir := s.T().TempDir() + configFile, argsFile := filepath.Join(dir, "config.json"), filepath.Join(dir, "args") + configJSON, err := json.Marshal(cfg) + s.Require().NoError(err) + s.Require().NoError(os.WriteFile(configFile, configJSON, 0o600)) + s.T().Setenv("DEVSY_TEST_CONFIG", configFile) + s.T().Setenv("DEVSY_TEST_ARGS", argsFile) + executable := filepath.Join(dir, "configured-cli") + script := `#!/bin/sh +[ "$1" = image ] && [ "$2" = inspect ] || exit 1 +printf '%s\n' "$@" > "$DEVSY_TEST_ARGS" +cat "$DEVSY_TEST_CONFIG" +` + s.Require().NoError(os.WriteFile(executable, []byte(script), 0o700)) + d := newDriver(newFakeClient(), nil, specDefaults{}) + d.dockerPath = executable + details, err := d.InspectImage(context.Background(), "final-image:latest") + s.Require().NoError(err) + s.Equal(cfg.User, details.Config.User) + s.Equal(cfg.Env, details.Config.Env) + s.Equal(cfg.Labels, details.Config.Labels) + s.Equal(cfg.Entrypoint, details.Config.Entrypoint) + s.Equal(cfg.Cmd, details.Config.Cmd) + args, err := os.ReadFile(argsFile) + s.Require().NoError(err) + s.Equal("image\ninspect\n--format\n{{json .Config}}\nfinal-image:latest\n", string(args)) + s.Require().NoError(os.WriteFile(configFile, []byte("invalid JSON"), 0o600)) + _, err = d.InspectImage(context.Background(), "final-image:latest") + s.ErrorContains(err, "parse cached image config") +} + func (s *imageUserSuite) layer(files map[string]string) v1.Layer { var buf bytes.Buffer writer := tar.NewWriter(&buf) diff --git a/pkg/driver/microsandbox/microsandbox.go b/pkg/driver/microsandbox/microsandbox.go index 04b3a9d019..15201519cf 100644 --- a/pkg/driver/microsandbox/microsandbox.go +++ b/pkg/driver/microsandbox/microsandbox.go @@ -244,27 +244,18 @@ func (d *microsandboxDriver) InspectImage( if _, err := d.TargetArchitecture(ctx, ""); err != nil { return nil, err } - img, cleanup, err := (filesystemUserResolver{dockerPath: d.dockerPath}).openImage( - ctx, - imageName, - false, - ) - if err != nil { - return nil, fmt.Errorf("get image for %s: %w", imageName, err) - } - defer cleanup() - cfg, err := img.ConfigFile() + cfg, err := inspectImageConfig(ctx, d.dockerPath, imageName) if err != nil { return nil, fmt.Errorf("get image config for %s: %w", imageName, err) } return &config.ImageDetails{ ID: imageName, Config: config.ImageDetailsConfig{ - User: cfg.Config.User, - Env: cfg.Config.Env, - Labels: cfg.Config.Labels, - Entrypoint: cfg.Config.Entrypoint, - Cmd: cfg.Config.Cmd, + User: cfg.User, + Env: cfg.Env, + Labels: cfg.Labels, + Entrypoint: cfg.Entrypoint, + Cmd: cfg.Cmd, }, }, nil }