From 85f220a9ac7fe894423103383f9357672f46903e Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 26 Jul 2023 12:37:57 -0600 Subject: [PATCH] fix: allow features to be a mapping of versions (#44) --- devcontainer/devcontainer.go | 27 ++++++++++-- devcontainer/devcontainer_test.go | 68 +++++++++++++++++++++++++++++++ devcontainer/features/features.go | 18 ++++++-- 3 files changed, 106 insertions(+), 7 deletions(-) diff --git a/devcontainer/devcontainer.go b/devcontainer/devcontainer.go index 3ff9e39a..6b86f950 100644 --- a/devcontainer/devcontainer.go +++ b/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 { diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index a4f6a4b1..a370449a 100644 --- a/devcontainer/devcontainer_test.go +++ b/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) { diff --git a/devcontainer/features/features.go b/devcontainer/features/features.go index 5e4e726f..e1afc2c6 100644 --- a/devcontainer/features/features.go +++ b/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 }