fix: allow features to be a mapping of versions (#44)

Author: kylecarbs

devcontainer/devcontainer.go

@@ -34,7 +34,7 @@ type Spec struct {
 	RemoteUser string            `json:"remoteUser"`
 	RemoteEnv  map[string]string `json:"remoteEnv"`
 	// Features is a map of feature names to feature configurations.
-	Features map[string]map[string]any `json:"features"`
+	Features map[string]any `json:"features"`
 
 	// Deprecated but still frequently used...
 	Dockerfile string `json:"dockerFile"`
@@ -186,15 +186,34 @@ func (s *Spec) compileFeatures(fs billy.Filesystem, scratchDir, remoteUser, dock
 	// is deterministic which allows for caching.
 	sort.Strings(featureOrder)
 
-	for _, featureRef := range featureOrder {
-		featureOpts := s.Features[featureRef]
+	for _, featureRefRaw := range featureOrder {
+		featureRefParsed, err := name.NewTag(featureRefRaw)
+		if err != nil {
+			return "", fmt.Errorf("parse feature ref %s: %w", featureRefRaw, err)
+		}
+		featureImage := featureRefParsed.Repository.Name()
+		featureTag := featureRefParsed.TagStr()
+
+		featureOpts := map[string]any{}
+		switch t := s.Features[featureRefRaw].(type) {
+		case string:
+			featureTag = t
+		case map[string]any:
+			featureOpts = t
+		}
+
+		featureRef := featureImage
+		if featureTag != "" {
+			featureRef += ":" + featureTag
+		}
+
 		// 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(featureRef))
-		featureName := strings.Split(filepath.Base(featureRef), ":")[0]
+		featureName := filepath.Base(featureImage)
 		featureDir := filepath.Join(featuresDir, fmt.Sprintf("%s-%x", featureName, featureSha[:4]))
 		err = fs.MkdirAll(featureDir, 0644)
 		if err != nil {

devcontainer/devcontainer_test.go

@@ -1,6 +1,7 @@
 package devcontainer_test
 
 import (
+	"crypto/md5"
 	"fmt"
 	"io"
 	"net/url"
@@ -11,6 +12,7 @@ import (
 
 	"github.com/coder/envbuilder"
 	"github.com/coder/envbuilder/devcontainer"
+	"github.com/coder/envbuilder/devcontainer/features"
 	"github.com/coder/envbuilder/registrytest"
 	"github.com/go-git/go-billy/v5/memfs"
 	"github.com/google/go-containerregistry/pkg/name"
@@ -36,6 +38,72 @@ func TestParse(t *testing.T) {
 	require.Equal(t, "Dockerfile", parsed.Build.Dockerfile)
 }
 
+func TestCompileWithFeatures(t *testing.T) {
+	t.Parallel()
+	registry := registrytest.New(t)
+	featureOne := registrytest.WriteContainer(t, registry, "coder/test: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, "coder/test: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",
+			},
+		},
+	})
+	// Update the tag to ensure it comes from the feature value!
+	featureTwoFake := strings.Join(append(strings.Split(featureTwo, ":")[:2], "faketag"), ":")
+
+	raw := `{
+  "build": {
+    "dockerfile": "Dockerfile",
+    "context": ".",
+  },
+  // Comments here!
+  "image": "codercom/code-server:latest",
+  "features": {
+	"` + featureOne + `": {},
+	"` + featureTwoFake + `": "potato"
+  }
+}`
+	dc, err := devcontainer.Parse([]byte(raw))
+	require.NoError(t, err)
+	fs := memfs.New()
+	params, err := dc.Compile(fs, "", envbuilder.MagicDir, "")
+	require.NoError(t, err)
+
+	// We have to SHA because we get a different MD5 every time!
+	featureOneMD5 := md5.Sum([]byte(featureOne))
+	featureOneSha := fmt.Sprintf("%x", featureOneMD5[:4])
+	featureTwoMD5 := md5.Sum([]byte(featureTwo))
+	featureTwoSha := fmt.Sprintf("%x", featureTwoMD5[:4])
+
+	require.Equal(t, `FROM codercom/code-server:latest
+
+USER root
+# Go potato - Example description!
+ENV POTATO=example
+RUN .envbuilder/features/test-`+featureTwoSha+`/install.sh
+# Rust tomato - Example description!
+ENV TOMATO=example
+RUN .envbuilder/features/test-`+featureOneSha+`/install.sh
+USER 1000`, params.DockerfileContent)
+}
+
 func TestCompileDevContainer(t *testing.T) {
 	t.Parallel()
 	t.Run("WithImage", func(t *testing.T) {

devcontainer/features/features.go

@@ -183,8 +183,20 @@ func (s *Spec) Compile(options map[string]any) (string, error) {
 	runDirective = append([]string{"RUN"}, runDirective...)
 	runDirective = append(runDirective, s.InstallScriptPath)
 
-	// Prefix and suffix with a newline to ensure the RUN command is on its own line.
-	lines := []string{"\n"}
+	comment := ""
+	if s.Name != "" {
+		comment += "# " + s.Name
+	}
+	if s.Version != "" {
+		comment += " " + s.Version
+	}
+	if s.Description != "" {
+		comment += " - " + s.Description
+	}
+	lines := []string{}
+	if comment != "" {
+		lines = append(lines, comment)
+	}
 	envKeys := make([]string, 0, len(s.ContainerEnv))
 	for key := range s.ContainerEnv {
 		envKeys = append(envKeys, key)
@@ -195,7 +207,7 @@ func (s *Spec) Compile(options map[string]any) (string, error) {
 	for _, key := range envKeys {
 		lines = append(lines, fmt.Sprintf("ENV %s=%s", key, s.ContainerEnv[key]))
 	}
-	lines = append(lines, strings.Join(runDirective, " "), "\n")
+	lines = append(lines, strings.Join(runDirective, " "))
 
 	return strings.Join(lines, "\n"), nil
 }