From ad9b748b1f3b0935c2db855c020e0a293b789a5c Mon Sep 17 00:00:00 2001 From: Samuel K Date: Wed, 8 Jul 2026 09:10:56 -0500 Subject: [PATCH 1/4] refactor(devcontainer): rebuild run.go with idiomatic Go structure Split the grab-bag run.go into cohesive files, unexport the runner struct fields, and simplify the initializeCommand and mount helpers. - run.go: Runner interface, runner type, Up dispatch, delegations - initialize.go: host-side initializeCommand hook execution - mount.go: workspace mount/consistency helpers Renames runner's exported fields (Driver, ID, WorkspaceConfig, ...) to unexported equivalents via type-aware rename across the package. Removes the context-mimicking initCmdContext struct and the redundant serial/parallel split, and de-duplicates the workspace mount-folder defaulting. --- pkg/devcontainer/build.go | 12 +- pkg/devcontainer/compose.go | 20 +- pkg/devcontainer/compose_test.go | 8 +- pkg/devcontainer/compose_up.go | 8 +- pkg/devcontainer/config.go | 34 +-- pkg/devcontainer/config_test.go | 14 +- pkg/devcontainer/delete.go | 24 +- pkg/devcontainer/delete_test.go | 6 +- pkg/devcontainer/initialize.go | 104 +++++++++ pkg/devcontainer/inspect.go | 6 +- pkg/devcontainer/mount.go | 86 +++++++ pkg/devcontainer/prebuild.go | 4 +- pkg/devcontainer/run.go | 386 ++++++++----------------------- pkg/devcontainer/setup.go | 96 ++++---- pkg/devcontainer/setup_test.go | 6 +- pkg/devcontainer/single.go | 74 +++--- 16 files changed, 438 insertions(+), 450 deletions(-) create mode 100644 pkg/devcontainer/initialize.go create mode 100644 pkg/devcontainer/mount.go diff --git a/pkg/devcontainer/build.go b/pkg/devcontainer/build.go index 2c137226a..d1e69f395 100644 --- a/pkg/devcontainer/build.go +++ b/pkg/devcontainer/build.go @@ -416,7 +416,7 @@ func (r *runner) buildImage( parsedConfig := params.parsedConfig options := params.options - targetArch, err := r.Driver.TargetArchitecture(ctx, r.ID) + targetArch, err := r.driver.TargetArchitecture(ctx, r.id) if err != nil { return nil, err } @@ -471,7 +471,7 @@ func (r *runner) executeBuild( ExtendedBuildInfo: params.extendedBuildInfo, DockerfilePath: params.dockerfilePath, DockerfileContent: params.dockerfileContent, - LocalWorkspaceFolder: r.LocalWorkspaceFolder, + LocalWorkspaceFolder: r.localWorkspaceFolder, Options: options, TargetArch: targetArch, }) @@ -484,16 +484,16 @@ func (r *runner) executeBuild( // check if we should fallback to dockerless. // This should only be OSS kubernetes as of March 06, 2025. - dockerDriver, ok := r.Driver.(driver.DockerDriver) + dockerDriver, ok := r.driver.(driver.DockerDriver) if options.ForceDockerless || !ok { - if r.WorkspaceConfig.Agent.Dockerless.Disabled == pkgconfig.BoolTrue { + if r.workspaceConfig.Agent.Dockerless.Disabled == pkgconfig.BoolTrue { return nil, fmt.Errorf( "cannot build devcontainer because driver is non-docker and dockerless fallback is disabled", ) } return dockerlessFallback(&dockerlessFallbackParams{ - localWorkspaceFolder: r.LocalWorkspaceFolder, + localWorkspaceFolder: r.localWorkspaceFolder, containerWorkspaceFolder: params.substitutionContext.ContainerWorkspaceFolder, parsedConfig: params.parsedConfig, buildInfo: params.buildInfo, @@ -509,7 +509,7 @@ func (r *runner) executeBuild( ExtendedBuildInfo: params.extendedBuildInfo, DockerfilePath: params.dockerfilePath, DockerfileContent: params.dockerfileContent, - LocalWorkspaceFolder: r.LocalWorkspaceFolder, + LocalWorkspaceFolder: r.localWorkspaceFolder, Options: options, }) } diff --git a/pkg/devcontainer/compose.go b/pkg/devcontainer/compose.go index 3caf83fd1..26d170c67 100644 --- a/pkg/devcontainer/compose.go +++ b/pkg/devcontainer/compose.go @@ -96,7 +96,7 @@ type composeUpParams struct { } func (r *runner) composeHelper() (*compose.ComposeHelper, error) { - dockerDriver, ok := r.Driver.(driver.DockerDriver) + dockerDriver, ok := r.driver.(driver.DockerDriver) if !ok { return nil, fmt.Errorf( "docker compose is not supported by this provider, choose a different one", @@ -112,7 +112,7 @@ func (r *runner) stopDockerCompose(ctx context.Context, projectName string) erro return fmt.Errorf("find docker compose: %w", err) } - parsedConfig, _, err := r.getSubstitutedConfig(r.WorkspaceConfig.CLIOptions) + parsedConfig, _, err := r.getSubstitutedConfig(r.workspaceConfig.CLIOptions) if err != nil { return fmt.Errorf("get parsed config: %w", err) } @@ -140,7 +140,7 @@ func (r *runner) deleteDockerCompose( return fmt.Errorf("find docker compose: %w", err) } - parsedConfig, _, err := r.getSubstitutedConfig(r.WorkspaceConfig.CLIOptions) + parsedConfig, _, err := r.getSubstitutedConfig(r.workspaceConfig.CLIOptions) if err != nil { return fmt.Errorf("get parsed config: %w", err) } @@ -235,7 +235,7 @@ func (r *runner) loadComposeProject( if err != nil { return nil, fmt.Errorf("load docker compose project: %w", err) } - project.Name = composeHelper.GetProjectName(r.ID) + project.Name = composeHelper.GetProjectName(r.id) log.Debugf("Loaded project %s", project.Name) if err := validateRunServices(parsedConfig.Config.RunServices, project); err != nil { @@ -375,7 +375,7 @@ func (r *runner) updateContainerUserUID( ctx context.Context, parsedConfig *config.DevContainerConfig, ) error { - dockerDriver, ok := r.Driver.(driver.DockerDriver) + dockerDriver, ok := r.driver.(driver.DockerDriver) if !ok { return nil } @@ -383,7 +383,7 @@ func (r *runner) updateContainerUserUID( defer func() { _ = writer.Close() }() if err := dockerDriver.UpdateContainerUserUID( ctx, - r.ID, + r.id, parsedConfig, writer, ); err != nil { @@ -604,7 +604,7 @@ func composeFileFromEnv(envFiles []string) (string, error) { func (r *runner) getEnvFiles() []string { var envFiles []string - envFile := path.Join(r.LocalWorkspaceFolder, ".env") + envFile := path.Join(r.localWorkspaceFolder, ".env") envFileStat, err := os.Stat(envFile) if err == nil && envFileStat.Mode().IsRegular() { envFiles = append(envFiles, envFile) @@ -873,11 +873,11 @@ func (r *runner) recreateDevContainer( ) error { log.Debugf("Deleting dev container %s due to --recreate", container.ID) - if err := r.Driver.StopDevContainer(ctx, r.ID); err != nil { + if err := r.driver.StopDevContainer(ctx, r.id); err != nil { return fmt.Errorf("stop dev container: %w", err) } - if err := r.Driver.DeleteDevContainer(ctx, r.ID); err != nil { + if err := r.driver.DeleteDevContainer(ctx, r.id); err != nil { return fmt.Errorf("delete dev container: %w", err) } return nil @@ -954,7 +954,7 @@ func getDockerComposeFolder(workspaceOriginFolder string) string { // docker-compose folder using a collision-safe unique name that retains the // given prefix (so checkForPersistedFile can still match it by prefix). func (r *runner) writeComposeOverrideFile(prefix string, data []byte) (string, error) { - dockerComposeFolder := getDockerComposeFolder(r.WorkspaceConfig.Origin) + dockerComposeFolder := getDockerComposeFolder(r.workspaceConfig.Origin) if err := os.MkdirAll(dockerComposeFolder, 0o750); err != nil { return "", err } diff --git a/pkg/devcontainer/compose_test.go b/pkg/devcontainer/compose_test.go index 845e08994..fd6e336b1 100644 --- a/pkg/devcontainer/compose_test.go +++ b/pkg/devcontainer/compose_test.go @@ -552,7 +552,7 @@ func TestEscapeComposeLabelValueRoundTrip(t *testing.T) { func TestBuildServiceLabels(t *testing.T) { t.Run("uses default ID label when no ID labels", func(t *testing.T) { r := &runner{} - r.ID = "workspace-id" + r.id = "workspace-id" labels := r.buildServiceLabels(nil) @@ -567,7 +567,7 @@ func TestBuildServiceLabels(t *testing.T) { t.Run("escapes dollars but preserves other characters", func(t *testing.T) { r := &runner{} - r.IDLabels = []string{"id.label=$value"} + r.idLabels = []string{"id.label=$value"} labels := r.buildServiceLabels(map[string]string{"extra": "it's $here"}) @@ -582,7 +582,7 @@ func TestBuildServiceLabels(t *testing.T) { t.Run("splits ID label only on first equals sign", func(t *testing.T) { r := &runner{} - r.IDLabels = []string{"id.label=a=b=c"} + r.idLabels = []string{"id.label=a=b=c"} labels := r.buildServiceLabels(nil) @@ -593,7 +593,7 @@ func TestBuildServiceLabels(t *testing.T) { t.Run("additional labels merge alongside ID labels", func(t *testing.T) { r := &runner{} - r.IDLabels = []string{"id.label=v"} + r.idLabels = []string{"id.label=v"} labels := r.buildServiceLabels(map[string]string{"k": "plain"}) diff --git a/pkg/devcontainer/compose_up.go b/pkg/devcontainer/compose_up.go index c6747351d..4cfa54a27 100644 --- a/pkg/devcontainer/compose_up.go +++ b/pkg/devcontainer/compose_up.go @@ -152,13 +152,13 @@ exec "$$@" // are merged in. All values are escaped to prevent compose variable expansion. func (r *runner) buildServiceLabels(additionalLabels map[string]string) composetypes.Labels { labels := composetypes.Labels{} - if len(r.IDLabels) > 0 { - for _, l := range r.IDLabels { + if len(r.idLabels) > 0 { + for _, l := range r.idLabels { k, v, _ := strings.Cut(l, "=") labels[k] = escapeComposeLabelValue(v) } } else { - labels[pkgconfig.DevcontainerIDLabel] = r.ID + labels[pkgconfig.DevcontainerIDLabel] = r.id } for k, v := range additionalLabels { labels.Add(k, escapeComposeLabelValue(v)) @@ -189,7 +189,7 @@ func namedVolumesFromMounts(mounts []*config.Mount) map[string]composetypes.Volu } func (r *runner) resolveComposeGPUAvailability(composeHelper *compose.ComposeHelper) bool { - switch r.WorkspaceConfig.CLIOptions.GPUAvailability { + switch r.workspaceConfig.CLIOptions.GPUAvailability { case stringTrue: return true case stringFalse: diff --git a/pkg/devcontainer/config.go b/pkg/devcontainer/config.go index 9c3f602f5..f67e58596 100644 --- a/pkg/devcontainer/config.go +++ b/pkg/devcontainer/config.go @@ -37,16 +37,16 @@ func (r *runner) getRawConfig(options provider.CLIOptions) (*config.DevContainer // rawConfigFromWorkspace returns the config embedded in the workspace metadata, // or nil when none is present. func (r *runner) rawConfigFromWorkspace() *config.DevContainerConfig { - if r.WorkspaceConfig.Workspace.DevContainerConfig == nil { + if r.workspaceConfig.Workspace.DevContainerConfig == nil { return nil } - rawConfig := config.CloneDevContainerConfig(r.WorkspaceConfig.Workspace.DevContainerConfig) - if devContainerPath := r.WorkspaceConfig.Workspace.DevContainerPath; devContainerPath != "" { - rawConfig.Origin = path.Join(filepath.ToSlash(r.LocalWorkspaceFolder), devContainerPath) + rawConfig := config.CloneDevContainerConfig(r.workspaceConfig.Workspace.DevContainerConfig) + if devContainerPath := r.workspaceConfig.Workspace.DevContainerPath; devContainerPath != "" { + rawConfig.Origin = path.Join(filepath.ToSlash(r.localWorkspaceFolder), devContainerPath) } else { rawConfig.Origin = path.Join( - filepath.ToSlash(r.LocalWorkspaceFolder), + filepath.ToSlash(r.localWorkspaceFolder), ".devcontainer."+pkgconfig.BinaryName+".json", ) } @@ -56,7 +56,7 @@ func (r *runner) rawConfigFromWorkspace() *config.DevContainerConfig { // rawConfigFromContainer returns a synthetic config for a running-container // source, or nil when the source is not a container. func (r *runner) rawConfigFromContainer() *config.DevContainerConfig { - containerID := r.WorkspaceConfig.Workspace.Source.Container + containerID := r.workspaceConfig.Workspace.Source.Container if containerID == "" { return nil } @@ -75,14 +75,14 @@ func (r *runner) rawConfigFromContainer() *config.DevContainerConfig { func (r *runner) rawConfigFromCrane( options provider.CLIOptions, ) (*config.DevContainerConfig, error) { - localWorkspaceFolder, err := crane.PullConfigFromSource(r.WorkspaceConfig, &options) + localWorkspaceFolder, err := crane.PullConfigFromSource(r.workspaceConfig, &options) if err != nil { return nil, err } return config.ParseDevContainerJSON( context.Background(), localWorkspaceFolder, - r.WorkspaceConfig.Workspace.DevContainerPath, + r.workspaceConfig.Workspace.DevContainerPath, ) } @@ -91,8 +91,8 @@ func (r *runner) rawConfigFromCrane( func (r *runner) rawConfigFromFilesystem( options provider.CLIOptions, ) (*config.DevContainerConfig, error) { - localWorkspaceFolder := r.LocalWorkspaceFolder - if subPath := r.WorkspaceConfig.Workspace.Source.GitSubPath; subPath != "" { + localWorkspaceFolder := r.localWorkspaceFolder + if subPath := r.workspaceConfig.Workspace.Source.GitSubPath; subPath != "" { localWorkspaceFolder = filepath.Join(localWorkspaceFolder, subPath) } @@ -109,7 +109,7 @@ func (r *runner) rawConfigFromFilesystem( rawConfig, err := config.ParseDevContainerJSONWithOptions( context.Background(), localWorkspaceFolder, - r.WorkspaceConfig.Workspace.DevContainerPath, + r.workspaceConfig.Workspace.DevContainerPath, opts, ) // A missing devcontainer.json is not an error: fall back to auto-detection. @@ -160,10 +160,10 @@ func (r *runner) getDefaultConfig( defaultConfig.ImageContainer = config.ImageContainer{Image: options.FallbackImage} } else { log.Infof("Try detecting project programming language") - defaultConfig = language.DefaultConfig(r.LocalWorkspaceFolder) + defaultConfig = language.DefaultConfig(r.localWorkspaceFolder) } - defaultConfig.Origin = path.Join(filepath.ToSlash(r.LocalWorkspaceFolder), ".devcontainer.json") + defaultConfig.Origin = path.Join(filepath.ToSlash(r.localWorkspaceFolder), ".devcontainer.json") if err := config.SaveDevContainerJSON(defaultConfig); err != nil { return nil, fmt.Errorf("write default devcontainer.json: %w", err) } @@ -214,8 +214,8 @@ func (r *runner) buildSubstitutionContext( ) *config.SubstitutionContext { configFile := rawParsedConfig.Origin workspaceMount, containerWorkspaceFolder := getWorkspace( - r.LocalWorkspaceFolder, - r.WorkspaceConfig.Workspace.ID, + r.localWorkspaceFolder, + r.workspaceConfig.Workspace.ID, rawParsedConfig, ) @@ -225,8 +225,8 @@ func (r *runner) buildSubstitutionContext( } return &config.SubstitutionContext{ - DevContainerID: config.DeriveDevContainerID(r.LocalWorkspaceFolder, configFile), - LocalWorkspaceFolder: r.LocalWorkspaceFolder, + DevContainerID: config.DeriveDevContainerID(r.localWorkspaceFolder, configFile), + LocalWorkspaceFolder: r.localWorkspaceFolder, ContainerWorkspaceFolder: containerWorkspaceFolder, Env: env, WorkspaceMount: workspaceMount, diff --git a/pkg/devcontainer/config_test.go b/pkg/devcontainer/config_test.go index 24bfe72fb..36c7b94a2 100644 --- a/pkg/devcontainer/config_test.go +++ b/pkg/devcontainer/config_test.go @@ -9,6 +9,8 @@ import ( "github.com/stretchr/testify/suite" ) +const testWorkspaceFolder = "/workspace" + type SubstituteTestSuite struct { suite.Suite runner *runner @@ -20,9 +22,9 @@ func TestSubstituteTestSuite(t *testing.T) { func (s *SubstituteTestSuite) SetupTest() { s.runner = &runner{ - ID: "test-id", - LocalWorkspaceFolder: "/workspace", - WorkspaceConfig: &provider2.AgentWorkspaceInfo{ + id: "test-id", + localWorkspaceFolder: testWorkspaceFolder, + workspaceConfig: &provider2.AgentWorkspaceInfo{ Workspace: &provider2.Workspace{ ID: "test-workspace", }, @@ -377,7 +379,7 @@ func TestWorkspaceMountFolderWarning(t *testing.T) { name: "both set", conf: &config.DevContainerConfig{ DevContainerConfigBase: config.DevContainerConfigBase{ - WorkspaceFolder: "/workspace", + WorkspaceFolder: testWorkspaceFolder, }, NonComposeBase: config.NonComposeBase{WorkspaceMount: new("source=v")}, }, @@ -394,7 +396,7 @@ func TestWorkspaceMountFolderWarning(t *testing.T) { name: "folder without mount", conf: &config.DevContainerConfig{ DevContainerConfigBase: config.DevContainerConfigBase{ - WorkspaceFolder: "/workspace", + WorkspaceFolder: testWorkspaceFolder, }, }, wantMsg: true, @@ -403,7 +405,7 @@ func TestWorkspaceMountFolderWarning(t *testing.T) { name: "empty-string mount satisfies the pairing", conf: &config.DevContainerConfig{ DevContainerConfigBase: config.DevContainerConfigBase{ - WorkspaceFolder: "/workspace", + WorkspaceFolder: testWorkspaceFolder, }, NonComposeBase: config.NonComposeBase{WorkspaceMount: new("")}, }, diff --git a/pkg/devcontainer/delete.go b/pkg/devcontainer/delete.go index eef09c4d0..137759e9c 100644 --- a/pkg/devcontainer/delete.go +++ b/pkg/devcontainer/delete.go @@ -10,7 +10,7 @@ import ( ) func (r *runner) Delete(ctx context.Context, options DeleteOptions) error { - containerDetails, err := r.Driver.FindDevContainer(ctx, r.ID) + containerDetails, err := r.driver.FindDevContainer(ctx, r.id) if err != nil { return fmt.Errorf("find dev container: %w", err) } @@ -27,13 +27,13 @@ func (r *runner) Delete(ctx context.Context, options DeleteOptions) error { } } else { if strings.ToLower(containerDetails.State.Status) == "running" { - err = r.Driver.StopDevContainer(ctx, r.ID) + err = r.driver.StopDevContainer(ctx, r.id) if err != nil { return err } } - err = r.Driver.DeleteDevContainer(ctx, r.ID) + err = r.driver.DeleteDevContainer(ctx, r.id) if err != nil { return err } @@ -45,13 +45,13 @@ func (r *runner) Delete(ctx context.Context, options DeleteOptions) error { // cleanupDeliveryVolume removes the devsy-managed volumes created for this // workspace. Best-effort: failures are logged, not returned. func (r *runner) cleanupDeliveryVolume(ctx context.Context) { - if err := r.newAgentDelivery().Cleanup(ctx, r.ID); err != nil { + if err := r.newAgentDelivery().Cleanup(ctx, r.id); err != nil { log.Debugf("best-effort delivery volume cleanup: %v", err) } } func (r *runner) Stop(ctx context.Context) error { - containerDetails, err := r.Driver.FindDevContainer(ctx, r.ID) + containerDetails, err := r.driver.FindDevContainer(ctx, r.id) if err != nil { return fmt.Errorf("find dev container: %w", err) } else if containerDetails == nil { @@ -72,18 +72,18 @@ func (r *runner) Stop(ctx context.Context) error { if isCompose { return r.stopDockerCompose(ctx, projectName) } - return r.Driver.StopDevContainer(ctx, r.ID) + return r.driver.StopDevContainer(ctx, r.id) default: - return r.Driver.StopDevContainer(ctx, r.ID) + return r.driver.StopDevContainer(ctx, r.id) } } func (r *runner) getShutdownAction(isCompose bool) string { - if r.WorkspaceConfig != nil && - r.WorkspaceConfig.LastDevContainerConfig != nil && - r.WorkspaceConfig.LastDevContainerConfig.Config != nil && - r.WorkspaceConfig.LastDevContainerConfig.Config.ShutdownAction != "" { - return r.WorkspaceConfig.LastDevContainerConfig.Config.ShutdownAction + if r.workspaceConfig != nil && + r.workspaceConfig.LastDevContainerConfig != nil && + r.workspaceConfig.LastDevContainerConfig.Config != nil && + r.workspaceConfig.LastDevContainerConfig.Config.ShutdownAction != "" { + return r.workspaceConfig.LastDevContainerConfig.Config.ShutdownAction } if isCompose { return config.ShutdownActionStopCompose diff --git a/pkg/devcontainer/delete_test.go b/pkg/devcontainer/delete_test.go index 74eedf091..8f881bf45 100644 --- a/pkg/devcontainer/delete_test.go +++ b/pkg/devcontainer/delete_test.go @@ -67,9 +67,9 @@ func (m *mockDriver) GetDevContainerLogs( func newTestRunner(d driver.Driver) *runner { return &runner{ - Driver: d, - ID: "test-workspace", - WorkspaceConfig: &provider.AgentWorkspaceInfo{ + driver: d, + id: "test-workspace", + workspaceConfig: &provider.AgentWorkspaceInfo{ Agent: provider.ProviderAgentConfig{ Driver: provider.CustomDriver, }, diff --git a/pkg/devcontainer/initialize.go b/pkg/devcontainer/initialize.go new file mode 100644 index 000000000..9dab4639d --- /dev/null +++ b/pkg/devcontainer/initialize.go @@ -0,0 +1,104 @@ +package devcontainer + +import ( + "errors" + "fmt" + "os" + "os/exec" + "runtime" + "strings" + "sync" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/log" +) + +// initializeCommand carries the shared execution state for the initializeCommand +// hook: the host shell, working directory, and extra environment. +type initializeCommand struct { + shell []string + workspaceFolder string + extraEnv []string +} + +// runInitializeCommand executes the devcontainer.json initializeCommand hook on +// the host. Named sub-commands run concurrently; their errors are collected and +// returned together. +func runInitializeCommand( + workspaceFolder string, + conf *config.DevContainerConfig, + extraEnv []string, +) error { + if len(conf.InitializeCommand) == 0 { + return nil + } + + init := &initializeCommand{ + shell: hostShell(), + workspaceFolder: workspaceFolder, + extraEnv: extraEnv, + } + + var ( + wg sync.WaitGroup + mu sync.Mutex + errs []error + ) + for name, cmd := range conf.InitializeCommand { + wg.Go(func() { + if err := init.run(name, cmd); err != nil { + mu.Lock() + errs = append(errs, err) + mu.Unlock() + } + }) + } + wg.Wait() + + return errors.Join(errs...) +} + +// hostShell returns the shell invocation used to run string-form commands. +// initializeCommand runs on the host; on Windows sh may not be on PATH, so we +// fall back to the default command interpreter. +func hostShell() []string { + if runtime.GOOS != "windows" { + return []string{"sh", "-c"} + } + if comSpec := os.Getenv("COMSPEC"); comSpec != "" { + return []string{comSpec, "/c"} + } + return []string{"cmd.exe", "/c"} +} + +// run executes a single named sub-command. A single-element command is run as a +// shell string; a multi-element command is executed argv-style. +func (c *initializeCommand) run(name string, cmd []string) error { + args := cmd + if len(cmd) == 1 { + args = append(append([]string{}, c.shell...), cmd[0]) + } + + log.Infof( + "Running initializeCommand %q from devcontainer.json: %q", + name, + strings.Join(args, " "), + ) + + stdout := log.Writer(log.LevelInfo) + stderr := log.Writer(log.LevelError) + defer func() { _ = stdout.Close() }() + defer func() { _ = stderr.Close() }() + + // args come from devcontainer.json initializeCommand, a trusted local config. + command := exec.Command(args[0], args[1:]...) //nolint:gosec // G204 + command.Dir = c.workspaceFolder + command.Env = append(command.Environ(), c.extraEnv...) + command.Stdout = stdout + command.Stderr = stderr + + if err := command.Run(); err != nil { + return fmt.Errorf("initializeCommand %q failed: %w", name, err) + } + return nil +} diff --git a/pkg/devcontainer/inspect.go b/pkg/devcontainer/inspect.go index 8353b484b..064582acb 100644 --- a/pkg/devcontainer/inspect.go +++ b/pkg/devcontainer/inspect.go @@ -10,13 +10,13 @@ import ( ) func (r *runner) inspectImage(ctx context.Context, imageName string) (*config.ImageDetails, error) { - dockerDriver, ok := r.Driver.(driver.DockerDriver) + dockerDriver, ok := r.driver.(driver.DockerDriver) if ok { return dockerDriver.InspectImage(ctx, imageName) } // Get target architecture from the driver - targetArch, err := r.Driver.TargetArchitecture(ctx, r.ID) + targetArch, err := r.driver.TargetArchitecture(ctx, r.id) if err != nil { return nil, fmt.Errorf("failed to get target architecture: %w", err) } @@ -44,7 +44,7 @@ func (r *runner) inspectImage(ctx context.Context, imageName string) (*config.Im } func (r *runner) getImageTag(ctx context.Context, imageID string) (string, error) { - dockerDriver, ok := r.Driver.(driver.DockerDriver) + dockerDriver, ok := r.driver.(driver.DockerDriver) if ok { return dockerDriver.GetImageTag(ctx, imageID) } diff --git a/pkg/devcontainer/mount.go b/pkg/devcontainer/mount.go new file mode 100644 index 000000000..82b66b737 --- /dev/null +++ b/pkg/devcontainer/mount.go @@ -0,0 +1,86 @@ +package devcontainer + +import ( + "fmt" + "runtime" + "strings" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" +) + +// defaultConsistency is applied to bind mounts on non-Linux hosts, where the +// file sharing layer benefits from an explicit consistency mode. +const defaultConsistency = "consistent" + +func needsDefaultConsistency() bool { + return runtime.GOOS != goosLinux +} + +// getWorkspace resolves the workspace bind-mount string and the container-side +// mount folder. The mount string is empty when the config suppresses the mount. +func getWorkspace( + workspaceFolder, workspaceID string, + conf *config.DevContainerConfig, +) (mount, containerFolder string) { + if conf.WorkspaceMount == nil { + containerFolder = containerMountFolder(conf, workspaceID) + return withDefaultConsistency(fmt.Sprintf( + "type=bind,source=%s,target=%s", + workspaceFolder, + containerFolder, + )), containerFolder + } + + if *conf.WorkspaceMount == "" { + return "", containerMountFolder(conf, workspaceID) + } + + mount = withDefaultConsistency(*conf.WorkspaceMount) + return mount, config.ParseMount(mount).Target +} + +func containerMountFolder(conf *config.DevContainerConfig, workspaceID string) string { + if conf.WorkspaceFolder != "" { + return conf.WorkspaceFolder + } + return "/workspaces/" + workspaceID +} + +// withDefaultConsistency adds the default consistency mode to a mount string on +// hosts that need it, unless the mount already specifies one. +func withDefaultConsistency(mount string) string { + if !needsDefaultConsistency() || mountHasConsistency(mount) { + return mount + } + return mount + ",consistency='" + defaultConsistency + "'" +} + +func mountHasConsistency(mount string) bool { + for part := range strings.SplitSeq(mount, ",") { + if strings.HasPrefix(part, "consistency=") { + return true + } + } + return false +} + +// mountSetConsistency returns the mount string with its consistency option set +// to value, replacing any existing value or appending one when absent. +func mountSetConsistency(mount, value string) string { + quoted := "consistency='" + value + "'" + + replaced := false + var parts []string + for part := range strings.SplitSeq(mount, ",") { + if strings.HasPrefix(part, "consistency=") { + parts = append(parts, quoted) + replaced = true + } else { + parts = append(parts, part) + } + } + if !replaced { + parts = append(parts, quoted) + } + return strings.Join(parts, ",") +} diff --git a/pkg/devcontainer/prebuild.go b/pkg/devcontainer/prebuild.go index 410263d54..facc6d30e 100644 --- a/pkg/devcontainer/prebuild.go +++ b/pkg/devcontainer/prebuild.go @@ -13,7 +13,7 @@ import ( ) func (r *runner) Build(ctx context.Context, options provider.BuildOptions) (string, error) { - dockerDriver, ok := r.Driver.(driver.DockerDriver) + dockerDriver, ok := r.driver.(driver.DockerDriver) if !ok { return "", fmt.Errorf("building only supported with docker driver") } @@ -47,7 +47,7 @@ func (r *runner) Build(ctx context.Context, options provider.BuildOptions) (stri } else if prebuildRepo != "" { prebuildImage = prebuildRepo + ":" + buildInfo.PrebuildHash } else { - prebuildImage = build.GetImageName(r.LocalWorkspaceFolder, buildInfo.PrebuildHash) + prebuildImage = build.GetImageName(r.localWorkspaceFolder, buildInfo.PrebuildHash) } if buildInfo.ImageName == prebuildImage { diff --git a/pkg/devcontainer/run.go b/pkg/devcontainer/run.go index d13287a54..827368864 100644 --- a/pkg/devcontainer/run.go +++ b/pkg/devcontainer/run.go @@ -2,14 +2,8 @@ package devcontainer import ( "context" - "errors" "fmt" "io" - "os" - "os/exec" - "runtime" - "strings" - "sync" "time" "github.com/devsy-org/devsy/pkg/devcontainer/config" @@ -18,22 +12,17 @@ import ( "github.com/devsy-org/devsy/pkg/encoding" "github.com/devsy-org/devsy/pkg/language" "github.com/devsy-org/devsy/pkg/log" - provider2 "github.com/devsy-org/devsy/pkg/provider" + "github.com/devsy-org/devsy/pkg/provider" ) +// Runner drives the lifecycle of a single workspace's dev container. type Runner interface { Up(ctx context.Context, options UpOptions, timeout time.Duration) (*config.Result, error) - - Build(ctx context.Context, options provider2.BuildOptions) (string, error) - + Build(ctx context.Context, options provider.BuildOptions) (string, error) Find(ctx context.Context) (*config.ContainerDetails, error) - Command(ctx context.Context, params CommandParams) error - Stop(ctx context.Context) error - Delete(ctx context.Context, options DeleteOptions) error - Logs(ctx context.Context, writer io.Writer) error } @@ -41,8 +30,7 @@ type DeleteOptions struct { RemoveVolumes bool } -// CommandParams groups the inputs for running a command inside the dev -// container. +// CommandParams groups the inputs for running a command inside the dev container. type CommandParams struct { User string Command string @@ -51,43 +39,9 @@ type CommandParams struct { Stderr io.Writer } -func NewRunner( - agentPath, agentDownloadURL string, - workspaceConfig *provider2.AgentWorkspaceInfo, -) (Runner, error) { - driver, err := drivercreate.NewDriver(workspaceConfig) - if err != nil { - return nil, err - } - - // we use the workspace uid as id to avoid conflicts between container names - return &runner{ - Driver: driver, - - AgentPath: agentPath, - AgentDownloadURL: agentDownloadURL, - LocalWorkspaceFolder: workspaceConfig.ContentFolder, - ID: GetRunnerIDFromWorkspace(workspaceConfig.Workspace), - IDLabels: workspaceConfig.CLIOptions.IDLabels, - WorkspaceConfig: workspaceConfig, - }, nil -} - -type runner struct { - Driver driver.Driver - - WorkspaceConfig *provider2.AgentWorkspaceInfo - AgentPath string - AgentDownloadURL string - - LocalWorkspaceFolder string - - ID string - IDLabels []string -} - +// UpOptions configures a single Up invocation. type UpOptions struct { - provider2.CLIOptions + provider.CLIOptions // NoBuild is set by the container tunnel to force pre-built images; it is // distinct from the embedded CLIOptions.NoBuild used by the build command. @@ -97,19 +51,17 @@ type UpOptions struct { RegistryCache string } -// toBuildOptions derives the BuildOptions for an up-triggered build. It carries -// the full embedded CLIOptions (so build flags like Pull/ForceBuild are never -// dropped) and applies the up-specific NoBuild/RegistryCache overrides. -func (o UpOptions) toBuildOptions() provider2.BuildOptions { - return provider2.BuildOptions{ +// toBuildOptions derives the BuildOptions for an up-triggered build, carrying +// the embedded CLIOptions and applying the up-specific overrides. +func (o UpOptions) toBuildOptions() provider.BuildOptions { + return provider.BuildOptions{ CLIOptions: o.CLIOptions, RegistryCache: o.RegistryCache, NoBuild: o.NoBuild, } } -// runContainerParams groups the inputs shared by the runSingleContainer, -// runDockerCompose, and runDefaultContainer dispatch methods. +// runContainerParams groups the inputs shared by the container dispatch methods. type runContainerParams struct { parsedConfig *config.SubstitutedConfig substitutionContext *config.SubstitutionContext @@ -117,6 +69,46 @@ type runContainerParams struct { timeout time.Duration } +type runner struct { + driver driver.Driver + + workspaceConfig *provider.AgentWorkspaceInfo + agentPath string + agentDownloadURL string + + localWorkspaceFolder string + + id string + idLabels []string +} + +func NewRunner( + agentPath, agentDownloadURL string, + workspaceConfig *provider.AgentWorkspaceInfo, +) (Runner, error) { + drv, err := drivercreate.NewDriver(workspaceConfig) + if err != nil { + return nil, err + } + + return &runner{ + driver: drv, + agentPath: agentPath, + agentDownloadURL: agentDownloadURL, + localWorkspaceFolder: workspaceConfig.ContentFolder, + id: GetRunnerIDFromWorkspace(workspaceConfig.Workspace), + idLabels: workspaceConfig.CLIOptions.IDLabels, + workspaceConfig: workspaceConfig, + }, nil +} + +func GetRunnerIDFromWorkspace(workspace *provider.Workspace) string { + if encoding.IsLegacyUID(workspace.UID) { + return workspace.ID + } + return workspace.UID +} + func (r *runner) Up( ctx context.Context, options UpOptions, @@ -124,7 +116,7 @@ func (r *runner) Up( ) (*config.Result, error) { log.Debugf( "Up devcontainer for workspace %q with timeout %s", - r.WorkspaceConfig.Workspace.ID, + r.workspaceConfig.Workspace.ID, timeout, ) @@ -134,20 +126,11 @@ func (r *runner) Up( } defer cleanupBuildInformation(substitutedConfig.Config) - // do not run initialize command in platform mode - if !options.Platform.Enabled { - if err := runInitializeCommand( - r.LocalWorkspaceFolder, - substitutedConfig.Config, - options.InitEnv, - ); err != nil { - return nil, err - } - } else if len(substitutedConfig.Config.InitializeCommand) > 0 { - log.Info("Skipping initializeCommand on platform") + if err := r.runInitializeCommand(substitutedConfig.Config, options); err != nil { + return nil, err } - runParams := &runContainerParams{ + params := &runContainerParams{ parsedConfig: substitutedConfig, substitutionContext: substitutionContext, options: options, @@ -158,17 +141,17 @@ func (r *runner) Up( case isDockerFileConfig(substitutedConfig.Config), substitutedConfig.Config.Image != "", substitutedConfig.Config.ContainerID != "": - return r.runSingleContainer(ctx, runParams) + return r.runSingleContainer(ctx, params) case isDockerComposeConfig(substitutedConfig.Config): - return r.runDockerCompose(ctx, runParams) + return r.runDockerCompose(ctx, params) default: - return r.runDefaultContainer(ctx, runParams) + return r.runDefaultContainer(ctx, params) } } func (r *runner) Command(ctx context.Context, params CommandParams) error { - return r.Driver.CommandDevContainer(ctx, &driver.CommandParams{ - WorkspaceID: r.ID, + return r.driver.CommandDevContainer(ctx, &driver.CommandParams{ + WorkspaceID: r.id, User: params.User, Command: params.Command, Stdin: params.Stdin, @@ -178,245 +161,58 @@ func (r *runner) Command(ctx context.Context, params CommandParams) error { } func (r *runner) Find(ctx context.Context) (*config.ContainerDetails, error) { - containerDetails, err := r.Driver.FindDevContainer(ctx, r.ID) + containerDetails, err := r.driver.FindDevContainer(ctx, r.id) if err != nil { return nil, fmt.Errorf("find dev container: %w", err) } - return containerDetails, nil } func (r *runner) Logs(ctx context.Context, writer io.Writer) error { - return r.Driver.GetDevContainerLogs(ctx, r.ID, writer, writer) + return r.driver.GetDevContainerLogs(ctx, r.id, writer, writer) } -func (r *runner) runDefaultContainer( - ctx context.Context, - params *runContainerParams, -) (*config.Result, error) { - options := params.options - substitutedConfig := params.parsedConfig - if options.FallbackImage != "" { - log.Warn( - "dev container config is missing one of \"image\", \"dockerFile\" or \"dockerComposeFile\" properties, " + - "using fallback image " + options.FallbackImage, - ) - - substitutedConfig.Config.ImageContainer = config.ImageContainer{ - Image: options.FallbackImage, - } - } else { - log.Warn( - "dev container config is missing one of \"image\", \"dockerFile\" or \"dockerComposeFile\" properties, " + - "defaulting to auto-detection", - ) - - lang, err := language.DetectLanguage(r.LocalWorkspaceFolder) - if err != nil { - return nil, fmt.Errorf( - "could not detect project language and dev container config is missing one of " + - "\"image\", \"dockerFile\" or \"dockerComposeFile\" properties", - ) - } - - if language.MapConfig[lang] == nil { - return nil, fmt.Errorf( - "could not detect project language and dev container config is missing one of " + - "\"image\", \"dockerFile\" or \"dockerComposeFile\" properties", - ) +// runInitializeCommand runs the host-side initializeCommand hook. The hook is +// never executed in platform mode. +func (r *runner) runInitializeCommand(conf *config.DevContainerConfig, options UpOptions) error { + if options.Platform.Enabled { + if len(conf.InitializeCommand) > 0 { + log.Info("Skipping initializeCommand on platform") } - substitutedConfig.Config.ImageContainer = language.MapConfig[lang].ImageContainer - } - - return r.runSingleContainer(ctx, params) -} - -func isDockerFileConfig(config *config.DevContainerConfig) bool { - return config.GetDockerfile() != "" -} - -// initCmdContext groups shared execution state for initializeCommand sub-commands. -type initCmdContext struct { - shellArgs []string - workspaceFolder string - extraEnvVars []string -} - -func runInitializeCommand( - workspaceFolder string, - conf *config.DevContainerConfig, - extraEnvVars []string, -) error { - if len(conf.InitializeCommand) == 0 { return nil } - - shellArgs := []string{"sh", "-c"} - // According to the devcontainer spec, `initializeCommand` needs to be run on the host. - // On Windows we can't assume everyone has `sh` added to their PATH so we need to use - // Windows default shell (usually cmd.exe). - if runtime.GOOS == "windows" { - comSpec := os.Getenv("COMSPEC") - if comSpec != "" { - shellArgs = []string{comSpec, "/c"} - } else { - shellArgs = []string{"cmd.exe", "/c"} - } - } - - ctx := &initCmdContext{ - shellArgs: shellArgs, - workspaceFolder: workspaceFolder, - extraEnvVars: extraEnvVars, - } - - if len(conf.InitializeCommand) > 1 { - return ctx.runParallel(conf.InitializeCommand) - } - - for name, cmd := range conf.InitializeCommand { - if err := ctx.runSingle(name, cmd); err != nil { - return err - } - } - return nil -} - -// runParallel executes all named sub-commands concurrently, collects errors, and returns them joined. -func (c *initCmdContext) runParallel(hook map[string][]string) error { - var ( - wg sync.WaitGroup - mu sync.Mutex - errs []error - ) - - wg.Add(len(hook)) - for name, cmd := range hook { - go func() { - defer wg.Done() - if err := c.runSingle(name, cmd); err != nil { - mu.Lock() - errs = append(errs, fmt.Errorf("named command %q failed: %w", name, err)) - mu.Unlock() - } - }() - } - - wg.Wait() - return errors.Join(errs...) + return runInitializeCommand(r.localWorkspaceFolder, conf, options.InitEnv) } -// runSingle executes a single initializeCommand sub-command. -func (c *initCmdContext) runSingle(name string, cmd []string) error { - var args []string - if len(cmd) == 1 { - args = make([]string, len(c.shellArgs)+1) - copy(args, c.shellArgs) - args[len(c.shellArgs)] = cmd[0] - } else { - args = cmd - } - - log.Infof( - "Running initializeCommand %q from devcontainer.json: %q", - name, - strings.Join(args, " "), - ) - - writer := log.Writer(log.LevelInfo) - errwriter := log.Writer(log.LevelError) - defer func() { _ = writer.Close() }() - defer func() { _ = errwriter.Close() }() - - // args come from devcontainer.json initializeCommand, a trusted local config. - c2 := exec.Command(args[0], args[1:]...) //nolint:gosec // G204 - env := c2.Environ() - env = append(env, c.extraEnvVars...) - - c2.Stdout = writer - c2.Stderr = errwriter - c2.Dir = c.workspaceFolder - c2.Env = env - if err := c2.Run(); err != nil { - return fmt.Errorf("initializeCommand %q failed: %w", name, err) - } - return nil -} - -func mountHasConsistency(mount string) bool { - for part := range strings.SplitSeq(mount, ",") { - if strings.HasPrefix(part, "consistency=") { - return true - } - } - return false -} - -func mountSetConsistency(mount, value string) string { - quoted := "consistency='" + value + "'" - var parts []string - for part := range strings.SplitSeq(mount, ",") { - if strings.HasPrefix(part, "consistency=") { - parts = append(parts, quoted) - } else { - parts = append(parts, part) - } - } - if !mountHasConsistency(mount) { - parts = append(parts, quoted) - } - return strings.Join(parts, ",") -} - -func needsDefaultConsistency() bool { - return runtime.GOOS != goosLinux -} +// runDefaultContainer handles configs missing image/dockerfile/compose by +// selecting a fallback image or auto-detecting the project language, then +// delegating to the single-container path. +func (r *runner) runDefaultContainer( + ctx context.Context, + params *runContainerParams, +) (*config.Result, error) { + conf := params.parsedConfig.Config -func getWorkspace( - workspaceFolder, workspaceID string, - conf *config.DevContainerConfig, -) (string, string) { - if conf.WorkspaceMount != nil { - // Explicit empty string means suppress the workspace mount entirely. - if *conf.WorkspaceMount == "" { - containerMountFolder := conf.WorkspaceFolder - if containerMountFolder == "" { - containerMountFolder = "/workspaces/" + workspaceID - } - return "", containerMountFolder - } + const missingProps = "dev container config is missing one of " + + "\"image\", \"dockerFile\" or \"dockerComposeFile\" properties" - mount := config.ParseMount(*conf.WorkspaceMount) - ws := *conf.WorkspaceMount - if needsDefaultConsistency() && !mountHasConsistency(ws) { - ws += ",consistency='consistent'" - } - return ws, mount.Target + if fallback := params.options.FallbackImage; fallback != "" { + log.Warn(missingProps + ", using fallback image " + fallback) + conf.ImageContainer = config.ImageContainer{Image: fallback} + return r.runSingleContainer(ctx, params) } - containerMountFolder := conf.WorkspaceFolder - if containerMountFolder == "" { - containerMountFolder = "/workspaces/" + workspaceID - } + log.Warn(missingProps + ", defaulting to auto-detection") - consistency := "" - if needsDefaultConsistency() { - consistency = ",consistency='consistent'" + lang, err := language.DetectLanguage(r.localWorkspaceFolder) + if err != nil || language.MapConfig[lang] == nil { + return nil, fmt.Errorf("could not detect project language and %s", missingProps) } + conf.ImageContainer = language.MapConfig[lang].ImageContainer - return fmt.Sprintf( - "type=bind,source=%s,target=%s%s", - workspaceFolder, - containerMountFolder, - consistency, - ), containerMountFolder + return r.runSingleContainer(ctx, params) } -func GetRunnerIDFromWorkspace(workspace *provider2.Workspace) string { - ID := workspace.UID - if encoding.IsLegacyUID(workspace.UID) { - ID = workspace.ID - } - - return ID +func isDockerFileConfig(config *config.DevContainerConfig) bool { + return config.GetDockerfile() != "" } diff --git a/pkg/devcontainer/setup.go b/pkg/devcontainer/setup.go index 28f666cb5..361058d44 100644 --- a/pkg/devcontainer/setup.go +++ b/pkg/devcontainer/setup.go @@ -115,29 +115,29 @@ type podExecCapableDriver interface { func (r *runner) newAgentDelivery() delivery.AgentDelivery { dockerCmd := "docker" var dockerEnv []string - if r.WorkspaceConfig.Agent.Docker.Path != "" { - dockerCmd = r.WorkspaceConfig.Agent.Docker.Path + if r.workspaceConfig.Agent.Docker.Path != "" { + dockerCmd = r.workspaceConfig.Agent.Docker.Path } - for k, v := range r.WorkspaceConfig.Agent.Docker.Env { + for k, v := range r.workspaceConfig.Agent.Docker.Env { dockerEnv = append(dockerEnv, k+"="+v) } - execFn := delivery.CommandFunc(r.Driver.CommandDevContainer, r.ID) + execFn := delivery.CommandFunc(r.driver.CommandDevContainer, r.id) var podExec delivery.PodExecFunc - if d, ok := r.Driver.(podExecCapableDriver); ok { + if d, ok := r.driver.(podExecCapableDriver); ok { podExec = func(ctx context.Context, argv []string, streams driver.Streams) error { - return d.CommandContainerArgv(ctx, r.ID, argv, streams) + return d.CommandContainerArgv(ctx, r.id, argv, streams) } } return delivery.NewAgentDelivery(delivery.FactoryOptions{ - WorkspaceConfig: r.WorkspaceConfig, - WorkspaceID: r.ID, + WorkspaceConfig: r.workspaceConfig, + WorkspaceID: r.id, DockerCommand: dockerCmd, DockerEnv: dockerEnv, - HelperImage: r.WorkspaceConfig.Agent.Docker.HelperImage, - ContainerID: r.ID, + HelperImage: r.workspaceConfig.Agent.Docker.HelperImage, + ContainerID: r.id, ExecFunc: execFn, PodExec: podExec, }) @@ -148,10 +148,10 @@ func (r *runner) newAgentDelivery() delivery.AgentDelivery { // than guessing the host arch: streaming a wrong-arch binary would succeed here // but fail when the agent starts, after the legacy fallback can no longer run. func (r *runner) deliveryArch(ctx context.Context) (string, error) { - if r.WorkspaceConfig.Agent.Driver != provider2.KubernetesDriver { + if r.workspaceConfig.Agent.Driver != provider2.KubernetesDriver { return runtime.GOARCH, nil } - arch, err := r.Driver.TargetArchitecture(ctx, r.ID) + arch, err := r.driver.TargetArchitecture(ctx, r.id) if err != nil { return "", fmt.Errorf("resolve cluster architecture: %w", err) } @@ -173,7 +173,7 @@ func (r *runner) deliverPostStart(ctx context.Context, strategy delivery.AgentDe } err = strategy.DeliverPostStart(ctx, delivery.PostStartOptions{ - WorkspaceID: r.ID, + WorkspaceID: r.id, BinarySource: binarySource, Arch: arch, }) @@ -184,7 +184,7 @@ func (r *runner) deliverPostStart(ctx context.Context, strategy delivery.AgentDe } func (r *runner) newBinarySource() (delivery.BinarySourceFunc, error) { - downloadURL := r.AgentDownloadURL + downloadURL := r.agentDownloadURL if downloadURL == "" { downloadURL = pkgconfig.DefaultAgentDownloadURL() } @@ -199,8 +199,8 @@ func (r *runner) legacyInject(ctx context.Context, timeout time.Duration) error err := agent.InjectAgent(&agent.InjectOptions{ Ctx: ctx, Exec: func(ctx context.Context, command string, stdin io.Reader, stdout io.Writer, stderr io.Writer) error { - return r.Driver.CommandDevContainer(ctx, &driver.CommandParams{ - WorkspaceID: r.ID, + return r.driver.CommandDevContainer(ctx, &driver.CommandParams{ + WorkspaceID: r.id, User: containerRootUser, Command: command, Stdin: stdin, @@ -244,7 +244,7 @@ func (r *runner) buildResult(params *setupContainerParams) *config.Result { result := &config.Result{ DevContainerConfigWithPath: &config.DevContainerConfigWithPath{ Config: params.rawConfig, - Path: getRelativeDevContainerJson(params.rawConfig.Origin, r.LocalWorkspaceFolder), + Path: getRelativeDevContainerJson(params.rawConfig.Origin, r.localWorkspaceFolder), }, MergedConfig: params.mergedConfig, SubstitutionContext: params.substitutionContext, @@ -252,21 +252,21 @@ func (r *runner) buildResult(params *setupContainerParams) *config.Result { HostWarnings: params.hostWarnings, } - if r.WorkspaceConfig.CLIOptions.DefaultUserEnvProbe != "" { - result.MergedConfig.UserEnvProbe = r.WorkspaceConfig.CLIOptions.DefaultUserEnvProbe + if r.workspaceConfig.CLIOptions.DefaultUserEnvProbe != "" { + result.MergedConfig.UserEnvProbe = r.workspaceConfig.CLIOptions.DefaultUserEnvProbe } - if r.WorkspaceConfig.CLIOptions.ContainerUser != "" { - result.MergedConfig.ContainerUser = r.WorkspaceConfig.CLIOptions.ContainerUser + if r.workspaceConfig.CLIOptions.ContainerUser != "" { + result.MergedConfig.ContainerUser = r.workspaceConfig.CLIOptions.ContainerUser } - if r.WorkspaceConfig.CLIOptions.RemoteUser != "" { - result.MergedConfig.RemoteUser = r.WorkspaceConfig.CLIOptions.RemoteUser + if r.workspaceConfig.CLIOptions.RemoteUser != "" { + result.MergedConfig.RemoteUser = r.workspaceConfig.CLIOptions.RemoteUser } - if r.WorkspaceConfig.Agent.Local == stringTrue && - r.WorkspaceConfig.CLIOptions.Platform.Enabled { + if r.workspaceConfig.Agent.Local == stringTrue && + r.workspaceConfig.CLIOptions.Platform.Enabled { result.MergedConfig.Mounts = filterWorkspaceMounts( result.MergedConfig.Mounts, - r.WorkspaceConfig.ContentFolder, + r.workspaceConfig.ContentFolder, ) } @@ -287,17 +287,17 @@ func (r *runner) compressResult(result *config.Result) (string, error) { func (r *runner) compressWorkspaceConfig() (string, error) { workspaceConfig := &provider2.ContainerWorkspaceInfo{ - IDE: r.WorkspaceConfig.Workspace.IDE, - CLIOptions: r.WorkspaceConfig.CLIOptions, - Dockerless: r.WorkspaceConfig.Agent.Dockerless, - ContainerTimeout: r.WorkspaceConfig.Agent.ContainerTimeout, - Source: r.WorkspaceConfig.Workspace.Source, - Agent: r.WorkspaceConfig.Agent, - ContentFolder: r.WorkspaceConfig.ContentFolder, + IDE: r.workspaceConfig.Workspace.IDE, + CLIOptions: r.workspaceConfig.CLIOptions, + Dockerless: r.workspaceConfig.Agent.Dockerless, + ContainerTimeout: r.workspaceConfig.Agent.ContainerTimeout, + Source: r.workspaceConfig.Workspace.Source, + Agent: r.workspaceConfig.Agent, + ContentFolder: r.workspaceConfig.ContentFolder, } workspaceConfig.PullFromInsideContainer = resolvePullFromInsideContainer( - r.WorkspaceConfig.CLIOptions, - r.WorkspaceConfig.Workspace.Source.GitRepository, + r.workspaceConfig.CLIOptions, + r.workspaceConfig.Workspace.Source.GitRepository, ) workspaceConfigRaw, err := json.Marshal(workspaceConfig) @@ -325,7 +325,7 @@ func (r *runner) buildSetupCommand(compressed, workspaceConfigCompressed string) } func (r *runner) addSetupFlags(args *[]string) { - _, isDockerDriver := r.Driver.(driver.DockerDriver) + _, isDockerDriver := r.driver.(driver.DockerDriver) r.addChownFlag(args, isDockerDriver) r.addDriverFlags(args, isDockerDriver) @@ -352,20 +352,20 @@ func shouldChownWorkspace(goos string, isDockerDriver, isPodman bool) bool { // isPodmanRuntime reports whether the docker driver is backed by the Podman // runtime (agent.docker.runtime: podman). func (r *runner) isPodmanRuntime() bool { - return strings.EqualFold(r.WorkspaceConfig.Agent.Docker.Runtime, string(docker.RuntimePodman)) + return strings.EqualFold(r.workspaceConfig.Agent.Docker.Runtime, string(docker.RuntimePodman)) } func (r *runner) addDriverFlags(args *[]string, isDockerDriver bool) { if !isDockerDriver { *args = append(*args, "--stream-mounts") } - if r.WorkspaceConfig.Agent.InjectGitCredentials != stringFalse { + if r.workspaceConfig.Agent.InjectGitCredentials != stringFalse { *args = append(*args, "--inject-git-credentials") } } func (r *runner) addPlatformFlags(args *[]string) { - platform := r.WorkspaceConfig.CLIOptions.Platform + platform := r.workspaceConfig.CLIOptions.Platform if platform.AccessKey != "" { *args = append(*args, "--access-key", shellescape.Quote(platform.AccessKey)) } @@ -378,7 +378,7 @@ func (r *runner) addPlatformFlags(args *[]string) { } func (r *runner) addDotfilesFlags(args *[]string) { - cli := r.WorkspaceConfig.CLIOptions + cli := r.workspaceConfig.CLIOptions if cli.DotfilesRepo != "" { *args = append(*args, "--dotfiles-repo", shellescape.Quote(cli.DotfilesRepo)) } @@ -388,7 +388,7 @@ func (r *runner) addDotfilesFlags(args *[]string) { } func (r *runner) addPrebuildFlag(args *[]string) { - if r.WorkspaceConfig.CLIOptions.Prebuild { + if r.workspaceConfig.CLIOptions.Prebuild { *args = append(*args, "--prebuild") } } @@ -413,10 +413,10 @@ func (r *runner) executeSetup( ctx, stdout, stdin, - r.WorkspaceConfig.Agent.InjectGitCredentials != stringFalse, - r.WorkspaceConfig.Agent.InjectDockerCredentials != stringFalse, + r.workspaceConfig.Agent.InjectGitCredentials != stringFalse, + r.workspaceConfig.Agent.InjectDockerCredentials != stringFalse, config.GetMounts(result), - tunnelserver.WithPlatformOptions(&r.WorkspaceConfig.CLIOptions.Platform), + tunnelserver.WithPlatformOptions(&r.workspaceConfig.CLIOptions.Platform), ) } @@ -428,8 +428,8 @@ func (r *runner) executeSetup( sshTunnelStdinReader, sshTunnelStdoutWriter *os.File, writer io.WriteCloser, ) error { - return r.Driver.CommandDevContainer(cancelCtx, &driver.CommandParams{ - WorkspaceID: r.ID, + return r.driver.CommandDevContainer(cancelCtx, &driver.CommandParams{ + WorkspaceID: r.id, User: containerRootUser, Command: sshCmd, Stdin: sshTunnelStdinReader, @@ -454,11 +454,11 @@ func (r *runner) buildSSHTunnelCommand() string { "internal", "ssh-server", "--stdio", } - if ide.ReusesAuthSock(r.WorkspaceConfig.Workspace.IDE.Name) { + if ide.ReusesAuthSock(r.workspaceConfig.Workspace.IDE.Name) { args = append( args, "--reuse-ssh-auth-sock", - shellescape.Quote(r.WorkspaceConfig.CLIOptions.SSHAuthSockID), + shellescape.Quote(r.workspaceConfig.CLIOptions.SSHAuthSockID), ) } if r.isDebugMode() { diff --git a/pkg/devcontainer/setup_test.go b/pkg/devcontainer/setup_test.go index be9821a25..b9d6434a6 100644 --- a/pkg/devcontainer/setup_test.go +++ b/pkg/devcontainer/setup_test.go @@ -76,7 +76,7 @@ func TestRunnerIsPodmanRuntime(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { r := &runner{ - WorkspaceConfig: &provider2.AgentWorkspaceInfo{ + workspaceConfig: &provider2.AgentWorkspaceInfo{ Agent: provider2.ProviderAgentConfig{ Docker: provider2.ProviderDockerDriverConfig{Runtime: c.runtime}, }, @@ -133,7 +133,7 @@ const testLoginInteractiveShell = "loginInteractiveShell" func TestBuildResult_DefaultUserEnvProbeOverride(t *testing.T) { r := &runner{ - WorkspaceConfig: &provider2.AgentWorkspaceInfo{ + workspaceConfig: &provider2.AgentWorkspaceInfo{ CLIOptions: provider2.CLIOptions{ DefaultUserEnvProbe: "none", }, @@ -158,7 +158,7 @@ func TestBuildResult_DefaultUserEnvProbeOverride(t *testing.T) { func TestBuildResult_DefaultUserEnvProbeEmpty(t *testing.T) { r := &runner{ - WorkspaceConfig: &provider2.AgentWorkspaceInfo{ + workspaceConfig: &provider2.AgentWorkspaceInfo{ CLIOptions: provider2.CLIOptions{}, }, } diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index 990dcffd6..dd920de6f 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -57,7 +57,7 @@ func (r *runner) runSingleContainer( options := runParams.options timeout := runParams.timeout - log.Debugf("starting devcontainer for workspace %s", r.ID) + log.Debugf("starting devcontainer for workspace %s", r.id) substitutionContext.Userns = options.Userns substitutionContext.UidMap = options.UidMap @@ -127,14 +127,14 @@ func (r *runner) findExistingDevContainer( ctx context.Context, ) (*config.ContainerDetails, error) { dockerCmd := "docker" - if r.WorkspaceConfig.Agent.Docker.Path != "" { - dockerCmd = r.WorkspaceConfig.Agent.Docker.Path + if r.workspaceConfig.Agent.Docker.Path != "" { + dockerCmd = r.workspaceConfig.Agent.Docker.Path } if !command.Exists(dockerCmd) { return nil, nil } - containerDetails, err := r.Driver.FindDevContainer(ctx, r.ID) + containerDetails, err := r.driver.FindDevContainer(ctx, r.id) if err != nil { return nil, fmt.Errorf("find dev container: %w", err) } @@ -185,7 +185,7 @@ func (r *runner) ensureRunning( return containerDetails, nil } - if err := r.Driver.StartDevContainer(ctx, r.ID); err != nil { + if err := r.driver.StartDevContainer(ctx, r.id); err != nil { return nil, err } return r.findRunningContainerOrFail(ctx, "start") @@ -243,12 +243,12 @@ func (r *runner) reprovisionIfNeeded( ctx context.Context, containerDetails *config.ContainerDetails, ) (*config.ContainerDetails, error) { - d, ok := r.Driver.(driver.ReprovisioningDriver) + d, ok := r.driver.(driver.ReprovisioningDriver) if !ok || !d.CanReprovision() { return containerDetails, nil } - if err := r.Driver.RunDevContainer(ctx, r.ID, nil); err != nil { + if err := r.driver.RunDevContainer(ctx, r.id, nil); err != nil { return nil, fmt.Errorf("runner driver run dev container: %w", err) } return r.findRunningContainerOrFail(ctx, "reprovision") @@ -364,14 +364,14 @@ func (r *runner) newContainerHostWarnings(p *resolveParams) ([]string, error) { // deleteForRecreate removes the existing container before recreating it. // Docker containers are fully deleted; other drivers stop the container. func (r *runner) deleteForRecreate(ctx context.Context) error { - if _, ok := r.Driver.(driver.DockerDriver); ok { + if _, ok := r.driver.(driver.DockerDriver); ok { if err := r.Delete(ctx, DeleteOptions{}); err != nil { return fmt.Errorf("delete devcontainer: %w", err) } return nil } - if err := r.Driver.StopDevContainer(ctx, r.ID); err != nil { + if err := r.driver.StopDevContainer(ctx, r.id); err != nil { return fmt.Errorf("stop devcontainer: %w", err) } return nil @@ -390,7 +390,7 @@ func (r *runner) injectDaemonEntrypoint( data, err := agent.GetEncodedWorkspaceDaemonConfig( p.options.Platform, - r.WorkspaceConfig.Workspace, + r.workspaceConfig.Workspace, p.substitutionContext, mergedConfig, ) @@ -409,12 +409,12 @@ func (r *runner) findRunningContainerOrFail( ctx context.Context, operation string, ) (*config.ContainerDetails, error) { - details, err := r.Driver.FindDevContainer(ctx, r.ID) + details, err := r.driver.FindDevContainer(ctx, r.id) if err != nil { return nil, fmt.Errorf("find dev container after %s: %w", operation, err) } if details == nil { - return nil, fmt.Errorf("dev container %s not found after %s", r.ID, operation) + return nil, fmt.Errorf("dev container %s not found after %s", r.id, operation) } return details, nil } @@ -464,7 +464,7 @@ func (r *runner) deliverPreStart(ctx context.Context, runOptions *driver.RunOpti } return strategy.DeliverPreStart(ctx, delivery.PreStartOptions{ - WorkspaceID: r.ID, + WorkspaceID: r.id, RunOptions: runOptions, BinarySource: binarySource, Arch: arch, @@ -477,10 +477,10 @@ func (r *runner) deliverPreStart(ctx context.Context, runOptions *driver.RunOpti // tree. It is skipped for git/image sources, bind mounts, and volumes devsy // does not manage. A reset removes the managed volume so it is re-seeded. func (r *runner) seedWorkspaceVolume(ctx context.Context, p *resolveParams) error { - if r.WorkspaceConfig == nil || r.WorkspaceConfig.Workspace == nil { + if r.workspaceConfig == nil || r.workspaceConfig.Workspace == nil { return nil } - if r.WorkspaceConfig.Workspace.Source.LocalFolder == "" { + if r.workspaceConfig.Workspace.Source.LocalFolder == "" { return nil } @@ -496,10 +496,10 @@ func (r *runner) seedWorkspaceVolume(ctx context.Context, p *resolveParams) erro } return seeder.SeedWorkspaceVolume(ctx, delivery.WorkspaceSeedOptions{ - WorkspaceID: r.ID, + WorkspaceID: r.id, VolumeName: mount.Source, - SourceDir: r.LocalWorkspaceFolder, - Reset: r.WorkspaceConfig.CLIOptions.Reset, + SourceDir: r.localWorkspaceFolder, + Reset: r.workspaceConfig.CLIOptions.Reset, }) } @@ -529,21 +529,21 @@ func (r *runner) runContainer( runOptions.Env = r.addExtraEnvVars(runOptions.Env) // check if docker - dockerDriver, ok := r.Driver.(driver.DockerDriver) + dockerDriver, ok := r.driver.(driver.DockerDriver) if ok { return dockerDriver.RunDockerDevContainer(ctx, &driver.RunDockerDevContainerParams{ - WorkspaceID: r.ID, + WorkspaceID: r.id, Options: runOptions, ParsedConfig: withResolvedUser(p.parsedConfig.Config, mergedConfig), - IDE: r.WorkspaceConfig.Workspace.IDE.Name, - IDEOptions: r.WorkspaceConfig.Workspace.IDE.Options, - LocalWorkspaceFolder: r.LocalWorkspaceFolder, - GPUAvailability: r.WorkspaceConfig.CLIOptions.GPUAvailability, + IDE: r.workspaceConfig.Workspace.IDE.Name, + IDEOptions: r.workspaceConfig.Workspace.IDE.Options, + LocalWorkspaceFolder: r.localWorkspaceFolder, + GPUAvailability: r.workspaceConfig.CLIOptions.GPUAvailability, }) } // build run options for regular driver - return r.Driver.RunDevContainer(ctx, r.ID, runOptions) + return r.driver.RunDevContainer(ctx, r.id, runOptions) } // withResolvedUser returns a copy of parsedConfig carrying the effective user @@ -573,8 +573,8 @@ func parseWorkspaceMount(substitutionContext *config.SubstitutionContext) *confi // workspaceUID returns the workspace UID, or an empty string when unavailable. func (r *runner) workspaceUID() string { - if r.WorkspaceConfig != nil && r.WorkspaceConfig.Workspace != nil { - return r.WorkspaceConfig.Workspace.UID + if r.workspaceConfig != nil && r.workspaceConfig.Workspace != nil { + return r.workspaceConfig.Workspace.UID } return "" } @@ -624,15 +624,15 @@ func (r *runner) getDockerlessRunOptions( } image := dockerlessImage - if r.WorkspaceConfig != nil && r.WorkspaceConfig.Agent.Dockerless.Image != "" { - image = r.WorkspaceConfig.Agent.Dockerless.Image + if r.workspaceConfig != nil && r.workspaceConfig.Agent.Dockerless.Image != "" { + image = r.workspaceConfig.Agent.Dockerless.Image } // we need to add an extra mount here, because otherwise the build config might get lost mounts := mergedConfig.Mounts mounts = append(mounts, &config.Mount{ Type: "volume", - Source: "dockerless-" + r.ID, + Source: "dockerless-" + r.id, Target: "/workspaces/.dockerless", }) @@ -732,7 +732,7 @@ func (r *runner) getRunOptions( Userns: substitutionContext.Userns, UidMap: substitutionContext.UidMap, GidMap: substitutionContext.GidMap, - Platform: r.WorkspaceConfig.CLIOptions.RunPlatform, + Platform: r.workspaceConfig.CLIOptions.RunPlatform, }, nil } @@ -745,13 +745,13 @@ func (r *runner) addExtraEnvVars(env map[string]string) map[string]string { env[DevsyExtraEnvVar] = stringTrue env[RemoteContainersExtraEnvVar] = stringTrue - if r.WorkspaceConfig != nil && r.WorkspaceConfig.Workspace != nil && - r.WorkspaceConfig.Workspace.ID != "" { - env[pkgconfig.EnvWorkspaceID] = r.WorkspaceConfig.Workspace.ID + if r.workspaceConfig != nil && r.workspaceConfig.Workspace != nil && + r.workspaceConfig.Workspace.ID != "" { + env[pkgconfig.EnvWorkspaceID] = r.workspaceConfig.Workspace.ID } - if r.WorkspaceConfig != nil && r.WorkspaceConfig.Workspace != nil && - r.WorkspaceConfig.Workspace.UID != "" { - env[pkgconfig.EnvWorkspaceUID] = r.WorkspaceConfig.Workspace.UID + if r.workspaceConfig != nil && r.workspaceConfig.Workspace != nil && + r.workspaceConfig.Workspace.UID != "" { + env[pkgconfig.EnvWorkspaceUID] = r.workspaceConfig.Workspace.UID } return env From 54e000c7f2d2d096b16f8d017099d3cf73c1bfde Mon Sep 17 00:00:00 2001 From: Samuel K Date: Wed, 8 Jul 2026 13:33:10 -0500 Subject: [PATCH 2/4] fix(devcontainer): guard empty initializeCommand entries and suppressed mounts Address PR review findings: - initializeCommand.run: return a named error for an empty command slice (e.g. `{"noop": []}`) instead of panicking on args[0]. - mountSetConsistency: keep a suppressed (empty) workspaceMount empty rather than synthesizing a malformed consistency-only mount string. Adds regression tests for both. --- pkg/devcontainer/initialize.go | 4 ++++ pkg/devcontainer/mount.go | 6 ++++++ pkg/devcontainer/run_test.go | 22 ++++++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/pkg/devcontainer/initialize.go b/pkg/devcontainer/initialize.go index 9dab4639d..3f9f319bb 100644 --- a/pkg/devcontainer/initialize.go +++ b/pkg/devcontainer/initialize.go @@ -74,6 +74,10 @@ func hostShell() []string { // run executes a single named sub-command. A single-element command is run as a // shell string; a multi-element command is executed argv-style. func (c *initializeCommand) run(name string, cmd []string) error { + if len(cmd) == 0 { + return fmt.Errorf("initializeCommand %q is empty", name) + } + args := cmd if len(cmd) == 1 { args = append(append([]string{}, c.shell...), cmd[0]) diff --git a/pkg/devcontainer/mount.go b/pkg/devcontainer/mount.go index 82b66b737..46efc90ba 100644 --- a/pkg/devcontainer/mount.go +++ b/pkg/devcontainer/mount.go @@ -67,6 +67,12 @@ func mountHasConsistency(mount string) bool { // mountSetConsistency returns the mount string with its consistency option set // to value, replacing any existing value or appending one when absent. func mountSetConsistency(mount, value string) string { + // An empty mount is the "suppress the workspace mount" signal; keep it empty + // rather than synthesizing a malformed consistency-only mount. + if mount == "" { + return "" + } + quoted := "consistency='" + value + "'" replaced := false diff --git a/pkg/devcontainer/run_test.go b/pkg/devcontainer/run_test.go index 76ed9038c..b4bc5b1ab 100644 --- a/pkg/devcontainer/run_test.go +++ b/pkg/devcontainer/run_test.go @@ -119,6 +119,22 @@ func TestRunInitializeCommand_Empty(t *testing.T) { } } +func TestRunInitializeCommand_EmptyEntry(t *testing.T) { + conf := &config.DevContainerConfig{} + conf.InitializeCommand = types.LifecycleHook{ + "noop": {}, + } + + // An empty command slice must produce a normal error, not a panic. + err := runInitializeCommand(t.TempDir(), conf, nil) + if err == nil { + t.Fatal("expected error for empty command entry") + } + if !contains(err.Error(), "noop") { + t.Fatalf("error should name the empty command, got: %v", err) + } +} + func contains(s, substr string) bool { return len(s) >= len(substr) && searchString(s, substr) } @@ -279,6 +295,12 @@ func TestMountSetConsistency(t *testing.T) { value: testConsistency, want: "type=bind,source=/s,target=/t," + wantSuffix, }, + { + name: "preserves suppressed (empty) mount", + mount: "", + value: testConsistency, + want: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 3519348090ff0179991233121d8e61e75289aaf7 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Wed, 8 Jul 2026 13:37:08 -0500 Subject: [PATCH 3/4] fix(devcontainer): propagate ctx to initializeCommand and escape logged image Address further PR review findings: - Thread the Up context through runInitializeCommand into exec.CommandContext so a cancelled/timed-out Up interrupts a blocking host-side hook. - Log the fallback image with %q to prevent CRLF log injection from CLI/config-derived values. --- pkg/devcontainer/initialize.go | 8 +++++--- pkg/devcontainer/run.go | 12 ++++++++---- pkg/devcontainer/run_test.go | 15 ++++++++------- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/pkg/devcontainer/initialize.go b/pkg/devcontainer/initialize.go index 3f9f319bb..252fea733 100644 --- a/pkg/devcontainer/initialize.go +++ b/pkg/devcontainer/initialize.go @@ -1,6 +1,7 @@ package devcontainer import ( + "context" "errors" "fmt" "os" @@ -25,6 +26,7 @@ type initializeCommand struct { // the host. Named sub-commands run concurrently; their errors are collected and // returned together. func runInitializeCommand( + ctx context.Context, workspaceFolder string, conf *config.DevContainerConfig, extraEnv []string, @@ -46,7 +48,7 @@ func runInitializeCommand( ) for name, cmd := range conf.InitializeCommand { wg.Go(func() { - if err := init.run(name, cmd); err != nil { + if err := init.run(ctx, name, cmd); err != nil { mu.Lock() errs = append(errs, err) mu.Unlock() @@ -73,7 +75,7 @@ func hostShell() []string { // run executes a single named sub-command. A single-element command is run as a // shell string; a multi-element command is executed argv-style. -func (c *initializeCommand) run(name string, cmd []string) error { +func (c *initializeCommand) run(ctx context.Context, name string, cmd []string) error { if len(cmd) == 0 { return fmt.Errorf("initializeCommand %q is empty", name) } @@ -95,7 +97,7 @@ func (c *initializeCommand) run(name string, cmd []string) error { defer func() { _ = stderr.Close() }() // args come from devcontainer.json initializeCommand, a trusted local config. - command := exec.Command(args[0], args[1:]...) //nolint:gosec // G204 + command := exec.CommandContext(ctx, args[0], args[1:]...) //nolint:gosec // G204 command.Dir = c.workspaceFolder command.Env = append(command.Environ(), c.extraEnv...) command.Stdout = stdout diff --git a/pkg/devcontainer/run.go b/pkg/devcontainer/run.go index 827368864..ec00a3152 100644 --- a/pkg/devcontainer/run.go +++ b/pkg/devcontainer/run.go @@ -126,7 +126,7 @@ func (r *runner) Up( } defer cleanupBuildInformation(substitutedConfig.Config) - if err := r.runInitializeCommand(substitutedConfig.Config, options); err != nil { + if err := r.runInitializeCommand(ctx, substitutedConfig.Config, options); err != nil { return nil, err } @@ -174,14 +174,18 @@ func (r *runner) Logs(ctx context.Context, writer io.Writer) error { // runInitializeCommand runs the host-side initializeCommand hook. The hook is // never executed in platform mode. -func (r *runner) runInitializeCommand(conf *config.DevContainerConfig, options UpOptions) error { +func (r *runner) runInitializeCommand( + ctx context.Context, + conf *config.DevContainerConfig, + options UpOptions, +) error { if options.Platform.Enabled { if len(conf.InitializeCommand) > 0 { log.Info("Skipping initializeCommand on platform") } return nil } - return runInitializeCommand(r.localWorkspaceFolder, conf, options.InitEnv) + return runInitializeCommand(ctx, r.localWorkspaceFolder, conf, options.InitEnv) } // runDefaultContainer handles configs missing image/dockerfile/compose by @@ -197,7 +201,7 @@ func (r *runner) runDefaultContainer( "\"image\", \"dockerFile\" or \"dockerComposeFile\" properties" if fallback := params.options.FallbackImage; fallback != "" { - log.Warn(missingProps + ", using fallback image " + fallback) + log.Warnf("%s, using fallback image %q", missingProps, fallback) conf.ImageContainer = config.ImageContainer{Image: fallback} return r.runSingleContainer(ctx, params) } diff --git a/pkg/devcontainer/run_test.go b/pkg/devcontainer/run_test.go index b4bc5b1ab..20181d9cb 100644 --- a/pkg/devcontainer/run_test.go +++ b/pkg/devcontainer/run_test.go @@ -1,6 +1,7 @@ package devcontainer import ( + "context" "os" "path/filepath" "testing" @@ -20,7 +21,7 @@ func TestRunInitializeCommand_ParallelTiming(t *testing.T) { } start := time.Now() - err := runInitializeCommand(tmpDir, conf, nil) + err := runInitializeCommand(context.Background(), tmpDir, conf, nil) elapsed := time.Since(start) if err != nil { @@ -41,7 +42,7 @@ func TestRunInitializeCommand_ParallelErrorCollection(t *testing.T) { "will-succeed": {"sh", "-c", "printf ok > " + markerFile}, } - err := runInitializeCommand(tmpDir, conf, nil) + err := runInitializeCommand(context.Background(), tmpDir, conf, nil) if err == nil { t.Fatal("expected error from failing command") } @@ -67,7 +68,7 @@ func TestRunInitializeCommand_SingleKey(t *testing.T) { "write-file": {"sh", "-c", "printf single > " + outFile}, } - err := runInitializeCommand(tmpDir, conf, nil) + err := runInitializeCommand(context.Background(), tmpDir, conf, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -91,7 +92,7 @@ func TestRunInitializeCommand_StringFormat(t *testing.T) { "": {"printf stringfmt > " + outFile}, } - err := runInitializeCommand(tmpDir, conf, nil) + err := runInitializeCommand(context.Background(), tmpDir, conf, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -108,13 +109,13 @@ func TestRunInitializeCommand_StringFormat(t *testing.T) { func TestRunInitializeCommand_Empty(t *testing.T) { // nil config conf := &config.DevContainerConfig{} - if err := runInitializeCommand(t.TempDir(), conf, nil); err != nil { + if err := runInitializeCommand(context.Background(), t.TempDir(), conf, nil); err != nil { t.Fatalf("nil InitializeCommand should return nil, got: %v", err) } // empty map conf.InitializeCommand = types.LifecycleHook{} - if err := runInitializeCommand(t.TempDir(), conf, nil); err != nil { + if err := runInitializeCommand(context.Background(), t.TempDir(), conf, nil); err != nil { t.Fatalf("empty InitializeCommand should return nil, got: %v", err) } } @@ -126,7 +127,7 @@ func TestRunInitializeCommand_EmptyEntry(t *testing.T) { } // An empty command slice must produce a normal error, not a panic. - err := runInitializeCommand(t.TempDir(), conf, nil) + err := runInitializeCommand(context.Background(), t.TempDir(), conf, nil) if err == nil { t.Fatal("expected error for empty command entry") } From f6deac5979f75a19c51f4f50b711cb0d97e224c5 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Wed, 8 Jul 2026 13:44:44 -0500 Subject: [PATCH 4/4] docs(devcontainer): correct defaultConsistency comment scope --- pkg/devcontainer/mount.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/devcontainer/mount.go b/pkg/devcontainer/mount.go index 46efc90ba..86fd060a4 100644 --- a/pkg/devcontainer/mount.go +++ b/pkg/devcontainer/mount.go @@ -8,8 +8,8 @@ import ( "github.com/devsy-org/devsy/pkg/devcontainer/config" ) -// defaultConsistency is applied to bind mounts on non-Linux hosts, where the -// file sharing layer benefits from an explicit consistency mode. +// defaultConsistency is applied to the workspace mount on non-Linux hosts, where +// the file sharing layer benefits from an explicit consistency mode. const defaultConsistency = "consistent" func needsDefaultConsistency() bool {