From 80010c23b95fa8bf5c18299c5a2fbd9cc2ad03a2 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Tue, 5 Mar 2024 13:08:42 -0800 Subject: [PATCH] Support variables substitutions in devcontainer.json fields (#99) --- devcontainer/devcontainer.go | 62 ++++++++++++++++++++----- devcontainer/devcontainer_test.go | 8 ++-- envbuilder.go | 77 ++++++++++++++++++------------- integration/integration_test.go | 17 ++++--- 4 files changed, 111 insertions(+), 53 deletions(-) diff --git a/devcontainer/devcontainer.go b/devcontainer/devcontainer.go index 98c9618..d083a08 100644 --- a/devcontainer/devcontainer.go +++ b/devcontainer/devcontainer.go @@ -66,8 +66,50 @@ type Compiled struct { BuildContext string BuildArgs []string - User string - Env []string + User string + ContainerEnv map[string]string + RemoteEnv map[string]string +} + +func SubstituteVars(s string, workspaceFolder string) string { + var buf string + for { + beforeOpen, afterOpen, ok := strings.Cut(s, "${") + if !ok { + return buf + s + } + varExpr, afterClose, ok := strings.Cut(afterOpen, "}") + if !ok { + return buf + s + } + + buf += beforeOpen + substitute(varExpr, workspaceFolder) + s = afterClose + } +} + +func substitute(varExpr string, workspaceFolder string) string { + parts := strings.Split(varExpr, ":") + if len(parts) == 1 { + switch varExpr { + case "localWorkspaceFolder", "containerWorkspaceFolder": + return workspaceFolder + case "localWorkspaceFolderBasename", "containerWorkspaceFolderBasename": + return filepath.Base(workspaceFolder) + default: + return os.Getenv(varExpr) + } + } + switch parts[0] { + case "env", "localEnv", "containerEnv": + if val, ok := os.LookupEnv(parts[1]); ok { + return val + } + if len(parts) == 3 { + return parts[2] + } + } + return "" } // HasImage returns true if the devcontainer.json specifies an image. @@ -85,16 +127,11 @@ func (s Spec) HasDockerfile() bool { // devcontainerDir is the path to the directory where the devcontainer.json file // is located. scratchDir is the path to the directory where the Dockerfile will // be written to if one doesn't exist. -func (s *Spec) Compile(fs billy.Filesystem, devcontainerDir, scratchDir, fallbackDockerfile string) (*Compiled, error) { - env := make([]string, 0) - for _, envMap := range []map[string]string{s.ContainerEnv, s.RemoteEnv} { - for key, value := range envMap { - env = append(env, key+"="+value) - } - } +func (s *Spec) Compile(fs billy.Filesystem, devcontainerDir, scratchDir, fallbackDockerfile, workspaceFolder string) (*Compiled, error) { params := &Compiled{ - User: s.ContainerUser, - Env: env, + User: s.ContainerUser, + ContainerEnv: s.ContainerEnv, + RemoteEnv: s.RemoteEnv, } if s.Image != "" { @@ -137,7 +174,8 @@ func (s *Spec) Compile(fs billy.Filesystem, devcontainerDir, scratchDir, fallbac buildArgs := make([]string, 0) for _, key := range buildArgkeys { - buildArgs = append(buildArgs, key+"="+s.Build.Args[key]) + val := SubstituteVars(s.Build.Args[key], workspaceFolder) + buildArgs = append(buildArgs, key+"="+val) } params.BuildArgs = buildArgs diff --git a/devcontainer/devcontainer_test.go b/devcontainer/devcontainer_test.go index e3bc020..7bb658a 100644 --- a/devcontainer/devcontainer_test.go +++ b/devcontainer/devcontainer_test.go @@ -86,7 +86,7 @@ func TestCompileWithFeatures(t *testing.T) { dc, err := devcontainer.Parse([]byte(raw)) require.NoError(t, err) fs := memfs.New() - params, err := dc.Compile(fs, "", envbuilder.MagicDir, "") + params, err := dc.Compile(fs, "", envbuilder.MagicDir, "", "") require.NoError(t, err) // We have to SHA because we get a different MD5 every time! @@ -117,7 +117,7 @@ func TestCompileDevContainer(t *testing.T) { dc := &devcontainer.Spec{ Image: "codercom/code-server:latest", } - params, err := dc.Compile(fs, "", envbuilder.MagicDir, "") + params, err := dc.Compile(fs, "", envbuilder.MagicDir, "", "") require.NoError(t, err) require.Equal(t, filepath.Join(envbuilder.MagicDir, "Dockerfile"), params.DockerfilePath) require.Equal(t, envbuilder.MagicDir, params.BuildContext) @@ -131,6 +131,7 @@ func TestCompileDevContainer(t *testing.T) { Context: ".", Args: map[string]string{ "ARG1": "value1", + "ARG2": "${localWorkspaceFolderBasename}", }, }, } @@ -142,9 +143,10 @@ func TestCompileDevContainer(t *testing.T) { _, err = io.WriteString(file, "FROM ubuntu") require.NoError(t, err) _ = file.Close() - params, err := dc.Compile(fs, dcDir, envbuilder.MagicDir, "") + params, err := dc.Compile(fs, dcDir, envbuilder.MagicDir, "", "/var/workspace") 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) }) diff --git a/envbuilder.go b/envbuilder.go index 45f2219..e1db216 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io" + "maps" "net" "net/http" "net/url" @@ -18,6 +19,7 @@ import ( "os/user" "path/filepath" "reflect" + "sort" "strconv" "strings" "syscall" @@ -445,7 +447,7 @@ func Run(ctx context.Context, options Options) error { logf(codersdk.LogLevelInfo, "No Dockerfile or image specified; falling back to the default image...") fallbackDockerfile = defaultParams.DockerfilePath } - buildParams, err = devContainer.Compile(options.Filesystem, devcontainerDir, MagicDir, fallbackDockerfile) + buildParams, err = devContainer.Compile(options.Filesystem, devcontainerDir, MagicDir, fallbackDockerfile, options.WorkspaceFolder) if err != nil { return fmt.Errorf("compile devcontainer.json: %w", err) } @@ -700,30 +702,14 @@ func Run(ctx context.Context, options Options) error { } _ = file.Close() - var exportEnvFile *os.File - // Do not export env if we skipped a rebuild, because ENV directives - // from the Dockerfile would not have been processed and we'd miss these - // in the export. We should have generated a complete set of environment - // on the intial build, so exporting environment variables a second time - // isn't useful anyway. - if options.ExportEnvFile != "" && !skippedRebuild { - exportEnvFile, err = os.Create(options.ExportEnvFile) - if err != nil { - return fmt.Errorf("failed to open EXPORT_ENV_FILE %q: %w", options.ExportEnvFile, err) - } - } - exportEnv := func(key, value string) { - if exportEnvFile == nil { - return - } - fmt.Fprintf(exportEnvFile, "%s=%s\n", key, value) - } - configFile, err := image.ConfigFile() if err != nil { return fmt.Errorf("get image config: %w", err) } + containerEnv := make(map[string]string) + remoteEnv := make(map[string]string) + // devcontainer metadata can be persisted through a standard label devContainerMetadata, exists := configFile.Config.Labels["devcontainer.metadata"] if exists { @@ -743,12 +729,8 @@ func Run(ctx context.Context, options Options) error { configFile.Config.User = container.RemoteUser } - for _, env := range []map[string]string{container.ContainerEnv, container.RemoteEnv} { - for key, value := range env { - os.Setenv(key, value) - exportEnv(key, value) - } - } + maps.Copy(containerEnv, container.ContainerEnv) + maps.Copy(remoteEnv, container.RemoteEnv) if !container.OnCreateCommand.IsEmpty() { scripts.OnCreateCommand = container.OnCreateCommand } @@ -784,19 +766,50 @@ func Run(ctx context.Context, options Options) error { } } + allEnvKeys := make(map[string]struct{}) + // It must be set in this parent process otherwise nothing will be found! for _, env := range configFile.Config.Env { pair := strings.SplitN(env, "=", 2) os.Setenv(pair[0], pair[1]) - exportEnv(pair[0], pair[1]) + allEnvKeys[pair[0]] = struct{}{} } - for _, env := range buildParams.Env { - pair := strings.SplitN(env, "=", 2) - os.Setenv(pair[0], pair[1]) - exportEnv(pair[0], pair[1]) + maps.Copy(containerEnv, buildParams.ContainerEnv) + maps.Copy(remoteEnv, buildParams.RemoteEnv) + + for _, env := range []map[string]string{containerEnv, remoteEnv} { + envKeys := make([]string, 0, len(env)) + for key := range env { + envKeys = append(envKeys, key) + allEnvKeys[key] = struct{}{} + } + sort.Strings(envKeys) + for _, envVar := range envKeys { + value := devcontainer.SubstituteVars(env[envVar], options.WorkspaceFolder) + os.Setenv(envVar, value) + } } - if exportEnvFile != nil { + // Do not export env if we skipped a rebuild, because ENV directives + // from the Dockerfile would not have been processed and we'd miss these + // in the export. We should have generated a complete set of environment + // on the intial build, so exporting environment variables a second time + // isn't useful anyway. + if options.ExportEnvFile != "" && !skippedRebuild { + exportEnvFile, err := os.Create(options.ExportEnvFile) + if err != nil { + return fmt.Errorf("failed to open EXPORT_ENV_FILE %q: %w", options.ExportEnvFile, err) + } + + envKeys := make([]string, 0, len(allEnvKeys)) + for key := range allEnvKeys { + envKeys = append(envKeys, key) + } + sort.Strings(envKeys) + for _, key := range envKeys { + fmt.Fprintf(exportEnvFile, "%s=%s\n", key, os.Getenv(key)) + } + exportEnvFile.Close() } diff --git a/integration/integration_test.go b/integration/integration_test.go index fca7f36..2bab8e0 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -418,7 +418,7 @@ func TestExitBuildOnFailure(t *testing.T) { require.ErrorContains(t, err, "parsing dockerfile") } -func TestExportEnvFile(t *testing.T) { +func TestContainerEnv(t *testing.T) { t.Parallel() // Ensures that a Git repository with a devcontainer.json is cloned and built. @@ -430,10 +430,13 @@ func TestExportEnvFile(t *testing.T) { "dockerfile": "Dockerfile" }, "containerEnv": { - "FROM_CONTAINER_ENV": "bar" + "FROM_CONTAINER_ENV": "bar", + "PATH": "/bin" }, "remoteEnv": { - "FROM_REMOTE_ENV": "baz" + "FROM_REMOTE_ENV": "baz", + "PATH": "/usr/local/bin:${containerEnv:PATH}:${containerEnv:GOPATH:/go/bin}:/opt", + "REMOTE_BAR": "${FROM_CONTAINER_ENV}" } }`, ".devcontainer/Dockerfile": "FROM alpine:latest\nENV FROM_DOCKERFILE=foo", @@ -447,9 +450,11 @@ func TestExportEnvFile(t *testing.T) { output := execContainer(t, ctr, "cat /env") require.Contains(t, strings.TrimSpace(output), - `FROM_DOCKERFILE=foo -FROM_CONTAINER_ENV=bar -FROM_REMOTE_ENV=baz`) + `FROM_CONTAINER_ENV=bar +FROM_DOCKERFILE=foo +FROM_REMOTE_ENV=baz +PATH=/usr/local/bin:/bin:/go/bin:/opt +REMOTE_BAR=bar`) } func TestLifecycleScripts(t *testing.T) {