Skip to content

Commit 467a6e8

Browse files
johnstcnjohnstcn
committed
env from devcontainer metadata
1 parent d52d0c1 commit 467a6e8

File tree

2 files changed

+137
-42
lines changed

2 files changed

+137
-42
lines changed

‎agent/agentcontainers/containers_dockercli.go

Lines changed: 79 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"os"
1010
"os/user"
11+
"slices"
1112
"sort"
1213
"strconv"
1314
"strings"
@@ -109,13 +110,14 @@ func EnvInfo(ctx context.Context, execer agentexec.Execer, container, containerU
109110
}
110111
cei.userShell = passwdFields[6]
111112

112-
// Finally, get the environment of the container.
113-
// TODO: For containers, we can't simply run `env` inside the container.
114113
// We need to inspect the container labels for remoteEnv and append these to
115114
// the resulting docker exec command.
116115
// ref: https://code.visualstudio.com/docs/devcontainers/attach-container
117-
// For now, we're simply returning the host environment.
118-
cei.env = make([]string, 0)
116+
env, err := devcontainerEnv(ctx, execer, container)
117+
if err != nil { // best effort.
118+
return nil, xerrors.Errorf("read devcontainer remoteEnv: %w", err)
119+
}
120+
cei.env = env
119121

120122
return &cei, nil
121123
}
@@ -158,12 +160,57 @@ func (cei *ContainerEnvInfoer) ModifyCommand(cmd string, args ...string) (string
158160
// Set the working directory to the user's home directory as a sane default.
159161
"--workdir",
160162
cei.user.HomeDir,
161-
cei.container,
162-
cmd,
163163
}
164+
165+
// Append the environment variables from the container.
166+
for _, e := range cei.env {
167+
dockerArgs = append(dockerArgs, "--env", e)
168+
}
169+
170+
// Append the container name and the command.
171+
dockerArgs = append(dockerArgs, cei.container, cmd)
164172
return "docker", append(dockerArgs, args...)
165173
}
166174

175+
// devcontainerEnv is a helper function that inspects the container labels to
176+
// find the required environment variables for running a command in the container.
177+
func devcontainerEnv(ctx context.Context, execer agentexec.Execer, container string) ([]string, error) {
178+
ins, stderr, err := runDockerInspect(ctx, execer, container)
179+
if err != nil {
180+
return nil, xerrors.Errorf("inspect container: %w: %q", err, stderr)
181+
}
182+
183+
if len(ins) != 1 {
184+
return nil, xerrors.Errorf("inspect container: expected 1 container, got %d", len(ins))
185+
}
186+
187+
in := ins[0]
188+
if in.Config.Labels == nil {
189+
return nil, nil
190+
}
191+
192+
// We want to look for the devcontainer metadata, which is in the
193+
// value of the label `devcontainer.metadata`.
194+
rawMeta, ok := in.Config.Labels["devcontainer.metadata"]
195+
if !ok {
196+
return nil, nil
197+
}
198+
meta := struct {
199+
RemoteEnv map[string]string `json:"remoteEnv"`
200+
}{}
201+
if err := json.Unmarshal([]byte(rawMeta), &meta); err != nil {
202+
return nil, xerrors.Errorf("unmarshal devcontainer.metadata: %w", err)
203+
}
204+
205+
// The environment variables are stored in the `remoteEnv` key.
206+
env := make([]string, 0, len(meta.RemoteEnv))
207+
for k, v := range meta.RemoteEnv {
208+
env = append(env, fmt.Sprintf("%s=%s", k, v))
209+
}
210+
slices.Sort(env)
211+
return env, nil
212+
}
213+
167214
// wrapDockerExec is a helper function that wraps the given command and arguments
168215
// with a docker exec command that runs as the given user in the given
169216
// container. This is used to fetch information about a container prior to
@@ -227,30 +274,16 @@ func (dcl *DockerCLILister) List(ctx context.Context) (codersdk.WorkspaceAgentLi
227274
}
228275

