From 63a38a7c2b7fadaf3c4f0da37cabe7cd29241706 Mon Sep 17 00:00:00 2001 From: Pablo Ulloa Date: Mon, 9 Mar 2026 21:02:22 +0000 Subject: [PATCH 1/4] devcontainer: implement overrideFeatureInstallOrder --- devcontainer/devcontainer.go | 44 +++++++++++++-- devcontainer/devcontainer_test.go | 91 +++++++++++++++++++++++++++++++ docs/devcontainer-spec-support.md | 2 +- 3 files changed, 130 insertions(+), 7 deletions(-) diff --git a/devcontainer/devcontainer.go b/devcontainer/devcontainer.go index ea07bfcd..e3c30555 100644 --- a/devcontainer/devcontainer.go +++ b/devcontainer/devcontainer.go @@ -40,6 +40,10 @@ type Spec struct { RemoteEnv map[string]string `json:"remoteEnv"` // Features is a map of feature names to feature configurations. Features map[string]any `json:"features"` + // OverrideFeatureInstallOrder overrides the order in which features are + // installed. Feature references not present in this list are installed + // after the listed ones, in alphabetical order. + OverrideFeatureInstallOrder []string `json:"overrideFeatureInstallOrder"` LifecycleScripts // Deprecated but still frequently used... @@ -234,15 +238,15 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir featureDirectives := []string{} featureContexts := make(map[string]string) - // TODO: Respect the installation order outlined by the spec: - // https://containers.dev/implementors/features/#installation-order - featureOrder := []string{} + featureOrder := make([]string, 0, len(s.Features)) for featureRef := range s.Features { featureOrder = append(featureOrder, featureRef) } - // It's critical we sort features prior to compilation so the Dockerfile - // is deterministic which allows for caching. - sort.Strings(featureOrder) + // applyInstallOrder places explicitly ordered features first (in declared + // order), then appends remaining features alphabetically. Alphabetical + // ordering for unconstrained features is critical for Dockerfile + // determinism, which allows for layer caching. + featureOrder = applyInstallOrder(featureOrder, s.OverrideFeatureInstallOrder) var lines []string for _, featureRefRaw := range featureOrder { @@ -306,6 +310,34 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir return strings.Join(lines, "\n"), featureContexts, err } +// applyInstallOrder returns features in the order specified by overrideOrder +// first, then any remaining features in alphabetical order. Entries in +// overrideOrder that don't match any feature are silently ignored. +func applyInstallOrder(features []string, overrideOrder []string) []string { + set := make(map[string]bool, len(features)) + for _, f := range features { + set[f] = true + } + + ordered := make([]string, 0, len(features)) + for _, f := range overrideOrder { + if set[f] { + ordered = append(ordered, f) + delete(set, f) + } + } + + // Collect and sort the remainder for determinism. + remaining := make([]string, 0, len(set)) + for _, f := range features { + if set[f] { + remaining = append(remaining, f) + } + } + sort.Strings(remaining) + return append(ordered, remaining...) +} + // BuildArgsMap converts a slice of "KEY=VALUE" strings to a map. func BuildArgsMap(buildArgs []string) map[string]string { m := make(map[string]string, len(buildArgs)) diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index 5b7fe033..a712b423 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -149,6 +149,97 @@ USER 1000`, params.DockerfileContent) }) } +func TestCompileWithFeaturesOverrideInstallOrder(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + featureOne := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/one:tomato", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "one", + Version: "tomato", + Name: "One", + }, + }) + featureTwo := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/two:potato", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "two", + Version: "potato", + Name: "Two", + }, + }) + featureThree := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/three:apple", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "three", + Version: "apple", + Name: "Three", + }, + }) + + featureOneMD5 := md5.Sum([]byte(featureOne)) + featureOneDir := fmt.Sprintf("/.envbuilder/features/one-%x", featureOneMD5[:4]) + featureTwoMD5 := md5.Sum([]byte(featureTwo)) + featureTwoDir := fmt.Sprintf("/.envbuilder/features/two-%x", featureTwoMD5[:4]) + featureThreeMD5 := md5.Sum([]byte(featureThree)) + featureThreeDir := fmt.Sprintf("/.envbuilder/features/three-%x", featureThreeMD5[:4]) + + t.Run("OverrideReverseOrder", func(t *testing.T) { + // featureThree then featureTwo are explicitly ordered first; featureOne + // is unconstrained and falls to the alphabetical remainder. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureOne + `": {}, + "` + featureTwo + `": {}, + "` + featureThree + `": {} + }, + "overrideFeatureInstallOrder": ["` + featureThree + `", "` + featureTwo + `"] +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + // featureThree and featureTwo come first (in override order), + // then featureOne last (alphabetical remainder). + require.Contains(t, params.DockerfileContent, "WORKDIR "+featureThreeDir+"\n") + require.Contains(t, params.DockerfileContent, "WORKDIR "+featureTwoDir+"\n") + require.Contains(t, params.DockerfileContent, "WORKDIR "+featureOneDir+"\n") + + threeIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureThreeDir) + twoIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTwoDir) + oneIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureOneDir) + require.Less(t, threeIdx, twoIdx, "three should be installed before two") + require.Less(t, twoIdx, oneIdx, "two should be installed before one") + }) + + t.Run("UnknownOverrideEntryIgnored", func(t *testing.T) { + // An entry in overrideFeatureInstallOrder that doesn't match any + // feature key should be silently ignored. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureOne + `": {}, + "` + featureTwo + `": {} + }, + "overrideFeatureInstallOrder": ["does-not-exist", "` + featureTwo + `"] +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + twoIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTwoDir) + oneIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureOneDir) + require.Less(t, twoIdx, oneIdx, "two should be installed before one") + }) +} + func TestCompileDevContainer(t *testing.T) { t.Parallel() t.Run("WithImage", func(t *testing.T) { diff --git a/docs/devcontainer-spec-support.md b/docs/devcontainer-spec-support.md index dd8bf0ff..4a54e289 100644 --- a/docs/devcontainer-spec-support.md +++ b/docs/devcontainer-spec-support.md @@ -33,7 +33,7 @@ Feel free to [create a new issue](https://github.com/coder/envbuilder/issues/new | ๐Ÿ”ด | `securityOpt` | Security options to add to the container (for example, `seccomp=unconfined`). | - | | ๐Ÿ”ด | `mounts` | Add additional mounts to the container. | [#220](https://github.com/coder/envbuilder/issues/220) | | ๐ŸŸข | `features` | Features to be added to the devcontainer. | - | -| ๐Ÿ”ด | `overrideFeatureInstallOrder` | Override the order in which features should be installed. | [#226](https://github.com/coder/envbuilder/issues/226) | +| ๏ฟฝ | `overrideFeatureInstallOrder` | Override the order in which features should be installed. | - | | ๐ŸŸ  | `customizations` | Product-specific properties, e.g., _VS Code_ settings and extensions. | Workaround in [#43](https://github.com/coder/envbuilder/issues/43) | ## Image or Dockerfile From 7cab1bfc76a6fb6df0fe1bb0f53fefb817362f6b Mon Sep 17 00:00:00 2001 From: Pablo Ulloa Date: Mon, 9 Mar 2026 21:29:37 +0000 Subject: [PATCH 2/4] devcontainer: implement installsAfter and dependsOn feature ordering --- devcontainer/devcontainer.go | 204 ++++- devcontainer/devcontainer_test.go | 1251 +++++++++++++++++------------ devcontainer/features/features.go | 9 + docs/devcontainer-spec-support.md | 246 +++--- 4 files changed, 1017 insertions(+), 693 deletions(-) diff --git a/devcontainer/devcontainer.go b/devcontainer/devcontainer.go index e3c30555..5aa601e4 100644 --- a/devcontainer/devcontainer.go +++ b/devcontainer/devcontainer.go @@ -231,25 +231,23 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir } featuresDir := filepath.Join(scratchDir, "features") - err := fs.MkdirAll(featuresDir, 0o644) - if err != nil { + if err := fs.MkdirAll(featuresDir, 0o644); err != nil { return "", nil, fmt.Errorf("create features directory: %w", err) } - featureDirectives := []string{} - featureContexts := make(map[string]string) - featureOrder := make([]string, 0, len(s.Features)) - for featureRef := range s.Features { - featureOrder = append(featureOrder, featureRef) + // Pass 1: resolve each raw ref to its canonical featureRef and extract + // the feature spec. We need all specs before we can resolve ordering + // since installsAfter/dependsOn live inside devcontainer-feature.json. + type extractedFeature struct { + featureRef string + featureName string + featureDir string + spec *features.Spec + opts map[string]any } - // applyInstallOrder places explicitly ordered features first (in declared - // order), then appends remaining features alphabetically. Alphabetical - // ordering for unconstrained features is critical for Dockerfile - // determinism, which allows for layer caching. - featureOrder = applyInstallOrder(featureOrder, s.OverrideFeatureInstallOrder) - - var lines []string - for _, featureRefRaw := range featureOrder { + extracted := make(map[string]*extractedFeature, len(s.Features)) + idToRef := make(map[string]string, len(s.Features)) // feature ID โ†’ refRaw + for featureRefRaw := range s.Features { var ( featureRef string ok bool @@ -265,7 +263,7 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir featureOpts := map[string]any{} switch t := s.Features[featureRefRaw].(type) { case string: - // As a shorthand, the value of the `features`` property can be provided as a + // As a shorthand, the value of the `features` property can be provided as a // single string. This string is mapped to an option called version. // https://containers.dev/implementors/features/#devcontainer-json-properties featureOpts["version"] = t @@ -288,13 +286,46 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir if err != nil { return "", nil, fmt.Errorf("extract feature %s: %w", featureRefRaw, err) } - fromDirective, directive, err := spec.Compile(featureRef, featureName, featureDir, containerUser, remoteUser, useBuildContexts, featureOpts) + extracted[featureRefRaw] = &extractedFeature{ + featureRef: featureRef, + featureName: featureName, + featureDir: featureDir, + spec: spec, + opts: featureOpts, + } + idToRef[spec.ID] = featureRefRaw + } + + // Resolve installation order, then validate hard dependencies. + refRaws := make([]string, 0, len(extracted)) + for refRaw := range extracted { + refRaws = append(refRaws, refRaw) + } + specsByRef := make(map[string]*features.Spec, len(extracted)) + for refRaw, ef := range extracted { + specsByRef[refRaw] = ef.spec + } + featureOrder, err := resolveInstallOrder(refRaws, specsByRef, idToRef, s.OverrideFeatureInstallOrder) + if err != nil { + return "", nil, err + } + if err := validateDependencies(specsByRef, idToRef); err != nil { + return "", nil, err + } + + // Pass 2: compile Dockerfile directives in the resolved order. + featureDirectives := make([]string, 0, len(featureOrder)) + featureContexts := make(map[string]string) + var lines []string + for _, featureRefRaw := range featureOrder { + ef := extracted[featureRefRaw] + fromDirective, directive, err := ef.spec.Compile(ef.featureRef, ef.featureName, ef.featureDir, containerUser, remoteUser, useBuildContexts, ef.opts) if err != nil { return "", nil, fmt.Errorf("compile feature %s: %w", featureRefRaw, err) } featureDirectives = append(featureDirectives, directive) if useBuildContexts { - featureContexts[featureRef] = featureDir + featureContexts[ef.featureRef] = ef.featureDir lines = append(lines, fromDirective) } } @@ -307,35 +338,132 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir // we're going to run as root. lines = append(lines, fmt.Sprintf("USER %s", remoteUser)) } - return strings.Join(lines, "\n"), featureContexts, err + return strings.Join(lines, "\n"), featureContexts, nil } -// applyInstallOrder returns features in the order specified by overrideOrder -// first, then any remaining features in alphabetical order. Entries in -// overrideOrder that don't match any feature are silently ignored. -func applyInstallOrder(features []string, overrideOrder []string) []string { - set := make(map[string]bool, len(features)) - for _, f := range features { - set[f] = true +// resolveInstallOrder determines the final feature installation order. +// +// Priority (highest to lowest): +// 1. overrideOrder entries (in declared order) โ€” user override wins unconditionally +// 2. installsAfter edges from devcontainer-feature.json โ€” soft ordering via +// Kahn's topological sort on the unconstrained remainder +// 3. Alphabetical โ€” tie-breaking for determinism and layer cache stability +// +// IDs in installsAfter that don't map to a feature present in the set are +// silently ignored (soft-dep semantics). Returns an error if a cycle is +// detected among the installsAfter edges. +// +// See https://containers.dev/implementors/features/#installation-order +func resolveInstallOrder(refRaws []string, specs map[string]*features.Spec, idToRef map[string]string, overrideOrder []string) ([]string, error) { + // Step 1: lock in override entries (in declared order), removing them + // from the free set so they are not subject to topo sorting. + free := make(map[string]bool, len(refRaws)) + for _, r := range refRaws { + free[r] = true + } + pinned := make([]string, 0, len(overrideOrder)) + for _, r := range overrideOrder { + if free[r] { + pinned = append(pinned, r) + delete(free, r) + } + } + + // Step 2: topological sort (Kahn's algorithm) on the free remainder, + // driven by installsAfter edges. An edge Aโ†’B means "B must come before A". + // Edges pointing outside the free set are ignored. + inDegree := make(map[string]int, len(free)) + deps := make(map[string][]string, len(free)) // refRaw โ†’ refRaws it must follow + for r := range free { + inDegree[r] = 0 + } + for r := range free { + for _, depID := range specs[r].InstallsAfter { + // Resolve the ID or ref to a refRaw present in the free set. + predRef, ok := idToRef[depID] + if !ok { + // depID might itself be a raw ref rather than a short ID. + if free[depID] { + predRef = depID + ok = true + } + } + if !ok || !free[predRef] { + // Predecessor absent or overridden โ€” soft dep, skip. + continue + } + deps[r] = append(deps[r], predRef) + inDegree[r]++ + } } - ordered := make([]string, 0, len(features)) - for _, f := range overrideOrder { - if set[f] { - ordered = append(ordered, f) - delete(set, f) + // Seed the ready queue with all zero-in-degree nodes, sorted alphabetically + // so tie-breaking is deterministic. + ready := make([]string, 0, len(free)) + for r := range free { + if inDegree[r] == 0 { + ready = append(ready, r) } } + sort.Strings(ready) - // Collect and sort the remainder for determinism. - remaining := make([]string, 0, len(set)) - for _, f := range features { - if set[f] { - remaining = append(remaining, f) + sorted := make([]string, 0, len(free)) + // successors maps predecessor โ†’ features that depend on it. + successors := make(map[string][]string, len(free)) + for r, preds := range deps { + for _, pred := range preds { + successors[pred] = append(successors[pred], r) + } + } + for len(ready) > 0 { + // Pop the first (alphabetically smallest) ready node. + r := ready[0] + ready = ready[1:] + sorted = append(sorted, r) + // Reduce in-degree for all features that installsAfter r. + newReady := []string{} + for _, succ := range successors[r] { + inDegree[succ]-- + if inDegree[succ] == 0 { + newReady = append(newReady, succ) + } + } + // Insert new ready nodes in sorted position to preserve alphabetical + // tie-breaking across the entire queue. + sort.Strings(newReady) + ready = append(ready, newReady...) + sort.Strings(ready) + } + + if len(sorted) != len(free) { + // Cycle detected โ€” identify the offending features. + cycled := make([]string, 0) + for r := range free { + if inDegree[r] > 0 { + cycled = append(cycled, r) + } + } + sort.Strings(cycled) + return nil, fmt.Errorf("cycle detected in feature installsAfter dependencies: %s", strings.Join(cycled, ", ")) + } + + return append(pinned, sorted...), nil +} + +// validateDependencies checks that every hard dependency declared via +// dependsOn in a feature's devcontainer-feature.json is satisfied by the +// set of installed features. +func validateDependencies(specs map[string]*features.Spec, idToRef map[string]string) error { + for refRaw, spec := range specs { + for _, depID := range spec.DependsOn { + _, byID := idToRef[depID] + _, byRef := specs[depID] + if !byID && !byRef { + return fmt.Errorf("feature %q (%s) requires feature %q which is not included", spec.ID, refRaw, depID) + } } } - sort.Strings(remaining) - return append(ordered, remaining...) + return nil } // BuildArgsMap converts a slice of "KEY=VALUE" strings to a map. diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index a712b423..99a9ab4a 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -1,532 +1,719 @@ -package devcontainer_test - -import ( - "crypto/md5" - "fmt" - "io" - "net/url" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/coder/envbuilder/devcontainer" - "github.com/coder/envbuilder/devcontainer/features" - "github.com/coder/envbuilder/testutil/registrytest" - "github.com/go-git/go-billy/v5/memfs" - "github.com/google/go-containerregistry/pkg/name" - v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1/partial" - "github.com/google/go-containerregistry/pkg/v1/remote" - "github.com/google/go-containerregistry/pkg/v1/types" - "github.com/stretchr/testify/require" -) - -const workingDir = "/.envbuilder" - -var emptyRemoteOpts []remote.Option - -func stubLookupEnv(string) (string, bool) { - return "", false -} - -func TestParse(t *testing.T) { - t.Parallel() - raw := `{ - "build": { - "dockerfile": "Dockerfile", - "context": ".", - }, - // Comments here! - "image": "codercom/code-server:latest" -}` - parsed, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - require.Equal(t, "Dockerfile", parsed.Build.Dockerfile) -} - -func TestCompileWithFeatures(t *testing.T) { - t.Parallel() - registry := registrytest.New(t) - featureOne := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/one:tomato", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "rust", - Version: "tomato", - Name: "Rust", - Description: "Example description!", - ContainerEnv: map[string]string{ - "TOMATO": "example", - }, - }, - }) - featureTwo := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/two:potato", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "go", - Version: "potato", - Name: "Go", - Description: "Example description!", - ContainerEnv: map[string]string{ - "POTATO": "example", - }, - Options: map[string]features.Option{ - "version": { - Type: "string", - }, - }, - }, - }) - - raw := `{ - "build": { - "dockerfile": "Dockerfile", - "context": ".", - }, - // Comments here! - "image": "localhost:5000/envbuilder-test-codercom-code-server:latest", - "features": { - "` + featureOne + `": {}, - "` + featureTwo + `": "potato" - } -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - featureOneMD5 := md5.Sum([]byte(featureOne)) - featureOneDir := fmt.Sprintf("/.envbuilder/features/one-%x", featureOneMD5[:4]) - featureTwoMD5 := md5.Sum([]byte(featureTwo)) - featureTwoDir := fmt.Sprintf("/.envbuilder/features/two-%x", featureTwoMD5[:4]) - - t.Run("WithoutBuildContexts", func(t *testing.T) { - params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err) - - require.Equal(t, `FROM localhost:5000/envbuilder-test-codercom-code-server:latest - -USER root -# Rust tomato - Example description! -WORKDIR `+featureOneDir+` -ENV TOMATO=example -RUN _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh -# Go potato - Example description! -WORKDIR `+featureTwoDir+` -ENV POTATO=example -RUN VERSION="potato" _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh -USER 1000`, params.DockerfileContent) - }) - - t.Run("WithBuildContexts", func(t *testing.T) { - params, err := dc.Compile(fs, "", workingDir, "", "", true, stubLookupEnv) - require.NoError(t, err) - - registryHost := strings.TrimPrefix(registry, "http://") - - require.Equal(t, `FROM scratch AS envbuilder_feature_one -COPY --from=`+registryHost+`/coder/one / / - -FROM scratch AS envbuilder_feature_two -COPY --from=`+registryHost+`/coder/two / / - -FROM localhost:5000/envbuilder-test-codercom-code-server:latest - -USER root -# Rust tomato - Example description! -WORKDIR /.envbuilder/features/one -ENV TOMATO=example -RUN --mount=type=bind,from=envbuilder_feature_one,target=/.envbuilder/features/one,rw _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh -# Go potato - Example description! -WORKDIR /.envbuilder/features/two -ENV POTATO=example -RUN --mount=type=bind,from=envbuilder_feature_two,target=/.envbuilder/features/two,rw VERSION="potato" _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh -USER 1000`, params.DockerfileContent) - - require.Equal(t, map[string]string{ - registryHost + "/coder/one": featureOneDir, - registryHost + "/coder/two": featureTwoDir, - }, params.FeatureContexts) - }) -} - -func TestCompileWithFeaturesOverrideInstallOrder(t *testing.T) { - t.Parallel() - registry := registrytest.New(t) - featureOne := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/one:tomato", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "one", - Version: "tomato", - Name: "One", - }, - }) - featureTwo := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/two:potato", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "two", - Version: "potato", - Name: "Two", - }, - }) - featureThree := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/three:apple", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "three", - Version: "apple", - Name: "Three", - }, - }) - - featureOneMD5 := md5.Sum([]byte(featureOne)) - featureOneDir := fmt.Sprintf("/.envbuilder/features/one-%x", featureOneMD5[:4]) - featureTwoMD5 := md5.Sum([]byte(featureTwo)) - featureTwoDir := fmt.Sprintf("/.envbuilder/features/two-%x", featureTwoMD5[:4]) - featureThreeMD5 := md5.Sum([]byte(featureThree)) - featureThreeDir := fmt.Sprintf("/.envbuilder/features/three-%x", featureThreeMD5[:4]) - - t.Run("OverrideReverseOrder", func(t *testing.T) { - // featureThree then featureTwo are explicitly ordered first; featureOne - // is unconstrained and falls to the alphabetical remainder. - raw := `{ - "image": "localhost:5000/envbuilder-test-ubuntu:latest", - "features": { - "` + featureOne + `": {}, - "` + featureTwo + `": {}, - "` + featureThree + `": {} - }, - "overrideFeatureInstallOrder": ["` + featureThree + `", "` + featureTwo + `"] -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err) - - // featureThree and featureTwo come first (in override order), - // then featureOne last (alphabetical remainder). - require.Contains(t, params.DockerfileContent, "WORKDIR "+featureThreeDir+"\n") - require.Contains(t, params.DockerfileContent, "WORKDIR "+featureTwoDir+"\n") - require.Contains(t, params.DockerfileContent, "WORKDIR "+featureOneDir+"\n") - - threeIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureThreeDir) - twoIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTwoDir) - oneIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureOneDir) - require.Less(t, threeIdx, twoIdx, "three should be installed before two") - require.Less(t, twoIdx, oneIdx, "two should be installed before one") - }) - - t.Run("UnknownOverrideEntryIgnored", func(t *testing.T) { - // An entry in overrideFeatureInstallOrder that doesn't match any - // feature key should be silently ignored. - raw := `{ - "image": "localhost:5000/envbuilder-test-ubuntu:latest", - "features": { - "` + featureOne + `": {}, - "` + featureTwo + `": {} - }, - "overrideFeatureInstallOrder": ["does-not-exist", "` + featureTwo + `"] -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err) - - twoIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTwoDir) - oneIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureOneDir) - require.Less(t, twoIdx, oneIdx, "two should be installed before one") - }) -} - -func TestCompileDevContainer(t *testing.T) { - t.Parallel() - t.Run("WithImage", func(t *testing.T) { - t.Parallel() - fs := memfs.New() - dc := &devcontainer.Spec{ - Image: "localhost:5000/envbuilder-test-ubuntu:latest", - } - params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err) - require.Equal(t, filepath.Join(workingDir, "Dockerfile"), params.DockerfilePath) - require.Equal(t, workingDir, params.BuildContext) - }) - t.Run("WithBuild", func(t *testing.T) { - t.Parallel() - fs := memfs.New() - dc := &devcontainer.Spec{ - Build: devcontainer.BuildSpec{ - Dockerfile: "Dockerfile", - Context: ".", - Args: map[string]string{ - "ARG1": "value1", - "ARG2": "${localWorkspaceFolderBasename}", - }, - }, - } - dcDir := "/workspaces/coder/.devcontainer" - err := fs.MkdirAll(dcDir, 0o755) - require.NoError(t, err) - file, err := fs.OpenFile(filepath.Join(dcDir, "Dockerfile"), os.O_CREATE|os.O_WRONLY, 0o644) - require.NoError(t, err) - _, err = io.WriteString(file, "FROM localhost:5000/envbuilder-test-ubuntu:latest") - require.NoError(t, err) - _ = file.Close() - params, err := dc.Compile(fs, dcDir, workingDir, "", "/var/workspace", false, stubLookupEnv) - require.NoError(t, err) - require.Equal(t, "ARG1=value1", params.BuildArgs[0]) - require.Equal(t, "ARG2=workspace", params.BuildArgs[1]) - require.Equal(t, filepath.Join(dcDir, "Dockerfile"), params.DockerfilePath) - require.Equal(t, dcDir, params.BuildContext) - }) -} - -func TestImageFromDockerfile(t *testing.T) { - t.Parallel() - for _, tc := range []struct { - content string - image string - }{{ - content: "FROM ubuntu", - image: "index.docker.io/library/ubuntu:latest", - }, { - content: "ARG VARIANT=bionic\nFROM ubuntu:$VARIANT", - image: "index.docker.io/library/ubuntu:bionic", - }, { - content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}", - image: "mcr.microsoft.com/devcontainers/python:0-3.10", - }, { - content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-$VARIANT ", - image: "mcr.microsoft.com/devcontainers/python:0-3.10", - }} { - tc := tc - t.Run(tc.image, func(t *testing.T) { - t.Parallel() - ref, err := devcontainer.ImageFromDockerfile(tc.content, nil) - require.NoError(t, err) - require.Equal(t, tc.image, ref.Name()) - }) - } -} - -func TestImageFromDockerfile_BuildArgs(t *testing.T) { - t.Parallel() - - // Test that build args override ARG defaults. - t.Run("OverridesDefault", func(t *testing.T) { - t.Parallel() - content := "ARG VARIANT=3.10\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}" - ref, err := devcontainer.ImageFromDockerfile(content, map[string]string{"VARIANT": "3.11-bookworm"}) - require.NoError(t, err) - require.Equal(t, "mcr.microsoft.com/devcontainers/python:0-3.11-bookworm", ref.Name()) - }) - - // Test that build args supply values for ARGs without defaults. - t.Run("SuppliesArgWithoutDefault", func(t *testing.T) { - t.Parallel() - content := "ARG VARIANT\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}" - ref, err := devcontainer.ImageFromDockerfile(content, map[string]string{"VARIANT": "3.11-bookworm"}) - require.NoError(t, err) - require.Equal(t, "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", ref.Name()) - }) -} - -func TestUserFromDockerfile_BuildArgs(t *testing.T) { - t.Parallel() - - t.Run("SubstitutesARGInFROM", func(t *testing.T) { - t.Parallel() - registry := registrytest.New(t) - image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ - Config: v1.Config{ - User: "testuser", - }, - }}) - require.NoError(t, err) - ref := strings.TrimPrefix(registry, "http://") + "/coder/test:latest" - parsed, err := name.ParseReference(ref) - require.NoError(t, err) - err = remote.Write(parsed, image) - require.NoError(t, err) - - // Dockerfile uses ARG without default for the image ref. - content := fmt.Sprintf("ARG TAG\nFROM %s/coder/test:${TAG}", strings.TrimPrefix(registry, "http://")) - user, err := devcontainer.UserFromDockerfile(content, map[string]string{"TAG": "latest"}) - require.NoError(t, err) - require.Equal(t, "testuser", user) - }) -} - -func TestUserFrom(t *testing.T) { - t.Parallel() - - t.Run("Image", func(t *testing.T) { - t.Parallel() - registry := registrytest.New(t) - image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ - Config: v1.Config{ - User: "example", - }, - }}) - require.NoError(t, err) - - parsed, err := url.Parse("http://" + registry) - require.NoError(t, err) - parsed.Path = "coder/test:latest" - ref, err := name.ParseReference(strings.TrimPrefix(parsed.String(), "http://")) - require.NoError(t, err) - err = remote.Write(ref, image) - require.NoError(t, err) - - user, err := devcontainer.UserFromImage(ref) - require.NoError(t, err) - require.Equal(t, "example", user) - }) - - t.Run("Dockerfile", func(t *testing.T) { - t.Parallel() - tests := []struct { - name string - content string - user string - }{ - { - name: "Empty", - content: "FROM scratch", - user: "", - }, - { - name: "User", - content: "FROM scratch\nUSER kyle", - user: "kyle", - }, - { - name: "Env with default", - content: "FROM scratch\nENV MYUSER=maf\nUSER ${MYUSER}", - user: "${MYUSER}", // This should be "maf" but the current implementation doesn't support this. - }, - { - name: "Env var with default", - content: "FROM scratch\nUSER ${MYUSER:-maf}", - user: "${MYUSER:-maf}", // This should be "maf" but the current implementation doesn't support this. - }, - { - name: "Arg", - content: "FROM scratch\nARG MYUSER\nUSER ${MYUSER}", - user: "${MYUSER}", // This should be "" or populated but the current implementation doesn't support this. - }, - { - name: "Arg with default", - content: "FROM scratch\nARG MYUSER=maf\nUSER ${MYUSER}", - user: "${MYUSER}", // This should be "maf" but the current implementation doesn't support this. - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - user, err := devcontainer.UserFromDockerfile(tt.content, nil) - require.NoError(t, err) - require.Equal(t, tt.user, user) - }) - } - }) - - t.Run("Multi-stage", func(t *testing.T) { - t.Parallel() - - registry := registrytest.New(t) - for tag, user := range map[string]string{ - "one": "maf", - "two": "fam", - } { - image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ - Config: v1.Config{ - User: user, - }, - }}) - require.NoError(t, err) - parsed, err := url.Parse("http://" + registry) - require.NoError(t, err) - parsed.Path = "coder/test:" + tag - ref, err := name.ParseReference(strings.TrimPrefix(parsed.String(), "http://")) - fmt.Println(ref) - require.NoError(t, err) - err = remote.Write(ref, image) - require.NoError(t, err) - } - - tests := []struct { - name string - images map[string]string - content string - user string - }{ - { - name: "Single", - content: "FROM coder/test:one", - user: "maf", - }, - { - name: "Multi", - content: "FROM ubuntu AS u\nFROM coder/test:two", - user: "fam", - }, - { - name: "Multi-2", - content: "FROM coder/test:two AS two\nUSER maffam\nFROM coder/test:one AS one", - user: "maf", - }, - { - name: "Multi-3", - content: "FROM coder/test:two AS two\nFROM coder/test:one AS one\nUSER fammaf", - user: "fammaf", - }, - { - name: "Multi-4", - content: `FROM ubuntu AS a -USER root -RUN useradd --create-home pickme -USER pickme -FROM a AS other -USER root -RUN useradd --create-home notme -USER notme -FROM a`, - user: "pickme", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - content := strings.ReplaceAll(tt.content, "coder/test", strings.TrimPrefix(registry, "http://")+"/coder/test") - - user, err := devcontainer.UserFromDockerfile(content, nil) - require.NoError(t, err) - require.Equal(t, tt.user, user) - }) - } - }) -} - -type emptyImage struct { - configFile *v1.ConfigFile -} - -func (i emptyImage) MediaType() (types.MediaType, error) { - return types.DockerManifestSchema2, nil -} - -func (i emptyImage) RawConfigFile() ([]byte, error) { - return partial.RawConfigFile(i) -} - -func (i emptyImage) ConfigFile() (*v1.ConfigFile, error) { - return i.configFile, nil -} - -func (i emptyImage) LayerByDiffID(h v1.Hash) (partial.UncompressedLayer, error) { - return nil, fmt.Errorf("LayerByDiffID(%s): empty image", h) -} +package devcontainer_test + +import ( + "crypto/md5" + "fmt" + "io" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/coder/envbuilder/devcontainer" + "github.com/coder/envbuilder/devcontainer/features" + "github.com/coder/envbuilder/testutil/registrytest" + "github.com/go-git/go-billy/v5/memfs" + "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/partial" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/types" + "github.com/stretchr/testify/require" +) + +const workingDir = "/.envbuilder" + +var emptyRemoteOpts []remote.Option + +func stubLookupEnv(string) (string, bool) { + return "", false +} + +func TestParse(t *testing.T) { + t.Parallel() + raw := `{ + "build": { + "dockerfile": "Dockerfile", + "context": ".", + }, + // Comments here! + "image": "codercom/code-server:latest" +}` + parsed, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + require.Equal(t, "Dockerfile", parsed.Build.Dockerfile) +} + +func TestCompileWithFeatures(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + featureOne := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/one:tomato", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "rust", + Version: "tomato", + Name: "Rust", + Description: "Example description!", + ContainerEnv: map[string]string{ + "TOMATO": "example", + }, + }, + }) + featureTwo := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/two:potato", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "go", + Version: "potato", + Name: "Go", + Description: "Example description!", + ContainerEnv: map[string]string{ + "POTATO": "example", + }, + Options: map[string]features.Option{ + "version": { + Type: "string", + }, + }, + }, + }) + + raw := `{ + "build": { + "dockerfile": "Dockerfile", + "context": ".", + }, + // Comments here! + "image": "localhost:5000/envbuilder-test-codercom-code-server:latest", + "features": { + "` + featureOne + `": {}, + "` + featureTwo + `": "potato" + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + featureOneMD5 := md5.Sum([]byte(featureOne)) + featureOneDir := fmt.Sprintf("/.envbuilder/features/one-%x", featureOneMD5[:4]) + featureTwoMD5 := md5.Sum([]byte(featureTwo)) + featureTwoDir := fmt.Sprintf("/.envbuilder/features/two-%x", featureTwoMD5[:4]) + + t.Run("WithoutBuildContexts", func(t *testing.T) { + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + require.Equal(t, `FROM localhost:5000/envbuilder-test-codercom-code-server:latest + +USER root +# Rust tomato - Example description! +WORKDIR `+featureOneDir+` +ENV TOMATO=example +RUN _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh +# Go potato - Example description! +WORKDIR `+featureTwoDir+` +ENV POTATO=example +RUN VERSION="potato" _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh +USER 1000`, params.DockerfileContent) + }) + + t.Run("WithBuildContexts", func(t *testing.T) { + params, err := dc.Compile(fs, "", workingDir, "", "", true, stubLookupEnv) + require.NoError(t, err) + + registryHost := strings.TrimPrefix(registry, "http://") + + require.Equal(t, `FROM scratch AS envbuilder_feature_one +COPY --from=`+registryHost+`/coder/one / / + +FROM scratch AS envbuilder_feature_two +COPY --from=`+registryHost+`/coder/two / / + +FROM localhost:5000/envbuilder-test-codercom-code-server:latest + +USER root +# Rust tomato - Example description! +WORKDIR /.envbuilder/features/one +ENV TOMATO=example +RUN --mount=type=bind,from=envbuilder_feature_one,target=/.envbuilder/features/one,rw _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh +# Go potato - Example description! +WORKDIR /.envbuilder/features/two +ENV POTATO=example +RUN --mount=type=bind,from=envbuilder_feature_two,target=/.envbuilder/features/two,rw VERSION="potato" _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh +USER 1000`, params.DockerfileContent) + + require.Equal(t, map[string]string{ + registryHost + "/coder/one": featureOneDir, + registryHost + "/coder/two": featureTwoDir, + }, params.FeatureContexts) + }) +} + +func TestCompileWithFeaturesOverrideInstallOrder(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + featureOne := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/one:tomato", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "one", + Version: "tomato", + Name: "One", + }, + }) + featureTwo := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/two:potato", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "two", + Version: "potato", + Name: "Two", + }, + }) + featureThree := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/three:apple", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "three", + Version: "apple", + Name: "Three", + }, + }) + + featureOneMD5 := md5.Sum([]byte(featureOne)) + featureOneDir := fmt.Sprintf("/.envbuilder/features/one-%x", featureOneMD5[:4]) + featureTwoMD5 := md5.Sum([]byte(featureTwo)) + featureTwoDir := fmt.Sprintf("/.envbuilder/features/two-%x", featureTwoMD5[:4]) + featureThreeMD5 := md5.Sum([]byte(featureThree)) + featureThreeDir := fmt.Sprintf("/.envbuilder/features/three-%x", featureThreeMD5[:4]) + + t.Run("OverrideReverseOrder", func(t *testing.T) { + // featureThree then featureTwo are explicitly ordered first; featureOne + // is unconstrained and falls to the alphabetical remainder. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureOne + `": {}, + "` + featureTwo + `": {}, + "` + featureThree + `": {} + }, + "overrideFeatureInstallOrder": ["` + featureThree + `", "` + featureTwo + `"] +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + // featureThree and featureTwo come first (in override order), + // then featureOne last (alphabetical remainder). + require.Contains(t, params.DockerfileContent, "WORKDIR "+featureThreeDir+"\n") + require.Contains(t, params.DockerfileContent, "WORKDIR "+featureTwoDir+"\n") + require.Contains(t, params.DockerfileContent, "WORKDIR "+featureOneDir+"\n") + + threeIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureThreeDir) + twoIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTwoDir) + oneIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureOneDir) + require.Less(t, threeIdx, twoIdx, "three should be installed before two") + require.Less(t, twoIdx, oneIdx, "two should be installed before one") + }) + + t.Run("UnknownOverrideEntryIgnored", func(t *testing.T) { + // An entry in overrideFeatureInstallOrder that doesn't match any + // feature key should be silently ignored. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureOne + `": {}, + "` + featureTwo + `": {} + }, + "overrideFeatureInstallOrder": ["does-not-exist", "` + featureTwo + `"] +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + twoIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTwoDir) + oneIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureOneDir) + require.Less(t, twoIdx, oneIdx, "two should be installed before one") + }) +} + +func TestCompileWithFeaturesInstallsAfter(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + + // featureBase has no deps. + featureBase := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/base:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "base", + Version: "1.0.0", + Name: "Base", + }, + }) + // featureTop declares installsAfter: ["base"], so it must come after featureBase. + featureTop := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/top:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "top", + Version: "1.0.0", + Name: "Top", + InstallsAfter: []string{"base"}, + }, + }) + + featureBaseMD5 := md5.Sum([]byte(featureBase)) + featureBaseDir := fmt.Sprintf("/.envbuilder/features/base-%x", featureBaseMD5[:4]) + featureTopMD5 := md5.Sum([]byte(featureTop)) + featureTopDir := fmt.Sprintf("/.envbuilder/features/top-%x", featureTopMD5[:4]) + + t.Run("InstallsAfterRespected", func(t *testing.T) { + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureTop + `": {}, + "` + featureBase + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + baseIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureBaseDir) + topIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTopDir) + require.Greater(t, baseIdx, -1, "base feature should be present") + require.Greater(t, topIdx, -1, "top feature should be present") + require.Less(t, baseIdx, topIdx, "base should be installed before top (installsAfter)") + }) + + t.Run("InstallsAfterAbsentDepIgnored", func(t *testing.T) { + // featureTop declares installsAfter: ["base"], but base is not in features. + // This is a soft dep โ€” should succeed with just featureTop. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureTop + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err, "absent installsAfter dep should not cause an error") + }) + + t.Run("OverrideWinsOverInstallsAfter", func(t *testing.T) { + // overrideFeatureInstallOrder forces top before base, contradicting + // top's installsAfter declaration. Override takes precedence. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureTop + `": {}, + "` + featureBase + `": {} + }, + "overrideFeatureInstallOrder": ["` + featureTop + `", "` + featureBase + `"] +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + topIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTopDir) + baseIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureBaseDir) + require.Less(t, topIdx, baseIdx, "override should force top before base") + }) +} + +func TestCompileWithFeaturesDependsOn(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + + featureA := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/a:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "a", + Version: "1.0.0", + Name: "A", + }, + }) + // featureB hard-depends on featureA. + featureB := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/b:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "b", + Version: "1.0.0", + Name: "B", + DependsOn: []string{"a"}, + }, + }) + + t.Run("DependsOnSatisfied", func(t *testing.T) { + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureA + `": {}, + "` + featureB + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + }) + + t.Run("DependsOnMissingErrors", func(t *testing.T) { + // featureB requires featureA, but featureA is not included. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureB + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.ErrorContains(t, err, `requires feature "a"`) + }) +} + +func TestResolveInstallOrderCycleDetection(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + + // featureX installsAfter featureY, featureY installsAfter featureX โ€” a cycle. + featureX := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/x:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "x", + Version: "1.0.0", + Name: "X", + InstallsAfter: []string{"y"}, + }, + }) + featureY := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/y:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "y", + Version: "1.0.0", + Name: "Y", + InstallsAfter: []string{"x"}, + }, + }) + + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureX + `": {}, + "` + featureY + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.ErrorContains(t, err, "cycle detected") +} + +func TestCompileDevContainer(t *testing.T) { + t.Parallel() + t.Run("WithImage", func(t *testing.T) { + t.Parallel() + fs := memfs.New() + dc := &devcontainer.Spec{ + Image: "localhost:5000/envbuilder-test-ubuntu:latest", + } + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + require.Equal(t, filepath.Join(workingDir, "Dockerfile"), params.DockerfilePath) + require.Equal(t, workingDir, params.BuildContext) + }) + t.Run("WithBuild", func(t *testing.T) { + t.Parallel() + fs := memfs.New() + dc := &devcontainer.Spec{ + Build: devcontainer.BuildSpec{ + Dockerfile: "Dockerfile", + Context: ".", + Args: map[string]string{ + "ARG1": "value1", + "ARG2": "${localWorkspaceFolderBasename}", + }, + }, + } + dcDir := "/workspaces/coder/.devcontainer" + err := fs.MkdirAll(dcDir, 0o755) + require.NoError(t, err) + file, err := fs.OpenFile(filepath.Join(dcDir, "Dockerfile"), os.O_CREATE|os.O_WRONLY, 0o644) + require.NoError(t, err) + _, err = io.WriteString(file, "FROM localhost:5000/envbuilder-test-ubuntu:latest") + require.NoError(t, err) + _ = file.Close() + params, err := dc.Compile(fs, dcDir, workingDir, "", "/var/workspace", false, stubLookupEnv) + require.NoError(t, err) + require.Equal(t, "ARG1=value1", params.BuildArgs[0]) + require.Equal(t, "ARG2=workspace", params.BuildArgs[1]) + require.Equal(t, filepath.Join(dcDir, "Dockerfile"), params.DockerfilePath) + require.Equal(t, dcDir, params.BuildContext) + }) +} + +func TestImageFromDockerfile(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + content string + image string + }{{ + content: "FROM ubuntu", + image: "index.docker.io/library/ubuntu:latest", + }, { + content: "ARG VARIANT=bionic\nFROM ubuntu:$VARIANT", + image: "index.docker.io/library/ubuntu:bionic", + }, { + content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}", + image: "mcr.microsoft.com/devcontainers/python:0-3.10", + }, { + content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-$VARIANT ", + image: "mcr.microsoft.com/devcontainers/python:0-3.10", + }} { + tc := tc + t.Run(tc.image, func(t *testing.T) { + t.Parallel() + ref, err := devcontainer.ImageFromDockerfile(tc.content, nil) + require.NoError(t, err) + require.Equal(t, tc.image, ref.Name()) + }) + } +} + +func TestImageFromDockerfile_BuildArgs(t *testing.T) { + t.Parallel() + + // Test that build args override ARG defaults. + t.Run("OverridesDefault", func(t *testing.T) { + t.Parallel() + content := "ARG VARIANT=3.10\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}" + ref, err := devcontainer.ImageFromDockerfile(content, map[string]string{"VARIANT": "3.11-bookworm"}) + require.NoError(t, err) + require.Equal(t, "mcr.microsoft.com/devcontainers/python:0-3.11-bookworm", ref.Name()) + }) + + // Test that build args supply values for ARGs without defaults. + t.Run("SuppliesArgWithoutDefault", func(t *testing.T) { + t.Parallel() + content := "ARG VARIANT\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}" + ref, err := devcontainer.ImageFromDockerfile(content, map[string]string{"VARIANT": "3.11-bookworm"}) + require.NoError(t, err) + require.Equal(t, "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", ref.Name()) + }) +} + +func TestUserFromDockerfile_BuildArgs(t *testing.T) { + t.Parallel() + + t.Run("SubstitutesARGInFROM", func(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ + Config: v1.Config{ + User: "testuser", + }, + }}) + require.NoError(t, err) + ref := strings.TrimPrefix(registry, "http://") + "/coder/test:latest" + parsed, err := name.ParseReference(ref) + require.NoError(t, err) + err = remote.Write(parsed, image) + require.NoError(t, err) + + // Dockerfile uses ARG without default for the image ref. + content := fmt.Sprintf("ARG TAG\nFROM %s/coder/test:${TAG}", strings.TrimPrefix(registry, "http://")) + user, err := devcontainer.UserFromDockerfile(content, map[string]string{"TAG": "latest"}) + require.NoError(t, err) + require.Equal(t, "testuser", user) + }) +} + +func TestUserFrom(t *testing.T) { + t.Parallel() + + t.Run("Image", func(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ + Config: v1.Config{ + User: "example", + }, + }}) + require.NoError(t, err) + + parsed, err := url.Parse("http://" + registry) + require.NoError(t, err) + parsed.Path = "coder/test:latest" + ref, err := name.ParseReference(strings.TrimPrefix(parsed.String(), "http://")) + require.NoError(t, err) + err = remote.Write(ref, image) + require.NoError(t, err) + + user, err := devcontainer.UserFromImage(ref) + require.NoError(t, err) + require.Equal(t, "example", user) + }) + + t.Run("Dockerfile", func(t *testing.T) { + t.Parallel() + tests := []struct { + name string + content string + user string + }{ + { + name: "Empty", + content: "FROM scratch", + user: "", + }, + { + name: "User", + content: "FROM scratch\nUSER kyle", + user: "kyle", + }, + { + name: "Env with default", + content: "FROM scratch\nENV MYUSER=maf\nUSER ${MYUSER}", + user: "${MYUSER}", // This should be "maf" but the current implementation doesn't support this. + }, + { + name: "Env var with default", + content: "FROM scratch\nUSER ${MYUSER:-maf}", + user: "${MYUSER:-maf}", // This should be "maf" but the current implementation doesn't support this. + }, + { + name: "Arg", + content: "FROM scratch\nARG MYUSER\nUSER ${MYUSER}", + user: "${MYUSER}", // This should be "" or populated but the current implementation doesn't support this. + }, + { + name: "Arg with default", + content: "FROM scratch\nARG MYUSER=maf\nUSER ${MYUSER}", + user: "${MYUSER}", // This should be "maf" but the current implementation doesn't support this. + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + user, err := devcontainer.UserFromDockerfile(tt.content, nil) + require.NoError(t, err) + require.Equal(t, tt.user, user) + }) + } + }) + + t.Run("Multi-stage", func(t *testing.T) { + t.Parallel() + + registry := registrytest.New(t) + for tag, user := range map[string]string{ + "one": "maf", + "two": "fam", + } { + image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ + Config: v1.Config{ + User: user, + }, + }}) + require.NoError(t, err) + parsed, err := url.Parse("http://" + registry) + require.NoError(t, err) + parsed.Path = "coder/test:" + tag + ref, err := name.ParseReference(strings.TrimPrefix(parsed.String(), "http://")) + fmt.Println(ref) + require.NoError(t, err) + err = remote.Write(ref, image) + require.NoError(t, err) + } + + tests := []struct { + name string + images map[string]string + content string + user string + }{ + { + name: "Single", + content: "FROM coder/test:one", + user: "maf", + }, + { + name: "Multi", + content: "FROM ubuntu AS u\nFROM coder/test:two", + user: "fam", + }, + { + name: "Multi-2", + content: "FROM coder/test:two AS two\nUSER maffam\nFROM coder/test:one AS one", + user: "maf", + }, + { + name: "Multi-3", + content: "FROM coder/test:two AS two\nFROM coder/test:one AS one\nUSER fammaf", + user: "fammaf", + }, + { + name: "Multi-4", + content: `FROM ubuntu AS a +USER root +RUN useradd --create-home pickme +USER pickme +FROM a AS other +USER root +RUN useradd --create-home notme +USER notme +FROM a`, + user: "pickme", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + content := strings.ReplaceAll(tt.content, "coder/test", strings.TrimPrefix(registry, "http://")+"/coder/test") + + user, err := devcontainer.UserFromDockerfile(content, nil) + require.NoError(t, err) + require.Equal(t, tt.user, user) + }) + } + }) +} + +type emptyImage struct { + configFile *v1.ConfigFile +} + +func (i emptyImage) MediaType() (types.MediaType, error) { + return types.DockerManifestSchema2, nil +} + +func (i emptyImage) RawConfigFile() ([]byte, error) { + return partial.RawConfigFile(i) +} + +func (i emptyImage) ConfigFile() (*v1.ConfigFile, error) { + return i.configFile, nil +} + +func (i emptyImage) LayerByDiffID(h v1.Hash) (partial.UncompressedLayer, error) { + return nil, fmt.Errorf("LayerByDiffID(%s): empty image", h) +} diff --git a/devcontainer/features/features.go b/devcontainer/features/features.go index bb044d5f..15d38539 100644 --- a/devcontainer/features/features.go +++ b/devcontainer/features/features.go @@ -188,6 +188,15 @@ type Spec struct { Keywords []string `json:"keywords"` Options map[string]Option `json:"options"` ContainerEnv map[string]string `json:"containerEnv"` + // InstallsAfter is a soft ordering hint: this feature prefers to be + // installed after the listed feature IDs or references, but will not + // fail if they are absent. + // See https://containers.dev/implementors/features/#installation-order + InstallsAfter []string `json:"installsAfter"` + // DependsOn is a hard dependency: this feature requires the listed + // feature IDs or references to also be present in the feature set. + // See https://containers.dev/implementors/features/#installation-order + DependsOn []string `json:"dependsOn"` } // Extract unpacks the feature from the image and returns a set of lines diff --git a/docs/devcontainer-spec-support.md b/docs/devcontainer-spec-support.md index 4a54e289..bf358b15 100644 --- a/docs/devcontainer-spec-support.md +++ b/docs/devcontainer-spec-support.md @@ -1,123 +1,123 @@ -# Support for Dev Container Specification - -> Refer to the full Dev Container specification [here](https://containers.dev/implementors/json_reference/) for more information on the below options. - -The symbols in the first column indicate the support status: - -- ๐ŸŸข Fully supported. -- ๐ŸŸ  Partially supported. -- ๐Ÿ”ด Not currently supported. - -The last column indicates any currently existing GitHub issue for tracking support for this feature. -Feel free to [create a new issue](https://github.com/coder/envbuilder/issues/new) if you'd like Envbuilder to support a particular feature. - -## General - -| Status | Name | Description | Known Issues | -| ------ | ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------ | -| ๐Ÿ”ด | `name` | Human-friendly name for the dev container. | - | -| ๐Ÿ”ด | `forwardPorts` | Port forwarding allows exposing container ports to the host, making services accessible. | [#48](https://github.com/coder/envbuilder/issues/48) | -| ๐Ÿ”ด | `portsAttributes` | Set port attributes for a `host:port`. | - | -| ๐Ÿ”ด | `otherPortsAttributes` | Other options for ports not configured using `portsAttributes`. | - | -| ๐ŸŸข | `containerEnv` | Environment variables to set inside the container. | - | -| ๐ŸŸข | `remoteEnv` | Override environment variables for tools, but not the whole container. | - | -| ๐ŸŸข\* | `remoteUser` | Override the user for tools, but not the whole container.
\*_Refer to [choosing a target user](./users.md#choosing-a-target-user), as behaviour may diverge from the spec._ | - | -| ๐ŸŸข\* | `containerUser` | Override the user for all operations run inside the container.
\*_Refer to [choosing a target user](./users.md#choosing-a-target-user), as behaviour may diverge from the spec._ | - | -| ๐Ÿ”ด | `updateRemoteUserUID` | Update the devcontainer UID/GID to match the local user. | - | -| ๐Ÿ”ด | `userEnvProbe` | Shell to use when probing for user environment variables. | - | -| ๐Ÿ”ด | `overrideCommand` | Override the default sleep command to be run by supporting services. | - | -| ๐Ÿ”ด | `shutdownAction` | Action to take when supporting tools are closed or shut down. | - | -| ๐Ÿ”ด | `init` | Adds a tiny init process to the container. | [#221](https://github.com/coder/envbuilder/issues/221) | -| ๐Ÿ”ด | `privileged` | Whether the container should be run in privileged mode. | - | -| ๐Ÿ”ด | `capAdd` | Capabilities to add to the container (for example, `SYS_PTRACE`). | - | -| ๐Ÿ”ด | `securityOpt` | Security options to add to the container (for example, `seccomp=unconfined`). | - | -| ๐Ÿ”ด | `mounts` | Add additional mounts to the container. | [#220](https://github.com/coder/envbuilder/issues/220) | -| ๐ŸŸข | `features` | Features to be added to the devcontainer. | - | -| ๏ฟฝ | `overrideFeatureInstallOrder` | Override the order in which features should be installed. | - | -| ๐ŸŸ  | `customizations` | Product-specific properties, e.g., _VS Code_ settings and extensions. | Workaround in [#43](https://github.com/coder/envbuilder/issues/43) | - -## Image or Dockerfile - -| Status | Name | Description | Known Issues | -| ------ | ------------------ | ------------------------------------------------------------------------------------------------------------- | ------------ | -| ๐ŸŸข | `image` | Name of an image to run. | - | -| ๐ŸŸข | `build.dockerfile` | Path to a Dockerfile to build relative to `devcontainer.json`. | - | -| ๐ŸŸข | `build.context` | Path to the build context relative to `devcontainer.json`. | - | -| ๐ŸŸข | `build.args` | Build args to use when building the Dockerfile. | - | -| ๐Ÿ”ด | `build.options` | Build options to pass to the Docker daemon. Envbuilder does not use a Docker daemon, so this is not relevant. | - | -| ๐ŸŸข | `build.target` | Target to be passed when building the Dockerfile. | - | -| ๐ŸŸข | `build.cacheFrom` | Images to use as caches when building the Dockerfile. | - | -| ๐Ÿ”ด | `appPort` | Ports to be published locally when the container is running. | - | -| ๐Ÿ”ด | `workspaceMount` | Overrides the default local mount point for the workspace when the container is created. | - | -| ๐Ÿ”ด | `workspaceFolder` | Default path to open when connecting to the container. | - | - -## Docker Compose - -| Status | Name | Description | Known Issues | -| ------ | ------------------- | ---------------------------------------------------------------------------- | ------------------------------------------------------ | -| ๐Ÿ”ด | `dockerComposeFile` | Path to a Docker Compose file related to the `devcontainer.json`. | [#236](https://github.com/coder/envbuilder/issues/236) | -| ๐Ÿ”ด | `service` | Name of the Docker Compose service to which supporting tools should connect. | [#236](https://github.com/coder/envbuilder/issues/236) | -| ๐Ÿ”ด | `runServices` | Docker Compose services to automatically start. | [#236](https://github.com/coder/envbuilder/issues/236) | - -## Lifecycle Scripts - -| Status | Name | Description | Known Issues | -| ------ | ---------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------ | -| ๐Ÿ”ด | `initializeCommand` | Command to run on the host machine when creating the container. | [#395](https://github.com/coder/envbuilder/issues/395) | -| ๐ŸŸข | `onCreateCommand` | Command to run inside container after first start. | | -| ๐ŸŸข | `updateContentCommand` | Command to run after `onCreateCommand` inside container. | | -| ๐ŸŸข | `postCreateCommand` | Command to run after `updateContentCommand` inside container. | | -| ๐ŸŸข\* | `postStartCommand` | Command to run each time the container is started.
\*_This may be specified by `ENVBUILDER_POST_START_SCRIPT`, in which case it is the responsibility of `ENVBUILDER_INIT_COMMAND` to run it._ | | -| ๐Ÿ”ด | `postAttachCommand` | Command to run each time a tool attaches to the container. | | -| ๐Ÿ”ด | `waitFor` | Specify the lifecycle command tools should wait to complete before connecting. | | - -## Minimum Host Requirements - -| Status | Name | Description | Known Issues | -| ------ | -------------------------- | -------------------------------- | ------------ | -| ๐Ÿ”ด | `hostRequirements.cpus` | Minimum number of CPUs required. | - | -| ๐Ÿ”ด | `hostRequirements.memory` | Minimum memory requirements. | - | -| ๐Ÿ”ด | `hostRequirements.storage` | Minimum storage requirements. | - | -| ๐Ÿ”ด | `hostRequirements.gpu` | Whether a GPU is required. | - | - -## Variable Substitution - -| Status | Name | Description | Known Issues | -| ------ | ------------------------------------- | --------------------------------------------------- | ------------ | -| ๐ŸŸข | `${localEnv:VARIABLE_NAME}` | Environment variable on the host machine. | - | -| ๐ŸŸข | `${containerEnv:VARIABLE_NAME}` | Existing environment variable inside the container. | - | -| ๐ŸŸข | `${localWorkspaceFolder}` | Path to the local workspace folder. | - | -| ๐ŸŸข | `${containerWorkspaceFolder}` | Path to the workspace folder inside the container. | - | -| ๐ŸŸข | `${localWorkspaceFolderBasename}` | Base name of `localWorkspaceFolder`. | - | -| ๐ŸŸข | `${containerWorkspaceFolderBasename}` | Base name of `containerWorkspaceFolder`. | - | -| ๐Ÿ”ด | `${devcontainerId}` | A stable unique identifier for the devcontainer. | - | - -## Features - -| Status | Name | Description | Known Issues | -| ------ | ------------------------ | ------------------------------------------------------------ | ------------ | -| ๐ŸŸข | `id` | Feature identifier | - | -| ๏ฟฝ | `version` | Feature version | - | -| ๐ŸŸข | `name` | Feature version | - | -| ๐ŸŸข | `description` | Description | - | -| ๐ŸŸข | `documentationURL` | Feature documentation URL | - | -| ๐ŸŸข | `licenseURL` | Feature license URL | - | -| ๐ŸŸข | `keywords` | Feature keywords | - | -| ๐ŸŸข | `options` | Map of options passed to the feature | - | -| ๐ŸŸข | `options[*].type` | Types of the option | - | -| ๐ŸŸข | `options[*].proposals` | Suggested values of the option | - | -| ๐ŸŸข | `options[*].enum` | Allowed string values of the option | - | -| ๐ŸŸข | `options[*].default` | Default value of the option | - | -| ๐ŸŸข | `options[*].description` | Description of the option | - | -| ๐ŸŸข | `containerEnv` | Environment variables to override | - | -| ๐Ÿ”ด | `privileged` | Set privileged mode for the container if the feature is used | - | -| ๐Ÿ”ด | `init` | Add `tiny init` when the feature is used | - | -| ๐Ÿ”ด | `capAdd` | Capabilities to add when the feature is used | - | -| ๐Ÿ”ด | `securityOpt` | Security options to add when the feature is used | - | -| ๐Ÿ”ด | `entrypoint` | Override entrypoint when the feature is used | - | -| ๐Ÿ”ด | `customizations` | Product-specific properties to add when the feature is used | - | -| ๐Ÿ”ด | `dependsOn` | Define a hard dependency on other features | - | -| ๐Ÿ”ด | `installsAfter` | Define a soft dependency on other features | - | -| ๐Ÿ”ด | `legacyIds` | Used when renaming a feature | - | -| ๐Ÿ”ด | `deprecated` | Whether the feature is deprecated | - | -| ๐Ÿ”ด | `mounts` | Cross-orchestrator mounts to add to the container | - | +# Support for Dev Container Specification + +> Refer to the full Dev Container specification [here](https://containers.dev/implementors/json_reference/) for more information on the below options. + +The symbols in the first column indicate the support status: + +- ๐ŸŸข Fully supported. +- ๐ŸŸ  Partially supported. +- ๐Ÿ”ด Not currently supported. + +The last column indicates any currently existing GitHub issue for tracking support for this feature. +Feel free to [create a new issue](https://github.com/coder/envbuilder/issues/new) if you'd like Envbuilder to support a particular feature. + +## General + +| Status | Name | Description | Known Issues | +| ------ | ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------ | +| ๐Ÿ”ด | `name` | Human-friendly name for the dev container. | - | +| ๐Ÿ”ด | `forwardPorts` | Port forwarding allows exposing container ports to the host, making services accessible. | [#48](https://github.com/coder/envbuilder/issues/48) | +| ๐Ÿ”ด | `portsAttributes` | Set port attributes for a `host:port`. | - | +| ๐Ÿ”ด | `otherPortsAttributes` | Other options for ports not configured using `portsAttributes`. | - | +| ๐ŸŸข | `containerEnv` | Environment variables to set inside the container. | - | +| ๐ŸŸข | `remoteEnv` | Override environment variables for tools, but not the whole container. | - | +| ๐ŸŸข\* | `remoteUser` | Override the user for tools, but not the whole container.
\*_Refer to [choosing a target user](./users.md#choosing-a-target-user), as behaviour may diverge from the spec._ | - | +| ๐ŸŸข\* | `containerUser` | Override the user for all operations run inside the container.
\*_Refer to [choosing a target user](./users.md#choosing-a-target-user), as behaviour may diverge from the spec._ | - | +| ๐Ÿ”ด | `updateRemoteUserUID` | Update the devcontainer UID/GID to match the local user. | - | +| ๐Ÿ”ด | `userEnvProbe` | Shell to use when probing for user environment variables. | - | +| ๐Ÿ”ด | `overrideCommand` | Override the default sleep command to be run by supporting services. | - | +| ๐Ÿ”ด | `shutdownAction` | Action to take when supporting tools are closed or shut down. | - | +| ๐Ÿ”ด | `init` | Adds a tiny init process to the container. | [#221](https://github.com/coder/envbuilder/issues/221) | +| ๐Ÿ”ด | `privileged` | Whether the container should be run in privileged mode. | - | +| ๐Ÿ”ด | `capAdd` | Capabilities to add to the container (for example, `SYS_PTRACE`). | - | +| ๐Ÿ”ด | `securityOpt` | Security options to add to the container (for example, `seccomp=unconfined`). | - | +| ๐Ÿ”ด | `mounts` | Add additional mounts to the container. | [#220](https://github.com/coder/envbuilder/issues/220) | +| ๐ŸŸข | `features` | Features to be added to the devcontainer. | - | +| ๏ฟฝ | `overrideFeatureInstallOrder` | Override the order in which features should be installed. | - | +| ๐ŸŸ  | `customizations` | Product-specific properties, e.g., _VS Code_ settings and extensions. | Workaround in [#43](https://github.com/coder/envbuilder/issues/43) | + +## Image or Dockerfile + +| Status | Name | Description | Known Issues | +| ------ | ------------------ | ------------------------------------------------------------------------------------------------------------- | ------------ | +| ๐ŸŸข | `image` | Name of an image to run. | - | +| ๐ŸŸข | `build.dockerfile` | Path to a Dockerfile to build relative to `devcontainer.json`. | - | +| ๐ŸŸข | `build.context` | Path to the build context relative to `devcontainer.json`. | - | +| ๐ŸŸข | `build.args` | Build args to use when building the Dockerfile. | - | +| ๐Ÿ”ด | `build.options` | Build options to pass to the Docker daemon. Envbuilder does not use a Docker daemon, so this is not relevant. | - | +| ๐ŸŸข | `build.target` | Target to be passed when building the Dockerfile. | - | +| ๐ŸŸข | `build.cacheFrom` | Images to use as caches when building the Dockerfile. | - | +| ๐Ÿ”ด | `appPort` | Ports to be published locally when the container is running. | - | +| ๐Ÿ”ด | `workspaceMount` | Overrides the default local mount point for the workspace when the container is created. | - | +| ๐Ÿ”ด | `workspaceFolder` | Default path to open when connecting to the container. | - | + +## Docker Compose + +| Status | Name | Description | Known Issues | +| ------ | ------------------- | ---------------------------------------------------------------------------- | ------------------------------------------------------ | +| ๐Ÿ”ด | `dockerComposeFile` | Path to a Docker Compose file related to the `devcontainer.json`. | [#236](https://github.com/coder/envbuilder/issues/236) | +| ๐Ÿ”ด | `service` | Name of the Docker Compose service to which supporting tools should connect. | [#236](https://github.com/coder/envbuilder/issues/236) | +| ๐Ÿ”ด | `runServices` | Docker Compose services to automatically start. | [#236](https://github.com/coder/envbuilder/issues/236) | + +## Lifecycle Scripts + +| Status | Name | Description | Known Issues | +| ------ | ---------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------ | +| ๐Ÿ”ด | `initializeCommand` | Command to run on the host machine when creating the container. | [#395](https://github.com/coder/envbuilder/issues/395) | +| ๐ŸŸข | `onCreateCommand` | Command to run inside container after first start. | | +| ๐ŸŸข | `updateContentCommand` | Command to run after `onCreateCommand` inside container. | | +| ๐ŸŸข | `postCreateCommand` | Command to run after `updateContentCommand` inside container. | | +| ๐ŸŸข\* | `postStartCommand` | Command to run each time the container is started.
\*_This may be specified by `ENVBUILDER_POST_START_SCRIPT`, in which case it is the responsibility of `ENVBUILDER_INIT_COMMAND` to run it._ | | +| ๐Ÿ”ด | `postAttachCommand` | Command to run each time a tool attaches to the container. | | +| ๐Ÿ”ด | `waitFor` | Specify the lifecycle command tools should wait to complete before connecting. | | + +## Minimum Host Requirements + +| Status | Name | Description | Known Issues | +| ------ | -------------------------- | -------------------------------- | ------------ | +| ๐Ÿ”ด | `hostRequirements.cpus` | Minimum number of CPUs required. | - | +| ๐Ÿ”ด | `hostRequirements.memory` | Minimum memory requirements. | - | +| ๐Ÿ”ด | `hostRequirements.storage` | Minimum storage requirements. | - | +| ๐Ÿ”ด | `hostRequirements.gpu` | Whether a GPU is required. | - | + +## Variable Substitution + +| Status | Name | Description | Known Issues | +| ------ | ------------------------------------- | --------------------------------------------------- | ------------ | +| ๐ŸŸข | `${localEnv:VARIABLE_NAME}` | Environment variable on the host machine. | - | +| ๐ŸŸข | `${containerEnv:VARIABLE_NAME}` | Existing environment variable inside the container. | - | +| ๐ŸŸข | `${localWorkspaceFolder}` | Path to the local workspace folder. | - | +| ๐ŸŸข | `${containerWorkspaceFolder}` | Path to the workspace folder inside the container. | - | +| ๐ŸŸข | `${localWorkspaceFolderBasename}` | Base name of `localWorkspaceFolder`. | - | +| ๐ŸŸข | `${containerWorkspaceFolderBasename}` | Base name of `containerWorkspaceFolder`. | - | +| ๐Ÿ”ด | `${devcontainerId}` | A stable unique identifier for the devcontainer. | - | + +## Features + +| Status | Name | Description | Known Issues | +| ------ | ------------------------ | ------------------------------------------------------------ | ------------ | +| ๐ŸŸข | `id` | Feature identifier | - | +| ๏ฟฝ | `version` | Feature version | - | +| ๐ŸŸข | `name` | Feature version | - | +| ๐ŸŸข | `description` | Description | - | +| ๐ŸŸข | `documentationURL` | Feature documentation URL | - | +| ๐ŸŸข | `licenseURL` | Feature license URL | - | +| ๐ŸŸข | `keywords` | Feature keywords | - | +| ๐ŸŸข | `options` | Map of options passed to the feature | - | +| ๐ŸŸข | `options[*].type` | Types of the option | - | +| ๐ŸŸข | `options[*].proposals` | Suggested values of the option | - | +| ๐ŸŸข | `options[*].enum` | Allowed string values of the option | - | +| ๐ŸŸข | `options[*].default` | Default value of the option | - | +| ๐ŸŸข | `options[*].description` | Description of the option | - | +| ๐ŸŸข | `containerEnv` | Environment variables to override | - | +| ๐Ÿ”ด | `privileged` | Set privileged mode for the container if the feature is used | - | +| ๐Ÿ”ด | `init` | Add `tiny init` when the feature is used | - | +| ๐Ÿ”ด | `capAdd` | Capabilities to add when the feature is used | - | +| ๐Ÿ”ด | `securityOpt` | Security options to add when the feature is used | - | +| ๐Ÿ”ด | `entrypoint` | Override entrypoint when the feature is used | - | +| ๐Ÿ”ด | `customizations` | Product-specific properties to add when the feature is used | - | +| ๏ฟฝ | `dependsOn` | Define a hard dependency on other features | - | +| ๏ฟฝ | `installsAfter` | Define a soft dependency on other features | - | +| ๐Ÿ”ด | `legacyIds` | Used when renaming a feature | - | +| ๐Ÿ”ด | `deprecated` | Whether the feature is deprecated | - | +| ๐Ÿ”ด | `mounts` | Cross-orchestrator mounts to add to the container | - | From ee3db529e3387395a04b1463d5b9cf6cfb20478b Mon Sep 17 00:00:00 2001 From: Pablo Ulloa Date: Mon, 9 Mar 2026 22:15:26 +0000 Subject: [PATCH 3/4] test(devcontainer): refactor and expand test suite --- devcontainer/devcontainer_test.go | 1438 ++++++++++++++--------------- 1 file changed, 719 insertions(+), 719 deletions(-) diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index 99a9ab4a..ac130733 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -1,719 +1,719 @@ -package devcontainer_test - -import ( - "crypto/md5" - "fmt" - "io" - "net/url" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/coder/envbuilder/devcontainer" - "github.com/coder/envbuilder/devcontainer/features" - "github.com/coder/envbuilder/testutil/registrytest" - "github.com/go-git/go-billy/v5/memfs" - "github.com/google/go-containerregistry/pkg/name" - v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1/partial" - "github.com/google/go-containerregistry/pkg/v1/remote" - "github.com/google/go-containerregistry/pkg/v1/types" - "github.com/stretchr/testify/require" -) - -const workingDir = "/.envbuilder" - -var emptyRemoteOpts []remote.Option - -func stubLookupEnv(string) (string, bool) { - return "", false -} - -func TestParse(t *testing.T) { - t.Parallel() - raw := `{ - "build": { - "dockerfile": "Dockerfile", - "context": ".", - }, - // Comments here! - "image": "codercom/code-server:latest" -}` - parsed, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - require.Equal(t, "Dockerfile", parsed.Build.Dockerfile) -} - -func TestCompileWithFeatures(t *testing.T) { - t.Parallel() - registry := registrytest.New(t) - featureOne := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/one:tomato", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "rust", - Version: "tomato", - Name: "Rust", - Description: "Example description!", - ContainerEnv: map[string]string{ - "TOMATO": "example", - }, - }, - }) - featureTwo := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/two:potato", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "go", - Version: "potato", - Name: "Go", - Description: "Example description!", - ContainerEnv: map[string]string{ - "POTATO": "example", - }, - Options: map[string]features.Option{ - "version": { - Type: "string", - }, - }, - }, - }) - - raw := `{ - "build": { - "dockerfile": "Dockerfile", - "context": ".", - }, - // Comments here! - "image": "localhost:5000/envbuilder-test-codercom-code-server:latest", - "features": { - "` + featureOne + `": {}, - "` + featureTwo + `": "potato" - } -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - featureOneMD5 := md5.Sum([]byte(featureOne)) - featureOneDir := fmt.Sprintf("/.envbuilder/features/one-%x", featureOneMD5[:4]) - featureTwoMD5 := md5.Sum([]byte(featureTwo)) - featureTwoDir := fmt.Sprintf("/.envbuilder/features/two-%x", featureTwoMD5[:4]) - - t.Run("WithoutBuildContexts", func(t *testing.T) { - params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err) - - require.Equal(t, `FROM localhost:5000/envbuilder-test-codercom-code-server:latest - -USER root -# Rust tomato - Example description! -WORKDIR `+featureOneDir+` -ENV TOMATO=example -RUN _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh -# Go potato - Example description! -WORKDIR `+featureTwoDir+` -ENV POTATO=example -RUN VERSION="potato" _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh -USER 1000`, params.DockerfileContent) - }) - - t.Run("WithBuildContexts", func(t *testing.T) { - params, err := dc.Compile(fs, "", workingDir, "", "", true, stubLookupEnv) - require.NoError(t, err) - - registryHost := strings.TrimPrefix(registry, "http://") - - require.Equal(t, `FROM scratch AS envbuilder_feature_one -COPY --from=`+registryHost+`/coder/one / / - -FROM scratch AS envbuilder_feature_two -COPY --from=`+registryHost+`/coder/two / / - -FROM localhost:5000/envbuilder-test-codercom-code-server:latest - -USER root -# Rust tomato - Example description! -WORKDIR /.envbuilder/features/one -ENV TOMATO=example -RUN --mount=type=bind,from=envbuilder_feature_one,target=/.envbuilder/features/one,rw _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh -# Go potato - Example description! -WORKDIR /.envbuilder/features/two -ENV POTATO=example -RUN --mount=type=bind,from=envbuilder_feature_two,target=/.envbuilder/features/two,rw VERSION="potato" _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh -USER 1000`, params.DockerfileContent) - - require.Equal(t, map[string]string{ - registryHost + "/coder/one": featureOneDir, - registryHost + "/coder/two": featureTwoDir, - }, params.FeatureContexts) - }) -} - -func TestCompileWithFeaturesOverrideInstallOrder(t *testing.T) { - t.Parallel() - registry := registrytest.New(t) - featureOne := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/one:tomato", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "one", - Version: "tomato", - Name: "One", - }, - }) - featureTwo := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/two:potato", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "two", - Version: "potato", - Name: "Two", - }, - }) - featureThree := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/three:apple", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "three", - Version: "apple", - Name: "Three", - }, - }) - - featureOneMD5 := md5.Sum([]byte(featureOne)) - featureOneDir := fmt.Sprintf("/.envbuilder/features/one-%x", featureOneMD5[:4]) - featureTwoMD5 := md5.Sum([]byte(featureTwo)) - featureTwoDir := fmt.Sprintf("/.envbuilder/features/two-%x", featureTwoMD5[:4]) - featureThreeMD5 := md5.Sum([]byte(featureThree)) - featureThreeDir := fmt.Sprintf("/.envbuilder/features/three-%x", featureThreeMD5[:4]) - - t.Run("OverrideReverseOrder", func(t *testing.T) { - // featureThree then featureTwo are explicitly ordered first; featureOne - // is unconstrained and falls to the alphabetical remainder. - raw := `{ - "image": "localhost:5000/envbuilder-test-ubuntu:latest", - "features": { - "` + featureOne + `": {}, - "` + featureTwo + `": {}, - "` + featureThree + `": {} - }, - "overrideFeatureInstallOrder": ["` + featureThree + `", "` + featureTwo + `"] -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err) - - // featureThree and featureTwo come first (in override order), - // then featureOne last (alphabetical remainder). - require.Contains(t, params.DockerfileContent, "WORKDIR "+featureThreeDir+"\n") - require.Contains(t, params.DockerfileContent, "WORKDIR "+featureTwoDir+"\n") - require.Contains(t, params.DockerfileContent, "WORKDIR "+featureOneDir+"\n") - - threeIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureThreeDir) - twoIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTwoDir) - oneIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureOneDir) - require.Less(t, threeIdx, twoIdx, "three should be installed before two") - require.Less(t, twoIdx, oneIdx, "two should be installed before one") - }) - - t.Run("UnknownOverrideEntryIgnored", func(t *testing.T) { - // An entry in overrideFeatureInstallOrder that doesn't match any - // feature key should be silently ignored. - raw := `{ - "image": "localhost:5000/envbuilder-test-ubuntu:latest", - "features": { - "` + featureOne + `": {}, - "` + featureTwo + `": {} - }, - "overrideFeatureInstallOrder": ["does-not-exist", "` + featureTwo + `"] -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err) - - twoIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTwoDir) - oneIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureOneDir) - require.Less(t, twoIdx, oneIdx, "two should be installed before one") - }) -} - -func TestCompileWithFeaturesInstallsAfter(t *testing.T) { - t.Parallel() - registry := registrytest.New(t) - - // featureBase has no deps. - featureBase := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/base:latest", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "base", - Version: "1.0.0", - Name: "Base", - }, - }) - // featureTop declares installsAfter: ["base"], so it must come after featureBase. - featureTop := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/top:latest", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "top", - Version: "1.0.0", - Name: "Top", - InstallsAfter: []string{"base"}, - }, - }) - - featureBaseMD5 := md5.Sum([]byte(featureBase)) - featureBaseDir := fmt.Sprintf("/.envbuilder/features/base-%x", featureBaseMD5[:4]) - featureTopMD5 := md5.Sum([]byte(featureTop)) - featureTopDir := fmt.Sprintf("/.envbuilder/features/top-%x", featureTopMD5[:4]) - - t.Run("InstallsAfterRespected", func(t *testing.T) { - raw := `{ - "image": "localhost:5000/envbuilder-test-ubuntu:latest", - "features": { - "` + featureTop + `": {}, - "` + featureBase + `": {} - } -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err) - - baseIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureBaseDir) - topIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTopDir) - require.Greater(t, baseIdx, -1, "base feature should be present") - require.Greater(t, topIdx, -1, "top feature should be present") - require.Less(t, baseIdx, topIdx, "base should be installed before top (installsAfter)") - }) - - t.Run("InstallsAfterAbsentDepIgnored", func(t *testing.T) { - // featureTop declares installsAfter: ["base"], but base is not in features. - // This is a soft dep โ€” should succeed with just featureTop. - raw := `{ - "image": "localhost:5000/envbuilder-test-ubuntu:latest", - "features": { - "` + featureTop + `": {} - } -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err, "absent installsAfter dep should not cause an error") - }) - - t.Run("OverrideWinsOverInstallsAfter", func(t *testing.T) { - // overrideFeatureInstallOrder forces top before base, contradicting - // top's installsAfter declaration. Override takes precedence. - raw := `{ - "image": "localhost:5000/envbuilder-test-ubuntu:latest", - "features": { - "` + featureTop + `": {}, - "` + featureBase + `": {} - }, - "overrideFeatureInstallOrder": ["` + featureTop + `", "` + featureBase + `"] -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err) - - topIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTopDir) - baseIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureBaseDir) - require.Less(t, topIdx, baseIdx, "override should force top before base") - }) -} - -func TestCompileWithFeaturesDependsOn(t *testing.T) { - t.Parallel() - registry := registrytest.New(t) - - featureA := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/a:latest", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "a", - Version: "1.0.0", - Name: "A", - }, - }) - // featureB hard-depends on featureA. - featureB := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/b:latest", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "b", - Version: "1.0.0", - Name: "B", - DependsOn: []string{"a"}, - }, - }) - - t.Run("DependsOnSatisfied", func(t *testing.T) { - raw := `{ - "image": "localhost:5000/envbuilder-test-ubuntu:latest", - "features": { - "` + featureA + `": {}, - "` + featureB + `": {} - } -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err) - }) - - t.Run("DependsOnMissingErrors", func(t *testing.T) { - // featureB requires featureA, but featureA is not included. - raw := `{ - "image": "localhost:5000/envbuilder-test-ubuntu:latest", - "features": { - "` + featureB + `": {} - } -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.ErrorContains(t, err, `requires feature "a"`) - }) -} - -func TestResolveInstallOrderCycleDetection(t *testing.T) { - t.Parallel() - registry := registrytest.New(t) - - // featureX installsAfter featureY, featureY installsAfter featureX โ€” a cycle. - featureX := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/x:latest", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "x", - Version: "1.0.0", - Name: "X", - InstallsAfter: []string{"y"}, - }, - }) - featureY := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/y:latest", features.TarLayerMediaType, map[string]any{ - "install.sh": "hey", - "devcontainer-feature.json": features.Spec{ - ID: "y", - Version: "1.0.0", - Name: "Y", - InstallsAfter: []string{"x"}, - }, - }) - - raw := `{ - "image": "localhost:5000/envbuilder-test-ubuntu:latest", - "features": { - "` + featureX + `": {}, - "` + featureY + `": {} - } -}` - dc, err := devcontainer.Parse([]byte(raw)) - require.NoError(t, err) - fs := memfs.New() - - _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.ErrorContains(t, err, "cycle detected") -} - -func TestCompileDevContainer(t *testing.T) { - t.Parallel() - t.Run("WithImage", func(t *testing.T) { - t.Parallel() - fs := memfs.New() - dc := &devcontainer.Spec{ - Image: "localhost:5000/envbuilder-test-ubuntu:latest", - } - params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.NoError(t, err) - require.Equal(t, filepath.Join(workingDir, "Dockerfile"), params.DockerfilePath) - require.Equal(t, workingDir, params.BuildContext) - }) - t.Run("WithBuild", func(t *testing.T) { - t.Parallel() - fs := memfs.New() - dc := &devcontainer.Spec{ - Build: devcontainer.BuildSpec{ - Dockerfile: "Dockerfile", - Context: ".", - Args: map[string]string{ - "ARG1": "value1", - "ARG2": "${localWorkspaceFolderBasename}", - }, - }, - } - dcDir := "/workspaces/coder/.devcontainer" - err := fs.MkdirAll(dcDir, 0o755) - require.NoError(t, err) - file, err := fs.OpenFile(filepath.Join(dcDir, "Dockerfile"), os.O_CREATE|os.O_WRONLY, 0o644) - require.NoError(t, err) - _, err = io.WriteString(file, "FROM localhost:5000/envbuilder-test-ubuntu:latest") - require.NoError(t, err) - _ = file.Close() - params, err := dc.Compile(fs, dcDir, workingDir, "", "/var/workspace", false, stubLookupEnv) - require.NoError(t, err) - require.Equal(t, "ARG1=value1", params.BuildArgs[0]) - require.Equal(t, "ARG2=workspace", params.BuildArgs[1]) - require.Equal(t, filepath.Join(dcDir, "Dockerfile"), params.DockerfilePath) - require.Equal(t, dcDir, params.BuildContext) - }) -} - -func TestImageFromDockerfile(t *testing.T) { - t.Parallel() - for _, tc := range []struct { - content string - image string - }{{ - content: "FROM ubuntu", - image: "index.docker.io/library/ubuntu:latest", - }, { - content: "ARG VARIANT=bionic\nFROM ubuntu:$VARIANT", - image: "index.docker.io/library/ubuntu:bionic", - }, { - content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}", - image: "mcr.microsoft.com/devcontainers/python:0-3.10", - }, { - content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-$VARIANT ", - image: "mcr.microsoft.com/devcontainers/python:0-3.10", - }} { - tc := tc - t.Run(tc.image, func(t *testing.T) { - t.Parallel() - ref, err := devcontainer.ImageFromDockerfile(tc.content, nil) - require.NoError(t, err) - require.Equal(t, tc.image, ref.Name()) - }) - } -} - -func TestImageFromDockerfile_BuildArgs(t *testing.T) { - t.Parallel() - - // Test that build args override ARG defaults. - t.Run("OverridesDefault", func(t *testing.T) { - t.Parallel() - content := "ARG VARIANT=3.10\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}" - ref, err := devcontainer.ImageFromDockerfile(content, map[string]string{"VARIANT": "3.11-bookworm"}) - require.NoError(t, err) - require.Equal(t, "mcr.microsoft.com/devcontainers/python:0-3.11-bookworm", ref.Name()) - }) - - // Test that build args supply values for ARGs without defaults. - t.Run("SuppliesArgWithoutDefault", func(t *testing.T) { - t.Parallel() - content := "ARG VARIANT\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}" - ref, err := devcontainer.ImageFromDockerfile(content, map[string]string{"VARIANT": "3.11-bookworm"}) - require.NoError(t, err) - require.Equal(t, "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", ref.Name()) - }) -} - -func TestUserFromDockerfile_BuildArgs(t *testing.T) { - t.Parallel() - - t.Run("SubstitutesARGInFROM", func(t *testing.T) { - t.Parallel() - registry := registrytest.New(t) - image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ - Config: v1.Config{ - User: "testuser", - }, - }}) - require.NoError(t, err) - ref := strings.TrimPrefix(registry, "http://") + "/coder/test:latest" - parsed, err := name.ParseReference(ref) - require.NoError(t, err) - err = remote.Write(parsed, image) - require.NoError(t, err) - - // Dockerfile uses ARG without default for the image ref. - content := fmt.Sprintf("ARG TAG\nFROM %s/coder/test:${TAG}", strings.TrimPrefix(registry, "http://")) - user, err := devcontainer.UserFromDockerfile(content, map[string]string{"TAG": "latest"}) - require.NoError(t, err) - require.Equal(t, "testuser", user) - }) -} - -func TestUserFrom(t *testing.T) { - t.Parallel() - - t.Run("Image", func(t *testing.T) { - t.Parallel() - registry := registrytest.New(t) - image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ - Config: v1.Config{ - User: "example", - }, - }}) - require.NoError(t, err) - - parsed, err := url.Parse("http://" + registry) - require.NoError(t, err) - parsed.Path = "coder/test:latest" - ref, err := name.ParseReference(strings.TrimPrefix(parsed.String(), "http://")) - require.NoError(t, err) - err = remote.Write(ref, image) - require.NoError(t, err) - - user, err := devcontainer.UserFromImage(ref) - require.NoError(t, err) - require.Equal(t, "example", user) - }) - - t.Run("Dockerfile", func(t *testing.T) { - t.Parallel() - tests := []struct { - name string - content string - user string - }{ - { - name: "Empty", - content: "FROM scratch", - user: "", - }, - { - name: "User", - content: "FROM scratch\nUSER kyle", - user: "kyle", - }, - { - name: "Env with default", - content: "FROM scratch\nENV MYUSER=maf\nUSER ${MYUSER}", - user: "${MYUSER}", // This should be "maf" but the current implementation doesn't support this. - }, - { - name: "Env var with default", - content: "FROM scratch\nUSER ${MYUSER:-maf}", - user: "${MYUSER:-maf}", // This should be "maf" but the current implementation doesn't support this. - }, - { - name: "Arg", - content: "FROM scratch\nARG MYUSER\nUSER ${MYUSER}", - user: "${MYUSER}", // This should be "" or populated but the current implementation doesn't support this. - }, - { - name: "Arg with default", - content: "FROM scratch\nARG MYUSER=maf\nUSER ${MYUSER}", - user: "${MYUSER}", // This should be "maf" but the current implementation doesn't support this. - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - user, err := devcontainer.UserFromDockerfile(tt.content, nil) - require.NoError(t, err) - require.Equal(t, tt.user, user) - }) - } - }) - - t.Run("Multi-stage", func(t *testing.T) { - t.Parallel() - - registry := registrytest.New(t) - for tag, user := range map[string]string{ - "one": "maf", - "two": "fam", - } { - image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ - Config: v1.Config{ - User: user, - }, - }}) - require.NoError(t, err) - parsed, err := url.Parse("http://" + registry) - require.NoError(t, err) - parsed.Path = "coder/test:" + tag - ref, err := name.ParseReference(strings.TrimPrefix(parsed.String(), "http://")) - fmt.Println(ref) - require.NoError(t, err) - err = remote.Write(ref, image) - require.NoError(t, err) - } - - tests := []struct { - name string - images map[string]string - content string - user string - }{ - { - name: "Single", - content: "FROM coder/test:one", - user: "maf", - }, - { - name: "Multi", - content: "FROM ubuntu AS u\nFROM coder/test:two", - user: "fam", - }, - { - name: "Multi-2", - content: "FROM coder/test:two AS two\nUSER maffam\nFROM coder/test:one AS one", - user: "maf", - }, - { - name: "Multi-3", - content: "FROM coder/test:two AS two\nFROM coder/test:one AS one\nUSER fammaf", - user: "fammaf", - }, - { - name: "Multi-4", - content: `FROM ubuntu AS a -USER root -RUN useradd --create-home pickme -USER pickme -FROM a AS other -USER root -RUN useradd --create-home notme -USER notme -FROM a`, - user: "pickme", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - content := strings.ReplaceAll(tt.content, "coder/test", strings.TrimPrefix(registry, "http://")+"/coder/test") - - user, err := devcontainer.UserFromDockerfile(content, nil) - require.NoError(t, err) - require.Equal(t, tt.user, user) - }) - } - }) -} - -type emptyImage struct { - configFile *v1.ConfigFile -} - -func (i emptyImage) MediaType() (types.MediaType, error) { - return types.DockerManifestSchema2, nil -} - -func (i emptyImage) RawConfigFile() ([]byte, error) { - return partial.RawConfigFile(i) -} - -func (i emptyImage) ConfigFile() (*v1.ConfigFile, error) { - return i.configFile, nil -} - -func (i emptyImage) LayerByDiffID(h v1.Hash) (partial.UncompressedLayer, error) { - return nil, fmt.Errorf("LayerByDiffID(%s): empty image", h) -} +package devcontainer_test + +import ( + "crypto/md5" + "fmt" + "io" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/coder/envbuilder/devcontainer" + "github.com/coder/envbuilder/devcontainer/features" + "github.com/coder/envbuilder/testutil/registrytest" + "github.com/go-git/go-billy/v5/memfs" + "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/partial" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/types" + "github.com/stretchr/testify/require" +) + +const workingDir = "/.envbuilder" + +var emptyRemoteOpts []remote.Option + +func stubLookupEnv(string) (string, bool) { + return "", false +} + +func TestParse(t *testing.T) { + t.Parallel() + raw := `{ + "build": { + "dockerfile": "Dockerfile", + "context": ".", + }, + // Comments here! + "image": "codercom/code-server:latest" +}` + parsed, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + require.Equal(t, "Dockerfile", parsed.Build.Dockerfile) +} + +func TestCompileWithFeatures(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + featureOne := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/one:tomato", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "rust", + Version: "tomato", + Name: "Rust", + Description: "Example description!", + ContainerEnv: map[string]string{ + "TOMATO": "example", + }, + }, + }) + featureTwo := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/two:potato", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "go", + Version: "potato", + Name: "Go", + Description: "Example description!", + ContainerEnv: map[string]string{ + "POTATO": "example", + }, + Options: map[string]features.Option{ + "version": { + Type: "string", + }, + }, + }, + }) + + raw := `{ + "build": { + "dockerfile": "Dockerfile", + "context": ".", + }, + // Comments here! + "image": "localhost:5000/envbuilder-test-codercom-code-server:latest", + "features": { + "` + featureOne + `": {}, + "` + featureTwo + `": "potato" + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + featureOneMD5 := md5.Sum([]byte(featureOne)) + featureOneDir := fmt.Sprintf("/.envbuilder/features/one-%x", featureOneMD5[:4]) + featureTwoMD5 := md5.Sum([]byte(featureTwo)) + featureTwoDir := fmt.Sprintf("/.envbuilder/features/two-%x", featureTwoMD5[:4]) + + t.Run("WithoutBuildContexts", func(t *testing.T) { + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + require.Equal(t, `FROM localhost:5000/envbuilder-test-codercom-code-server:latest + +USER root +# Rust tomato - Example description! +WORKDIR `+featureOneDir+` +ENV TOMATO=example +RUN _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh +# Go potato - Example description! +WORKDIR `+featureTwoDir+` +ENV POTATO=example +RUN VERSION="potato" _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh +USER 1000`, params.DockerfileContent) + }) + + t.Run("WithBuildContexts", func(t *testing.T) { + params, err := dc.Compile(fs, "", workingDir, "", "", true, stubLookupEnv) + require.NoError(t, err) + + registryHost := strings.TrimPrefix(registry, "http://") + + require.Equal(t, `FROM scratch AS envbuilder_feature_one +COPY --from=`+registryHost+`/coder/one / / + +FROM scratch AS envbuilder_feature_two +COPY --from=`+registryHost+`/coder/two / / + +FROM localhost:5000/envbuilder-test-codercom-code-server:latest + +USER root +# Rust tomato - Example description! +WORKDIR /.envbuilder/features/one +ENV TOMATO=example +RUN --mount=type=bind,from=envbuilder_feature_one,target=/.envbuilder/features/one,rw _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh +# Go potato - Example description! +WORKDIR /.envbuilder/features/two +ENV POTATO=example +RUN --mount=type=bind,from=envbuilder_feature_two,target=/.envbuilder/features/two,rw VERSION="potato" _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh +USER 1000`, params.DockerfileContent) + + require.Equal(t, map[string]string{ + registryHost + "/coder/one": featureOneDir, + registryHost + "/coder/two": featureTwoDir, + }, params.FeatureContexts) + }) +} + +func TestCompileWithFeaturesOverrideInstallOrder(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + featureOne := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/one:tomato", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "one", + Version: "tomato", + Name: "One", + }, + }) + featureTwo := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/two:potato", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "two", + Version: "potato", + Name: "Two", + }, + }) + featureThree := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/three:apple", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "three", + Version: "apple", + Name: "Three", + }, + }) + + featureOneMD5 := md5.Sum([]byte(featureOne)) + featureOneDir := fmt.Sprintf("/.envbuilder/features/one-%x", featureOneMD5[:4]) + featureTwoMD5 := md5.Sum([]byte(featureTwo)) + featureTwoDir := fmt.Sprintf("/.envbuilder/features/two-%x", featureTwoMD5[:4]) + featureThreeMD5 := md5.Sum([]byte(featureThree)) + featureThreeDir := fmt.Sprintf("/.envbuilder/features/three-%x", featureThreeMD5[:4]) + + t.Run("OverrideReverseOrder", func(t *testing.T) { + // featureThree then featureTwo are explicitly ordered first; featureOne + // is unconstrained and falls to the alphabetical remainder. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureOne + `": {}, + "` + featureTwo + `": {}, + "` + featureThree + `": {} + }, + "overrideFeatureInstallOrder": ["` + featureThree + `", "` + featureTwo + `"] +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + // featureThree and featureTwo come first (in override order), + // then featureOne last (alphabetical remainder). + require.Contains(t, params.DockerfileContent, "WORKDIR "+featureThreeDir+"\n") + require.Contains(t, params.DockerfileContent, "WORKDIR "+featureTwoDir+"\n") + require.Contains(t, params.DockerfileContent, "WORKDIR "+featureOneDir+"\n") + + threeIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureThreeDir) + twoIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTwoDir) + oneIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureOneDir) + require.Less(t, threeIdx, twoIdx, "three should be installed before two") + require.Less(t, twoIdx, oneIdx, "two should be installed before one") + }) + + t.Run("UnknownOverrideEntryIgnored", func(t *testing.T) { + // An entry in overrideFeatureInstallOrder that doesn't match any + // feature key should be silently ignored. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureOne + `": {}, + "` + featureTwo + `": {} + }, + "overrideFeatureInstallOrder": ["does-not-exist", "` + featureTwo + `"] +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + twoIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTwoDir) + oneIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureOneDir) + require.Less(t, twoIdx, oneIdx, "two should be installed before one") + }) +} + +func TestCompileWithFeaturesInstallsAfter(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + + // featureBase has no deps. + featureBase := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/base:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "base", + Version: "1.0.0", + Name: "Base", + }, + }) + // featureTop declares installsAfter: ["base"], so it must come after featureBase. + featureTop := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/top:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "top", + Version: "1.0.0", + Name: "Top", + InstallsAfter: []string{"base"}, + }, + }) + + featureBaseMD5 := md5.Sum([]byte(featureBase)) + featureBaseDir := fmt.Sprintf("/.envbuilder/features/base-%x", featureBaseMD5[:4]) + featureTopMD5 := md5.Sum([]byte(featureTop)) + featureTopDir := fmt.Sprintf("/.envbuilder/features/top-%x", featureTopMD5[:4]) + + t.Run("InstallsAfterRespected", func(t *testing.T) { + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureTop + `": {}, + "` + featureBase + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + baseIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureBaseDir) + topIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTopDir) + require.Greater(t, baseIdx, -1, "base feature should be present") + require.Greater(t, topIdx, -1, "top feature should be present") + require.Less(t, baseIdx, topIdx, "base should be installed before top (installsAfter)") + }) + + t.Run("InstallsAfterAbsentDepIgnored", func(t *testing.T) { + // featureTop declares installsAfter: ["base"], but base is not in features. + // This is a soft dep โ€” should succeed with just featureTop. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureTop + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err, "absent installsAfter dep should not cause an error") + }) + + t.Run("OverrideWinsOverInstallsAfter", func(t *testing.T) { + // overrideFeatureInstallOrder forces top before base, contradicting + // top's installsAfter declaration. Override takes precedence. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureTop + `": {}, + "` + featureBase + `": {} + }, + "overrideFeatureInstallOrder": ["` + featureTop + `", "` + featureBase + `"] +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + topIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTopDir) + baseIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureBaseDir) + require.Less(t, topIdx, baseIdx, "override should force top before base") + }) +} + +func TestCompileWithFeaturesDependsOn(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + + featureA := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/a:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "a", + Version: "1.0.0", + Name: "A", + }, + }) + // featureB hard-depends on featureA. + featureB := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/b:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "b", + Version: "1.0.0", + Name: "B", + DependsOn: []string{"a"}, + }, + }) + + t.Run("DependsOnSatisfied", func(t *testing.T) { + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureA + `": {}, + "` + featureB + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + }) + + t.Run("DependsOnMissingErrors", func(t *testing.T) { + // featureB requires featureA, but featureA is not included. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureB + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.ErrorContains(t, err, `requires feature "a"`) + }) +} + +func TestResolveInstallOrderCycleDetection(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + + // featureX installsAfter featureY, featureY installsAfter featureX โ€” a cycle. + featureX := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/x:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "x", + Version: "1.0.0", + Name: "X", + InstallsAfter: []string{"y"}, + }, + }) + featureY := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/y:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "y", + Version: "1.0.0", + Name: "Y", + InstallsAfter: []string{"x"}, + }, + }) + + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureX + `": {}, + "` + featureY + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.ErrorContains(t, err, "cycle detected") +} + +func TestCompileDevContainer(t *testing.T) { + t.Parallel() + t.Run("WithImage", func(t *testing.T) { + t.Parallel() + fs := memfs.New() + dc := &devcontainer.Spec{ + Image: "localhost:5000/envbuilder-test-ubuntu:latest", + } + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + require.Equal(t, filepath.Join(workingDir, "Dockerfile"), params.DockerfilePath) + require.Equal(t, workingDir, params.BuildContext) + }) + t.Run("WithBuild", func(t *testing.T) { + t.Parallel() + fs := memfs.New() + dc := &devcontainer.Spec{ + Build: devcontainer.BuildSpec{ + Dockerfile: "Dockerfile", + Context: ".", + Args: map[string]string{ + "ARG1": "value1", + "ARG2": "${localWorkspaceFolderBasename}", + }, + }, + } + dcDir := "/workspaces/coder/.devcontainer" + err := fs.MkdirAll(dcDir, 0o755) + require.NoError(t, err) + file, err := fs.OpenFile(filepath.Join(dcDir, "Dockerfile"), os.O_CREATE|os.O_WRONLY, 0o644) + require.NoError(t, err) + _, err = io.WriteString(file, "FROM localhost:5000/envbuilder-test-ubuntu:latest") + require.NoError(t, err) + _ = file.Close() + params, err := dc.Compile(fs, dcDir, workingDir, "", "/var/workspace", false, stubLookupEnv) + require.NoError(t, err) + require.Equal(t, "ARG1=value1", params.BuildArgs[0]) + require.Equal(t, "ARG2=workspace", params.BuildArgs[1]) + require.Equal(t, filepath.Join(dcDir, "Dockerfile"), params.DockerfilePath) + require.Equal(t, dcDir, params.BuildContext) + }) +} + +func TestImageFromDockerfile(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + content string + image string + }{{ + content: "FROM ubuntu", + image: "index.docker.io/library/ubuntu:latest", + }, { + content: "ARG VARIANT=bionic\nFROM ubuntu:$VARIANT", + image: "index.docker.io/library/ubuntu:bionic", + }, { + content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}", + image: "mcr.microsoft.com/devcontainers/python:0-3.10", + }, { + content: "ARG VARIANT=\"3.10\"\nFROM mcr.microsoft.com/devcontainers/python:0-$VARIANT ", + image: "mcr.microsoft.com/devcontainers/python:0-3.10", + }} { + tc := tc + t.Run(tc.image, func(t *testing.T) { + t.Parallel() + ref, err := devcontainer.ImageFromDockerfile(tc.content, nil) + require.NoError(t, err) + require.Equal(t, tc.image, ref.Name()) + }) + } +} + +func TestImageFromDockerfile_BuildArgs(t *testing.T) { + t.Parallel() + + // Test that build args override ARG defaults. + t.Run("OverridesDefault", func(t *testing.T) { + t.Parallel() + content := "ARG VARIANT=3.10\nFROM mcr.microsoft.com/devcontainers/python:0-${VARIANT}" + ref, err := devcontainer.ImageFromDockerfile(content, map[string]string{"VARIANT": "3.11-bookworm"}) + require.NoError(t, err) + require.Equal(t, "mcr.microsoft.com/devcontainers/python:0-3.11-bookworm", ref.Name()) + }) + + // Test that build args supply values for ARGs without defaults. + t.Run("SuppliesArgWithoutDefault", func(t *testing.T) { + t.Parallel() + content := "ARG VARIANT\nFROM mcr.microsoft.com/devcontainers/python:1-${VARIANT}" + ref, err := devcontainer.ImageFromDockerfile(content, map[string]string{"VARIANT": "3.11-bookworm"}) + require.NoError(t, err) + require.Equal(t, "mcr.microsoft.com/devcontainers/python:1-3.11-bookworm", ref.Name()) + }) +} + +func TestUserFromDockerfile_BuildArgs(t *testing.T) { + t.Parallel() + + t.Run("SubstitutesARGInFROM", func(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ + Config: v1.Config{ + User: "testuser", + }, + }}) + require.NoError(t, err) + ref := strings.TrimPrefix(registry, "http://") + "/coder/test:latest" + parsed, err := name.ParseReference(ref) + require.NoError(t, err) + err = remote.Write(parsed, image) + require.NoError(t, err) + + // Dockerfile uses ARG without default for the image ref. + content := fmt.Sprintf("ARG TAG\nFROM %s/coder/test:${TAG}", strings.TrimPrefix(registry, "http://")) + user, err := devcontainer.UserFromDockerfile(content, map[string]string{"TAG": "latest"}) + require.NoError(t, err) + require.Equal(t, "testuser", user) + }) +} + +func TestUserFrom(t *testing.T) { + t.Parallel() + + t.Run("Image", func(t *testing.T) { + t.Parallel() + registry := registrytest.New(t) + image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ + Config: v1.Config{ + User: "example", + }, + }}) + require.NoError(t, err) + + parsed, err := url.Parse("http://" + registry) + require.NoError(t, err) + parsed.Path = "coder/test:latest" + ref, err := name.ParseReference(strings.TrimPrefix(parsed.String(), "http://")) + require.NoError(t, err) + err = remote.Write(ref, image) + require.NoError(t, err) + + user, err := devcontainer.UserFromImage(ref) + require.NoError(t, err) + require.Equal(t, "example", user) + }) + + t.Run("Dockerfile", func(t *testing.T) { + t.Parallel() + tests := []struct { + name string + content string + user string + }{ + { + name: "Empty", + content: "FROM scratch", + user: "", + }, + { + name: "User", + content: "FROM scratch\nUSER kyle", + user: "kyle", + }, + { + name: "Env with default", + content: "FROM scratch\nENV MYUSER=maf\nUSER ${MYUSER}", + user: "${MYUSER}", // This should be "maf" but the current implementation doesn't support this. + }, + { + name: "Env var with default", + content: "FROM scratch\nUSER ${MYUSER:-maf}", + user: "${MYUSER:-maf}", // This should be "maf" but the current implementation doesn't support this. + }, + { + name: "Arg", + content: "FROM scratch\nARG MYUSER\nUSER ${MYUSER}", + user: "${MYUSER}", // This should be "" or populated but the current implementation doesn't support this. + }, + { + name: "Arg with default", + content: "FROM scratch\nARG MYUSER=maf\nUSER ${MYUSER}", + user: "${MYUSER}", // This should be "maf" but the current implementation doesn't support this. + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + user, err := devcontainer.UserFromDockerfile(tt.content, nil) + require.NoError(t, err) + require.Equal(t, tt.user, user) + }) + } + }) + + t.Run("Multi-stage", func(t *testing.T) { + t.Parallel() + + registry := registrytest.New(t) + for tag, user := range map[string]string{ + "one": "maf", + "two": "fam", + } { + image, err := partial.UncompressedToImage(emptyImage{configFile: &v1.ConfigFile{ + Config: v1.Config{ + User: user, + }, + }}) + require.NoError(t, err) + parsed, err := url.Parse("http://" + registry) + require.NoError(t, err) + parsed.Path = "coder/test:" + tag + ref, err := name.ParseReference(strings.TrimPrefix(parsed.String(), "http://")) + fmt.Println(ref) + require.NoError(t, err) + err = remote.Write(ref, image) + require.NoError(t, err) + } + + tests := []struct { + name string + images map[string]string + content string + user string + }{ + { + name: "Single", + content: "FROM coder/test:one", + user: "maf", + }, + { + name: "Multi", + content: "FROM ubuntu AS u\nFROM coder/test:two", + user: "fam", + }, + { + name: "Multi-2", + content: "FROM coder/test:two AS two\nUSER maffam\nFROM coder/test:one AS one", + user: "maf", + }, + { + name: "Multi-3", + content: "FROM coder/test:two AS two\nFROM coder/test:one AS one\nUSER fammaf", + user: "fammaf", + }, + { + name: "Multi-4", + content: `FROM ubuntu AS a +USER root +RUN useradd --create-home pickme +USER pickme +FROM a AS other +USER root +RUN useradd --create-home notme +USER notme +FROM a`, + user: "pickme", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + content := strings.ReplaceAll(tt.content, "coder/test", strings.TrimPrefix(registry, "http://")+"/coder/test") + + user, err := devcontainer.UserFromDockerfile(content, nil) + require.NoError(t, err) + require.Equal(t, tt.user, user) + }) + } + }) +} + +type emptyImage struct { + configFile *v1.ConfigFile +} + +func (i emptyImage) MediaType() (types.MediaType, error) { + return types.DockerManifestSchema2, nil +} + +func (i emptyImage) RawConfigFile() ([]byte, error) { + return partial.RawConfigFile(i) +} + +func (i emptyImage) ConfigFile() (*v1.ConfigFile, error) { + return i.configFile, nil +} + +func (i emptyImage) LayerByDiffID(h v1.Hash) (partial.UncompressedLayer, error) { + return nil, fmt.Errorf("LayerByDiffID(%s): empty image", h) +} From abcd9f4328ac621f612160e051c5b34605f20024 Mon Sep 17 00:00:00 2001 From: Pablo Ulloa Date: Wed, 11 Mar 2026 00:35:50 -0300 Subject: [PATCH 4/4] fear: enforce dependsOn ordering and canonical ref resolution --- devcontainer/devcontainer.go | 428 ++++++++++++++------ devcontainer/devcontainer_test.go | 232 ++++++++++- devcontainer/features/features.go | 4 +- devcontainer/install_order_internal_test.go | 156 +++++++ 4 files changed, 690 insertions(+), 130 deletions(-) create mode 100644 devcontainer/install_order_internal_test.go diff --git a/devcontainer/devcontainer.go b/devcontainer/devcontainer.go index 5aa601e4..00183f9b 100644 --- a/devcontainer/devcontainer.go +++ b/devcontainer/devcontainer.go @@ -238,16 +238,31 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir // Pass 1: resolve each raw ref to its canonical featureRef and extract // the feature spec. We need all specs before we can resolve ordering // since installsAfter/dependsOn live inside devcontainer-feature.json. + // + // A worklist is used to recursively resolve dependsOn hard dependencies: + // each extracted feature's dependsOn entries are added to the worklist so + // that transitive dependencies are automatically fetched and installed, + // matching the spec requirement that the install set is the union of + // user-declared features and all their transitive dependsOn dependencies. type extractedFeature struct { featureRef string featureName string featureDir string spec *features.Spec opts map[string]any + // fromDep is true when this feature was added automatically to satisfy + // a dependsOn hard dependency (not explicitly listed by the user). + fromDep bool } extracted := make(map[string]*extractedFeature, len(s.Features)) idToRef := make(map[string]string, len(s.Features)) // feature ID โ†’ refRaw - for featureRefRaw := range s.Features { + canonicalToRefs := make(map[string][]string, len(s.Features)) + + // extractOne extracts a single feature and registers it in the tables. + extractOne := func(featureRefRaw string, opts map[string]any, fromDep bool) error { + if _, already := extracted[featureRefRaw]; already { + return nil + } var ( featureRef string ok bool @@ -255,48 +270,137 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir if _, featureRef, ok = strings.Cut(featureRefRaw, "./"); !ok { featureRefParsed, err := name.ParseReference(featureRefRaw) if err != nil { - return "", nil, fmt.Errorf("parse feature ref %s: %w", featureRefRaw, err) + return fmt.Errorf("parse feature ref %s: %w", featureRefRaw, err) } featureRef = featureRefParsed.Context().Name() } - featureOpts := map[string]any{} - switch t := s.Features[featureRefRaw].(type) { - case string: - // As a shorthand, the value of the `features` property can be provided as a - // single string. This string is mapped to an option called version. - // https://containers.dev/implementors/features/#devcontainer-json-properties - featureOpts["version"] = t - case map[string]any: - featureOpts = t - } - - // It's important for caching that this directory is static. - // If it changes on each run then the container will not be cached. - // - // devcontainers/cli has a very complex method of computing the feature - // name from the feature reference. We're just going to hash it for simplicity. featureSha := md5.Sum([]byte(featureRefRaw)) - featureName := filepath.Base(featureRef) - featureDir := filepath.Join(featuresDir, fmt.Sprintf("%s-%x", featureName, featureSha[:4])) + featureName := fmt.Sprintf("%s-%x", filepath.Base(featureRef), featureSha[:4]) + featureDir := filepath.Join(featuresDir, featureName) if err := fs.MkdirAll(featureDir, 0o644); err != nil { - return "", nil, err + return err } spec, err := features.Extract(fs, devcontainerDir, featureDir, featureRefRaw) if err != nil { - return "", nil, fmt.Errorf("extract feature %s: %w", featureRefRaw, err) + return fmt.Errorf("extract feature %s: %w", featureRefRaw, err) } extracted[featureRefRaw] = &extractedFeature{ featureRef: featureRef, featureName: featureName, featureDir: featureDir, spec: spec, - opts: featureOpts, + opts: opts, + fromDep: fromDep, } idToRef[spec.ID] = featureRefRaw + canonicalToRefs[featureRef] = append(canonicalToRefs[featureRef], featureRefRaw) + return nil + } + + // Seed the worklist with user-declared features. + type workItem struct { + ref string + opts map[string]any + fromDep bool + } + worklist := make([]workItem, 0, len(s.Features)) + for featureRefRaw := range s.Features { + opts := map[string]any{} + switch t := s.Features[featureRefRaw].(type) { + case string: + // As a shorthand, the value of the `features` property can be provided as a + // single string. This string is mapped to an option called version. + // https://containers.dev/implementors/features/#devcontainer-json-properties + opts["version"] = t + case map[string]any: + opts = t + } + worklist = append(worklist, workItem{ref: featureRefRaw, opts: opts, fromDep: false}) + } + + // Phase 1: extract all user-declared features. This populates idToRef and + // canonicalToRefs fully before we follow any dependsOn edges, so that dep + // refs expressed as feature IDs or canonical names can be resolved without + // trying to fetch them as bare OCI references. + for len(worklist) > 0 { + item := worklist[0] + worklist = worklist[1:] + if err := extractOne(item.ref, item.opts, item.fromDep); err != nil { + return "", nil, err + } } - // Resolve installation order, then validate hard dependencies. + // Phase 2: follow dependsOn for every extracted feature and auto-add any + // transitive deps that are not yet in the install set. + // + // depCovered returns true when depRef already maps to an extracted feature, + // checked by exact key, by feature ID (via idToRef), or by canonical name + // (via canonicalToRefs โ€” handles "host/repo" matching "host/repo:latest"). + depCovered := func(depRef string) bool { + if _, ok := extracted[depRef]; ok { + return true + } + if ref, ok := idToRef[depRef]; ok { + if _, ok := extracted[ref]; ok { + return true + } + } + if refs, ok := canonicalToRefs[depRef]; ok && len(refs) > 0 { + return true + } + return false + } + + // enqueueNewDeps adds any un-covered deps of ef to the worklist. + enqueueNewDeps := func(ef *extractedFeature) { + for depRef, depOpts := range ef.spec.DependsOn { + if depCovered(depRef) { + continue + } + // Use the full ref from idToRef if this is a bare feature ID. + resolvedRef := depRef + if ref, ok := idToRef[depRef]; ok { + resolvedRef = ref + } + depOptsCopy := make(map[string]any, len(depOpts)) + for k, v := range depOpts { + depOptsCopy[k] = v + } + worklist = append(worklist, workItem{ref: resolvedRef, opts: depOptsCopy, fromDep: true}) + } + } + + for _, ef := range extracted { + enqueueNewDeps(ef) + } + for len(worklist) > 0 { + item := worklist[0] + worklist = worklist[1:] + if _, already := extracted[item.ref]; already { + continue + } + if err := extractOne(item.ref, item.opts, item.fromDep); err != nil { + return "", nil, err + } + enqueueNewDeps(extracted[item.ref]) + } + + canonicalToRef, ambiguousCanonicals := buildCanonicalToRef(canonicalToRefs) + + // When build contexts are enabled, each canonical ref produces a Docker + // stage alias and context key. Duplicates would generate an invalid + // Dockerfile, so reject them early. + if useBuildContexts { + for canonical, refs := range ambiguousCanonicals { + return "", nil, fmt.Errorf("multiple configured features share canonical reference %q (%s); this produces duplicate build stages when build contexts are enabled", canonical, strings.Join(refs, ", ")) + } + } + + // Validate hard dependencies: every dependsOn entry must resolve to a + // feature in the extracted set. After the worklist above, all transitive + // dependencies that could be fetched as OCI refs are present; this catches + // the case where a dep ref is unresolvable (e.g. ambiguous canonical). refRaws := make([]string, 0, len(extracted)) for refRaw := range extracted { refRaws = append(refRaws, refRaw) @@ -305,13 +409,10 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir for refRaw, ef := range extracted { specsByRef[refRaw] = ef.spec } - featureOrder, err := resolveInstallOrder(refRaws, specsByRef, idToRef, s.OverrideFeatureInstallOrder) + featureOrder, err := resolveInstallOrder(refRaws, specsByRef, idToRef, canonicalToRef, ambiguousCanonicals, s.OverrideFeatureInstallOrder) if err != nil { return "", nil, err } - if err := validateDependencies(specsByRef, idToRef); err != nil { - return "", nil, err - } // Pass 2: compile Dockerfile directives in the resolved order. featureDirectives := make([]string, 0, len(featureOrder)) @@ -343,127 +444,222 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, devcontainerDir, scratchDir // resolveInstallOrder determines the final feature installation order. // -// Priority (highest to lowest): -// 1. overrideOrder entries (in declared order) โ€” user override wins unconditionally -// 2. installsAfter edges from devcontainer-feature.json โ€” soft ordering via -// Kahn's topological sort on the unconstrained remainder -// 3. Alphabetical โ€” tie-breaking for determinism and layer cache stability +// The algorithm follows the spec's round-based dependency sort: +// 1. Build a DAG with dependsOn (hard) and installsAfter (soft) edges. +// 2. Assign a roundPriority from overrideFeatureInstallOrder: the i-th entry +// (0-based) receives priority (n - i), all others get 0. +// 3. Execute rounds: each round, collect all features whose deps are fully +// satisfied (in-degree 0). Of those, commit only the ones with the maximum +// roundPriority. Tie-break within the committed set alphabetically. +// Return uncommitted candidates to the worklist for the next round. +// 4. Cycle โ†’ error. // -// IDs in installsAfter that don't map to a feature present in the set are -// silently ignored (soft-dep semantics). Returns an error if a cycle is -// detected among the installsAfter edges. +// This correctly handles overrideFeatureInstallOrder: a pinned feature with +// a free dependency cannot be committed until that dependency's round completes, +// matching the spec requirement that overrides cannot "pull forward" a Feature +// past its own dependency graph. +// +// IDs in installsAfter that don't map to a present feature are silently +// ignored (soft-dep semantics). // // See https://containers.dev/implementors/features/#installation-order -func resolveInstallOrder(refRaws []string, specs map[string]*features.Spec, idToRef map[string]string, overrideOrder []string) ([]string, error) { - // Step 1: lock in override entries (in declared order), removing them - // from the free set so they are not subject to topo sorting. - free := make(map[string]bool, len(refRaws)) +func resolveInstallOrder(refRaws []string, specs map[string]*features.Spec, idToRef, canonicalToRef map[string]string, ambiguousCanonicals map[string][]string, overrideOrder []string) ([]string, error) { + n := len(refRaws) + all := make(map[string]bool, n) for _, r := range refRaws { - free[r] = true + all[r] = true } - pinned := make([]string, 0, len(overrideOrder)) - for _, r := range overrideOrder { - if free[r] { - pinned = append(pinned, r) - delete(free, r) + + // Assign roundPriority from overrideFeatureInstallOrder. + // Entry at index i gets priority (len - i) so earlier entries have higher + // priority. + roundPriority := make(map[string]int, len(overrideOrder)) + pinnedSet := make(map[string]bool, len(overrideOrder)) + for i, r := range overrideOrder { + if all[r] { + roundPriority[r] = len(overrideOrder) - i + pinnedSet[r] = true } } - // Step 2: topological sort (Kahn's algorithm) on the free remainder, - // driven by installsAfter edges. An edge Aโ†’B means "B must come before A". - // Edges pointing outside the free set are ignored. - inDegree := make(map[string]int, len(free)) - deps := make(map[string][]string, len(free)) // refRaw โ†’ refRaws it must follow - for r := range free { + // Build the dependency graph: inDegree and successors. + inDegree := make(map[string]int, n) + for _, r := range refRaws { inDegree[r] = 0 } - for r := range free { + // preds maps refRaw โ†’ set of refRaws it must follow. + preds := make(map[string]map[string]struct{}, n) + for _, r := range refRaws { + preds[r] = make(map[string]struct{}) + } + addEdge := func(from, to string) { + // "from" must come after "to" + if _, ok := preds[from][to]; ok { + return + } + preds[from][to] = struct{}{} + inDegree[from]++ + } + + for _, r := range refRaws { + for dep := range specs[r].DependsOn { + predRef, ok, err := resolveDependencyRef(dep, specs, idToRef, canonicalToRef, ambiguousCanonicals) + if err != nil { + return nil, err + } + if !ok || !all[predRef] { + continue + } + addEdge(r, predRef) + } + // installsAfter is a soft dep: only respected when the feature is NOT + // in overrideFeatureInstallOrder. Pinned features have their install + // order dictated by the override list; their installsAfter hints are + // ignored per the spec ("soft dependencies are respected for Features + // not in overrideFeatureInstallOrder"). + if pinnedSet[r] { + continue + } for _, depID := range specs[r].InstallsAfter { - // Resolve the ID or ref to a refRaw present in the free set. - predRef, ok := idToRef[depID] - if !ok { - // depID might itself be a raw ref rather than a short ID. - if free[depID] { - predRef = depID - ok = true - } + predRef, ok, err := resolveDependencyRef(depID, specs, idToRef, canonicalToRef, ambiguousCanonicals) + if err != nil { + return nil, err } - if !ok || !free[predRef] { - // Predecessor absent or overridden โ€” soft dep, skip. + if !ok || !all[predRef] { + // Soft dep: only applies when predecessor is in the install set. continue } - deps[r] = append(deps[r], predRef) - inDegree[r]++ + addEdge(r, predRef) } } - // Seed the ready queue with all zero-in-degree nodes, sorted alphabetically - // so tie-breaking is deterministic. - ready := make([]string, 0, len(free)) - for r := range free { - if inDegree[r] == 0 { - ready = append(ready, r) + // successors maps predecessor โ†’ features that depend on it. + successors := make(map[string][]string, n) + for r, ps := range preds { + for p := range ps { + successors[p] = append(successors[p], r) } } - sort.Strings(ready) - sorted := make([]string, 0, len(free)) - // successors maps predecessor โ†’ features that depend on it. - successors := make(map[string][]string, len(free)) - for r, preds := range deps { - for _, pred := range preds { - successors[pred] = append(successors[pred], r) - } - } - for len(ready) > 0 { - // Pop the first (alphabetically smallest) ready node. - r := ready[0] - ready = ready[1:] - sorted = append(sorted, r) - // Reduce in-degree for all features that installsAfter r. - newReady := []string{} - for _, succ := range successors[r] { - inDegree[succ]-- - if inDegree[succ] == 0 { - newReady = append(newReady, succ) + // Validate that overrideFeatureInstallOrder is consistent with the + // dependency graph: for any two pinned features A and B where A is listed + // before B in overrideOrder, A must not (transitively or directly) depend + // on B. + pinnedList := make([]string, 0, len(overrideOrder)) + for _, r := range overrideOrder { + if all[r] { + pinnedList = append(pinnedList, r) + } + } + pinnedIndex := make(map[string]int, len(pinnedList)) + for i, r := range pinnedList { + pinnedIndex[r] = i + } + for _, r := range pinnedList { + for dep := range specs[r].DependsOn { + depRef, ok, err := resolveDependencyRef(dep, specs, idToRef, canonicalToRef, ambiguousCanonicals) + if err != nil { + return nil, err } + if !ok { + continue + } + if depIdx, isPinned := pinnedIndex[depRef]; isPinned { + if depIdx > pinnedIndex[r] { + return nil, fmt.Errorf("overrideFeatureInstallOrder violates dependsOn: %q must be installed before %q", depRef, r) + } + } + // If dep is not pinned, the round-based sort will handle it correctly + // by not committing r until dep is in installationOrder. } - // Insert new ready nodes in sorted position to preserve alphabetical - // tie-breaking across the entire queue. - sort.Strings(newReady) - ready = append(ready, newReady...) - sort.Strings(ready) } - if len(sorted) != len(free) { - // Cycle detected โ€” identify the offending features. - cycled := make([]string, 0) - for r := range free { - if inDegree[r] > 0 { + // Round-based sort (spec ยง3). + worklist := make(map[string]bool, n) + for _, r := range refRaws { + worklist[r] = true + } + installationOrder := make([]string, 0, n) + installed := make(map[string]bool, n) + + for len(worklist) > 0 { + // Collect all candidates whose dependencies are fully installed. + round := make([]string, 0) + for r := range worklist { + if inDegree[r] == 0 { + round = append(round, r) + } + } + if len(round) == 0 { + // No progress โ€” cycle. + cycled := make([]string, 0, len(worklist)) + for r := range worklist { cycled = append(cycled, r) } + sort.Strings(cycled) + return nil, fmt.Errorf("cycle detected in feature dependency graph: %s", strings.Join(cycled, ", ")) + } + + // Find the maximum roundPriority among candidates. + maxPriority := 0 + for _, r := range round { + if roundPriority[r] > maxPriority { + maxPriority = roundPriority[r] + } + } + + // Commit only those with the max priority; return the rest to the + // worklist for subsequent rounds. + toCommit := make([]string, 0, len(round)) + for _, r := range round { + if roundPriority[r] == maxPriority { + toCommit = append(toCommit, r) + } + } + sort.Strings(toCommit) // alphabetical tie-break within a round + + for _, r := range toCommit { + installationOrder = append(installationOrder, r) + installed[r] = true + delete(worklist, r) + // Reduce in-degree for successors. + for _, succ := range successors[r] { + inDegree[succ]-- + } } - sort.Strings(cycled) - return nil, fmt.Errorf("cycle detected in feature installsAfter dependencies: %s", strings.Join(cycled, ", ")) } - return append(pinned, sorted...), nil + return installationOrder, nil } -// validateDependencies checks that every hard dependency declared via -// dependsOn in a feature's devcontainer-feature.json is satisfied by the -// set of installed features. -func validateDependencies(specs map[string]*features.Spec, idToRef map[string]string) error { - for refRaw, spec := range specs { - for _, depID := range spec.DependsOn { - _, byID := idToRef[depID] - _, byRef := specs[depID] - if !byID && !byRef { - return fmt.Errorf("feature %q (%s) requires feature %q which is not included", spec.ID, refRaw, depID) - } +func resolveDependencyRef(dep string, specs map[string]*features.Spec, idToRef, canonicalToRef map[string]string, ambiguousCanonicals map[string][]string) (string, bool, error) { + if refRaw, ok := idToRef[dep]; ok { + return refRaw, true, nil + } + if _, ok := specs[dep]; ok { + return dep, true, nil + } + if refRaw, ok := canonicalToRef[dep]; ok { + return refRaw, true, nil + } + if refRaws, ok := ambiguousCanonicals[dep]; ok { + return "", false, fmt.Errorf("ambiguous canonical feature reference %q matches multiple configured features: %s", dep, strings.Join(refRaws, ", ")) + } + return "", false, nil +} + +func buildCanonicalToRef(canonicalToRefs map[string][]string) (map[string]string, map[string][]string) { + canonicalToRef := make(map[string]string, len(canonicalToRefs)) + ambiguous := make(map[string][]string) + for canonicalRef, refRaws := range canonicalToRefs { + sort.Strings(refRaws) + if len(refRaws) > 1 { + ambiguous[canonicalRef] = refRaws + continue } + canonicalToRef[canonicalRef] = refRaws[0] } - return nil + return canonicalToRef, ambiguous } // BuildArgsMap converts a slice of "KEY=VALUE" strings to a map. diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index ac130733..bc39c59a 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -95,9 +95,11 @@ func TestCompileWithFeatures(t *testing.T) { fs := memfs.New() featureOneMD5 := md5.Sum([]byte(featureOne)) - featureOneDir := fmt.Sprintf("/.envbuilder/features/one-%x", featureOneMD5[:4]) + featureOneName := fmt.Sprintf("one-%x", featureOneMD5[:4]) + featureOneDir := "/.envbuilder/features/" + featureOneName featureTwoMD5 := md5.Sum([]byte(featureTwo)) - featureTwoDir := fmt.Sprintf("/.envbuilder/features/two-%x", featureTwoMD5[:4]) + featureTwoName := fmt.Sprintf("two-%x", featureTwoMD5[:4]) + featureTwoDir := "/.envbuilder/features/" + featureTwoName t.Run("WithoutBuildContexts", func(t *testing.T) { params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) @@ -123,23 +125,23 @@ USER 1000`, params.DockerfileContent) registryHost := strings.TrimPrefix(registry, "http://") - require.Equal(t, `FROM scratch AS envbuilder_feature_one + require.Equal(t, `FROM scratch AS envbuilder_feature_`+featureOneName+` COPY --from=`+registryHost+`/coder/one / / -FROM scratch AS envbuilder_feature_two +FROM scratch AS envbuilder_feature_`+featureTwoName+` COPY --from=`+registryHost+`/coder/two / / FROM localhost:5000/envbuilder-test-codercom-code-server:latest USER root # Rust tomato - Example description! -WORKDIR /.envbuilder/features/one +WORKDIR /.envbuilder/features/`+featureOneName+` ENV TOMATO=example -RUN --mount=type=bind,from=envbuilder_feature_one,target=/.envbuilder/features/one,rw _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh +RUN --mount=type=bind,from=envbuilder_feature_`+featureOneName+`,target=/.envbuilder/features/`+featureOneName+`,rw _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh # Go potato - Example description! -WORKDIR /.envbuilder/features/two +WORKDIR /.envbuilder/features/`+featureTwoName+` ENV POTATO=example -RUN --mount=type=bind,from=envbuilder_feature_two,target=/.envbuilder/features/two,rw VERSION="potato" _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh +RUN --mount=type=bind,from=envbuilder_feature_`+featureTwoName+`,target=/.envbuilder/features/`+featureTwoName+`,rw VERSION="potato" _CONTAINER_USER="1000" _REMOTE_USER="1000" ./install.sh USER 1000`, params.DockerfileContent) require.Equal(t, map[string]string{ @@ -263,11 +265,25 @@ func TestCompileWithFeaturesInstallsAfter(t *testing.T) { InstallsAfter: []string{"base"}, }, }) + baseRef, err := name.ParseReference(featureBase) + require.NoError(t, err) + baseCanonical := baseRef.Context().Name() + featureTopByRef := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/top-by-ref:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "top-by-ref", + Version: "1.0.0", + Name: "TopByRef", + InstallsAfter: []string{baseCanonical}, + }, + }) featureBaseMD5 := md5.Sum([]byte(featureBase)) featureBaseDir := fmt.Sprintf("/.envbuilder/features/base-%x", featureBaseMD5[:4]) featureTopMD5 := md5.Sum([]byte(featureTop)) featureTopDir := fmt.Sprintf("/.envbuilder/features/top-%x", featureTopMD5[:4]) + featureTopByRefMD5 := md5.Sum([]byte(featureTopByRef)) + featureTopByRefDir := fmt.Sprintf("/.envbuilder/features/top-by-ref-%x", featureTopByRefMD5[:4]) t.Run("InstallsAfterRespected", func(t *testing.T) { raw := `{ @@ -328,8 +344,32 @@ func TestCompileWithFeaturesInstallsAfter(t *testing.T) { topIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTopDir) baseIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureBaseDir) + require.Greater(t, topIdx, -1, "top feature should be present") + require.Greater(t, baseIdx, -1, "base feature should be present") require.Less(t, topIdx, baseIdx, "override should force top before base") }) + + t.Run("InstallsAfterCanonicalRefRespected", func(t *testing.T) { + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureTopByRef + `": {}, + "` + featureBase + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + baseIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureBaseDir) + topIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureTopByRefDir) + require.Greater(t, baseIdx, -1, "base feature should be present") + require.Greater(t, topIdx, -1, "top-by-ref feature should be present") + require.Less(t, baseIdx, topIdx, "base should be installed before top-by-ref (installsAfter by canonical ref)") + }) } func TestCompileWithFeaturesDependsOn(t *testing.T) { @@ -344,17 +384,69 @@ func TestCompileWithFeaturesDependsOn(t *testing.T) { Name: "A", }, }) - // featureB hard-depends on featureA. + // featureB hard-depends on featureA. The dependsOn key uses the full OCI + // reference of featureA so that auto-add (DependsOnAutoAdded) can fetch it + // from the registry when it is not explicitly declared in features. featureB := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/b:latest", features.TarLayerMediaType, map[string]any{ "install.sh": "hey", "devcontainer-feature.json": features.Spec{ ID: "b", Version: "1.0.0", Name: "B", - DependsOn: []string{"a"}, + DependsOn: map[string]map[string]any{featureA: {}}, + }, + }) + featureEarly := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/aaa-early:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "early", + Version: "1.0.0", + Name: "Early", + DependsOn: map[string]map[string]any{"late": {}}, + }, + }) + featureLate := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/zzz-late:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "late", + Version: "1.0.0", + Name: "Late", + }, + }) + lateRef, err := name.ParseReference(featureLate) + require.NoError(t, err) + lateCanonical := lateRef.Context().Name() + featureByRef := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/by-ref:latest", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "by-ref", + Version: "1.0.0", + Name: "ByRef", + DependsOn: map[string]map[string]any{lateCanonical: {}}, + }, + }) + featureLateV1 := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/zzz-late:1.0.0", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "late-v1", + Version: "1.0.0", + Name: "LateV1", + }, + }) + featureLateV2 := registrytest.WriteContainer(t, registry, emptyRemoteOpts, "coder/zzz-late:2.0.0", features.TarLayerMediaType, map[string]any{ + "install.sh": "hey", + "devcontainer-feature.json": features.Spec{ + ID: "late-v2", + Version: "2.0.0", + Name: "LateV2", }, }) + featureEarlyMD5 := md5.Sum([]byte(featureEarly)) + featureEarlyDir := fmt.Sprintf("/.envbuilder/features/aaa-early-%x", featureEarlyMD5[:4]) + featureLateMD5 := md5.Sum([]byte(featureLate)) + featureLateDir := fmt.Sprintf("/.envbuilder/features/zzz-late-%x", featureLateMD5[:4]) + t.Run("DependsOnSatisfied", func(t *testing.T) { raw := `{ "image": "localhost:5000/envbuilder-test-ubuntu:latest", @@ -371,8 +463,10 @@ func TestCompileWithFeaturesDependsOn(t *testing.T) { require.NoError(t, err) }) - t.Run("DependsOnMissingErrors", func(t *testing.T) { - // featureB requires featureA, but featureA is not included. + t.Run("DependsOnAutoAdded", func(t *testing.T) { + // featureB requires featureA, but featureA is not explicitly listed. + // Per spec, featureA should be automatically fetched and added to the + // install set. raw := `{ "image": "localhost:5000/envbuilder-test-ubuntu:latest", "features": { @@ -383,8 +477,120 @@ func TestCompileWithFeaturesDependsOn(t *testing.T) { require.NoError(t, err) fs := memfs.New() + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + // featureA (dep of featureB) must be present and installed before featureB. + featureAMD5 := md5.Sum([]byte(featureA)) + featureADir := fmt.Sprintf("/.envbuilder/features/a-%x", featureAMD5[:4]) + featureBMD5 := md5.Sum([]byte(featureB)) + featureBDir := fmt.Sprintf("/.envbuilder/features/b-%x", featureBMD5[:4]) + aIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureADir) + bIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureBDir) + require.Greater(t, aIdx, -1, "auto-added featureA should be present in Dockerfile") + require.Greater(t, bIdx, -1, "featureB should be present in Dockerfile") + require.Less(t, aIdx, bIdx, "featureA should be installed before featureB (dependsOn)") + }) + + t.Run("DependsOnEnforcesInstallOrder", func(t *testing.T) { + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureEarly + `": {}, + "` + featureLate + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + earlyIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureEarlyDir) + lateIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureLateDir) + require.Greater(t, earlyIdx, -1, "early feature should be present") + require.Greater(t, lateIdx, -1, "late feature should be present") + require.Less(t, lateIdx, earlyIdx, "late should be installed before early due to dependsOn") + }) + + t.Run("OverridePinnedFreeDependsOnSucceeds", func(t *testing.T) { + // featureEarly is pinned via overrideFeatureInstallOrder; it depends on + // featureLate which is in the free (topo-sorted) set. Per spec, the + // free set is installed before pinned features, so this is valid. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureEarly + `": {}, + "` + featureLate + `": {} + }, + "overrideFeatureInstallOrder": ["` + featureEarly + `"] +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + params, err := dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + + // featureLate (free/topo) must appear before featureEarly (pinned). + earlyIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureEarlyDir) + lateIdx := strings.Index(params.DockerfileContent, "WORKDIR "+featureLateDir) + require.Greater(t, earlyIdx, -1, "early feature should be present") + require.Greater(t, lateIdx, -1, "late feature should be present") + require.Less(t, lateIdx, earlyIdx, "late (free) must be installed before early (pinned)") + }) + + t.Run("OverridePinnedBeforePinnedDepErrors", func(t *testing.T) { + // Both featureEarly and featureLate are pinned, but featureEarly + // (which depends on featureLate) is listed first โ€” a true violation. + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureEarly + `": {}, + "` + featureLate + `": {} + }, + "overrideFeatureInstallOrder": ["` + featureEarly + `", "` + featureLate + `"] +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.ErrorContains(t, err, "overrideFeatureInstallOrder violates dependsOn") + }) + + t.Run("DependsOnCanonicalRefResolved", func(t *testing.T) { + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureByRef + `": {}, + "` + featureLate + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + + _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) + require.NoError(t, err) + }) + + t.Run("DependsOnCanonicalRefAmbiguousErrors", func(t *testing.T) { + raw := `{ + "image": "localhost:5000/envbuilder-test-ubuntu:latest", + "features": { + "` + featureByRef + `": {}, + "` + featureLateV1 + `": {}, + "` + featureLateV2 + `": {} + } +}` + dc, err := devcontainer.Parse([]byte(raw)) + require.NoError(t, err) + fs := memfs.New() + _, err = dc.Compile(fs, "", workingDir, "", "", false, stubLookupEnv) - require.ErrorContains(t, err, `requires feature "a"`) + require.ErrorContains(t, err, "ambiguous canonical feature reference") }) } diff --git a/devcontainer/features/features.go b/devcontainer/features/features.go index 15d38539..5d340ed3 100644 --- a/devcontainer/features/features.go +++ b/devcontainer/features/features.go @@ -195,8 +195,10 @@ type Spec struct { InstallsAfter []string `json:"installsAfter"` // DependsOn is a hard dependency: this feature requires the listed // feature IDs or references to also be present in the feature set. + // Each key is a feature reference and the value is the options object + // for that feature (same semantics as the features object in devcontainer.json). // See https://containers.dev/implementors/features/#installation-order - DependsOn []string `json:"dependsOn"` + DependsOn map[string]map[string]any `json:"dependsOn"` } // Extract unpacks the feature from the image and returns a set of lines diff --git a/devcontainer/install_order_internal_test.go b/devcontainer/install_order_internal_test.go new file mode 100644 index 00000000..17ab9fea --- /dev/null +++ b/devcontainer/install_order_internal_test.go @@ -0,0 +1,156 @@ +package devcontainer + +import ( + "testing" + + "github.com/coder/envbuilder/devcontainer/features" + "github.com/stretchr/testify/require" +) + +// These tests intentionally live in package devcontainer (not +// devcontainer_test) so they can exercise unexported helper behavior directly. +// The external-package tests in devcontainer_test.go continue to validate +// user-facing behavior through the public API. + +func TestBuildCanonicalToRefUnique(t *testing.T) { + t.Parallel() + + canonicalToRefs := map[string][]string{ + "ghcr.io/example/features/a": {"ghcr.io/example/features/a:1"}, + "ghcr.io/example/features/b": {"ghcr.io/example/features/b:2"}, + } + + canonicalToRef, ambiguous := buildCanonicalToRef(canonicalToRefs) + require.Empty(t, ambiguous) + require.Equal(t, "ghcr.io/example/features/a:1", canonicalToRef["ghcr.io/example/features/a"]) + require.Equal(t, "ghcr.io/example/features/b:2", canonicalToRef["ghcr.io/example/features/b"]) +} + +func TestBuildCanonicalToRefAmbiguousDeferred(t *testing.T) { + t.Parallel() + + canonicalToRefs := map[string][]string{ + "ghcr.io/example/features/late": { + "ghcr.io/example/features/late:2.0.0", + "ghcr.io/example/features/late:1.0.0", + }, + } + + canonicalToRef, ambiguous := buildCanonicalToRef(canonicalToRefs) + // buildCanonicalToRef no longer errors; ambiguity is deferred. + require.Empty(t, canonicalToRef) + require.Contains(t, ambiguous, "ghcr.io/example/features/late") + require.Equal(t, []string{ + "ghcr.io/example/features/late:1.0.0", + "ghcr.io/example/features/late:2.0.0", + }, ambiguous["ghcr.io/example/features/late"]) + + // Ambiguity error surfaces only when the canonical is actually resolved. + specs := map[string]*features.Spec{} + idToRef := map[string]string{} + _, _, err := resolveDependencyRef("ghcr.io/example/features/late", specs, idToRef, canonicalToRef, ambiguous) + require.ErrorContains(t, err, "ambiguous canonical feature reference \"ghcr.io/example/features/late\"") + require.ErrorContains(t, err, "ghcr.io/example/features/late:1.0.0, ghcr.io/example/features/late:2.0.0") +} + +// TestResolveInstallOrderPinnedFreeDepOK confirms that a pinned feature whose +// dependsOn target is in the free (topo-sorted) set does NOT produce an error. +// The free set is always installed before pinned features, so the ordering +// constraint is automatically satisfied. +func TestResolveInstallOrderPinnedFreeDepOK(t *testing.T) { + t.Parallel() + + // early depends on late. early is pinned via overrideOrder; late is free. + specs := map[string]*features.Spec{ + "early:latest": {ID: "early", Version: "1.0.0", Name: "Early", DependsOn: map[string]map[string]any{"late": {}}}, + "late:latest": {ID: "late", Version: "1.0.0", Name: "Late"}, + } + idToRef := map[string]string{ + "early": "early:latest", + "late": "late:latest", + } + + order, err := resolveInstallOrder( + []string{"early:latest", "late:latest"}, + specs, idToRef, + map[string]string{}, map[string][]string{}, + []string{"early:latest"}, // pin early, leaving late free + ) + require.NoError(t, err) + // late (free/topo) must precede early (pinned). + lateIdx := -1 + earlyIdx := -1 + for i, r := range order { + if r == "late:latest" { + lateIdx = i + } + if r == "early:latest" { + earlyIdx = i + } + } + require.Greater(t, lateIdx, -1, "late should be in output") + require.Greater(t, earlyIdx, -1, "early should be in output") + require.Less(t, lateIdx, earlyIdx, "late (free) must come before early (pinned)") +} + +// TestResolveInstallOrderBothPinnedViolationErrors confirms that when both the +// dependent feature AND its dependency are pinned and ordered incorrectly, an +// error is returned. +func TestResolveInstallOrderBothPinnedViolationErrors(t *testing.T) { + t.Parallel() + + specs := map[string]*features.Spec{ + "early:latest": {ID: "early", Version: "1.0.0", Name: "Early", DependsOn: map[string]map[string]any{"late": {}}}, + "late:latest": {ID: "late", Version: "1.0.0", Name: "Late"}, + } + idToRef := map[string]string{ + "early": "early:latest", + "late": "late:latest", + } + + _, err := resolveInstallOrder( + []string{"early:latest", "late:latest"}, + specs, idToRef, + map[string]string{}, map[string][]string{}, + []string{"early:latest", "late:latest"}, // both pinned, wrong order + ) + require.ErrorContains(t, err, "overrideFeatureInstallOrder violates dependsOn") +} + +// TestResolveInstallOrderPinnedIgnoresInstallsAfter confirms that a pinned +// feature's installsAfter hints are ignored: the override takes precedence +// over soft dependencies per spec. +func TestResolveInstallOrderPinnedIgnoresInstallsAfter(t *testing.T) { + t.Parallel() + + // top declares installsAfter: ["base"], but top is pinned first in the + // override. The override should win; base must come AFTER top. + specs := map[string]*features.Spec{ + "top:latest": {ID: "top", Version: "1.0.0", Name: "Top", InstallsAfter: []string{"base"}}, + "base:latest": {ID: "base", Version: "1.0.0", Name: "Base"}, + } + idToRef := map[string]string{ + "top": "top:latest", + "base": "base:latest", + } + + order, err := resolveInstallOrder( + []string{"top:latest", "base:latest"}, + specs, idToRef, + map[string]string{}, map[string][]string{}, + []string{"top:latest", "base:latest"}, // override: top first + ) + require.NoError(t, err) + topIdx, baseIdx := -1, -1 + for i, r := range order { + if r == "top:latest" { + topIdx = i + } + if r == "base:latest" { + baseIdx = i + } + } + require.Greater(t, topIdx, -1, "top should be in output") + require.Greater(t, baseIdx, -1, "base should be in output") + require.Less(t, topIdx, baseIdx, "override must place top before base despite installsAfter") +}