diff --git a/pkg/devcontainer/compose.go b/pkg/devcontainer/compose.go index 12a6a06c3..15c0be9f5 100644 --- a/pkg/devcontainer/compose.go +++ b/pkg/devcontainer/compose.go @@ -45,7 +45,7 @@ func (r *runner) stopDockerCompose(ctx context.Context, projectName string) erro return errors.Wrap(err, "find docker compose") } - parsedConfig, _, err := r.getSubstitutedConfig(r.WorkspaceConfig.CLIOptions) + parsedConfig, _, err := r.getSubstitutedConfig(r.WorkspaceConfig.CLIOptions, map[string]string{}) if err != nil { return errors.Wrap(err, "get parsed config") } @@ -69,7 +69,7 @@ func (r *runner) deleteDockerCompose(ctx context.Context, projectName string) er return errors.Wrap(err, "find docker compose") } - parsedConfig, _, err := r.getSubstitutedConfig(r.WorkspaceConfig.CLIOptions) + parsedConfig, _, err := r.getSubstitutedConfig(r.WorkspaceConfig.CLIOptions, map[string]string{}) if err != nil { return errors.Wrap(err, "get parsed config") } diff --git a/pkg/devcontainer/config.go b/pkg/devcontainer/config.go index d4a172111..2a54184ae 100644 --- a/pkg/devcontainer/config.go +++ b/pkg/devcontainer/config.go @@ -89,18 +89,19 @@ func (r *runner) getDefaultConfig(options provider2.CLIOptions) (*config.DevCont return defaultConfig, nil } -func (r *runner) getSubstitutedConfig(options provider2.CLIOptions) (*config.SubstitutedConfig, *config.SubstitutionContext, error) { +func (r *runner) getSubstitutedConfig(options provider2.CLIOptions, probedEnv map[string]string) (*config.SubstitutedConfig, *config.SubstitutionContext, error) { rawConfig, err := r.getRawConfig(options) if err != nil { return nil, nil, err } - return r.substitute(options, rawConfig) + return r.substitute(options, rawConfig, probedEnv) } func (r *runner) substitute( options provider2.CLIOptions, rawParsedConfig *config.DevContainerConfig, + probedEnv map[string]string, ) (*config.SubstitutedConfig, *config.SubstitutionContext, error) { configFile := rawParsedConfig.Origin @@ -110,11 +111,17 @@ func (r *runner) substitute( r.WorkspaceConfig.Workspace.ID, rawParsedConfig, ) + // merge probed environment with os.Environ() + env := config.ListToObject(os.Environ()) + for k, v := range probedEnv { + env[k] = v + } + substitutionContext := &config.SubstitutionContext{ DevContainerID: r.ID, LocalWorkspaceFolder: r.LocalWorkspaceFolder, ContainerWorkspaceFolder: containerWorkspaceFolder, - Env: config.ListToObject(os.Environ()), + Env: env, WorkspaceMount: workspaceMount, } diff --git a/pkg/devcontainer/config/userenvprobe.go b/pkg/devcontainer/config/userenvprobe.go index 88072381e..cbc6cf06c 100644 --- a/pkg/devcontainer/config/userenvprobe.go +++ b/pkg/devcontainer/config/userenvprobe.go @@ -48,6 +48,10 @@ func NewUserEnvProbe(probe string) (UserEnvProbe, error) { } func ProbeUserEnv(ctx context.Context, probe string, userName string, log log.Logger) (map[string]string, error) { + return ProbeUserEnvWithUserSwitch(ctx, probe, userName, true, log) +} + +func ProbeUserEnvWithUserSwitch(ctx context.Context, probe string, userName string, switchUser bool, log log.Logger) (map[string]string, error) { userEnvProbe, err := NewUserEnvProbe(probe) if err != nil { log.Warnf("Get user env probe: %v", err) @@ -66,12 +70,12 @@ func ProbeUserEnv(ctx context.Context, probe string, userName string, log log.Lo log.Debugf("running user env probe with shell \"%s\", probe \"%s\", user \"%s\" and command \"%s\"", strings.Join(preferredShell, " "), string(userEnvProbe), userName, "cat /proc/self/environ") - probedEnv, err := doProbe(ctx, userEnvProbe, preferredShell, userName, "cat /proc/self/environ", '\x00', log) + probedEnv, err := doProbeWithUserSwitch(ctx, userEnvProbe, preferredShell, userName, "cat /proc/self/environ", '\x00', switchUser, log) if err != nil { log.Debugf("running user env probe with shell \"%s\", probe \"%s\", user \"%s\" and command \"%s\"", strings.Join(preferredShell, " "), string(userEnvProbe), userName, "printenv") - newProbedEnv, newErr := doProbe(ctx, userEnvProbe, preferredShell, userName, "printenv", '\n', log) + newProbedEnv, newErr := doProbeWithUserSwitch(ctx, userEnvProbe, preferredShell, userName, "printenv", '\n', switchUser, log) if newErr != nil { log.Warnf("failed to probe user environment variables: %v, %v", err, newErr) } else { @@ -86,6 +90,10 @@ func ProbeUserEnv(ctx context.Context, probe string, userName string, log log.Lo } func doProbe(ctx context.Context, userEnvProbe UserEnvProbe, preferredShell []string, userName string, probeCmd string, sep byte, log log.Logger) (map[string]string, error) { + return doProbeWithUserSwitch(ctx, userEnvProbe, preferredShell, userName, probeCmd, sep, true, log) +} + +func doProbeWithUserSwitch(ctx context.Context, userEnvProbe UserEnvProbe, preferredShell []string, userName string, probeCmd string, sep byte, switchUser bool, log log.Logger) (map[string]string, error) { args := preferredShell args = append(args, getShellArgs(userEnvProbe, userName, probeCmd)...) @@ -93,9 +101,11 @@ func doProbe(ctx context.Context, userEnvProbe UserEnvProbe, preferredShell []st defer cancel() cmd := exec.CommandContext(timeoutCtx, args[0], args[1:]...) - err := PrepareCmdUser(cmd, userName) - if err != nil { - return nil, fmt.Errorf("prepare probe: %w", err) + if switchUser { + err := PrepareCmdUser(cmd, userName) + if err != nil { + return nil, fmt.Errorf("prepare probe: %w", err) + } } out, err := cmd.Output() @@ -114,7 +124,7 @@ func doProbe(ctx context.Context, userEnvProbe UserEnvProbe, preferredShell []st log.Debugf("failed to split env var: %s", line) continue } - retEnv[tokens[0]] = tokens[1] + retEnv[tokens[0]] = strings.Join(tokens[1:], "=") } if scanner.Err() != nil { return nil, fmt.Errorf("scan shell output: %w", err) diff --git a/pkg/devcontainer/prebuild.go b/pkg/devcontainer/prebuild.go index 8e27e7b65..25b6c01cd 100644 --- a/pkg/devcontainer/prebuild.go +++ b/pkg/devcontainer/prebuild.go @@ -21,7 +21,7 @@ func (r *runner) Build(ctx context.Context, options provider.BuildOptions) (stri return "", fmt.Errorf("building only supported with docker driver") } - substitutedConfig, substitutionContext, err := r.getSubstitutedConfig(options.CLIOptions) + substitutedConfig, substitutionContext, err := r.getSubstitutedConfig(options.CLIOptions, map[string]string{}) if err != nil { return "", err } diff --git a/pkg/devcontainer/run.go b/pkg/devcontainer/run.go index aa627d24f..bf2a404d6 100644 --- a/pkg/devcontainer/run.go +++ b/pkg/devcontainer/run.go @@ -92,7 +92,14 @@ type UpOptions struct { func (r *runner) Up(ctx context.Context, options UpOptions, timeout time.Duration) (*config.Result, error) { r.Log.Debugf("Up devcontainer for workspace '%s' with timeout %s", r.WorkspaceConfig.Workspace.ID, timeout) - substitutedConfig, substitutionContext, err := r.getSubstitutedConfig(options.CLIOptions) + // probe local user environment for localEnv substitution + probedEnv, err := config.ProbeUserEnvWithUserSwitch(ctx, string(config.DefaultUserEnvProbe), "", false, r.Log) + if err != nil { + r.Log.Warnf("failed to probe local user environment, localEnv variables may not work: %v", err) + probedEnv = map[string]string{} + } + + substitutedConfig, substitutionContext, err := r.getSubstitutedConfig(options.CLIOptions, probedEnv) if err != nil { return nil, err } diff --git a/pkg/devcontainer/setup/reproduce_issue_test.go b/pkg/devcontainer/setup/reproduce_issue_test.go new file mode 100644 index 000000000..42ada4ff8 --- /dev/null +++ b/pkg/devcontainer/setup/reproduce_issue_test.go @@ -0,0 +1,81 @@ +package setup + +import ( + "os" + "os/user" + "path/filepath" + "testing" + + "github.com/loft-sh/devpod/pkg/devcontainer/config" + "github.com/loft-sh/log" +) + +func TestChownMounts(t *testing.T) { + // Create temp directories for mounts + tempDir := t.TempDir() + mountTarget1 := filepath.Join(tempDir, "mount1") + mountTarget2 := filepath.Join(tempDir, "mount2") + err := os.Mkdir(mountTarget1, 0755) + if err != nil { + t.Fatalf("Failed to create temp dir mount1: %v", err) + } + err = os.Mkdir(mountTarget2, 0755) + if err != nil { + t.Fatalf("Failed to create temp dir mount2: %v", err) + } + + // Set MarkerBaseDir to temp dir to avoid permission issues + oldMarkerBaseDir := MarkerBaseDir + MarkerBaseDir = t.TempDir() + defer func() { MarkerBaseDir = oldMarkerBaseDir }() + + // Get current user + currentUser, err := user.Current() + if err != nil { + t.Fatalf("Failed to get current user: %v", err) + } + + // Create a mock result with bind mounts + result := &config.Result{ + MergedConfig: &config.MergedDevContainerConfig{ + DevContainerConfigBase: config.DevContainerConfigBase{ + RemoteUser: currentUser.Username, + }, + NonComposeBase: config.NonComposeBase{ + Mounts: []*config.Mount{ + { + Source: "/local/path", + Target: mountTarget1, + Type: "bind", + }, + }, + }, + }, + SubstitutionContext: &config.SubstitutionContext{ + WorkspaceMount: "source=/ws/src,target=" + mountTarget2 + ",type=bind", + ContainerWorkspaceFolder: mountTarget2, + }, + } + + // Mock logger + logger := log.Discard + + // Call ChownMounts + // We expect it to succeed for the existing directories with current user + err = ChownMounts(result, logger) + if err != nil { + t.Errorf("ChownMounts failed: %v", err) + } + + // Verify marker file created + markerPath := filepath.Join(MarkerBaseDir, "chownMounts.marker") + if _, err := os.Stat(markerPath); os.IsNotExist(err) { + t.Errorf("Marker file not created at %s", markerPath) + } + + // Call again, should be skipped (log logic inside, but we rely on function returning nil and not erroring) + err = ChownMounts(result, logger) + if err != nil { + t.Errorf("ChownMounts second call failed: %v", err) + } +} diff --git a/pkg/devcontainer/setup/setup.go b/pkg/devcontainer/setup/setup.go index d367373ae..21cbbc271 100644 --- a/pkg/devcontainer/setup/setup.go +++ b/pkg/devcontainer/setup/setup.go @@ -26,7 +26,12 @@ import ( ) const ( - ResultLocation = "/var/run/devpod/result.json" + DefaultResultLocation = "/var/run/devpod/result.json" +) + +var ( + ResultLocation = DefaultResultLocation + MarkerBaseDir = "/var/devpod" ) func SetupContainer(ctx context.Context, setupInfo *config.Result, extraWorkspaceEnv []string, chownProjects bool, platformOptions *devpod.PlatformOptions, tunnelClient tunnel.TunnelClient, log log.Logger) error { @@ -39,6 +44,12 @@ func SetupContainer(ctx context.Context, setupInfo *config.Result, extraWorkspac return errors.Wrap(err, "chown workspace") } + // chown mounts + err = ChownMounts(setupInfo, log) + if err != nil { + log.Warnf("Error chowning mounts: %v", err) + } + // patch remote env log.Debugf("Patch etc environment & profile...") err = PatchEtcEnvironment(setupInfo.MergedConfig, log) @@ -179,6 +190,40 @@ func ChownWorkspace(setupInfo *config.Result, recursive bool, log log.Logger) er return nil } +func ChownMounts(setupInfo *config.Result, log log.Logger) error { + user := config.GetRemoteUser(setupInfo) + exists, err := markerFileExists("chownMounts", "") + if err != nil { + return err + } else if exists { + return nil + } + + // check if we have any mounts to chown + mounts := config.GetMounts(setupInfo) + if len(mounts) == 0 { + return nil + } + + log.Infof("Chown mounts...") + for _, m := range mounts { + if m.Type == "bind" && m.Target != "" { + // check if it is the workspace folder + if strings.HasPrefix(m.Target, setupInfo.SubstitutionContext.ContainerWorkspaceFolder) { + continue + } + + log.Debugf("Chown mount %s...", m.Target) + err = copy2.Chown(m.Target, user) + if err != nil { + // Just log warning as some mounts might not be chownable or might not exist in the same way + log.Debugf("Failed to chown mount %s: %v", m.Target, err) + } + } + } + return nil +} + func PatchEtcProfile() error { exists, err := markerFileExists("patchEtcProfile", "") if err != nil { @@ -330,7 +375,7 @@ func SetupKubeConfig(ctx context.Context, setupInfo *config.Result, tunnelClient } func markerFileExists(markerName string, markerContent string) (bool, error) { - markerName = filepath.Join("/var/devpod", markerName+".marker") + markerName = filepath.Join(MarkerBaseDir, markerName+".marker") t, err := os.ReadFile(markerName) if err != nil && !os.IsNotExist(err) { return false, err diff --git a/pkg/devcontainer/setup/setup_test.go b/pkg/devcontainer/setup/setup_test.go new file mode 100644 index 000000000..42ada4ff8 --- /dev/null +++ b/pkg/devcontainer/setup/setup_test.go @@ -0,0 +1,81 @@ +package setup + +import ( + "os" + "os/user" + "path/filepath" + "testing" + + "github.com/loft-sh/devpod/pkg/devcontainer/config" + "github.com/loft-sh/log" +) + +func TestChownMounts(t *testing.T) { + // Create temp directories for mounts + tempDir := t.TempDir() + mountTarget1 := filepath.Join(tempDir, "mount1") + mountTarget2 := filepath.Join(tempDir, "mount2") + err := os.Mkdir(mountTarget1, 0755) + if err != nil { + t.Fatalf("Failed to create temp dir mount1: %v", err) + } + err = os.Mkdir(mountTarget2, 0755) + if err != nil { + t.Fatalf("Failed to create temp dir mount2: %v", err) + } + + // Set MarkerBaseDir to temp dir to avoid permission issues + oldMarkerBaseDir := MarkerBaseDir + MarkerBaseDir = t.TempDir() + defer func() { MarkerBaseDir = oldMarkerBaseDir }() + + // Get current user + currentUser, err := user.Current() + if err != nil { + t.Fatalf("Failed to get current user: %v", err) + } + + // Create a mock result with bind mounts + result := &config.Result{ + MergedConfig: &config.MergedDevContainerConfig{ + DevContainerConfigBase: config.DevContainerConfigBase{ + RemoteUser: currentUser.Username, + }, + NonComposeBase: config.NonComposeBase{ + Mounts: []*config.Mount{ + { + Source: "/local/path", + Target: mountTarget1, + Type: "bind", + }, + }, + }, + }, + SubstitutionContext: &config.SubstitutionContext{ + WorkspaceMount: "source=/ws/src,target=" + mountTarget2 + ",type=bind", + ContainerWorkspaceFolder: mountTarget2, + }, + } + + // Mock logger + logger := log.Discard + + // Call ChownMounts + // We expect it to succeed for the existing directories with current user + err = ChownMounts(result, logger) + if err != nil { + t.Errorf("ChownMounts failed: %v", err) + } + + // Verify marker file created + markerPath := filepath.Join(MarkerBaseDir, "chownMounts.marker") + if _, err := os.Stat(markerPath); os.IsNotExist(err) { + t.Errorf("Marker file not created at %s", markerPath) + } + + // Call again, should be skipped (log logic inside, but we rely on function returning nil and not erroring) + err = ChownMounts(result, logger) + if err != nil { + t.Errorf("ChownMounts second call failed: %v", err) + } +}