devcontainer: implement overrideFeatureInstallOrder

Author: prulloac

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))

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) {

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