diff --git a/cmd/ci/ci.go b/cmd/ci/ci.go index 2024812b6a..975ffc8418 100644 --- a/cmd/ci/ci.go +++ b/cmd/ci/ci.go @@ -291,6 +291,8 @@ func (cmd *CICmd) resolveWorkspace( ProviderUserOptions: cmd.ProviderOptions, DevContainerImage: cmd.DevContainerImage, DevContainerPath: cmd.DevContainerPath, + DevContainerID: cmd.DevContainerID, + DevContainerSource: cmd.DevContainerSource, SSHConfigPath: sshConfigPath, UID: cmd.UID, Owner: cmd.Owner, diff --git a/cmd/workspace/up/agent.go b/cmd/workspace/up/agent.go index 5a4733945b..671a07cd53 100644 --- a/cmd/workspace/up/agent.go +++ b/cmd/workspace/up/agent.go @@ -145,7 +145,10 @@ func (cmd *UpCmd) devsyUpDaemon( func (cmd *UpCmd) buildWorkspaceOptions(workspace *provider2.Workspace) provider2.CLIOptions { baseOptions := cmd.CLIOptions baseOptions.ID = workspace.ID - baseOptions.DevContainerPath = workspace.DevContainerPath + if workspace.DevContainerConfig == nil { + baseOptions.DevContainerPath = workspace.DevContainerPath + baseOptions.DevContainerID = workspace.DevContainerID + } baseOptions.DevContainerImage = workspace.DevContainerImage baseOptions.DevContainerSource = workspace.DevContainerSource baseOptions.IDE = workspace.IDE.Name diff --git a/cmd/workspace/up/up_client.go b/cmd/workspace/up/up_client.go index cda32f8896..544ca93537 100644 --- a/cmd/workspace/up/up_client.go +++ b/cmd/workspace/up/up_client.go @@ -217,6 +217,7 @@ func (cmd *UpCmd) resolveParams( ReconfigureProvider: cmd.Reconfigure, DevContainerImage: cmd.DevContainerImage, DevContainerPath: cmd.DevContainerPath, + DevContainerID: cmd.DevContainerID, DevContainerSource: cmd.DevContainerSource, SSHConfigPath: cmd.SSHConfigPath, SSHConfigIncludePath: devsyConfig.ContextOption( diff --git a/e2e/tests/up-docker-compose/config.go b/e2e/tests/up-docker-compose/config.go index f394fcdc95..f29342a1b5 100644 --- a/e2e/tests/up-docker-compose/config.go +++ b/e2e/tests/up-docker-compose/config.go @@ -17,6 +17,7 @@ import ( pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/devcontainer/config" docker "github.com/devsy-org/devsy/pkg/docker" + "github.com/devsy-org/devsy/pkg/flags/names" provider2 "github.com/devsy-org/devsy/pkg/provider" "github.com/devsy-org/devsy/pkg/status" "github.com/docker/docker/api/types/container" @@ -142,6 +143,43 @@ var _ = ginkgo.Describe( gomega.Expect(restartIds).To(gomega.HaveLen(1), "1 compose container after restart") }, ginkgo.SpecTimeout(framework.TimeoutLong())) + ginkgo.It("multi-profile lifecycle retains selected profile", func(ctx context.Context) { + tempDir, err := setupWorkspace( + "tests/up-docker-compose/testdata/docker-compose-multi-profile", + tc.initialDir, + tc.f, + ) + framework.ExpectNoError(err) + + err = tc.f.DevsyUp(ctx, names.Flag(names.DevContainer), "id:max", tempDir) + framework.ExpectNoError(err) + + err = tc.f.DevsyWorkspaceStop(ctx, tempDir) + framework.ExpectNoError(err) + + err = tc.f.DevsyUp(ctx, tempDir) + framework.ExpectNoError(err) + + workspace, err := tc.f.FindWorkspace(ctx, tempDir) + framework.ExpectNoError(err) + + // Distinct markers verify the persisted selector survived restart. + err = tc.f.ExecCommand( + ctx, + true, + true, + "[max]", + []string{ + cmdWorkspace, cmdSSH, flagCommand, + "echo \"[$SELECTED_PROFILE]\"", workspace.ID, + }, + ) + framework.ExpectNoError(err) + + err = tc.f.DevsyWorkspaceDelete(ctx, tempDir) + framework.ExpectNoError(err) + }, ginkgo.SpecTimeout(framework.TimeoutLong())) + ginkgo.It("environment variables", func(ctx context.Context) { _, workspace, err := tc.setupAndStartWorkspace( ctx, diff --git a/e2e/tests/up-docker-compose/testdata/docker-compose-multi-profile/.devcontainer/compose.yaml b/e2e/tests/up-docker-compose/testdata/docker-compose-multi-profile/.devcontainer/compose.yaml new file mode 100644 index 0000000000..2b9da8bdff --- /dev/null +++ b/e2e/tests/up-docker-compose/testdata/docker-compose-multi-profile/.devcontainer/compose.yaml @@ -0,0 +1,4 @@ +services: + app: + image: ghcr.io/devsy-org/test-images/go:1 + command: sleep infinity diff --git a/e2e/tests/up-docker-compose/testdata/docker-compose-multi-profile/.devcontainer/max-nvidia/devcontainer.json b/e2e/tests/up-docker-compose/testdata/docker-compose-multi-profile/.devcontainer/max-nvidia/devcontainer.json new file mode 100644 index 0000000000..855b7ea29a --- /dev/null +++ b/e2e/tests/up-docker-compose/testdata/docker-compose-multi-profile/.devcontainer/max-nvidia/devcontainer.json @@ -0,0 +1,9 @@ +{ + "name": "max-nvidia", + "dockerComposeFile": "../compose.yaml", + "service": "app", + "workspaceFolder": "/workspaces", + "containerEnv": { + "SELECTED_PROFILE": "max-nvidia" + } +} diff --git a/e2e/tests/up-docker-compose/testdata/docker-compose-multi-profile/.devcontainer/max/devcontainer.json b/e2e/tests/up-docker-compose/testdata/docker-compose-multi-profile/.devcontainer/max/devcontainer.json new file mode 100644 index 0000000000..200bc2f735 --- /dev/null +++ b/e2e/tests/up-docker-compose/testdata/docker-compose-multi-profile/.devcontainer/max/devcontainer.json @@ -0,0 +1,9 @@ +{ + "name": "max", + "dockerComposeFile": "../compose.yaml", + "service": "app", + "workspaceFolder": "/workspaces", + "containerEnv": { + "SELECTED_PROFILE": "max" + } +} diff --git a/e2e/tests/up/helper.go b/e2e/tests/up/helper.go index b2ec878c5f..7cd789f04a 100644 --- a/e2e/tests/up/helper.go +++ b/e2e/tests/up/helper.go @@ -116,6 +116,15 @@ func probeSSH( return out, err } +func readLifecycleFile(workspaceDir, name string) (string, error) { + //nolint:gosec // G304: test-controlled path inside workspace + data, err := os.ReadFile(filepath.Join(workspaceDir, name)) + if err != nil { + return "", err + } + return strings.TrimSpace(string(data)), nil +} + // lifecycleMarkerCount reads a marker file in workspaceDir and returns the count // of non-empty lines. If the file does not exist, it returns 0, nil. // diff --git a/e2e/tests/up/provider_docker.go b/e2e/tests/up/provider_docker.go index aee7fedb06..9a0197dc2f 100644 --- a/e2e/tests/up/provider_docker.go +++ b/e2e/tests/up/provider_docker.go @@ -394,47 +394,31 @@ var _ = ginkgo.Describe( err = dtc.f.DevsyUp(ctx, tempDir) framework.ExpectNoError(err) - // onCreateCommand and updateContentCommand should have run (foreground). - out, err := dtc.execSSH(ctx, tempDir, "cat $HOME/on-create.out") + out, err := readLifecycleFile(tempDir, "on-create.out") framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("onCreateDone")) + gomega.Expect(out).To(gomega.Equal("onCreateDone")) - out, err = dtc.execSSH(ctx, tempDir, "cat $HOME/update-content.out") + out, err = readLifecycleFile(tempDir, "update-content.out") framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("updateContentDone")) + gomega.Expect(out).To(gomega.Equal("updateContentDone")) - // postCreateCommand runs as a deferred hook in the background. - // Wait for it to complete and verify the marker file + env substitution. - gomega.Eventually(func() string { - out, err := dtc.execSSH(ctx, tempDir, "cat $HOME/deferred.marker 2>/dev/null") - if err != nil { - return "" - } - return strings.TrimSpace(out) + // Deferred hooks run in the background; verify completion and env substitution. + gomega.Eventually(func() (string, error) { + return readLifecycleFile(tempDir, "deferred.marker") }).WithTimeout(30*time.Second).WithPolling(2*time.Second).Should( gomega.Equal("postCreateDone"), "deferred postCreateCommand should eventually complete in background", ) - // Verify the deferred hook received substituted env vars, not literals. - envPath, err := dtc.execSSH(ctx, tempDir, "cat $HOME/deferred-env-path.out") + envPath, err := readLifecycleFile(tempDir, "deferred-env-path.out") framework.ExpectNoError(err) gomega.Expect(envPath).To(gomega.ContainSubstring("/usr/local/bin"), "deferred hook should receive resolved PATH, not ${containerEnv:PATH}") gomega.Expect(envPath).NotTo(gomega.ContainSubstring("${containerEnv:"), "deferred hook should not contain literal variable references") - // postStartCommand also deferred — verify it ran. - gomega.Eventually(func() string { - out, err := dtc.execSSH( - ctx, - tempDir, - "cat $HOME/post-start-deferred.out 2>/dev/null", - ) - if err != nil { - return "" - } - return strings.TrimSpace(out) + gomega.Eventually(func() (string, error) { + return readLifecycleFile(tempDir, "post-start-deferred.out") }).WithTimeout(30*time.Second).WithPolling(2*time.Second).Should( gomega.Equal("postStartDone"), "deferred postStartCommand should eventually complete in background", diff --git a/e2e/tests/up/provider_podman_rootful_lifecycle.go b/e2e/tests/up/provider_podman_rootful_lifecycle.go index b6ef84a7c8..583384330e 100644 --- a/e2e/tests/up/provider_podman_rootful_lifecycle.go +++ b/e2e/tests/up/provider_podman_rootful_lifecycle.go @@ -108,45 +108,29 @@ var _ = ginkgo.Describe( err = f.DevsyUp(ctx, tempDir) framework.ExpectNoError(err) - out, err := f.DevsySSH(ctx, tempDir, "cat $HOME/on-create.out") + out, err := readLifecycleFile(tempDir, "on-create.out") framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("onCreateDone")) + gomega.Expect(out).To(gomega.Equal("onCreateDone")) - out, err = f.DevsySSH(ctx, tempDir, "cat $HOME/update-content.out") + out, err = readLifecycleFile(tempDir, "update-content.out") framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("updateContentDone")) + gomega.Expect(out).To(gomega.Equal("updateContentDone")) - gomega.Eventually(func() string { - out, err := probeSSH(f, - ctx, tempDir, "cat $HOME/deferred.marker 2>/dev/null", - ) - if err != nil { - return "" - } - return strings.TrimSpace(out) + gomega.Eventually(func() (string, error) { + return readLifecycleFile(tempDir, "deferred.marker") }).WithTimeout(30 * time.Second).WithPolling(2 * time.Second).Should( gomega.Equal("postCreateDone"), ) - envPath, err := f.DevsySSH( - ctx, tempDir, "cat $HOME/deferred-env-path.out", - ) + envPath, err := readLifecycleFile(tempDir, "deferred-env-path.out") framework.ExpectNoError(err) gomega.Expect(envPath).To( gomega.ContainSubstring("/usr/local/bin"), ) gomega.Expect(envPath).NotTo(gomega.ContainSubstring("${containerEnv:")) - gomega.Eventually(func() string { - out, err := probeSSH(f, - ctx, - tempDir, - "cat $HOME/post-start-deferred.out 2>/dev/null", - ) - if err != nil { - return "" - } - return strings.TrimSpace(out) + gomega.Eventually(func() (string, error) { + return readLifecycleFile(tempDir, "post-start-deferred.out") }).WithTimeout(30 * time.Second).WithPolling(2 * time.Second).Should( gomega.Equal("postStartDone"), ) diff --git a/e2e/tests/up/provider_podman_rootless_lifecycle.go b/e2e/tests/up/provider_podman_rootless_lifecycle.go index 2b172d43d1..e0caa0e02b 100644 --- a/e2e/tests/up/provider_podman_rootless_lifecycle.go +++ b/e2e/tests/up/provider_podman_rootless_lifecycle.go @@ -101,45 +101,29 @@ var _ = ginkgo.Describe( err = f.DevsyUp(ctx, tempDir) framework.ExpectNoError(err) - out, err := f.DevsySSH(ctx, tempDir, "cat $HOME/on-create.out") + out, err := readLifecycleFile(tempDir, "on-create.out") framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("onCreateDone")) + gomega.Expect(out).To(gomega.Equal("onCreateDone")) - out, err = f.DevsySSH(ctx, tempDir, "cat $HOME/update-content.out") + out, err = readLifecycleFile(tempDir, "update-content.out") framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("updateContentDone")) + gomega.Expect(out).To(gomega.Equal("updateContentDone")) - gomega.Eventually(func() string { - out, err := probeSSH(f, - ctx, tempDir, "cat $HOME/deferred.marker 2>/dev/null", - ) - if err != nil { - return "" - } - return strings.TrimSpace(out) + gomega.Eventually(func() (string, error) { + return readLifecycleFile(tempDir, "deferred.marker") }).WithTimeout(30 * time.Second).WithPolling(2 * time.Second).Should( gomega.Equal("postCreateDone"), ) - envPath, err := f.DevsySSH( - ctx, tempDir, "cat $HOME/deferred-env-path.out", - ) + envPath, err := readLifecycleFile(tempDir, "deferred-env-path.out") framework.ExpectNoError(err) gomega.Expect(envPath).To( gomega.ContainSubstring("/usr/local/bin"), ) gomega.Expect(envPath).NotTo(gomega.ContainSubstring("${containerEnv:")) - gomega.Eventually(func() string { - out, err := probeSSH(f, - ctx, - tempDir, - "cat $HOME/post-start-deferred.out 2>/dev/null", - ) - if err != nil { - return "" - } - return strings.TrimSpace(out) + gomega.Eventually(func() (string, error) { + return readLifecycleFile(tempDir, "post-start-deferred.out") }).WithTimeout(30 * time.Second).WithPolling(2 * time.Second).Should( gomega.Equal("postStartDone"), ) diff --git a/e2e/tests/up/testdata/docker-waitfor/.devcontainer.json b/e2e/tests/up/testdata/docker-waitfor/.devcontainer.json index 56c24e1fd3..34467c941b 100644 --- a/e2e/tests/up/testdata/docker-waitfor/.devcontainer.json +++ b/e2e/tests/up/testdata/docker-waitfor/.devcontainer.json @@ -4,8 +4,8 @@ "remoteEnv": { "CONTAINER_ENV_PATH": "${containerEnv:PATH}" }, - "onCreateCommand": "echo onCreateDone > $HOME/on-create.out", - "updateContentCommand": "echo updateContentDone > $HOME/update-content.out", - "postCreateCommand": "echo -n ${CONTAINER_ENV_PATH} > $HOME/deferred-env-path.out && echo postCreateDone > $HOME/deferred.marker", - "postStartCommand": "echo postStartDone > $HOME/post-start-deferred.out" + "onCreateCommand": "echo onCreateDone > on-create.out", + "updateContentCommand": "echo updateContentDone > update-content.out", + "postCreateCommand": "echo -n ${CONTAINER_ENV_PATH} > deferred-env-path.out && echo postCreateDone > deferred.marker", + "postStartCommand": "echo postStartDone > post-start-deferred.out" } diff --git a/pkg/devcontainer/config.go b/pkg/devcontainer/config.go index c886bea5ee..4b1c8d074f 100644 --- a/pkg/devcontainer/config.go +++ b/pkg/devcontainer/config.go @@ -43,35 +43,166 @@ func (r *runner) getRawConfigWithContext( ctx context.Context, options provider.CLIOptions, ) (*config.DevContainerConfig, error) { - source := options.DevContainerSource - if source == "" { - source = r.workspaceConfig.Workspace.DevContainerSource + selection := r.effectiveDevContainerSelection(options) + if selection.source != "" { + return r.rawConfigFromSourceWithContext(ctx, selection.source, options) } - if source != "" { - return r.rawConfigFromSourceWithContext(ctx, source, options) - } - if conf := r.rawConfigFromWorkspace(); conf != nil { - return conf, nil + if selection.path == "" && selection.id == "" { + if conf := r.rawConfigFromWorkspace(); conf != nil { + return conf, nil + } } if conf := r.rawConfigFromContainer(); conf != nil { return conf, nil } if crane.ShouldUse(&options) { - return r.rawConfigFromCraneWithContext(ctx, options) + return r.rawConfigFromCraneWithContext(ctx, options, selection) + } + return r.rawConfigFromFilesystemWithContext(ctx, options, selection) +} + +type devContainerSelection struct { + source string + path string + id string +} + +// effectiveDevContainerSelection returns the one effective selector for this +// operation. Current CLI input wins over persisted workspace state, followed +// by the last resolved path for compatibility with older workspaces. +func (r *runner) effectiveDevContainerSelection( + options provider.CLIOptions, +) devContainerSelection { + if selection, ok := newDevContainerSelection( + options.DevContainerSource, + options.DevContainerPath, + options.DevContainerID, + ); ok { + return selection + } + + if r.workspaceConfig != nil && r.workspaceConfig.Workspace != nil { + if selection, ok := r.persistedDevContainerSelection(); ok { + return selection + } + } + + if selection, ok := r.lastConfigPathSelection(); ok { + return selection + } + + return devContainerSelection{} +} + +// lastConfigPathSelection supports workspaces created before selection was +// persisted. Embedded configs take precedence, and reset-created stale paths +// fall back to discovery when their recorded file no longer exists. +func (r *runner) lastConfigPathSelection() (devContainerSelection, bool) { + if r.workspaceConfig == nil || r.workspaceConfig.LastDevContainerConfig == nil { + return devContainerSelection{}, false + } + workspace := r.workspaceConfig.Workspace + if workspace != nil && workspace.DevContainerConfig != nil { + return devContainerSelection{}, false + } + path := r.workspaceConfig.LastDevContainerConfig.Path + if path == "" { + return devContainerSelection{}, false + } + relativePath := r.workspaceRelativeLastConfigPath(path) + if !devContainerConfigExists(r.workspaceFolder(), relativePath) { + return devContainerSelection{}, false + } + return devContainerSelection{path: relativePath}, true +} + +func devContainerConfigExists(workspaceFolder, relativePath string) bool { + _, err := os.Stat(filepath.Join(workspaceFolder, filepath.FromSlash(relativePath))) + // Only a missing file means absent. Other stat errors (permissions, a + // file where a directory should be, ...) count as present so the parse + // step surfaces the real filesystem error instead of silently skipping + // the recorded path. + return err == nil || !os.IsNotExist(err) +} + +func (r *runner) persistedDevContainerSelection() (devContainerSelection, bool) { + workspace := r.workspaceConfig.Workspace + switch { + case workspace.DevContainerSource != "": + return devContainerSelection{source: workspace.DevContainerSource}, true + case workspace.DevContainerConfig != nil: + // Preserve pre-persistence precedence for embedded configs. + return devContainerSelection{}, false + case workspace.DevContainerPath != "": + return devContainerSelection{path: workspace.DevContainerPath}, true + case workspace.DevContainerID != "": + return devContainerSelection{id: workspace.DevContainerID}, true + default: + return devContainerSelection{}, false + } +} + +// workspaceRelativeLastConfigPath converts a content-root-relative last path +// to the workspace folder (the content root plus the git subpath). Paths +// outside the subpath remain unchanged. +func (r *runner) workspaceRelativeLastConfigPath(lastPath string) string { + if r.workspaceConfig == nil || r.workspaceConfig.Workspace == nil { + return lastPath + } + + // The stored subpath can carry a leading slash (@subpath:/x/y); the last + // path never does, so the comparison needs the repo-relative form. + subPath := filepath.Clean(filepath.FromSlash(r.workspaceConfig.Workspace.Source.GitSubPath)) + subPath = strings.TrimPrefix(subPath, string(filepath.Separator)) + if subPath == "." || subPath == "" { + return lastPath + } + + relativePath, err := filepath.Rel(subPath, filepath.FromSlash(lastPath)) + if err != nil || relativePath == ".." || + strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) { + return lastPath + } + return filepath.ToSlash(relativePath) +} + +// workspaceFolder is the content root plus the git subpath, when present. +// Workspace.DevContainerPath and CLI paths are relative to this folder. +func (r *runner) workspaceFolder() string { + if r.workspaceConfig == nil || r.workspaceConfig.Workspace == nil { + return r.localWorkspaceFolder + } + subPath := r.workspaceConfig.Workspace.Source.GitSubPath + if subPath == "" { + return r.localWorkspaceFolder + } + return filepath.Join(r.localWorkspaceFolder, filepath.FromSlash(subPath)) +} + +func newDevContainerSelection(source, path, id string) (devContainerSelection, bool) { + switch { + case source != "": + return devContainerSelection{source: source}, true + case path != "": + return devContainerSelection{path: path}, true + case id != "": + return devContainerSelection{id: id}, true + default: + return devContainerSelection{}, false } - return r.rawConfigFromFilesystemWithContext(ctx, options) } // 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 == nil || r.workspaceConfig.Workspace == nil || + 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.Origin = path.Join(filepath.ToSlash(r.workspaceFolder()), devContainerPath) } else { rawConfig.Origin = path.Join( filepath.ToSlash(r.localWorkspaceFolder), @@ -103,12 +234,15 @@ func (r *runner) rawConfigFromContainer() *config.DevContainerConfig { func (r *runner) rawConfigFromCrane( options provider.CLIOptions, ) (*config.DevContainerConfig, error) { - return r.rawConfigFromCraneWithContext(context.Background(), options) + return r.rawConfigFromCraneWithContext( + context.Background(), options, r.effectiveDevContainerSelection(options), + ) } func (r *runner) rawConfigFromCraneWithContext( ctx context.Context, options provider.CLIOptions, + selection devContainerSelection, ) (*config.DevContainerConfig, error) { localWorkspaceFolder, err := crane.PullConfigFromSourceWithContext( ctx, @@ -118,10 +252,20 @@ func (r *runner) rawConfigFromCraneWithContext( if err != nil { return nil, err } - return config.ParseDevContainerJSON( + opts := config.ParseOptions{} + if selection.id != "" { + // As on the filesystem path, an explicit id must not be shadowed by a + // root config, and a mismatch must error rather than fall back. + opts = config.ParseOptions{ + Selector: config.SelectByID(selection.id), + ForceSelect: true, + } + } + return config.ParseDevContainerJSONWithOptions( ctx, localWorkspaceFolder, - r.workspaceConfig.Workspace.DevContainerPath, + selection.path, + opts, ) } @@ -130,24 +274,24 @@ func (r *runner) rawConfigFromCraneWithContext( func (r *runner) rawConfigFromFilesystem( options provider.CLIOptions, ) (*config.DevContainerConfig, error) { - return r.rawConfigFromFilesystemWithContext(context.Background(), options) + return r.rawConfigFromFilesystemWithContext( + context.Background(), options, r.effectiveDevContainerSelection(options), + ) } func (r *runner) rawConfigFromFilesystemWithContext( ctx context.Context, options provider.CLIOptions, + selection devContainerSelection, ) (*config.DevContainerConfig, error) { - localWorkspaceFolder := r.localWorkspaceFolder - if subPath := r.workspaceConfig.Workspace.Source.GitSubPath; subPath != "" { - localWorkspaceFolder = filepath.Join(localWorkspaceFolder, subPath) - } + localWorkspaceFolder := r.workspaceFolder() opts := config.ParseOptions{Selector: config.SelectSingle(localWorkspaceFolder)} - if options.DevContainerID != "" { + if selection.id != "" { // An explicit id must not be shadowed by a root config, and a mismatch // must error rather than silently fall back. opts = config.ParseOptions{ - Selector: config.SelectByID(options.DevContainerID), + Selector: config.SelectByID(selection.id), ForceSelect: true, } } @@ -155,7 +299,7 @@ func (r *runner) rawConfigFromFilesystemWithContext( rawConfig, err := config.ParseDevContainerJSONWithOptions( ctx, localWorkspaceFolder, - r.workspaceConfig.Workspace.DevContainerPath, + selection.path, opts, ) // A missing devcontainer.json is not an error: fall back to auto-detection. diff --git a/pkg/devcontainer/config/result.go b/pkg/devcontainer/config/result.go index 9fdddf33b4..9ca102eff1 100644 --- a/pkg/devcontainer/config/result.go +++ b/pkg/devcontainer/config/result.go @@ -42,7 +42,7 @@ type DevContainerConfigWithPath struct { // Config is the devcontainer.json config Config *DevContainerConfig `json:"config,omitempty"` - // Path is the relative path to the devcontainer.json from the workspace folder + // Path is relative to the content root, before any Git subpath. Path string `json:"path,omitempty"` } diff --git a/pkg/devcontainer/config_test.go b/pkg/devcontainer/config_test.go index 66b6037ab7..e450fb825d 100644 --- a/pkg/devcontainer/config_test.go +++ b/pkg/devcontainer/config_test.go @@ -3,6 +3,7 @@ package devcontainer import ( "os" "path/filepath" + "strings" "testing" "github.com/devsy-org/devsy/pkg/devcontainer/config" @@ -10,7 +11,12 @@ import ( "github.com/stretchr/testify/suite" ) -const testWorkspaceFolder = "/workspace" +const ( + testWorkspaceFolder = "/workspace" + testDevContainerProfile = "max" + testEmbeddedImage = "embedded" + testNestedSubPath = "app" +) type SubstituteTestSuite struct { suite.Suite @@ -478,6 +484,31 @@ func seedAmbiguousProfiles(t *testing.T, folder string) { } } +func seedConfigAt(t *testing.T, folder, relativePath string) { + t.Helper() + file := filepath.Join(folder, filepath.FromSlash(relativePath)) + if err := os.MkdirAll(filepath.Dir(file), 0o750); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(file, []byte(`{"image":"seed"}`), 0o600); err != nil { + t.Fatal(err) + } +} + +func seedNamedProfiles(t *testing.T, folder string, ids ...string) { + t.Helper() + for _, id := range ids { + dir := filepath.Join(folder, ".devcontainer", id) + if err := os.MkdirAll(dir, 0o750); err != nil { + t.Fatal(err) + } + body := []byte(`{"image":"` + id + `"}`) + if err := os.WriteFile(filepath.Join(dir, "devcontainer.json"), body, 0o600); err != nil { + t.Fatal(err) + } + } +} + func TestGetRawConfig_SourceImageBypassesDiscovery(t *testing.T) { folder := t.TempDir() seedAmbiguousProfiles(t, folder) @@ -571,6 +602,263 @@ func TestGetRawConfig_CLISourceOverridesPersisted(t *testing.T) { } } +func TestGetRawConfig_PersistedIDSelectsProfile(t *testing.T) { + folder := t.TempDir() + seedNamedProfiles(t, folder, testDevContainerProfile, "max-nvidia", "max-vaapi") + r := newRunnerAt(folder) + r.workspaceConfig.Workspace.DevContainerID = testDevContainerProfile + + conf, err := r.getRawConfig(provider2.CLIOptions{}) + if err != nil { + t.Fatalf("getRawConfig: %v", err) + } + if conf.Image != testDevContainerProfile { + t.Errorf("Image = %q, want %s", conf.Image, testDevContainerProfile) + } +} + +func TestGetRawConfig_ExplicitSelectorOverridesPersistedSelector(t *testing.T) { + folder := t.TempDir() + seedNamedProfiles(t, folder, testDevContainerProfile, "max-nvidia") + r := newRunnerAt(folder) + r.workspaceConfig.Workspace.DevContainerSource = "image:persisted" + + conf, err := r.getRawConfig(provider2.CLIOptions{DevContainerID: "max-nvidia"}) + if err != nil { + t.Fatalf("getRawConfig: %v", err) + } + if conf.Image != "max-nvidia" { + t.Errorf("Image = %q, want max-nvidia", conf.Image) + } +} + +func TestGetRawConfig_ExplicitPathOverridesPersistedID(t *testing.T) { + folder := t.TempDir() + seedNamedProfiles(t, folder, testDevContainerProfile, "max-vaapi") + r := newRunnerAt(folder) + r.workspaceConfig.Workspace.DevContainerID = testDevContainerProfile + + conf, err := r.getRawConfig(provider2.CLIOptions{ + DevContainerPath: ".devcontainer/max-vaapi/devcontainer.json", + }) + if err != nil { + t.Fatalf("getRawConfig: %v", err) + } + if conf.Image != "max-vaapi" { + t.Errorf("Image = %q, want max-vaapi", conf.Image) + } +} + +func TestGetRawConfig_LastConfigPathCompatibilityFallback(t *testing.T) { + folder := t.TempDir() + seedNamedProfiles(t, folder, "max", "max-nvidia") + r := newRunnerAt(folder) + r.workspaceConfig.LastDevContainerConfig = &config.DevContainerConfigWithPath{ + Path: ".devcontainer/" + testDevContainerProfile + "/devcontainer.json", + } + + conf, err := r.getRawConfig(provider2.CLIOptions{}) + if err != nil { + t.Fatalf("getRawConfig: %v", err) + } + if conf.Image != testDevContainerProfile { + t.Errorf("Image = %q, want %s", conf.Image, testDevContainerProfile) + } +} + +func TestEffectiveDevContainerSelection_LastPathStripsGitSubPath(t *testing.T) { + folder := t.TempDir() + seedConfigAt(t, folder, "devsy/jupyter-notebook-hello-world/.devcontainer/devcontainer.json") + r := newRunnerAt(folder) + r.workspaceConfig.Workspace.Source.GitSubPath = "devsy/jupyter-notebook-hello-world" + r.workspaceConfig.LastDevContainerConfig = &config.DevContainerConfigWithPath{ + Path: "devsy/jupyter-notebook-hello-world/.devcontainer/devcontainer.json", + } + + selection := r.effectiveDevContainerSelection(provider2.CLIOptions{}) + if selection.path != ".devcontainer/devcontainer.json" { + t.Fatalf("selection path = %q, want .devcontainer/devcontainer.json", selection.path) + } +} + +// Last-path conversion must strip only the applied subpath. +func TestEffectiveDevContainerSelection_LastPathRepeatedSubPathSegment(t *testing.T) { + folder := t.TempDir() + seedConfigAt(t, folder, testNestedSubPath+"/app/.devcontainer/devcontainer.json") + r := newRunnerAt(folder) + r.workspaceConfig.Workspace.Source.GitSubPath = testNestedSubPath + r.workspaceConfig.LastDevContainerConfig = &config.DevContainerConfigWithPath{ + Path: testNestedSubPath + "/app/.devcontainer/devcontainer.json", + } + + selection := r.effectiveDevContainerSelection(provider2.CLIOptions{}) + if selection.path != "app/.devcontainer/devcontainer.json" { + t.Fatalf( + "selection path = %q, want app/.devcontainer/devcontainer.json", + selection.path, + ) + } +} + +// Stored git subpaths may have a leading slash, while last paths do not. +func TestEffectiveDevContainerSelection_LastPathLeadingSlashSubPath(t *testing.T) { + folder := t.TempDir() + seedConfigAt(t, folder, "devsy/jupyter-notebook-hello-world/.devcontainer/devcontainer.json") + r := newRunnerAt(folder) + r.workspaceConfig.Workspace.Source.GitSubPath = "/devsy/jupyter-notebook-hello-world" + r.workspaceConfig.LastDevContainerConfig = &config.DevContainerConfigWithPath{ + Path: "devsy/jupyter-notebook-hello-world/.devcontainer/devcontainer.json", + } + + selection := r.effectiveDevContainerSelection(provider2.CLIOptions{}) + if selection.path != ".devcontainer/devcontainer.json" { + t.Fatalf("selection path = %q, want .devcontainer/devcontainer.json", selection.path) + } +} + +// An embedded config (from the provider protocol) outranks a persisted +// path or id to preserve pre-persistence behavior. +func TestGetRawConfig_EmbeddedConfigWinsOverPersistedPath(t *testing.T) { + folder := t.TempDir() + seedNamedProfiles(t, folder, testDevContainerProfile) + r := newRunnerAt(folder) + r.workspaceConfig.Workspace.DevContainerConfig = &config.DevContainerConfig{ + ImageContainer: config.ImageContainer{Image: testEmbeddedImage}, + } + r.workspaceConfig.Workspace.DevContainerPath = + ".devcontainer/" + testDevContainerProfile + "/devcontainer.json" + + conf, err := r.getRawConfig(provider2.CLIOptions{}) + if err != nil { + t.Fatalf("getRawConfig: %v", err) + } + if conf.Image != testEmbeddedImage { + t.Errorf( + "Image = %q, want embedded (persisted path must not override the embedded config)", + conf.Image, + ) + } +} + +func TestGetRawConfig_EmbeddedConfigWinsOverPersistedID(t *testing.T) { + folder := t.TempDir() + seedNamedProfiles(t, folder, testDevContainerProfile) + r := newRunnerAt(folder) + r.workspaceConfig.Workspace.DevContainerConfig = &config.DevContainerConfig{ + ImageContainer: config.ImageContainer{Image: testEmbeddedImage}, + } + r.workspaceConfig.Workspace.DevContainerID = testDevContainerProfile + + conf, err := r.getRawConfig(provider2.CLIOptions{}) + if err != nil { + t.Fatalf("getRawConfig: %v", err) + } + if conf.Image != testEmbeddedImage { + t.Errorf( + "Image = %q, want embedded (persisted id must not override the embedded config)", + conf.Image, + ) + } +} + +func TestDevContainerConfigExists_TreatsOnlyNotExistAsAbsent(t *testing.T) { + folder := t.TempDir() + // A regular file where a directory should be makes stat fail with + // ENOTDIR. That must count as present so the parse step surfaces the + // real filesystem error instead of silently skipping the recorded path. + if err := os.WriteFile(filepath.Join(folder, "cfg"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + if !devContainerConfigExists(folder, "cfg/devcontainer.json") { + t.Error("ENOTDIR stat error treated as absent; want present") + } + if devContainerConfigExists(folder, "missing/devcontainer.json") { + t.Error("missing file treated as present; want absent") + } + if !devContainerConfigExists(folder, "cfg") { + t.Error("existing file treated as absent; want present") + } +} + +func TestGetRawConfig_LastPathFallbackSkipsMissingFile(t *testing.T) { + r := newRunnerAt(t.TempDir()) + r.workspaceConfig.Workspace.Source.GitSubPath = "devsy/jupyter-notebook-hello-world" + r.workspaceConfig.LastDevContainerConfig = &config.DevContainerConfigWithPath{ + Path: "devsy/jupyter-notebook-hello-world/.devcontainer.json", + } + + conf, err := r.getRawConfig(provider2.CLIOptions{}) + if err != nil { + t.Fatalf("getRawConfig: %v", err) + } + const defaultImage = "mcr.microsoft.com/devcontainers/base:ubuntu" + if conf.Image != defaultImage { + t.Errorf("Image = %q, want auto-detected default %s", conf.Image, defaultImage) + } +} + +func TestGetRawConfig_EmbeddedConfigWinsOverLastPathFallback(t *testing.T) { + folder := t.TempDir() + seedNamedProfiles(t, folder, testDevContainerProfile) + r := newRunnerAt(folder) + r.workspaceConfig.Workspace.DevContainerConfig = &config.DevContainerConfig{ + ImageContainer: config.ImageContainer{Image: testEmbeddedImage}, + } + r.workspaceConfig.LastDevContainerConfig = &config.DevContainerConfigWithPath{ + Path: ".devcontainer/" + testDevContainerProfile + "/devcontainer.json", + } + + conf, err := r.getRawConfig(provider2.CLIOptions{}) + if err != nil { + t.Fatalf("getRawConfig: %v", err) + } + if conf.Image != testEmbeddedImage { + t.Errorf( + "Image = %q, want embedded (last path must not override the embedded config)", + conf.Image, + ) + } +} + +// Persisted paths are workspace-folder-relative and must not be stripped. +func TestGetRawConfig_PersistedPathRepeatedSubPathSegment(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, testNestedSubPath, testNestedSubPath, ".devcontainer") + if err := os.MkdirAll(dir, 0o750); err != nil { + t.Fatal(err) + } + configFile := filepath.Join(dir, "devcontainer.json") + if err := os.WriteFile(configFile, []byte(`{"image":"nested"}`), 0o600); err != nil { + t.Fatal(err) + } + r := newRunnerAt(root) + r.workspaceConfig.Workspace.Source.GitSubPath = testNestedSubPath + r.workspaceConfig.Workspace.DevContainerPath = "app/.devcontainer/devcontainer.json" + + conf, err := r.getRawConfig(provider2.CLIOptions{}) + if err != nil { + t.Fatalf("getRawConfig: %v", err) + } + if conf.Image != "nested" { + t.Errorf("Image = %q, want nested", conf.Image) + } +} + +func TestGetRawConfig_InvalidPersistedID(t *testing.T) { + folder := t.TempDir() + seedNamedProfiles(t, folder, testDevContainerProfile, "max-nvidia") + r := newRunnerAt(folder) + r.workspaceConfig.Workspace.DevContainerID = "missing" + + _, err := r.getRawConfig(provider2.CLIOptions{}) + if err == nil { + t.Fatal("expected invalid persisted ID to fail") + } + if !strings.Contains(err.Error(), `devcontainer with ID "missing" not found`) { + t.Fatalf("error = %q, want missing ID error", err) + } +} + func TestGetRawConfig_SourceNoneWithImageBypassesDiscovery(t *testing.T) { folder := t.TempDir() seedAmbiguousProfiles(t, folder) diff --git a/pkg/provider/workspace.go b/pkg/provider/workspace.go index da3ed2d1f5..08d2e78297 100644 --- a/pkg/provider/workspace.go +++ b/pkg/provider/workspace.go @@ -47,9 +47,14 @@ type Workspace struct { // DevContainerImage is the container image to use, overriding whatever is in the devcontainer.json DevContainerImage string `json:"devContainerImage,omitempty"` - // DevContainerPath is the relative path where the devcontainer.json is located. + // DevContainerPath is the path to the devcontainer.json relative to the + // workspace folder: the content root including the git subpath, if any. DevContainerPath string `json:"devContainerPath,omitempty"` + // DevContainerID is the selected named devcontainer profile. It is persisted + // so lifecycle operations reuse the same profile instead of rediscovering it. + DevContainerID string `json:"devContainerID,omitempty"` + // DevContainerSource is the devcontainer source override (e.g. "image:" // or "none") that ignores the project's devcontainer.json. It is persisted so // restarts reuse the same override instead of falling back to discovery. diff --git a/pkg/workspace/workspace.go b/pkg/workspace/workspace.go index 21a5abdb67..687f3541fc 100644 --- a/pkg/workspace/workspace.go +++ b/pkg/workspace/workspace.go @@ -54,6 +54,7 @@ type ResolveParams struct { ReconfigureProvider bool DevContainerImage string DevContainerPath string + DevContainerID string DevContainerSource string SSHConfigPath string SSHConfigIncludePath string @@ -142,20 +143,37 @@ func applyDevContainerOverrides(workspace *providerpkg.Workspace, params Resolve } func applyDevContainerFields(workspace *providerpkg.Workspace, params ResolveParams) bool { - changed := false + changed := applyDevContainerSelection(workspace, params) if params.DevContainerImage != "" && workspace.DevContainerImage != params.DevContainerImage { workspace.DevContainerImage = params.DevContainerImage changed = true } - if params.DevContainerPath != "" && workspace.DevContainerPath != params.DevContainerPath { - workspace.DevContainerPath = params.DevContainerPath - changed = true - } - if params.DevContainerSource != "" && - workspace.DevContainerSource != params.DevContainerSource { - workspace.DevContainerSource = params.DevContainerSource - changed = true + return changed +} + +func applyDevContainerSelection(workspace *providerpkg.Workspace, params ResolveParams) bool { + switch { + case params.DevContainerSource != "": + return setDevContainerSelection(workspace, params.DevContainerSource, "", "") + case params.DevContainerPath != "": + return setDevContainerSelection(workspace, "", params.DevContainerPath, "") + case params.DevContainerID != "": + return setDevContainerSelection(workspace, "", "", params.DevContainerID) + default: + return false } +} + +func setDevContainerSelection( + workspace *providerpkg.Workspace, + source, path, id string, +) bool { + changed := workspace.DevContainerSource != source || + workspace.DevContainerPath != path || + workspace.DevContainerID != id + workspace.DevContainerSource = source + workspace.DevContainerPath = path + workspace.DevContainerID = id return changed } diff --git a/pkg/workspace/workspace_test.go b/pkg/workspace/workspace_test.go index b8042055e7..5c4e45b2af 100644 --- a/pkg/workspace/workspace_test.go +++ b/pkg/workspace/workspace_test.go @@ -202,6 +202,7 @@ func TestApplyDevContainerOverrides_NoOp(t *testing.T) { require.NoError(t, applyDevContainerOverrides(ws, ResolveParams{})) assert.Empty(t, ws.DevContainerImage) assert.Empty(t, ws.DevContainerPath) + assert.Empty(t, ws.DevContainerID) } func TestApplyDevContainerOverrides_SetsImageAndPath(t *testing.T) { @@ -239,3 +240,41 @@ func TestApplyDevContainerOverrides_PersistsSource(t *testing.T) { require.NoError(t, err) assert.Equal(t, source, loaded.DevContainerSource) } + +func TestApplyDevContainerOverrides_PersistsIDAndClearsOtherSelectors(t *testing.T) { + setupTestPathManager(t) + + ws := &providerpkg.Workspace{ + ID: "ws-id", + Context: testDefaultContext, + DevContainerPath: ".devcontainer/old/devcontainer.json", + DevContainerSource: "image:old", + } + require.NoError(t, applyDevContainerOverrides(ws, ResolveParams{DevContainerID: "max"})) + + assert.Equal(t, "max", ws.DevContainerID) + assert.Empty(t, ws.DevContainerPath) + assert.Empty(t, ws.DevContainerSource) + + loaded, err := providerpkg.LoadWorkspaceConfig(testDefaultContext, ws.ID) + require.NoError(t, err) + assert.Equal(t, "max", loaded.DevContainerID) + assert.Empty(t, loaded.DevContainerPath) + assert.Empty(t, loaded.DevContainerSource) +} + +func TestApplyDevContainerOverrides_SelectorReplacement(t *testing.T) { + ws := &providerpkg.Workspace{DevContainerID: "max"} + + changed := applyDevContainerFields(ws, ResolveParams{DevContainerPath: testDevContainerPath}) + assert.True(t, changed) + assert.Empty(t, ws.DevContainerID) + assert.Equal(t, testDevContainerPath, ws.DevContainerPath) + assert.Empty(t, ws.DevContainerSource) + + changed = applyDevContainerFields(ws, ResolveParams{DevContainerSource: "image:python"}) + assert.True(t, changed) + assert.Empty(t, ws.DevContainerID) + assert.Empty(t, ws.DevContainerPath) + assert.Equal(t, "image:python", ws.DevContainerSource) +}