From 31c7bffd5d87876add52605cbc839f7ef283d1f4 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 10 Apr 2025 13:28:11 +0000 Subject: [PATCH] refactor(agent/agentcontainers): Implement API service --- agent/agentcontainers/api.go | 205 +++++++++ agent/agentcontainers/api_internal_test.go | 161 +++++++ agent/agentcontainers/api_test.go | 171 +++++++ agent/agentcontainers/containers.go | 194 -------- agent/agentcontainers/containers_dockercli.go | 6 +- .../containers_internal_test.go | 421 ------------------ agent/agentcontainers/containers_test.go | 416 +++++++++++------ agent/agentcontainers/devcontainer.go | 1 - agent/api.go | 11 +- 9 files changed, 821 insertions(+), 765 deletions(-) create mode 100644 agent/agentcontainers/api.go create mode 100644 agent/agentcontainers/api_internal_test.go create mode 100644 agent/agentcontainers/api_test.go diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go new file mode 100644 index 0000000000000..81354457d0730 --- /dev/null +++ b/agent/agentcontainers/api.go @@ -0,0 +1,205 @@ +package agentcontainers + +import ( + "context" + "errors" + "net/http" + "slices" + "time" + + "github.com/go-chi/chi/v5" + "golang.org/x/xerrors" + + "cdr.dev/slog" + "github.com/coder/coder/v2/agent/agentexec" + "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/quartz" +) + +const ( + defaultGetContainersCacheDuration = 10 * time.Second + dockerCreatedAtTimeFormat = "2006-01-02 15:04:05 -0700 MST" + getContainersTimeout = 5 * time.Second +) + +// API is responsible for container-related operations in the agent. +// It provides methods to list and manage containers. +type API struct { + cacheDuration time.Duration + cl Lister + dccli DevcontainerCLI + clock quartz.Clock + + // lockCh protects the below fields. We use a channel instead of a mutex so we + // can handle cancellation properly. + lockCh chan struct{} + containers codersdk.WorkspaceAgentListContainersResponse + mtime time.Time +} + +// Option is a functional option for API. +type Option func(*API) + +// WithLister sets the agentcontainers.Lister implementation to use. +// The default implementation uses the Docker CLI to list containers. +func WithLister(cl Lister) Option { + return func(api *API) { + api.cl = cl + } +} + +func WithDevcontainerCLI(dccli DevcontainerCLI) Option { + return func(api *API) { + api.dccli = dccli + } +} + +// NewAPI returns a new API with the given options applied. +func NewAPI(logger slog.Logger, options ...Option) *API { + api := &API{ + clock: quartz.NewReal(), + cacheDuration: defaultGetContainersCacheDuration, + lockCh: make(chan struct{}, 1), + } + for _, opt := range options { + opt(api) + } + if api.cl == nil { + api.cl = &DockerCLILister{} + } + if api.dccli == nil { + api.dccli = NewDevcontainerCLI(logger, agentexec.DefaultExecer) + } + + return api +} + +// Routes returns the HTTP handler for container-related routes. +func (api *API) Routes() http.Handler { + r := chi.NewRouter() + r.Get("/", api.handleList) + r.Post("/{id}/recreate", api.handleRecreate) + return r +} + +// handleList handles the HTTP request to list containers. +func (api *API) handleList(rw http.ResponseWriter, r *http.Request) { + select { + case <-r.Context().Done(): + // Client went away. + return + default: + ct, err := api.getContainers(r.Context()) + if err != nil { + if errors.Is(err, context.Canceled) { + httpapi.Write(r.Context(), rw, http.StatusRequestTimeout, codersdk.Response{ + Message: "Could not get containers.", + Detail: "Took too long to list containers.", + }) + return + } + httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Could not get containers.", + Detail: err.Error(), + }) + return + } + + httpapi.Write(r.Context(), rw, http.StatusOK, ct) + } +} + +func copyListContainersResponse(resp codersdk.WorkspaceAgentListContainersResponse) codersdk.WorkspaceAgentListContainersResponse { + return codersdk.WorkspaceAgentListContainersResponse{ + Containers: slices.Clone(resp.Containers), + Warnings: slices.Clone(resp.Warnings), + } +} + +func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) { + select { + case <-ctx.Done(): + return codersdk.WorkspaceAgentListContainersResponse{}, ctx.Err() + default: + api.lockCh <- struct{}{} + } + defer func() { + <-api.lockCh + }() + + now := api.clock.Now() + if now.Sub(api.mtime) < api.cacheDuration { + return copyListContainersResponse(api.containers), nil + } + + timeoutCtx, timeoutCancel := context.WithTimeout(ctx, getContainersTimeout) + defer timeoutCancel() + updated, err := api.cl.List(timeoutCtx) + if err != nil { + return codersdk.WorkspaceAgentListContainersResponse{}, xerrors.Errorf("get containers: %w", err) + } + api.containers = updated + api.mtime = now + + return copyListContainersResponse(api.containers), nil +} + +// handleRecreate handles the HTTP request to recreate a container. +func (api *API) handleRecreate(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + id := chi.URLParam(r, "id") + + if id == "" { + httpapi.Write(ctx, w, http.StatusBadRequest, codersdk.Response{ + Message: "Missing container ID or name", + Detail: "Container ID or name is required to recreate a devcontainer.", + }) + return + } + + containers, err := api.cl.List(ctx) + if err != nil { + httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ + Message: "Could not list containers", + Detail: err.Error(), + }) + return + } + + containerIdx := slices.IndexFunc(containers.Containers, func(c codersdk.WorkspaceAgentContainer) bool { + return c.Match(id) + }) + if containerIdx == -1 { + httpapi.Write(ctx, w, http.StatusNotFound, codersdk.Response{ + Message: "Container not found", + Detail: "Container ID or name not found in the list of containers.", + }) + return + } + + container := containers.Containers[containerIdx] + workspaceFolder := container.Labels[DevcontainerLocalFolderLabel] + configPath := container.Labels[DevcontainerConfigFileLabel] + + // Workspace folder is required to recreate a container, we don't verify + // the config path here because it's optional. + if workspaceFolder == "" { + httpapi.Write(ctx, w, http.StatusBadRequest, codersdk.Response{ + Message: "Missing workspace folder label", + Detail: "The workspace folder label is required to recreate a devcontainer.", + }) + return + } + + _, err = api.dccli.Up(ctx, workspaceFolder, configPath, WithRemoveExistingContainer()) + if err != nil { + httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ + Message: "Could not recreate devcontainer", + Detail: err.Error(), + }) + return + } + + w.WriteHeader(http.StatusNoContent) +} diff --git a/agent/agentcontainers/api_internal_test.go b/agent/agentcontainers/api_internal_test.go new file mode 100644 index 0000000000000..756526d341d68 --- /dev/null +++ b/agent/agentcontainers/api_internal_test.go @@ -0,0 +1,161 @@ +package agentcontainers + +import ( + "math/rand" + "strings" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/agent/agentcontainers/acmock" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" + "github.com/coder/quartz" +) + +func TestAPI(t *testing.T) { + t.Parallel() + + // List tests the API.getContainers method using a mock + // implementation. It specifically tests caching behavior. + t.Run("List", func(t *testing.T) { + t.Parallel() + + fakeCt := fakeContainer(t) + fakeCt2 := fakeContainer(t) + makeResponse := func(cts ...codersdk.WorkspaceAgentContainer) codersdk.WorkspaceAgentListContainersResponse { + return codersdk.WorkspaceAgentListContainersResponse{Containers: cts} + } + + // Each test case is called multiple times to ensure idempotency + for _, tc := range []struct { + name string + // data to be stored in the handler + cacheData codersdk.WorkspaceAgentListContainersResponse + // duration of cache + cacheDur time.Duration + // relative age of the cached data + cacheAge time.Duration + // function to set up expectations for the mock + setupMock func(*acmock.MockLister) + // expected result + expected codersdk.WorkspaceAgentListContainersResponse + // expected error + expectedErr string + }{ + { + name: "no cache", + setupMock: func(mcl *acmock.MockLister) { + mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt), nil).AnyTimes() + }, + expected: makeResponse(fakeCt), + }, + { + name: "no data", + cacheData: makeResponse(), + cacheAge: 2 * time.Second, + cacheDur: time.Second, + setupMock: func(mcl *acmock.MockLister) { + mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt), nil).AnyTimes() + }, + expected: makeResponse(fakeCt), + }, + { + name: "cached data", + cacheAge: time.Second, + cacheData: makeResponse(fakeCt), + cacheDur: 2 * time.Second, + expected: makeResponse(fakeCt), + }, + { + name: "lister error", + setupMock: func(mcl *acmock.MockLister) { + mcl.EXPECT().List(gomock.Any()).Return(makeResponse(), assert.AnError).AnyTimes() + }, + expectedErr: assert.AnError.Error(), + }, + { + name: "stale cache", + cacheAge: 2 * time.Second, + cacheData: makeResponse(fakeCt), + cacheDur: time.Second, + setupMock: func(mcl *acmock.MockLister) { + mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt2), nil).AnyTimes() + }, + expected: makeResponse(fakeCt2), + }, + } { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var ( + ctx = testutil.Context(t, testutil.WaitShort) + clk = quartz.NewMock(t) + ctrl = gomock.NewController(t) + mockLister = acmock.NewMockLister(ctrl) + now = time.Now().UTC() + logger = slogtest.Make(t, nil).Leveled(slog.LevelDebug) + api = NewAPI(logger, WithLister(mockLister)) + ) + api.cacheDuration = tc.cacheDur + api.clock = clk + api.containers = tc.cacheData + if tc.cacheAge != 0 { + api.mtime = now.Add(-tc.cacheAge) + } + if tc.setupMock != nil { + tc.setupMock(mockLister) + } + + clk.Set(now).MustWait(ctx) + + // Repeat the test to ensure idempotency + for i := 0; i < 2; i++ { + actual, err := api.getContainers(ctx) + if tc.expectedErr != "" { + require.Empty(t, actual, "expected no data (attempt %d)", i) + require.ErrorContains(t, err, tc.expectedErr, "expected error (attempt %d)", i) + } else { + require.NoError(t, err, "expected no error (attempt %d)", i) + require.Equal(t, tc.expected, actual, "expected containers to be equal (attempt %d)", i) + } + } + }) + } + }) +} + +func fakeContainer(t *testing.T, mut ...func(*codersdk.WorkspaceAgentContainer)) codersdk.WorkspaceAgentContainer { + t.Helper() + ct := codersdk.WorkspaceAgentContainer{ + CreatedAt: time.Now().UTC(), + ID: uuid.New().String(), + FriendlyName: testutil.GetRandomName(t), + Image: testutil.GetRandomName(t) + ":" + strings.Split(uuid.New().String(), "-")[0], + Labels: map[string]string{ + testutil.GetRandomName(t): testutil.GetRandomName(t), + }, + Running: true, + Ports: []codersdk.WorkspaceAgentContainerPort{ + { + Network: "tcp", + Port: testutil.RandomPortNoListen(t), + HostPort: testutil.RandomPortNoListen(t), + //nolint:gosec // this is a test + HostIP: []string{"127.0.0.1", "[::1]", "localhost", "0.0.0.0", "[::]", testutil.GetRandomName(t)}[rand.Intn(6)], + }, + }, + Status: testutil.MustRandString(t, 10), + Volumes: map[string]string{testutil.GetRandomName(t): testutil.GetRandomName(t)}, + } + for _, m := range mut { + m(&ct) + } + return ct +} diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go new file mode 100644 index 0000000000000..76a88f4fc1da4 --- /dev/null +++ b/agent/agentcontainers/api_test.go @@ -0,0 +1,171 @@ +package agentcontainers_test + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/agent/agentcontainers" + "github.com/coder/coder/v2/codersdk" +) + +// fakeLister implements the agentcontainers.Lister interface for +// testing. +type fakeLister struct { + containers codersdk.WorkspaceAgentListContainersResponse + err error +} + +func (f *fakeLister) List(_ context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) { + return f.containers, f.err +} + +// fakeDevcontainerCLI implements the agentcontainers.DevcontainerCLI +// interface for testing. +type fakeDevcontainerCLI struct { + id string + err error +} + +func (f *fakeDevcontainerCLI) Up(_ context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) { + return f.id, f.err +} + +func TestAPI(t *testing.T) { + t.Parallel() + + t.Run("Recreate", func(t *testing.T) { + t.Parallel() + + validContainer := codersdk.WorkspaceAgentContainer{ + ID: "container-id", + FriendlyName: "container-name", + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/workspace", + agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json", + }, + } + + missingFolderContainer := codersdk.WorkspaceAgentContainer{ + ID: "missing-folder-container", + FriendlyName: "missing-folder-container", + Labels: map[string]string{}, + } + + tests := []struct { + name string + containerID string + lister *fakeLister + devcontainerCLI *fakeDevcontainerCLI + wantStatus int + wantBody string + }{ + { + name: "Missing ID", + containerID: "", + lister: &fakeLister{}, + devcontainerCLI: &fakeDevcontainerCLI{}, + wantStatus: http.StatusBadRequest, + wantBody: "Missing container ID or name", + }, + { + name: "List error", + containerID: "container-id", + lister: &fakeLister{ + err: xerrors.New("list error"), + }, + devcontainerCLI: &fakeDevcontainerCLI{}, + wantStatus: http.StatusInternalServerError, + wantBody: "Could not list containers", + }, + { + name: "Container not found", + containerID: "nonexistent-container", + lister: &fakeLister{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{validContainer}, + }, + }, + devcontainerCLI: &fakeDevcontainerCLI{}, + wantStatus: http.StatusNotFound, + wantBody: "Container not found", + }, + { + name: "Missing workspace folder label", + containerID: "missing-folder-container", + lister: &fakeLister{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{missingFolderContainer}, + }, + }, + devcontainerCLI: &fakeDevcontainerCLI{}, + wantStatus: http.StatusBadRequest, + wantBody: "Missing workspace folder label", + }, + { + name: "Devcontainer CLI error", + containerID: "container-id", + lister: &fakeLister{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{validContainer}, + }, + }, + devcontainerCLI: &fakeDevcontainerCLI{ + err: xerrors.New("devcontainer CLI error"), + }, + wantStatus: http.StatusInternalServerError, + wantBody: "Could not recreate devcontainer", + }, + { + name: "OK", + containerID: "container-id", + lister: &fakeLister{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{validContainer}, + }, + }, + devcontainerCLI: &fakeDevcontainerCLI{}, + wantStatus: http.StatusNoContent, + wantBody: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug) + + // Setup router with the handler under test. + r := chi.NewRouter() + api := agentcontainers.NewAPI( + logger, + agentcontainers.WithLister(tt.lister), + agentcontainers.WithDevcontainerCLI(tt.devcontainerCLI), + ) + r.Mount("/containers", api.Routes()) + + // Simulate HTTP request to the recreate endpoint. + req := httptest.NewRequest(http.MethodPost, "/containers/"+tt.containerID+"/recreate", nil) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + + // Check the response status code and body. + require.Equal(t, tt.wantStatus, rec.Code, "status code mismatch") + if tt.wantBody != "" { + assert.Contains(t, rec.Body.String(), tt.wantBody, "response body mismatch") + } else if tt.wantStatus == http.StatusNoContent { + assert.Empty(t, rec.Body.String(), "expected empty response body") + } + }) + } + }) +} diff --git a/agent/agentcontainers/containers.go b/agent/agentcontainers/containers.go index edd099dd842c5..5be288781d480 100644 --- a/agent/agentcontainers/containers.go +++ b/agent/agentcontainers/containers.go @@ -2,146 +2,10 @@ package agentcontainers import ( "context" - "errors" - "net/http" - "slices" - "time" - "golang.org/x/xerrors" - - "github.com/go-chi/chi/v5" - - "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/codersdk" - "github.com/coder/quartz" -) - -const ( - defaultGetContainersCacheDuration = 10 * time.Second - dockerCreatedAtTimeFormat = "2006-01-02 15:04:05 -0700 MST" - getContainersTimeout = 5 * time.Second ) -type Handler struct { - cacheDuration time.Duration - cl Lister - dccli DevcontainerCLI - clock quartz.Clock - - // lockCh protects the below fields. We use a channel instead of a mutex so we - // can handle cancellation properly. - lockCh chan struct{} - containers *codersdk.WorkspaceAgentListContainersResponse - mtime time.Time -} - -// Option is a functional option for Handler. -type Option func(*Handler) - -// WithLister sets the agentcontainers.Lister implementation to use. -// The default implementation uses the Docker CLI to list containers. -func WithLister(cl Lister) Option { - return func(ch *Handler) { - ch.cl = cl - } -} - -func WithDevcontainerCLI(dccli DevcontainerCLI) Option { - return func(ch *Handler) { - ch.dccli = dccli - } -} - -// New returns a new Handler with the given options applied. -func New(options ...Option) *Handler { - ch := &Handler{ - lockCh: make(chan struct{}, 1), - } - for _, opt := range options { - opt(ch) - } - return ch -} - -func (ch *Handler) List(rw http.ResponseWriter, r *http.Request) { - select { - case <-r.Context().Done(): - // Client went away. - return - default: - ct, err := ch.getContainers(r.Context()) - if err != nil { - if errors.Is(err, context.Canceled) { - httpapi.Write(r.Context(), rw, http.StatusRequestTimeout, codersdk.Response{ - Message: "Could not get containers.", - Detail: "Took too long to list containers.", - }) - return - } - httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Could not get containers.", - Detail: err.Error(), - }) - return - } - - httpapi.Write(r.Context(), rw, http.StatusOK, ct) - } -} - -func (ch *Handler) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) { - select { - case <-ctx.Done(): - return codersdk.WorkspaceAgentListContainersResponse{}, ctx.Err() - default: - ch.lockCh <- struct{}{} - } - defer func() { - <-ch.lockCh - }() - - // make zero-value usable - if ch.cacheDuration == 0 { - ch.cacheDuration = defaultGetContainersCacheDuration - } - if ch.cl == nil { - ch.cl = &DockerCLILister{} - } - if ch.containers == nil { - ch.containers = &codersdk.WorkspaceAgentListContainersResponse{} - } - if ch.clock == nil { - ch.clock = quartz.NewReal() - } - - now := ch.clock.Now() - if now.Sub(ch.mtime) < ch.cacheDuration { - // Return a copy of the cached data to avoid accidental modification by the caller. - cpy := codersdk.WorkspaceAgentListContainersResponse{ - Containers: slices.Clone(ch.containers.Containers), - Warnings: slices.Clone(ch.containers.Warnings), - } - return cpy, nil - } - - timeoutCtx, timeoutCancel := context.WithTimeout(ctx, getContainersTimeout) - defer timeoutCancel() - updated, err := ch.cl.List(timeoutCtx) - if err != nil { - return codersdk.WorkspaceAgentListContainersResponse{}, xerrors.Errorf("get containers: %w", err) - } - ch.containers = &updated - ch.mtime = now - - // Return a copy of the cached data to avoid accidental modification by the - // caller. - cpy := codersdk.WorkspaceAgentListContainersResponse{ - Containers: slices.Clone(ch.containers.Containers), - Warnings: slices.Clone(ch.containers.Warnings), - } - return cpy, nil -} - // Lister is an interface for listing containers visible to the // workspace agent. type Lister interface { @@ -158,61 +22,3 @@ var _ Lister = NoopLister{} func (NoopLister) List(_ context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) { return codersdk.WorkspaceAgentListContainersResponse{}, nil } - -func (ch *Handler) Recreate(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - id := chi.URLParam(r, "id") - - if id == "" { - httpapi.Write(ctx, w, http.StatusBadRequest, codersdk.Response{ - Message: "Missing container ID or name", - Detail: "Container ID or name is required to recreate a devcontainer.", - }) - return - } - - containers, err := ch.cl.List(ctx) - if err != nil { - httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ - Message: "Could not list containers", - Detail: err.Error(), - }) - return - } - - containerIdx := slices.IndexFunc(containers.Containers, func(c codersdk.WorkspaceAgentContainer) bool { - return c.Match(id) - }) - if containerIdx == -1 { - httpapi.Write(ctx, w, http.StatusNotFound, codersdk.Response{ - Message: "Container not found", - Detail: "Container ID or name not found in the list of containers.", - }) - return - } - - container := containers.Containers[containerIdx] - workspaceFolder := container.Labels[DevcontainerLocalFolderLabel] - configPath := container.Labels[DevcontainerConfigFileLabel] - - // Workspace folder is required to recreate a container, we don't verify - // the config path here because it's optional. - if workspaceFolder == "" { - httpapi.Write(ctx, w, http.StatusBadRequest, codersdk.Response{ - Message: "Missing workspace folder label", - Detail: "The workspace folder label is required to recreate a devcontainer.", - }) - return - } - - _, err = ch.dccli.Up(ctx, workspaceFolder, configPath, WithRemoveExistingContainer()) - if err != nil { - httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ - Message: "Could not recreate devcontainer", - Detail: err.Error(), - }) - return - } - - w.WriteHeader(http.StatusNoContent) -} diff --git a/agent/agentcontainers/containers_dockercli.go b/agent/agentcontainers/containers_dockercli.go index b29f1e974bf3b..208c3ec2ea89b 100644 --- a/agent/agentcontainers/containers_dockercli.go +++ b/agent/agentcontainers/containers_dockercli.go @@ -14,14 +14,14 @@ import ( "strings" "time" + "golang.org/x/exp/maps" + "golang.org/x/xerrors" + "github.com/coder/coder/v2/agent/agentcontainers/dcspec" "github.com/coder/coder/v2/agent/agentexec" "github.com/coder/coder/v2/agent/usershell" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" - - "golang.org/x/exp/maps" - "golang.org/x/xerrors" ) // DockerCLILister is a ContainerLister that lists containers using the docker CLI diff --git a/agent/agentcontainers/containers_internal_test.go b/agent/agentcontainers/containers_internal_test.go index 6b59da407f789..eeb6a5d0374d1 100644 --- a/agent/agentcontainers/containers_internal_test.go +++ b/agent/agentcontainers/containers_internal_test.go @@ -1,163 +1,18 @@ package agentcontainers import ( - "fmt" - "math/rand" "os" "path/filepath" - "slices" - "strconv" - "strings" "testing" "time" - "go.uber.org/mock/gomock" - "github.com/google/go-cmp/cmp" - "github.com/google/uuid" - "github.com/ory/dockertest/v3" - "github.com/ory/dockertest/v3/docker" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/coder/coder/v2/agent/agentcontainers/acmock" - "github.com/coder/coder/v2/agent/agentexec" "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/pty" - "github.com/coder/coder/v2/testutil" - "github.com/coder/quartz" ) -// TestIntegrationDocker tests agentcontainers functionality using a real -// Docker container. It starts a container with a known -// label, lists the containers, and verifies that the expected container is -// returned. It also executes a sample command inside the container. -// The container is deleted after the test is complete. -// As this test creates containers, it is skipped by default. -// It can be run manually as follows: -// -// CODER_TEST_USE_DOCKER=1 go test ./agent/agentcontainers -run TestDockerCLIContainerLister -// -//nolint:paralleltest // This test tends to flake when lots of containers start and stop in parallel. -func TestIntegrationDocker(t *testing.T) { - if ctud, ok := os.LookupEnv("CODER_TEST_USE_DOCKER"); !ok || ctud != "1" { - t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test") - } - - pool, err := dockertest.NewPool("") - require.NoError(t, err, "Could not connect to docker") - testLabelValue := uuid.New().String() - // Create a temporary directory to validate that we surface mounts correctly. - testTempDir := t.TempDir() - // Pick a random port to expose for testing port bindings. - testRandPort := testutil.RandomPortNoListen(t) - ct, err := pool.RunWithOptions(&dockertest.RunOptions{ - Repository: "busybox", - Tag: "latest", - Cmd: []string{"sleep", "infnity"}, - Labels: map[string]string{ - "com.coder.test": testLabelValue, - "devcontainer.metadata": `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`, - }, - Mounts: []string{testTempDir + ":" + testTempDir}, - ExposedPorts: []string{fmt.Sprintf("%d/tcp", testRandPort)}, - PortBindings: map[docker.Port][]docker.PortBinding{ - docker.Port(fmt.Sprintf("%d/tcp", testRandPort)): { - { - HostIP: "0.0.0.0", - HostPort: strconv.FormatInt(int64(testRandPort), 10), - }, - }, - }, - }, func(config *docker.HostConfig) { - config.AutoRemove = true - config.RestartPolicy = docker.RestartPolicy{Name: "no"} - }) - require.NoError(t, err, "Could not start test docker container") - t.Logf("Created container %q", ct.Container.Name) - t.Cleanup(func() { - assert.NoError(t, pool.Purge(ct), "Could not purge resource %q", ct.Container.Name) - t.Logf("Purged container %q", ct.Container.Name) - }) - // Wait for container to start - require.Eventually(t, func() bool { - ct, ok := pool.ContainerByName(ct.Container.Name) - return ok && ct.Container.State.Running - }, testutil.WaitShort, testutil.IntervalSlow, "Container did not start in time") - - dcl := NewDocker(agentexec.DefaultExecer) - ctx := testutil.Context(t, testutil.WaitShort) - actual, err := dcl.List(ctx) - require.NoError(t, err, "Could not list containers") - require.Empty(t, actual.Warnings, "Expected no warnings") - var found bool - for _, foundContainer := range actual.Containers { - if foundContainer.ID == ct.Container.ID { - found = true - assert.Equal(t, ct.Container.Created, foundContainer.CreatedAt) - // ory/dockertest pre-pends a forward slash to the container name. - assert.Equal(t, strings.TrimPrefix(ct.Container.Name, "/"), foundContainer.FriendlyName) - // ory/dockertest returns the sha256 digest of the image. - assert.Equal(t, "busybox:latest", foundContainer.Image) - assert.Equal(t, ct.Container.Config.Labels, foundContainer.Labels) - assert.True(t, foundContainer.Running) - assert.Equal(t, "running", foundContainer.Status) - if assert.Len(t, foundContainer.Ports, 1) { - assert.Equal(t, testRandPort, foundContainer.Ports[0].Port) - assert.Equal(t, "tcp", foundContainer.Ports[0].Network) - } - if assert.Len(t, foundContainer.Volumes, 1) { - assert.Equal(t, testTempDir, foundContainer.Volumes[testTempDir]) - } - // Test that EnvInfo is able to correctly modify a command to be - // executed inside the container. - dei, err := EnvInfo(ctx, agentexec.DefaultExecer, ct.Container.ID, "") - require.NoError(t, err, "Expected no error from DockerEnvInfo()") - ptyWrappedCmd, ptyWrappedArgs := dei.ModifyCommand("/bin/sh", "--norc") - ptyCmd, ptyPs, err := pty.Start(agentexec.DefaultExecer.PTYCommandContext(ctx, ptyWrappedCmd, ptyWrappedArgs...)) - require.NoError(t, err, "failed to start pty command") - t.Cleanup(func() { - _ = ptyPs.Kill() - _ = ptyCmd.Close() - }) - tr := testutil.NewTerminalReader(t, ptyCmd.OutputReader()) - matchPrompt := func(line string) bool { - return strings.Contains(line, "#") - } - matchHostnameCmd := func(line string) bool { - return strings.Contains(strings.TrimSpace(line), "hostname") - } - matchHostnameOuput := func(line string) bool { - return strings.Contains(strings.TrimSpace(line), ct.Container.Config.Hostname) - } - matchEnvCmd := func(line string) bool { - return strings.Contains(strings.TrimSpace(line), "env") - } - matchEnvOutput := func(line string) bool { - return strings.Contains(line, "FOO=bar") || strings.Contains(line, "MULTILINE=foo") - } - require.NoError(t, tr.ReadUntil(ctx, matchPrompt), "failed to match prompt") - t.Logf("Matched prompt") - _, err = ptyCmd.InputWriter().Write([]byte("hostname\r\n")) - require.NoError(t, err, "failed to write to pty") - t.Logf("Wrote hostname command") - require.NoError(t, tr.ReadUntil(ctx, matchHostnameCmd), "failed to match hostname command") - t.Logf("Matched hostname command") - require.NoError(t, tr.ReadUntil(ctx, matchHostnameOuput), "failed to match hostname output") - t.Logf("Matched hostname output") - _, err = ptyCmd.InputWriter().Write([]byte("env\r\n")) - require.NoError(t, err, "failed to write to pty") - t.Logf("Wrote env command") - require.NoError(t, tr.ReadUntil(ctx, matchEnvCmd), "failed to match env command") - t.Logf("Matched env command") - require.NoError(t, tr.ReadUntil(ctx, matchEnvOutput), "failed to match env output") - t.Logf("Matched env output") - break - } - } - assert.True(t, found, "Expected to find container with label 'com.coder.test=%s'", testLabelValue) -} - func TestWrapDockerExec(t *testing.T) { t.Parallel() tests := []struct { @@ -196,120 +51,6 @@ func TestWrapDockerExec(t *testing.T) { } } -// TestContainersHandler tests the containersHandler.getContainers method using -// a mock implementation. It specifically tests caching behavior. -func TestContainersHandler(t *testing.T) { - t.Parallel() - - t.Run("list", func(t *testing.T) { - t.Parallel() - - fakeCt := fakeContainer(t) - fakeCt2 := fakeContainer(t) - makeResponse := func(cts ...codersdk.WorkspaceAgentContainer) codersdk.WorkspaceAgentListContainersResponse { - return codersdk.WorkspaceAgentListContainersResponse{Containers: cts} - } - - // Each test case is called multiple times to ensure idempotency - for _, tc := range []struct { - name string - // data to be stored in the handler - cacheData codersdk.WorkspaceAgentListContainersResponse - // duration of cache - cacheDur time.Duration - // relative age of the cached data - cacheAge time.Duration - // function to set up expectations for the mock - setupMock func(*acmock.MockLister) - // expected result - expected codersdk.WorkspaceAgentListContainersResponse - // expected error - expectedErr string - }{ - { - name: "no cache", - setupMock: func(mcl *acmock.MockLister) { - mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt), nil).AnyTimes() - }, - expected: makeResponse(fakeCt), - }, - { - name: "no data", - cacheData: makeResponse(), - cacheAge: 2 * time.Second, - cacheDur: time.Second, - setupMock: func(mcl *acmock.MockLister) { - mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt), nil).AnyTimes() - }, - expected: makeResponse(fakeCt), - }, - { - name: "cached data", - cacheAge: time.Second, - cacheData: makeResponse(fakeCt), - cacheDur: 2 * time.Second, - expected: makeResponse(fakeCt), - }, - { - name: "lister error", - setupMock: func(mcl *acmock.MockLister) { - mcl.EXPECT().List(gomock.Any()).Return(makeResponse(), assert.AnError).AnyTimes() - }, - expectedErr: assert.AnError.Error(), - }, - { - name: "stale cache", - cacheAge: 2 * time.Second, - cacheData: makeResponse(fakeCt), - cacheDur: time.Second, - setupMock: func(mcl *acmock.MockLister) { - mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt2), nil).AnyTimes() - }, - expected: makeResponse(fakeCt2), - }, - } { - tc := tc - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - var ( - ctx = testutil.Context(t, testutil.WaitShort) - clk = quartz.NewMock(t) - ctrl = gomock.NewController(t) - mockLister = acmock.NewMockLister(ctrl) - now = time.Now().UTC() - ch = Handler{ - cacheDuration: tc.cacheDur, - cl: mockLister, - clock: clk, - containers: &tc.cacheData, - lockCh: make(chan struct{}, 1), - } - ) - if tc.cacheAge != 0 { - ch.mtime = now.Add(-tc.cacheAge) - } - if tc.setupMock != nil { - tc.setupMock(mockLister) - } - - clk.Set(now).MustWait(ctx) - - // Repeat the test to ensure idempotency - for i := 0; i < 2; i++ { - actual, err := ch.getContainers(ctx) - if tc.expectedErr != "" { - require.Empty(t, actual, "expected no data (attempt %d)", i) - require.ErrorContains(t, err, tc.expectedErr, "expected error (attempt %d)", i) - } else { - require.NoError(t, err, "expected no error (attempt %d)", i) - require.Equal(t, tc.expected, actual, "expected containers to be equal (attempt %d)", i) - } - } - }) - } - }) -} - func TestConvertDockerPort(t *testing.T) { t.Parallel() @@ -675,165 +416,3 @@ func TestConvertDockerInspect(t *testing.T) { }) } } - -// TestDockerEnvInfoer tests the ability of EnvInfo to extract information from -// running containers. Containers are deleted after the test is complete. -// As this test creates containers, it is skipped by default. -// It can be run manually as follows: -// -// CODER_TEST_USE_DOCKER=1 go test ./agent/agentcontainers -run TestDockerEnvInfoer -// -//nolint:paralleltest // This test tends to flake when lots of containers start and stop in parallel. -func TestDockerEnvInfoer(t *testing.T) { - if ctud, ok := os.LookupEnv("CODER_TEST_USE_DOCKER"); !ok || ctud != "1" { - t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test") - } - - pool, err := dockertest.NewPool("") - require.NoError(t, err, "Could not connect to docker") - // nolint:paralleltest // variable recapture no longer required - for idx, tt := range []struct { - image string - labels map[string]string - expectedEnv []string - containerUser string - expectedUsername string - expectedUserShell string - }{ - { - image: "busybox:latest", - labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`}, - - expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, - expectedUsername: "root", - expectedUserShell: "/bin/sh", - }, - { - image: "busybox:latest", - labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`}, - expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, - containerUser: "root", - expectedUsername: "root", - expectedUserShell: "/bin/sh", - }, - { - image: "codercom/enterprise-minimal:ubuntu", - labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`}, - expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, - expectedUsername: "coder", - expectedUserShell: "/bin/bash", - }, - { - image: "codercom/enterprise-minimal:ubuntu", - labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`}, - expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, - containerUser: "coder", - expectedUsername: "coder", - expectedUserShell: "/bin/bash", - }, - { - image: "codercom/enterprise-minimal:ubuntu", - labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`}, - expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, - containerUser: "root", - expectedUsername: "root", - expectedUserShell: "/bin/bash", - }, - { - image: "codercom/enterprise-minimal:ubuntu", - labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar"}},{"remoteEnv": {"MULTILINE": "foo\nbar\nbaz"}}]`}, - expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, - containerUser: "root", - expectedUsername: "root", - expectedUserShell: "/bin/bash", - }, - } { - //nolint:paralleltest // variable recapture no longer required - t.Run(fmt.Sprintf("#%d", idx), func(t *testing.T) { - // Start a container with the given image - // and environment variables - image := strings.Split(tt.image, ":")[0] - tag := strings.Split(tt.image, ":")[1] - ct, err := pool.RunWithOptions(&dockertest.RunOptions{ - Repository: image, - Tag: tag, - Cmd: []string{"sleep", "infinity"}, - Labels: tt.labels, - }, func(config *docker.HostConfig) { - config.AutoRemove = true - config.RestartPolicy = docker.RestartPolicy{Name: "no"} - }) - require.NoError(t, err, "Could not start test docker container") - t.Logf("Created container %q", ct.Container.Name) - t.Cleanup(func() { - assert.NoError(t, pool.Purge(ct), "Could not purge resource %q", ct.Container.Name) - t.Logf("Purged container %q", ct.Container.Name) - }) - - ctx := testutil.Context(t, testutil.WaitShort) - dei, err := EnvInfo(ctx, agentexec.DefaultExecer, ct.Container.ID, tt.containerUser) - require.NoError(t, err, "Expected no error from DockerEnvInfo()") - - u, err := dei.User() - require.NoError(t, err, "Expected no error from CurrentUser()") - require.Equal(t, tt.expectedUsername, u.Username, "Expected username to match") - - hd, err := dei.HomeDir() - require.NoError(t, err, "Expected no error from UserHomeDir()") - require.NotEmpty(t, hd, "Expected user homedir to be non-empty") - - sh, err := dei.Shell(tt.containerUser) - require.NoError(t, err, "Expected no error from UserShell()") - require.Equal(t, tt.expectedUserShell, sh, "Expected user shell to match") - - // We don't need to test the actual environment variables here. - environ := dei.Environ() - require.NotEmpty(t, environ, "Expected environ to be non-empty") - - // Test that the environment variables are present in modified command - // output. - envCmd, envArgs := dei.ModifyCommand("env") - for _, env := range tt.expectedEnv { - require.Subset(t, envArgs, []string{"--env", env}) - } - // Run the command in the container and check the output - // HACK: we remove the --tty argument because we're not running in a tty - envArgs = slices.DeleteFunc(envArgs, func(s string) bool { return s == "--tty" }) - stdout, stderr, err := run(ctx, agentexec.DefaultExecer, envCmd, envArgs...) - require.Empty(t, stderr, "Expected no stderr output") - require.NoError(t, err, "Expected no error from running command") - for _, env := range tt.expectedEnv { - require.Contains(t, stdout, env) - } - }) - } -} - -func fakeContainer(t *testing.T, mut ...func(*codersdk.WorkspaceAgentContainer)) codersdk.WorkspaceAgentContainer { - t.Helper() - ct := codersdk.WorkspaceAgentContainer{ - CreatedAt: time.Now().UTC(), - ID: uuid.New().String(), - FriendlyName: testutil.GetRandomName(t), - Image: testutil.GetRandomName(t) + ":" + strings.Split(uuid.New().String(), "-")[0], - Labels: map[string]string{ - testutil.GetRandomName(t): testutil.GetRandomName(t), - }, - Running: true, - Ports: []codersdk.WorkspaceAgentContainerPort{ - { - Network: "tcp", - Port: testutil.RandomPortNoListen(t), - HostPort: testutil.RandomPortNoListen(t), - //nolint:gosec // this is a test - HostIP: []string{"127.0.0.1", "[::1]", "localhost", "0.0.0.0", "[::]", testutil.GetRandomName(t)}[rand.Intn(6)], - }, - }, - Status: testutil.MustRandString(t, 10), - Volumes: map[string]string{testutil.GetRandomName(t): testutil.GetRandomName(t)}, - } - for _, m := range mut { - m(&ct) - } - return ct -} diff --git a/agent/agentcontainers/containers_test.go b/agent/agentcontainers/containers_test.go index ac479de25419a..59befb2fd2be0 100644 --- a/agent/agentcontainers/containers_test.go +++ b/agent/agentcontainers/containers_test.go @@ -2,165 +2,295 @@ package agentcontainers_test import ( "context" - "net/http" - "net/http/httptest" + "fmt" + "os" + "slices" + "strconv" + "strings" "testing" - "github.com/go-chi/chi/v5" + "github.com/google/uuid" + "github.com/ory/dockertest/v3" + "github.com/ory/dockertest/v3/docker" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "golang.org/x/xerrors" "github.com/coder/coder/v2/agent/agentcontainers" - "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/agent/agentexec" + "github.com/coder/coder/v2/pty" + "github.com/coder/coder/v2/testutil" ) -// fakeLister implements the agentcontainers.Lister interface for -// testing. -type fakeLister struct { - containers codersdk.WorkspaceAgentListContainersResponse - err error -} +// TestIntegrationDocker tests agentcontainers functionality using a real +// Docker container. It starts a container with a known +// label, lists the containers, and verifies that the expected container is +// returned. It also executes a sample command inside the container. +// The container is deleted after the test is complete. +// As this test creates containers, it is skipped by default. +// It can be run manually as follows: +// +// CODER_TEST_USE_DOCKER=1 go test ./agent/agentcontainers -run TestDockerCLIContainerLister +// +//nolint:paralleltest // This test tends to flake when lots of containers start and stop in parallel. +func TestIntegrationDocker(t *testing.T) { + if ctud, ok := os.LookupEnv("CODER_TEST_USE_DOCKER"); !ok || ctud != "1" { + t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test") + } -func (f *fakeLister) List(_ context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) { - return f.containers, f.err -} + pool, err := dockertest.NewPool("") + require.NoError(t, err, "Could not connect to docker") + testLabelValue := uuid.New().String() + // Create a temporary directory to validate that we surface mounts correctly. + testTempDir := t.TempDir() + // Pick a random port to expose for testing port bindings. + testRandPort := testutil.RandomPortNoListen(t) + ct, err := pool.RunWithOptions(&dockertest.RunOptions{ + Repository: "busybox", + Tag: "latest", + Cmd: []string{"sleep", "infnity"}, + Labels: map[string]string{ + "com.coder.test": testLabelValue, + "devcontainer.metadata": `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`, + }, + Mounts: []string{testTempDir + ":" + testTempDir}, + ExposedPorts: []string{fmt.Sprintf("%d/tcp", testRandPort)}, + PortBindings: map[docker.Port][]docker.PortBinding{ + docker.Port(fmt.Sprintf("%d/tcp", testRandPort)): { + { + HostIP: "0.0.0.0", + HostPort: strconv.FormatInt(int64(testRandPort), 10), + }, + }, + }, + }, func(config *docker.HostConfig) { + config.AutoRemove = true + config.RestartPolicy = docker.RestartPolicy{Name: "no"} + }) + require.NoError(t, err, "Could not start test docker container") + t.Logf("Created container %q", ct.Container.Name) + t.Cleanup(func() { + assert.NoError(t, pool.Purge(ct), "Could not purge resource %q", ct.Container.Name) + t.Logf("Purged container %q", ct.Container.Name) + }) + // Wait for container to start + require.Eventually(t, func() bool { + ct, ok := pool.ContainerByName(ct.Container.Name) + return ok && ct.Container.State.Running + }, testutil.WaitShort, testutil.IntervalSlow, "Container did not start in time") -// fakeDevcontainerCLI implements the agentcontainers.DevcontainerCLI -// interface for testing. -type fakeDevcontainerCLI struct { - id string - err error + dcl := agentcontainers.NewDocker(agentexec.DefaultExecer) + ctx := testutil.Context(t, testutil.WaitShort) + actual, err := dcl.List(ctx) + require.NoError(t, err, "Could not list containers") + require.Empty(t, actual.Warnings, "Expected no warnings") + var found bool + for _, foundContainer := range actual.Containers { + if foundContainer.ID == ct.Container.ID { + found = true + assert.Equal(t, ct.Container.Created, foundContainer.CreatedAt) + // ory/dockertest pre-pends a forward slash to the container name. + assert.Equal(t, strings.TrimPrefix(ct.Container.Name, "/"), foundContainer.FriendlyName) + // ory/dockertest returns the sha256 digest of the image. + assert.Equal(t, "busybox:latest", foundContainer.Image) + assert.Equal(t, ct.Container.Config.Labels, foundContainer.Labels) + assert.True(t, foundContainer.Running) + assert.Equal(t, "running", foundContainer.Status) + if assert.Len(t, foundContainer.Ports, 1) { + assert.Equal(t, testRandPort, foundContainer.Ports[0].Port) + assert.Equal(t, "tcp", foundContainer.Ports[0].Network) + } + if assert.Len(t, foundContainer.Volumes, 1) { + assert.Equal(t, testTempDir, foundContainer.Volumes[testTempDir]) + } + // Test that EnvInfo is able to correctly modify a command to be + // executed inside the container. + dei, err := agentcontainers.EnvInfo(ctx, agentexec.DefaultExecer, ct.Container.ID, "") + require.NoError(t, err, "Expected no error from DockerEnvInfo()") + ptyWrappedCmd, ptyWrappedArgs := dei.ModifyCommand("/bin/sh", "--norc") + ptyCmd, ptyPs, err := pty.Start(agentexec.DefaultExecer.PTYCommandContext(ctx, ptyWrappedCmd, ptyWrappedArgs...)) + require.NoError(t, err, "failed to start pty command") + t.Cleanup(func() { + _ = ptyPs.Kill() + _ = ptyCmd.Close() + }) + tr := testutil.NewTerminalReader(t, ptyCmd.OutputReader()) + matchPrompt := func(line string) bool { + return strings.Contains(line, "#") + } + matchHostnameCmd := func(line string) bool { + return strings.Contains(strings.TrimSpace(line), "hostname") + } + matchHostnameOuput := func(line string) bool { + return strings.Contains(strings.TrimSpace(line), ct.Container.Config.Hostname) + } + matchEnvCmd := func(line string) bool { + return strings.Contains(strings.TrimSpace(line), "env") + } + matchEnvOutput := func(line string) bool { + return strings.Contains(line, "FOO=bar") || strings.Contains(line, "MULTILINE=foo") + } + require.NoError(t, tr.ReadUntil(ctx, matchPrompt), "failed to match prompt") + t.Logf("Matched prompt") + _, err = ptyCmd.InputWriter().Write([]byte("hostname\r\n")) + require.NoError(t, err, "failed to write to pty") + t.Logf("Wrote hostname command") + require.NoError(t, tr.ReadUntil(ctx, matchHostnameCmd), "failed to match hostname command") + t.Logf("Matched hostname command") + require.NoError(t, tr.ReadUntil(ctx, matchHostnameOuput), "failed to match hostname output") + t.Logf("Matched hostname output") + _, err = ptyCmd.InputWriter().Write([]byte("env\r\n")) + require.NoError(t, err, "failed to write to pty") + t.Logf("Wrote env command") + require.NoError(t, tr.ReadUntil(ctx, matchEnvCmd), "failed to match env command") + t.Logf("Matched env command") + require.NoError(t, tr.ReadUntil(ctx, matchEnvOutput), "failed to match env output") + t.Logf("Matched env output") + break + } + } + assert.True(t, found, "Expected to find container with label 'com.coder.test=%s'", testLabelValue) } -func (f *fakeDevcontainerCLI) Up(_ context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) { - return f.id, f.err -} +// TestDockerEnvInfoer tests the ability of EnvInfo to extract information from +// running containers. Containers are deleted after the test is complete. +// As this test creates containers, it is skipped by default. +// It can be run manually as follows: +// +// CODER_TEST_USE_DOCKER=1 go test ./agent/agentcontainers -run TestDockerEnvInfoer +// +//nolint:paralleltest // This test tends to flake when lots of containers start and stop in parallel. +func TestDockerEnvInfoer(t *testing.T) { + if ctud, ok := os.LookupEnv("CODER_TEST_USE_DOCKER"); !ok || ctud != "1" { + t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test") + } -func TestHandler(t *testing.T) { - t.Parallel() + pool, err := dockertest.NewPool("") + require.NoError(t, err, "Could not connect to docker") + // nolint:paralleltest // variable recapture no longer required + for idx, tt := range []struct { + image string + labels map[string]string + expectedEnv []string + containerUser string + expectedUsername string + expectedUserShell string + }{ + { + image: "busybox:latest", + labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`}, - t.Run("Recreate", func(t *testing.T) { - t.Parallel() + expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, + expectedUsername: "root", + expectedUserShell: "/bin/sh", + }, + { + image: "busybox:latest", + labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`}, + expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, + containerUser: "root", + expectedUsername: "root", + expectedUserShell: "/bin/sh", + }, + { + image: "codercom/enterprise-minimal:ubuntu", + labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`}, + expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, + expectedUsername: "coder", + expectedUserShell: "/bin/bash", + }, + { + image: "codercom/enterprise-minimal:ubuntu", + labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`}, + expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, + containerUser: "coder", + expectedUsername: "coder", + expectedUserShell: "/bin/bash", + }, + { + image: "codercom/enterprise-minimal:ubuntu", + labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar", "MULTILINE": "foo\nbar\nbaz"}}]`}, + expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, + containerUser: "root", + expectedUsername: "root", + expectedUserShell: "/bin/bash", + }, + { + image: "codercom/enterprise-minimal:ubuntu", + labels: map[string]string{`devcontainer.metadata`: `[{"remoteEnv": {"FOO": "bar"}},{"remoteEnv": {"MULTILINE": "foo\nbar\nbaz"}}]`}, + expectedEnv: []string{"FOO=bar", "MULTILINE=foo\nbar\nbaz"}, + containerUser: "root", + expectedUsername: "root", + expectedUserShell: "/bin/bash", + }, + } { + //nolint:paralleltest // variable recapture no longer required + t.Run(fmt.Sprintf("#%d", idx), func(t *testing.T) { + // Start a container with the given image + // and environment variables + image := strings.Split(tt.image, ":")[0] + tag := strings.Split(tt.image, ":")[1] + ct, err := pool.RunWithOptions(&dockertest.RunOptions{ + Repository: image, + Tag: tag, + Cmd: []string{"sleep", "infinity"}, + Labels: tt.labels, + }, func(config *docker.HostConfig) { + config.AutoRemove = true + config.RestartPolicy = docker.RestartPolicy{Name: "no"} + }) + require.NoError(t, err, "Could not start test docker container") + t.Logf("Created container %q", ct.Container.Name) + t.Cleanup(func() { + assert.NoError(t, pool.Purge(ct), "Could not purge resource %q", ct.Container.Name) + t.Logf("Purged container %q", ct.Container.Name) + }) - validContainer := codersdk.WorkspaceAgentContainer{ - ID: "container-id", - FriendlyName: "container-name", - Labels: map[string]string{ - agentcontainers.DevcontainerLocalFolderLabel: "/workspace", - agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json", - }, - } + ctx := testutil.Context(t, testutil.WaitShort) + dei, err := agentcontainers.EnvInfo(ctx, agentexec.DefaultExecer, ct.Container.ID, tt.containerUser) + require.NoError(t, err, "Expected no error from DockerEnvInfo()") - missingFolderContainer := codersdk.WorkspaceAgentContainer{ - ID: "missing-folder-container", - FriendlyName: "missing-folder-container", - Labels: map[string]string{}, - } + u, err := dei.User() + require.NoError(t, err, "Expected no error from CurrentUser()") + require.Equal(t, tt.expectedUsername, u.Username, "Expected username to match") - tests := []struct { - name string - containerID string - lister *fakeLister - devcontainerCLI *fakeDevcontainerCLI - wantStatus int - wantBody string - }{ - { - name: "Missing ID", - containerID: "", - lister: &fakeLister{}, - devcontainerCLI: &fakeDevcontainerCLI{}, - wantStatus: http.StatusBadRequest, - wantBody: "Missing container ID or name", - }, - { - name: "List error", - containerID: "container-id", - lister: &fakeLister{ - err: xerrors.New("list error"), - }, - devcontainerCLI: &fakeDevcontainerCLI{}, - wantStatus: http.StatusInternalServerError, - wantBody: "Could not list containers", - }, - { - name: "Container not found", - containerID: "nonexistent-container", - lister: &fakeLister{ - containers: codersdk.WorkspaceAgentListContainersResponse{ - Containers: []codersdk.WorkspaceAgentContainer{validContainer}, - }, - }, - devcontainerCLI: &fakeDevcontainerCLI{}, - wantStatus: http.StatusNotFound, - wantBody: "Container not found", - }, - { - name: "Missing workspace folder label", - containerID: "missing-folder-container", - lister: &fakeLister{ - containers: codersdk.WorkspaceAgentListContainersResponse{ - Containers: []codersdk.WorkspaceAgentContainer{missingFolderContainer}, - }, - }, - devcontainerCLI: &fakeDevcontainerCLI{}, - wantStatus: http.StatusBadRequest, - wantBody: "Missing workspace folder label", - }, - { - name: "Devcontainer CLI error", - containerID: "container-id", - lister: &fakeLister{ - containers: codersdk.WorkspaceAgentListContainersResponse{ - Containers: []codersdk.WorkspaceAgentContainer{validContainer}, - }, - }, - devcontainerCLI: &fakeDevcontainerCLI{ - err: xerrors.New("devcontainer CLI error"), - }, - wantStatus: http.StatusInternalServerError, - wantBody: "Could not recreate devcontainer", - }, - { - name: "OK", - containerID: "container-id", - lister: &fakeLister{ - containers: codersdk.WorkspaceAgentListContainersResponse{ - Containers: []codersdk.WorkspaceAgentContainer{validContainer}, - }, - }, - devcontainerCLI: &fakeDevcontainerCLI{}, - wantStatus: http.StatusNoContent, - wantBody: "", - }, - } + hd, err := dei.HomeDir() + require.NoError(t, err, "Expected no error from UserHomeDir()") + require.NotEmpty(t, hd, "Expected user homedir to be non-empty") - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - // Setup router with the handler under test. - r := chi.NewRouter() - handler := agentcontainers.New( - agentcontainers.WithLister(tt.lister), - agentcontainers.WithDevcontainerCLI(tt.devcontainerCLI), - ) - r.Post("/containers/{id}/recreate", handler.Recreate) - - // Simulate HTTP request to the recreate endpoint. - req := httptest.NewRequest(http.MethodPost, "/containers/"+tt.containerID+"/recreate", nil) - rec := httptest.NewRecorder() - r.ServeHTTP(rec, req) - - // Check the response status code and body. - require.Equal(t, tt.wantStatus, rec.Code, "status code mismatch") - if tt.wantBody != "" { - assert.Contains(t, rec.Body.String(), tt.wantBody, "response body mismatch") - } else if tt.wantStatus == http.StatusNoContent { - assert.Empty(t, rec.Body.String(), "expected empty response body") - } - }) - } - }) + sh, err := dei.Shell(tt.containerUser) + require.NoError(t, err, "Expected no error from UserShell()") + require.Equal(t, tt.expectedUserShell, sh, "Expected user shell to match") + + // We don't need to test the actual environment variables here. + environ := dei.Environ() + require.NotEmpty(t, environ, "Expected environ to be non-empty") + + // Test that the environment variables are present in modified command + // output. + envCmd, envArgs := dei.ModifyCommand("env") + for _, env := range tt.expectedEnv { + require.Subset(t, envArgs, []string{"--env", env}) + } + // Run the command in the container and check the output + // HACK: we remove the --tty argument because we're not running in a tty + envArgs = slices.DeleteFunc(envArgs, func(s string) bool { return s == "--tty" }) + stdout, stderr, err := run(ctx, agentexec.DefaultExecer, envCmd, envArgs...) + require.Empty(t, stderr, "Expected no stderr output") + require.NoError(t, err, "Expected no error from running command") + for _, env := range tt.expectedEnv { + require.Contains(t, stdout, env) + } + }) + } +} + +func run(ctx context.Context, execer agentexec.Execer, cmd string, args ...string) (stdout, stderr string, err error) { + var stdoutBuf, stderrBuf strings.Builder + execCmd := execer.CommandContext(ctx, cmd, args...) + execCmd.Stdout = &stdoutBuf + execCmd.Stderr = &stderrBuf + err = execCmd.Run() + stdout = strings.TrimSpace(stdoutBuf.String()) + stderr = strings.TrimSpace(stderrBuf.String()) + return stdout, stderr, err } diff --git a/agent/agentcontainers/devcontainer.go b/agent/agentcontainers/devcontainer.go index f93e0722c75b9..cbf42e150d240 100644 --- a/agent/agentcontainers/devcontainer.go +++ b/agent/agentcontainers/devcontainer.go @@ -8,7 +8,6 @@ import ( "strings" "cdr.dev/slog" - "github.com/coder/coder/v2/codersdk" ) diff --git a/agent/api.go b/agent/api.go index 375338acfab18..bb357d1b87da2 100644 --- a/agent/api.go +++ b/agent/api.go @@ -36,10 +36,15 @@ func (a *agent) apiHandler() http.Handler { ignorePorts: cpy, cacheDuration: cacheDuration, } - ch := agentcontainers.New(agentcontainers.WithLister(a.lister)) + + containerAPI := agentcontainers.NewAPI( + a.logger.Named("containers"), + agentcontainers.WithLister(a.lister), + ) + promHandler := PrometheusMetricsHandler(a.prometheusRegistry, a.logger) - r.Get("/api/v0/containers", ch.List) - r.Post("/api/v0/containers/{id}/recreate", ch.Recreate) + + r.Mount("/api/v0/containers", containerAPI.Routes()) r.Get("/api/v0/listening-ports", lp.handler) r.Get("/api/v0/netcheck", a.HandleNetcheck) r.Post("/api/v0/list-directory", a.HandleLS)