diff --git a/e2e/tests/extends/extends.go b/e2e/tests/extends/extends.go index c3145dd13..b3ec142ef 100644 --- a/e2e/tests/extends/extends.go +++ b/e2e/tests/extends/extends.go @@ -129,6 +129,34 @@ var _ = ginkgo.Describe("extends property", ginkgo.Label("extends"), func() { framework.ExpectError(err) }, ginkgo.SpecTimeout(framework.TimeoutShort())) + ginkgo.It("resolves array extends with multiple parents", func(ctx context.Context) { + f := framework.NewDefaultFramework(initialDir + "/bin") + tempDir, err := framework.CopyToTempDirWithoutChdir( + "tests/extends/testdata/array-extends", + ) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(func() { _ = os.RemoveAll(tempDir) }) + + stdout, _, err := readConfiguration(ctx, f, tempDir) + framework.ExpectNoError(err) + + config := parseConfigFromOutput(stdout) + gomega.Expect(config).To(gomega.HaveKeyWithValue("name", "Array Extends Child")) + gomega.Expect(config).To( + gomega.HaveKeyWithValue("image", "mcr.microsoft.com/devcontainers/base:ubuntu"), + ) + gomega.Expect(config).To(gomega.HaveKeyWithValue("remoteUser", "vscode")) + + containerEnv, ok := config["containerEnv"].(map[string]any) + gomega.Expect(ok).To(gomega.BeTrue(), "containerEnv should be an object") + gomega.Expect(containerEnv).To(gomega.HaveKeyWithValue("FROM_BASE", "base-value")) + gomega.Expect(containerEnv).To( + gomega.HaveKeyWithValue("FROM_MIDDLEWARE", "middleware-value"), + ) + gomega.Expect(containerEnv).To(gomega.HaveKeyWithValue("FROM_CHILD", "child-value")) + gomega.Expect(containerEnv).To(gomega.HaveKeyWithValue("SHARED", "from-middleware")) + }, ginkgo.SpecTimeout(framework.TimeoutShort())) + ginkgo.It("returns error when extends references missing file", func(ctx context.Context) { f := framework.NewDefaultFramework(initialDir + "/bin") tempDir, err := framework.CopyToTempDirWithoutChdir( diff --git a/e2e/tests/extends/testdata/array-extends/.devcontainer/base.json b/e2e/tests/extends/testdata/array-extends/.devcontainer/base.json new file mode 100644 index 000000000..c131d81ba --- /dev/null +++ b/e2e/tests/extends/testdata/array-extends/.devcontainer/base.json @@ -0,0 +1,7 @@ +{ + "image": "mcr.microsoft.com/devcontainers/base:ubuntu", + "containerEnv": { + "FROM_BASE": "base-value", + "SHARED": "from-base" + } +} diff --git a/e2e/tests/extends/testdata/array-extends/.devcontainer/devcontainer.json b/e2e/tests/extends/testdata/array-extends/.devcontainer/devcontainer.json new file mode 100644 index 000000000..4dc738b00 --- /dev/null +++ b/e2e/tests/extends/testdata/array-extends/.devcontainer/devcontainer.json @@ -0,0 +1,7 @@ +{ + "extends": ["base.json", "middleware.json"], + "name": "Array Extends Child", + "containerEnv": { + "FROM_CHILD": "child-value" + } +} diff --git a/e2e/tests/extends/testdata/array-extends/.devcontainer/middleware.json b/e2e/tests/extends/testdata/array-extends/.devcontainer/middleware.json new file mode 100644 index 000000000..a93f08bd5 --- /dev/null +++ b/e2e/tests/extends/testdata/array-extends/.devcontainer/middleware.json @@ -0,0 +1,7 @@ +{ + "remoteUser": "vscode", + "containerEnv": { + "FROM_MIDDLEWARE": "middleware-value", + "SHARED": "from-middleware" + } +} diff --git a/pkg/devcontainer/config/config.go b/pkg/devcontainer/config/config.go index 66f8fac36..cd3d2b661 100644 --- a/pkg/devcontainer/config/config.go +++ b/pkg/devcontainer/config/config.go @@ -45,8 +45,8 @@ func CloneDevContainerConfig(config *DevContainerConfig) *DevContainerConfig { } type DevContainerConfigBase struct { - // Path to another devcontainer.json to inherit from. - Extends string `json:"extends,omitempty"` + // Path(s) to other devcontainer.json files to inherit from. + Extends ExtendsRef `json:"extends,omitempty"` // A name for the dev container which can be displayed to the user. Name string `json:"name,omitempty"` diff --git a/pkg/devcontainer/config/extends.go b/pkg/devcontainer/config/extends.go index 063486345..46b4a950b 100644 --- a/pkg/devcontainer/config/extends.go +++ b/pkg/devcontainer/config/extends.go @@ -10,14 +10,62 @@ import ( "github.com/tailscale/hujson" ) -// resolveExtends resolves the extends chain for a devcontainer.json file. -// It returns the fully resolved parent config (with its own extends already merged). -// visited tracks files already in the resolution chain for cycle detection. -func resolveExtends( +// ExtendsRef holds one or more paths to parent devcontainer.json files. +// JSON accepts either a single string or an array of strings. +type ExtendsRef []string + +func (e ExtendsRef) IsEmpty() bool { + return len(e) == 0 +} + +func (e ExtendsRef) MarshalJSON() ([]byte, error) { + if len(e) == 1 { + return json.Marshal(e[0]) + } + return json.Marshal([]string(e)) +} + +func (e *ExtendsRef) UnmarshalJSON(data []byte) error { + var s string + if err := json.Unmarshal(data, &s); err == nil { + *e = ExtendsRef{s} + return nil + } + var arr []string + if err := json.Unmarshal(data, &arr); err == nil { + *e = ExtendsRef(arr) + return nil + } + return fmt.Errorf("extends: must be a string or array of strings") +} + +// resolveExtendsArray resolves multiple extends refs left-to-right, +// merging each on top of the previous result. The final merged config +// is returned as the combined parent for the declaring file. +func resolveExtendsArray( + refs ExtendsRef, declaringDir string, + visited map[string]bool, +) (*DevContainerConfig, error) { + var merged *DevContainerConfig + for _, ref := range refs { + resolved, err := resolveExtendsSingle(ref, declaringDir, visited) + if err != nil { + return nil, err + } + if merged == nil { + merged = resolved + } else { + merged = mergeExtendsConfigs(merged, resolved) + } + } + return merged, nil +} + +// resolveExtendsSingle resolves a single extends reference. +func resolveExtendsSingle( extendsRef, declaringDir string, visited map[string]bool, ) (*DevContainerConfig, error) { - // Resolve relative path against the declaring file's directory refPath := extendsRef if !filepath.IsAbs(refPath) { refPath = filepath.Join(declaringDir, refPath) @@ -28,7 +76,6 @@ func resolveExtends( return nil, fmt.Errorf("extends: resolve path %q: %w", extendsRef, err) } - // Cycle detection if visited[absPath] { return nil, fmt.Errorf("extends: cycle detected, %q already in chain", absPath) } @@ -67,9 +114,9 @@ func parseDevContainerJSONFileWithVisited( devContainer.Origin = absPath // Recursively resolve extends - if devContainer.Extends != "" { + if !devContainer.Extends.IsEmpty() { declaringDir := filepath.Dir(absPath) - parent, err := resolveExtends(devContainer.Extends, declaringDir, visited) + parent, err := resolveExtendsArray(devContainer.Extends, declaringDir, visited) if err != nil { return nil, err } @@ -91,7 +138,7 @@ func mergeExtendsConfigs(parent, child *DevContainerConfig) *DevContainerConfig // Special result.Origin = child.Origin - result.Extends = "" + result.Extends = nil return result } diff --git a/pkg/devcontainer/config/extends_test.go b/pkg/devcontainer/config/extends_test.go index 24e17b422..54b20b011 100644 --- a/pkg/devcontainer/config/extends_test.go +++ b/pkg/devcontainer/config/extends_test.go @@ -15,6 +15,7 @@ const ( testUserRoot = "root" testOriginParent = "/tmp/parent.json" testOriginChild = "/tmp/child.json" + testFileBase = "base.json" ) func writeJSON(t *testing.T, dir, filename, content string) string { @@ -54,8 +55,8 @@ func TestExtends_BasicScalarOverride(t *testing.T) { if cfg.RemoteUser != "vscode" { t.Errorf("expected remoteUser 'vscode', got %q", cfg.RemoteUser) } - if cfg.Extends != "" { - t.Errorf("expected extends to be cleared, got %q", cfg.Extends) + if !cfg.Extends.IsEmpty() { + t.Errorf("expected extends to be cleared, got %v", cfg.Extends) } } @@ -388,8 +389,8 @@ func TestMergeExtendsConfigs_Scalars(t *testing.T) { if result.Origin != testOriginChild { t.Errorf("Origin: got %q, want %q", result.Origin, testOriginChild) } - if result.Extends != "" { - t.Errorf("Extends: should be cleared, got %q", result.Extends) + if !result.Extends.IsEmpty() { + t.Errorf("Extends: should be cleared, got %v", result.Extends) } } @@ -499,6 +500,183 @@ func TestMergeExtendsConfigs_ArraysAndHooks(t *testing.T) { } } +func TestExtends_ArraySingleRef(t *testing.T) { + tmpDir := t.TempDir() + writeJSON(t, tmpDir, testFileBase, `{ + "name": "base", + "image": "ubuntu:20.04", + "remoteUser": "vscode" + }`) + childPath := writeJSON(t, tmpDir, "devcontainer.json", `{ + "extends": ["base.json"], + "name": "child" + }`) + + cfg, err := ParseDevContainerJSONFile(childPath) + if err != nil { + t.Fatal(err) + } + if cfg.Name != testNameChild { + t.Errorf("expected name 'child', got %q", cfg.Name) + } + if cfg.Image != testImageUbuntu { + t.Errorf("expected image 'ubuntu:20.04', got %q", cfg.Image) + } +} + +func TestExtends_ArrayMultipleRefs_Scalars(t *testing.T) { + tmpDir := t.TempDir() + writeJSON(t, tmpDir, testFileBase, `{ + "image": "ubuntu:20.04", + "containerEnv": {"FROM_BASE": "base-val", "SHARED": "from-base"} + }`) + writeJSON(t, tmpDir, "middle.json", `{ + "remoteUser": "vscode", + "containerEnv": {"FROM_MIDDLE": "mid-val", "SHARED": "from-middle"} + }`) + childPath := writeJSON(t, tmpDir, "devcontainer.json", `{ + "extends": ["base.json", "middle.json"], + "name": "child", + "containerEnv": {"FROM_CHILD": "child-val"} + }`) + + cfg, err := ParseDevContainerJSONFile(childPath) + if err != nil { + t.Fatal(err) + } + if cfg.Name != testNameChild { + t.Errorf("expected name 'child', got %q", cfg.Name) + } + if cfg.Image != testImageUbuntu { + t.Errorf("expected image from base, got %q", cfg.Image) + } + if cfg.RemoteUser != "vscode" { + t.Errorf("expected remoteUser from middle, got %q", cfg.RemoteUser) + } +} + +func TestExtends_ArrayMultipleRefs_EnvMerge(t *testing.T) { + tmpDir := t.TempDir() + writeJSON(t, tmpDir, testFileBase, `{ + "image": "ubuntu:20.04", + "containerEnv": {"FROM_BASE": "base-val", "SHARED": "from-base"} + }`) + writeJSON(t, tmpDir, "middle.json", `{ + "remoteUser": "vscode", + "containerEnv": {"FROM_MIDDLE": "mid-val", "SHARED": "from-middle"} + }`) + childPath := writeJSON(t, tmpDir, "devcontainer.json", `{ + "extends": ["base.json", "middle.json"], + "name": "child", + "containerEnv": {"FROM_CHILD": "child-val"} + }`) + + cfg, err := ParseDevContainerJSONFile(childPath) + if err != nil { + t.Fatal(err) + } + if cfg.ContainerEnv["FROM_BASE"] != "base-val" { + t.Error("missing FROM_BASE from base") + } + if cfg.ContainerEnv["FROM_MIDDLE"] != "mid-val" { + t.Error("missing FROM_MIDDLE from middle") + } + if cfg.ContainerEnv["FROM_CHILD"] != "child-val" { + t.Error("missing FROM_CHILD from child") + } + if cfg.ContainerEnv["SHARED"] != "from-middle" { + t.Errorf("SHARED: got %q, want from-middle", cfg.ContainerEnv["SHARED"]) + } +} + +func TestExtends_ArrayOrderMatters(t *testing.T) { + tmpDir := t.TempDir() + writeJSON(t, tmpDir, "a.json", `{"remoteUser": "a-user", "image": "img-a"}`) + writeJSON(t, tmpDir, "b.json", `{"remoteUser": "b-user"}`) + childPath := writeJSON(t, tmpDir, "devcontainer.json", `{ + "extends": ["a.json", "b.json"], + "name": "child" + }`) + + cfg, err := ParseDevContainerJSONFile(childPath) + if err != nil { + t.Fatal(err) + } + if cfg.RemoteUser != "b-user" { + t.Errorf("later ref should override: got %q, want 'b-user'", cfg.RemoteUser) + } + if cfg.Image != "img-a" { + t.Errorf("image from a should remain: got %q", cfg.Image) + } +} + +func TestExtends_ArrayCycleDetection(t *testing.T) { + tmpDir := t.TempDir() + writeJSON(t, tmpDir, "a.json", `{"extends": "b.json", "name": "a"}`) + writeJSON(t, tmpDir, "b.json", `{"extends": "a.json", "name": "b"}`) + childPath := writeJSON(t, tmpDir, "devcontainer.json", `{ + "extends": ["a.json"] + }`) + + _, err := ParseDevContainerJSONFile(childPath) + if err == nil { + t.Fatal("expected cycle error") + } + if !strings.Contains(err.Error(), "cycle") { + t.Errorf("expected 'cycle' in error, got: %v", err) + } +} + +func TestExtendsRef_UnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + want ExtendsRef + }{ + {"single string", `"base.json"`, ExtendsRef{testFileBase}}, + {"array single", `["base.json"]`, ExtendsRef{testFileBase}}, + {"array multi", `["a.json","b.json"]`, ExtendsRef{"a.json", "b.json"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var got ExtendsRef + if err := got.UnmarshalJSON([]byte(tc.input)); err != nil { + t.Fatal(err) + } + if len(got) != len(tc.want) { + t.Fatalf("len: got %d, want %d", len(got), len(tc.want)) + } + for i := range got { + if got[i] != tc.want[i] { + t.Errorf("[%d]: got %q, want %q", i, got[i], tc.want[i]) + } + } + }) + } +} + +func TestExtendsRef_MarshalJSON(t *testing.T) { + tests := []struct { + name string + input ExtendsRef + want string + }{ + {"single", ExtendsRef{testFileBase}, `"base.json"`}, + {"multi", ExtendsRef{"a.json", "b.json"}, `["a.json","b.json"]`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := tc.input.MarshalJSON() + if err != nil { + t.Fatal(err) + } + if string(got) != tc.want { + t.Errorf("got %q, want %q", string(got), tc.want) + } + }) + } +} + func strPtr(s string) *string { return &s } diff --git a/pkg/devcontainer/config/parse.go b/pkg/devcontainer/config/parse.go index d2f70108e..61e0335d1 100644 --- a/pkg/devcontainer/config/parse.go +++ b/pkg/devcontainer/config/parse.go @@ -97,10 +97,10 @@ func ParseDevContainerJSONFile(jsonFilePath string) (*DevContainerConfig, error) devContainer.Origin = path // Resolve extends before applying legacy transforms - if devContainer.Extends != "" { + if !devContainer.Extends.IsEmpty() { visited := map[string]bool{path: true} declaringDir := filepath.Dir(path) - parent, err := resolveExtends(devContainer.Extends, declaringDir, visited) + parent, err := resolveExtendsArray(devContainer.Extends, declaringDir, visited) if err != nil { return nil, err }