229276
// now we can get the detailed information for each container
230-
// Run `docker inspect` on each container ID
231-
stdoutBuf.Reset()
232-
stderrBuf.Reset()
233-
// nolint: gosec // We are not executing user input, these IDs come from
234-
// `docker ps`.
235-
cmd = dcl.execer.CommandContext(ctx, "docker", append([]string{"inspect"}, ids...)...)
236-
cmd.Stdout = &stdoutBuf
237-
cmd.Stderr = &stderrBuf
238-
if err := cmd.Run(); err != nil {
239-
return codersdk.WorkspaceAgentListContainersResponse{}, xerrors.Errorf("run docker inspect: %w: %s", err, strings.TrimSpace(stderrBuf.String()))
240-
}
241-
242-
dockerInspectStderr := strings.TrimSpace(stderrBuf.String())
243-
277+
// Run `docker inspect` on each container ID.
244278
// NOTE: There is an unavoidable potential race condition where a
245279
// container is removed between `docker ps` and `docker inspect`.
246280
// In this case, stderr will contain an error message but stdout
247281
// will still contain valid JSON. We will just end up missing
248282
// information about the removed container. We could potentially
249283
// log this error, but I'm not sure it's worth it.
250-
ins := make([]dockerInspect, 0, len(ids))
251-
if err := json.NewDecoder(&stdoutBuf).Decode(&ins); err != nil {
252-
// However, if we just get invalid JSON, we should absolutely return an error.
253-
return codersdk.WorkspaceAgentListContainersResponse{}, xerrors.Errorf("decode docker inspect output: %w", err)
284+
ins, dockerInspectStderr, err := runDockerInspect(ctx, dcl.execer, ids...)
285+
if err != nil {
286+
return codersdk.WorkspaceAgentListContainersResponse{}, xerrors.Errorf("run docker inspect: %w", err)
254287
}
255288

256289
res := codersdk.WorkspaceAgentListContainersResponse{
@@ -272,6 +305,28 @@ func (dcl *DockerCLILister) List(ctx context.Context) (codersdk.WorkspaceAgentLi
272305
return res, nil
273306
}
274307

308+
// runDockerInspect is a helper function that runs `docker inspect` on the given
309+
// container IDs and returns the parsed output.
310+
// The stderr output is also returned for logging purposes.
311+
func runDockerInspect(ctx context.Context, execer agentexec.Execer, ids ...string) ([]dockerInspect, string, error) {
312+
var stdoutBuf, stderrBuf bytes.Buffer
313+
cmd := execer.CommandContext(ctx, "docker", append([]string{"inspect"}, ids...)...)
314+
cmd.Stdout = &stdoutBuf
315+
cmd.Stderr = &stderrBuf
316+
err := cmd.Run()
317+
stderr := strings.TrimSpace(stderrBuf.String())
318+
if err != nil {
319+
return nil, stderr, err
320+
}
321+
322+
var ins []dockerInspect
323+
if err := json.NewDecoder(&stdoutBuf).Decode(&ins); err != nil {
324+
return nil, stderr, xerrors.Errorf("decode docker inspect output: %w", err)
325+
}
326+
327+
return ins, stderr, nil
328+
}
329+
275330
// To avoid a direct dependency on the Docker API, we use the docker CLI
276331
// to fetch information about containers.
277332
type dockerInspect struct {

‎agent/agentcontainers/containers_internal_test.go

Lines changed: 58 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package agentcontainers
33
import (
44
"fmt"
55
"os"
6+
"slices"
67
"strconv"
78
"strings"
89
"testing"
@@ -47,10 +48,13 @@ func TestIntegrationDocker(t *testing.T) {
4748
// Pick a random port to expose for testing port bindings.
4849
testRandPort := testutil.RandomPortNoListen(t)
4950
ct, err := pool.RunWithOptions(&dockertest.RunOptions{
50-
Repository: "busybox",
51-
Tag: "latest",
52-
Cmd: []string{"sleep", "infnity"},
53-
Labels: map[string]string{"com.coder.test": testLabelValue},
51+
Repository: "busybox",
52+
Tag: "latest",
53+
Cmd: []string{"sleep", "infnity"},
54+
Labels: map[string]string{
55+
"com.coder.test": testLabelValue,
56+
"devcontainer.metadata": `{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}`,
57+
},
5458
Mounts: []string{testTempDir + ":" + testTempDir},
5559
ExposedPorts: []string{fmt.Sprintf("%d/tcp", testRandPort)},
5660
PortBindings: map[docker.Port][]docker.PortBinding{
@@ -122,6 +126,12 @@ func TestIntegrationDocker(t *testing.T) {
122126
matchHostnameOuput := func(line string) bool {
123127
return strings.Contains(strings.TrimSpace(line), ct.Container.Config.Hostname)
124128
}
129+
matchEnvCmd := func(line string) bool {
130+
return strings.Contains(strings.TrimSpace(line), "env")
131+
}
132+
matchEnvOutput := func(line string) bool {
133+
return strings.Contains(line, "FOO=bar") || strings.Contains(line, "MULTILINE=foo")
134+
}
125135
require.NoError(t, tr.ReadUntil(ctx, matchPrompt), "failed to match prompt")
126136
t.Logf("Matched prompt")
127137
_, err = ptyCmd.InputWriter().Write([]byte("hostname\r\n"))
@@ -131,6 +141,13 @@ func TestIntegrationDocker(t *testing.T) {
131141
t.Logf("Matched hostname command")
132142
require.NoError(t, tr.ReadUntil(ctx, matchHostnameOuput), "failed to match hostname output")
133143
t.Logf("Matched hostname output")
144+
_, err = ptyCmd.InputWriter().Write([]byte("env\r\n"))
145+
require.NoError(t, err, "failed to write to pty")
146+
t.Logf("Wrote env command")
147+
require.NoError(t, tr.ReadUntil(ctx, matchEnvCmd), "failed to match env command")
148+
t.Logf("Matched env command")
149+
require.NoError(t, tr.ReadUntil(ctx, matchEnvOutput), "failed to match env output")
150+
t.Logf("Matched env output")
134151
break
135152
}
136153
}
@@ -403,49 +420,56 @@ func TestConvertDockerVolume(t *testing.T) {
403420
// CODER_TEST_USE_DOCKER=1 go test ./agent/agentcontainers -run TestDockerEnvInfoer
404421
func TestDockerEnvInfoer(t *testing.T) {
405422
t.Parallel()
406-
if ctud, ok := os.LookupEnv("CODER_TEST_USE_DOCKER"); !ok || ctud != "1" {
407-
t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test")
408-
}
423+
// if ctud, ok := os.LookupEnv("CODER_TEST_USE_DOCKER"); !ok || ctud != "1" {
424+
// t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test")
425+
// }
409426

410427
pool, err := dockertest.NewPool("")
411428
require.NoError(t, err, "Could not connect to docker")
412429
// nolint:paralleltest // variable recapture no longer required
413430
for idx, tt := range []struct {
414431
image string
415-
env []string
432+
labels map[string]string
433+
expectedEnv []string
416434
containerUser string
417435
expectedUsername string
418436
expectedUserShell string
419437
}{
420438
{
421-
image: "busybox:latest",
422-
env: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"},
439+
image: "busybox:latest",
440+
labels: map[string]string{`devcontainer.metadata`: `{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}`},
441+
442+
expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"},
423443
expectedUsername: "root",
424444
expectedUserShell: "/bin/sh",
425445
},
426446
{
427447
image: "busybox:latest",
428-
env: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"},
448+
labels: map[string]string{`devcontainer.metadata`: `{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}`},
449+
expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"},
429450
containerUser: "root",
430451
expectedUsername: "root",
431452
expectedUserShell: "/bin/sh",
432453
},
433454
{
434455
image: "codercom/enterprise-minimal:ubuntu",
435-
env: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"},
456+
labels: map[string]string{`devcontainer.metadata`: `{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}`},
457+
expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"},
436458
expectedUsername: "coder",
437459
expectedUserShell: "/bin/bash",
438460
},
439461
{
440462
image: "codercom/enterprise-minimal:ubuntu",
441-
env: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"},
463+
labels: map[string]string{`devcontainer.metadata`: `{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}`},
464+
expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"},
442465
containerUser: "coder",
443466
expectedUsername: "coder",
444467
expectedUserShell: "/bin/bash",
445468
},
446469
{
447470
image: "codercom/enterprise-minimal:ubuntu",
448-
env: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"},
471+
labels: map[string]string{`devcontainer.metadata`: `{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}`},
472+
expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"},
449473
containerUser: "root",
450474
expectedUsername: "root",
451475
expectedUserShell: "/bin/bash",
@@ -462,7 +486,7 @@ func TestDockerEnvInfoer(t *testing.T) {
462486
Repository: image,
463487
Tag: tag,
464488
Cmd: []string{"sleep", "infinity"},
465-
Env: tt.env,
489+
Labels: tt.labels,
466490
}, func(config *docker.HostConfig) {
467491
config.AutoRemove = true
468492
config.RestartPolicy = docker.RestartPolicy{Name: "no"}
@@ -490,9 +514,25 @@ func TestDockerEnvInfoer(t *testing.T) {
490514
require.NoError(t, err, "Expected no error from UserShell()")
491515
require.Equal(t, tt.expectedUserShell, sh, "Expected user shell to match")
492516

493-
// TODO: environment from devcontainer labels.
494-
// environ := dei.Environ()
495-
// require.Subset(t, environ, tt.env, "Expected environment to match")
517+
// We don't need to test the actual environment variables here.
518+
environ := dei.Environ()
519+
require.NotEmpty(t, environ, "Expected environ to be non-empty")
520+
521+
// Test that the environment variables are present in modified command
522+
// output.
523+
envCmd, envArgs := dei.ModifyCommand("env")
524+
for _, env := range tt.expectedEnv {
525+
require.Subset(t, envArgs, []string{"--env", env})
526+
}
527+
// Run the command in the container and check the output
528+
// HACK: we remove the --tty argument because we're not running in a tty
529+
envArgs = slices.DeleteFunc(envArgs, func(s string) bool { return s == "--tty" })
530+
stdout, stderr, err := run(ctx, agentexec.DefaultExecer, envCmd, envArgs...)
531+
require.Empty(t, stderr, "Expected no stderr output")
532+
require.NoError(t, err, "Expected no error from running command")
533+
for _, env := range tt.expectedEnv {
534+
require.Contains(t, stdout, env)
535+
}
496536
})
497537
}
498538
}

0 commit comments

Comments
 (0)