diff --git a/cmd/workspace/up/up_devcontainer_source_test.go b/cmd/workspace/up/up_devcontainer_source_test.go index 99f560e50..41abb6791 100644 --- a/cmd/workspace/up/up_devcontainer_source_test.go +++ b/cmd/workspace/up/up_devcontainer_source_test.go @@ -28,6 +28,16 @@ func TestResolveDevContainerSource_Path(t *testing.T) { assert.Empty(t, cmd.DevContainerSource) } +func TestResolveDevContainerSource_ExternalPathPassThrough(t *testing.T) { + cmd := &UpCmd{GlobalFlags: &flags.GlobalFlags{}} + cmd.DevContainerSource = "/abs/external/devcontainer.json" + + require.NoError(t, cmd.resolveDevContainerSource()) + assert.Equal(t, "/abs/external/devcontainer.json", cmd.DevContainerSource) + assert.Empty(t, cmd.DevContainerPath) + assert.Empty(t, cmd.DevContainerID) +} + func TestResolveDevContainerSource_NoneAndImagePassThrough(t *testing.T) { for _, spec := range []string{srcNone, "image:python"} { cmd := &UpCmd{GlobalFlags: &flags.GlobalFlags{}} diff --git a/cmd/workspace/up/up_validate.go b/cmd/workspace/up/up_validate.go index a576b1140..88795ab34 100644 --- a/cmd/workspace/up/up_validate.go +++ b/cmd/workspace/up/up_validate.go @@ -61,8 +61,10 @@ func (cmd *UpCmd) resolveDevContainerSource() error { cmd.DevContainerID = spec.ID cmd.DevContainerSource = "" case devcontainer.SourcePath: - cmd.DevContainerPath = spec.Path - cmd.DevContainerSource = "" + if !filepath.IsAbs(spec.Path) { + cmd.DevContainerPath = spec.Path + cmd.DevContainerSource = "" + } case devcontainer.SourceNone, devcontainer.SourceImage: } return nil diff --git a/pkg/devcontainer/config.go b/pkg/devcontainer/config.go index 9165217fa..d6e32de50 100644 --- a/pkg/devcontainer/config.go +++ b/pkg/devcontainer/config.go @@ -9,14 +9,27 @@ import ( "path" "path/filepath" "slices" + "strings" pkgconfig "github.com/devsy-org/devsy/pkg/config" + copypkg "github.com/devsy-org/devsy/pkg/copy" "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/devcontainer/crane" "github.com/devsy-org/devsy/pkg/flags/names" "github.com/devsy-org/devsy/pkg/language" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/provider" + "github.com/devsy-org/devsy/pkg/random" +) + +var ( + importedProfileParent = filepath.ToSlash(devcontainerProfileParent) + importedProfileMarker = "." + pkgconfig.BinaryName + "-imported" +) + +const ( + devcontainerProfileParent = ".devcontainer" + importedProfileName = pkgconfig.BinaryName ) // getRawConfig resolves the raw devcontainer config for the workspace, trying @@ -180,11 +193,149 @@ func (r *runner) rawConfigFromSource( defaultConfig = language.DefaultConfig(r.localWorkspaceFolder) } return r.saveSynthesizedConfig(defaultConfig) + case SourcePath: + return r.importExternalDevContainer(spec.Path) + case SourceID: + return nil, fmt.Errorf("devcontainer id source must be resolved before build") default: return nil, fmt.Errorf("unsupported devcontainer source kind %q", spec.Kind) } } +func (r *runner) importExternalDevContainer(srcPath string) (*config.DevContainerConfig, error) { + absPath, err := filepath.Abs(srcPath) + if err != nil { + return nil, fmt.Errorf("resolve devcontainer path %s: %w", srcPath, err) + } + srcPath = absPath + if _, err := os.Stat(srcPath); err != nil { + return nil, fmt.Errorf("devcontainer path %s does not exist: %w", srcPath, err) + } + + relDir := r.importedProfileRelDir() + destDir := filepath.Join(r.localWorkspaceFolder, filepath.FromSlash(relDir)) + if err := copyExternalDevContainer(srcPath, destDir); err != nil { + return nil, err + } + if err := os.WriteFile(filepath.Join(destDir, importedProfileMarker), nil, 0o600); err != nil { + return nil, fmt.Errorf("mark imported devcontainer: %w", err) + } + if err := r.excludeFromGit(relDir); err != nil { + log.Debugf("could not add imported devcontainer to git exclude: %v", err) + } + + origin := filepath.Join(destDir, filepath.Base(srcPath)) + rawConfig, err := config.ParseDevContainerJSONFile(context.Background(), origin) + if err != nil { + return nil, fmt.Errorf("parse imported devcontainer.json: %w", err) + } + return rawConfig, nil +} + +func (r *runner) importedProfileRelDir() string { + base := path.Join(importedProfileParent, importedProfileName) + plain := filepath.Join(r.localWorkspaceFolder, filepath.FromSlash(base)) + if isImportedProfileDir(plain) || !dirExists(plain) { + return base + } + return base + "_" + random.String(6) +} + +func copyExternalDevContainer(srcPath, destDir string) error { + if err := os.RemoveAll(destDir); err != nil { + return fmt.Errorf("clear imported devcontainer dir: %w", err) + } + if isSelfContainedDevContainer(srcPath) { + if err := copypkg.Directory(filepath.Dir(srcPath), destDir); err != nil { + return fmt.Errorf("copy devcontainer folder: %w", err) + } + return nil + } + if err := copypkg.CreateIfNotExists(destDir, 0o755); err != nil { + return fmt.Errorf("create imported devcontainer dir: %w", err) + } + dest := filepath.Join(destDir, filepath.Base(srcPath)) + if err := copypkg.File(srcPath, dest, 0o644); err != nil { + return fmt.Errorf("copy devcontainer file: %w", err) + } + return nil +} + +func dirExists(path string) bool { + info, err := os.Stat(path) + return err == nil && info.IsDir() +} + +// isSelfContainedDevContainer reports whether srcPath lives in a dedicated +// devcontainer directory whose sibling assets (Dockerfile, features, compose) +// belong with the config and must be copied alongside it. To avoid pulling in +// arbitrary trees, this is limited to the spec's own layout: the config's +// parent directory is ".devcontainer", or a profile directory nested directly +// under ".devcontainer" (".devcontainer//devcontainer.json"). A config +// that sits anywhere else (e.g. a project root) is treated as a bare file. +func isSelfContainedDevContainer(srcPath string) bool { + dir := filepath.Dir(srcPath) + if filepath.Base(dir) == devcontainerProfileParent { + return true + } + return filepath.Base(filepath.Dir(dir)) == devcontainerProfileParent +} + +func isImportedProfileDir(dir string) bool { + _, err := os.Stat(filepath.Join(dir, importedProfileMarker)) + return err == nil +} + +func (r *runner) excludeFromGit(relPath string) error { + excludePath := filepath.Join(r.localWorkspaceFolder, ".git", "info", "exclude") + if _, err := os.Stat(filepath.Dir(excludePath)); err != nil { + return nil // nothing to do + } + // #nosec G304 -- path is under the workspace .git dir + existing, err := os.ReadFile(excludePath) + if err != nil && !os.IsNotExist(err) { + return err + } + entry := "/" + filepath.ToSlash(relPath) + for line := range strings.SplitSeq(string(existing), "\n") { + if strings.TrimSpace(line) == entry { + return nil // already excluded + } + } + // #nosec G304 -- path is under the workspace .git dir + f, err := os.OpenFile(excludePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) + if err != nil { + return err + } + defer func() { _ = f.Close() }() + _, err = fmt.Fprintf(f, "%s\n", entry) + return err +} + +func CleanupImportedDevContainers(workspaceFolder string) error { + parent := filepath.Join(workspaceFolder, filepath.FromSlash(importedProfileParent)) + entries, err := os.ReadDir(parent) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + for _, e := range entries { + if !e.IsDir() { + continue + } + dir := filepath.Join(parent, e.Name()) + if !isImportedProfileDir(dir) { + continue + } + if err := os.RemoveAll(dir); err != nil { + return fmt.Errorf("remove imported devcontainer %s: %w", dir, err) + } + } + return nil +} + func (r *runner) saveSynthesizedConfig( c *config.DevContainerConfig, ) (*config.DevContainerConfig, error) { diff --git a/pkg/devcontainer/delete.go b/pkg/devcontainer/delete.go index 137759e9c..2b1b61adf 100644 --- a/pkg/devcontainer/delete.go +++ b/pkg/devcontainer/delete.go @@ -15,6 +15,7 @@ func (r *runner) Delete(ctx context.Context, options DeleteOptions) error { return fmt.Errorf("find dev container: %w", err) } defer r.cleanupDeliveryVolume(ctx) + defer r.cleanupImportedDevContainer() if containerDetails == nil { return nil } @@ -42,11 +43,20 @@ func (r *runner) Delete(ctx context.Context, options DeleteOptions) error { return nil } -// 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 { - log.Debugf("best-effort delivery volume cleanup: %v", err) + log.Debugf("delivery volume cleanup: %v", err) + } +} + +func (r *runner) cleanupImportedDevContainer() { + if r.workspaceConfig == nil || + r.workspaceConfig.Workspace == nil || + r.workspaceConfig.Workspace.Source.LocalFolder == "" { + return + } + if err := CleanupImportedDevContainers(r.localWorkspaceFolder); err != nil { + log.Debugf("imported devcontainer cleanup: %v", err) } } diff --git a/pkg/devcontainer/delete_test.go b/pkg/devcontainer/delete_test.go index 8f881bf45..21229eae6 100644 --- a/pkg/devcontainer/delete_test.go +++ b/pkg/devcontainer/delete_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io" + "path/filepath" "testing" "github.com/devsy-org/devsy/pkg/devcontainer/config" @@ -175,3 +176,36 @@ func TestCleanupDeliveryVolume_DoesNotPanic(t *testing.T) { r.cleanupDeliveryVolume(context.Background()) } + +func TestDelete_RemovesImportedDevContainer(t *testing.T) { + ws := t.TempDir() + external := filepath.Join(t.TempDir(), "devcontainer.json") + writeFile(t, external, `{"image":"alpine"}`) + + r := newTestRunner(&mockDriver{findResult: nil}) + r.localWorkspaceFolder = ws + r.workspaceConfig.Workspace = &provider.Workspace{ + Source: provider.WorkspaceSource{LocalFolder: ws}, + } + + if _, err := r.importExternalDevContainer(external); err != nil { + t.Fatalf("import failed: %v", err) + } + if !dirExists(importedProfilePath(ws)) { + t.Fatal("expected imported profile before delete") + } + + if err := r.Delete(context.Background(), DeleteOptions{}); err != nil { + t.Fatalf("Delete failed: %v", err) + } + if dirExists(importedProfilePath(ws)) { + t.Error("imported profile should be removed after Delete") + } +} + +func TestDelete_NonLocalSource_KeepsNothingToClean(t *testing.T) { + r := newTestRunner(&mockDriver{findResult: nil}) + if err := r.Delete(context.Background(), DeleteOptions{}); err != nil { + t.Fatalf("Delete failed: %v", err) + } +} diff --git a/pkg/devcontainer/import_external_test.go b/pkg/devcontainer/import_external_test.go new file mode 100644 index 000000000..26eddb069 --- /dev/null +++ b/pkg/devcontainer/import_external_test.go @@ -0,0 +1,191 @@ +package devcontainer + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeFile(t *testing.T, path, body string) { + t.Helper() + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o750)) + require.NoError(t, os.WriteFile(path, []byte(body), 0o600)) +} + +func importedProfilePath(ws string) string { + return filepath.Join(ws, filepath.FromSlash(importedProfileParent), importedProfileName) +} + +func TestImportExternalDevContainer_SelfContainedFolder(t *testing.T) { + // A dedicated .devcontainer/ folder with a sibling Dockerfile. + external := filepath.Join(t.TempDir(), ".devcontainer") + writeFile(t, filepath.Join(external, "devcontainer.json"), + `{"name":"ext","build":{"dockerfile":"Dockerfile"}}`) + writeFile(t, filepath.Join(external, "Dockerfile"), "FROM alpine\n") + + ws := t.TempDir() + r := &runner{localWorkspaceFolder: ws} + + cfg, err := r.importExternalDevContainer(filepath.Join(external, "devcontainer.json")) + require.NoError(t, err) + + imported := importedProfilePath(ws) + assert.FileExists(t, filepath.Join(imported, "devcontainer.json")) + assert.FileExists(t, filepath.Join(imported, "Dockerfile")) + + assert.Equal(t, filepath.Join(imported, "devcontainer.json"), cfg.Origin) + assert.Equal(t, "Dockerfile", cfg.GetDockerfile()) + dockerfile := filepath.Join(filepath.Dir(cfg.Origin), cfg.GetDockerfile()) + assert.FileExists(t, dockerfile) +} + +func TestImportExternalDevContainer_ProjectRootCopiesOnlyConfig(t *testing.T) { + // A config that lives at a project root (parent is NOT .devcontainer) must + // copy only the config file — never the surrounding tree (secrets, .git, + // node_modules, ...). + external := t.TempDir() + writeFile(t, filepath.Join(external, "devcontainer.json"), `{"image":"alpine"}`) + writeFile(t, filepath.Join(external, "secret.env"), "TOKEN=shh") + writeFile(t, filepath.Join(external, ".git", "config"), "[core]") + + ws := t.TempDir() + r := &runner{localWorkspaceFolder: ws} + + _, err := r.importExternalDevContainer(filepath.Join(external, "devcontainer.json")) + require.NoError(t, err) + + imported := importedProfilePath(ws) + assert.FileExists(t, filepath.Join(imported, "devcontainer.json")) + assert.NoFileExists(t, filepath.Join(imported, "secret.env"), + "must not copy sibling files from a non-.devcontainer parent") + assert.NoDirExists(t, filepath.Join(imported, ".git"), + "must not copy sibling dirs from a non-.devcontainer parent") +} + +func TestImportExternalDevContainer_ProfileFolderIsSelfContained(t *testing.T) { + // A named profile (.devcontainer//devcontainer.json) is self-contained + // and its siblings travel with it. + external := filepath.Join(t.TempDir(), ".devcontainer", "backend") + writeFile(t, filepath.Join(external, "devcontainer.json"), + `{"build":{"dockerfile":"Dockerfile"}}`) + writeFile(t, filepath.Join(external, "Dockerfile"), "FROM alpine\n") + + ws := t.TempDir() + r := &runner{localWorkspaceFolder: ws} + + _, err := r.importExternalDevContainer(filepath.Join(external, "devcontainer.json")) + require.NoError(t, err) + assert.FileExists(t, filepath.Join(importedProfilePath(ws), "Dockerfile")) +} + +func TestImportExternalDevContainer_BareFile(t *testing.T) { + external := t.TempDir() + writeFile(t, filepath.Join(external, "devcontainer.json"), `{"image":"alpine"}`) + + ws := t.TempDir() + r := &runner{localWorkspaceFolder: ws} + + cfg, err := r.importExternalDevContainer(filepath.Join(external, "devcontainer.json")) + require.NoError(t, err) + assert.FileExists(t, filepath.Join(importedProfilePath(ws), "devcontainer.json")) + assert.Equal(t, "alpine", cfg.Image) +} + +func TestImportExternalDevContainer_WritesMarker(t *testing.T) { + external := t.TempDir() + writeFile(t, filepath.Join(external, "devcontainer.json"), `{"image":"alpine"}`) + + ws := t.TempDir() + r := &runner{localWorkspaceFolder: ws} + + _, err := r.importExternalDevContainer(filepath.Join(external, "devcontainer.json")) + require.NoError(t, err) + assert.True(t, isImportedProfileDir(importedProfilePath(ws)), + "imported profile must carry the marker file") +} + +func TestImportExternalDevContainer_CollisionGetsSuffix(t *testing.T) { + external := t.TempDir() + writeFile(t, filepath.Join(external, "devcontainer.json"), `{"image":"alpine"}`) + + ws := t.TempDir() + writeFile(t, filepath.Join(importedProfilePath(ws), "devcontainer.json"), + `{"image":"project-owned"}`) + r := &runner{localWorkspaceFolder: ws} + + cfg, err := r.importExternalDevContainer(filepath.Join(external, "devcontainer.json")) + require.NoError(t, err) + + assert.Equal(t, "alpine", cfg.Image) + base := filepath.Base(filepath.Dir(cfg.Origin)) + assert.True(t, strings.HasPrefix(base, importedProfileName+"_"), + "expected a suffixed dir, got %q", base) +} + +func TestImportExternalDevContainer_ReusesOwnProfile(t *testing.T) { + external := t.TempDir() + writeFile(t, filepath.Join(external, "devcontainer.json"), `{"image":"alpine"}`) + + ws := t.TempDir() + r := &runner{localWorkspaceFolder: ws} + + first, err := r.importExternalDevContainer(filepath.Join(external, "devcontainer.json")) + require.NoError(t, err) + second, err := r.importExternalDevContainer(filepath.Join(external, "devcontainer.json")) + require.NoError(t, err) + assert.Equal(t, first.Origin, second.Origin) +} + +func TestImportExternalDevContainer_AddsGitExclude(t *testing.T) { + external := t.TempDir() + writeFile(t, filepath.Join(external, "devcontainer.json"), `{"image":"alpine"}`) + + ws := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(ws, ".git", "info"), 0o750)) + r := &runner{localWorkspaceFolder: ws} + + _, err := r.importExternalDevContainer(filepath.Join(external, "devcontainer.json")) + require.NoError(t, err) + + // #nosec G304 -- test path + exclude, err := os.ReadFile(filepath.Join(ws, ".git", "info", "exclude")) + require.NoError(t, err) + assert.Contains(t, string(exclude), importedProfileParent+"/"+importedProfileName) +} + +func TestImportExternalDevContainer_NonGitRepoSkipsExclude(t *testing.T) { + external := t.TempDir() + writeFile(t, filepath.Join(external, "devcontainer.json"), `{"image":"alpine"}`) + + ws := t.TempDir() // no .git + r := &runner{localWorkspaceFolder: ws} + + _, err := r.importExternalDevContainer(filepath.Join(external, "devcontainer.json")) + require.NoError(t, err, "import must succeed even outside a git repo") +} + +func TestCleanupImportedDevContainers(t *testing.T) { + external := t.TempDir() + writeFile(t, filepath.Join(external, "devcontainer.json"), `{"image":"alpine"}`) + + ws := t.TempDir() + writeFile(t, filepath.Join(ws, ".devcontainer", "app", "devcontainer.json"), `{}`) + + r := &runner{localWorkspaceFolder: ws} + _, err := r.importExternalDevContainer(filepath.Join(external, "devcontainer.json")) + require.NoError(t, err) + require.DirExists(t, importedProfilePath(ws)) + + require.NoError(t, CleanupImportedDevContainers(ws)) + assert.NoDirExists(t, importedProfilePath(ws), "imported profile must be removed") + assert.DirExists(t, filepath.Join(ws, ".devcontainer", "app"), + "project-owned profile must be preserved") +} + +func TestCleanupImportedDevContainers_NoDevcontainerDir(t *testing.T) { + assert.NoError(t, CleanupImportedDevContainers(t.TempDir())) +}