From eb8a4698131c6d768a4032be99fad680be99427c Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 12 Jun 2025 16:16:46 +0000 Subject: [PATCH 01/16] feat(agent/agentcontainers): support apps for dev container agents --- agent/agentcontainers/acmock/acmock.go | 8 +- agent/agentcontainers/api.go | 28 ++- agent/agentcontainers/api_test.go | 97 +++++++- agent/agentcontainers/devcontainercli.go | 10 +- agent/agentcontainers/devcontainercli_test.go | 2 +- agent/agentcontainers/subagent.go | 77 +++++- agent/agentcontainers/subagent_test.go | 230 ++++++++++++++++++ agent/agenttest/client.go | 25 ++ 8 files changed, 461 insertions(+), 16 deletions(-) diff --git a/agent/agentcontainers/acmock/acmock.go b/agent/agentcontainers/acmock/acmock.go index 990a243a33ddf..b6bb4a9523fb6 100644 --- a/agent/agentcontainers/acmock/acmock.go +++ b/agent/agentcontainers/acmock/acmock.go @@ -150,9 +150,9 @@ func (mr *MockDevcontainerCLIMockRecorder) Exec(ctx, workspaceFolder, configPath } // ReadConfig mocks base method. -func (m *MockDevcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, configPath string, opts ...agentcontainers.DevcontainerCLIReadConfigOptions) (agentcontainers.DevcontainerConfig, error) { +func (m *MockDevcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, configPath string, env []string, opts ...agentcontainers.DevcontainerCLIReadConfigOptions) (agentcontainers.DevcontainerConfig, error) { m.ctrl.T.Helper() - varargs := []any{ctx, workspaceFolder, configPath} + varargs := []any{ctx, workspaceFolder, configPath, env} for _, a := range opts { varargs = append(varargs, a) } @@ -163,9 +163,9 @@ func (m *MockDevcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, c } // ReadConfig indicates an expected call of ReadConfig. -func (mr *MockDevcontainerCLIMockRecorder) ReadConfig(ctx, workspaceFolder, configPath any, opts ...any) *gomock.Call { +func (mr *MockDevcontainerCLIMockRecorder) ReadConfig(ctx, workspaceFolder, configPath, env any, opts ...any) *gomock.Call { mr.mock.ctrl.T.Helper() - varargs := append([]any{ctx, workspaceFolder, configPath}, opts...) + varargs := append([]any{ctx, workspaceFolder, configPath, env}, opts...) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReadConfig", reflect.TypeOf((*MockDevcontainerCLI)(nil).ReadConfig), varargs...) } diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 785d87bf3654e..41d112788f684 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -64,6 +64,9 @@ type API struct { subAgentURL string subAgentEnv []string + userName string + workspaceName string + mu sync.RWMutex closed bool containers codersdk.WorkspaceAgentListContainersResponse // Output from the last list operation. @@ -153,6 +156,20 @@ func WithSubAgentEnv(env ...string) Option { } } +// WithWorkspaceName sets the workspace name for the sub-agent. +func WithWorkspaceName(name string) Option { + return func(api *API) { + api.workspaceName = name + } +} + +// WithUserName sets the workspace name for the sub-agent. +func WithUserName(name string) Option { + return func(api *API) { + api.userName = name + } +} + // WithDevcontainers sets the known devcontainers for the API. This // allows the API to be aware of devcontainers defined in the workspace // agent manifest. @@ -1127,7 +1144,14 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c codersdk.DisplayAppPortForward: true, } - if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath); err != nil { + var apps []SubAgentApp + + if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath, []string{ + fmt.Sprintf("CODER_AGENT_NAME=%s", dc.Name), + fmt.Sprintf("CODER_USER_NAME=%s", api.userName), + fmt.Sprintf("CODER_WORKSPACE_NAME=%s", api.workspaceName), + fmt.Sprintf("CODER_DEPLOYMENT_URL=%s", api.subAgentURL), + }); err != nil { api.logger.Error(ctx, "unable to read devcontainer config", slog.Error(err)) } else { coderCustomization := config.MergedConfiguration.Customizations.Coder @@ -1143,6 +1167,8 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c } displayAppsMap[app] = enabled } + + apps = append(apps, customization.Apps...) } } diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 8dc1f83dc916b..bb803ffc4c689 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -26,6 +26,7 @@ import ( "github.com/coder/coder/v2/agent/agentcontainers" "github.com/coder/coder/v2/agent/agentcontainers/acmock" "github.com/coder/coder/v2/agent/agentcontainers/watcher" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" "github.com/coder/quartz" @@ -68,7 +69,7 @@ type fakeDevcontainerCLI struct { execErrC chan func(cmd string, args ...string) error // If set, send fn to return err, nil or close to return execErr. readConfig agentcontainers.DevcontainerConfig readConfigErr error - readConfigErrC chan error + readConfigErrC chan func(envs []string) (agentcontainers.DevcontainerConfig, error) } func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) { @@ -99,14 +100,14 @@ func (f *fakeDevcontainerCLI) Exec(ctx context.Context, _, _ string, cmd string, return f.execErr } -func (f *fakeDevcontainerCLI) ReadConfig(ctx context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIReadConfigOptions) (agentcontainers.DevcontainerConfig, error) { +func (f *fakeDevcontainerCLI) ReadConfig(ctx context.Context, _, _ string, envs []string, _ ...agentcontainers.DevcontainerCLIReadConfigOptions) (agentcontainers.DevcontainerConfig, error) { if f.readConfigErrC != nil { select { case <-ctx.Done(): return agentcontainers.DevcontainerConfig{}, ctx.Err() - case err, ok := <-f.readConfigErrC: + case fn, ok := <-f.readConfigErrC: if ok { - return f.readConfig, err + return fn(envs) } } } @@ -1253,7 +1254,8 @@ func TestAPI(t *testing.T) { deleteErrC: make(chan error, 1), } fakeDCCLI = &fakeDevcontainerCLI{ - execErrC: make(chan func(cmd string, args ...string) error, 1), + execErrC: make(chan func(cmd string, args ...string) error, 1), + readConfigErrC: make(chan func(envs []string) (agentcontainers.DevcontainerConfig, error), 1), } testContainer = codersdk.WorkspaceAgentContainer{ @@ -1293,6 +1295,8 @@ func TestAPI(t *testing.T) { agentcontainers.WithSubAgentClient(fakeSAC), agentcontainers.WithSubAgentURL("test-subagent-url"), agentcontainers.WithDevcontainerCLI(fakeDCCLI), + agentcontainers.WithUserName("test-user"), + agentcontainers.WithWorkspaceName("test-workspace"), ) apiClose := func() { closeOnce.Do(func() { @@ -1300,6 +1304,7 @@ func TestAPI(t *testing.T) { close(fakeSAC.createErrC) close(fakeSAC.deleteErrC) close(fakeDCCLI.execErrC) + defer close(fakeDCCLI.readConfigErrC) _ = api.Close() }) @@ -1313,6 +1318,13 @@ func TestAPI(t *testing.T) { assert.Empty(t, args) return nil }) // Exec pwd. + testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) (agentcontainers.DevcontainerConfig, error) { + assert.Contains(t, envs, "CODER_AGENT_NAME=test-container") + assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") + assert.Contains(t, envs, "CODER_USER_NAME=test-user") + assert.Contains(t, envs, "CODER_DEPLOYMENT_URL=test-subagent-url") + return agentcontainers.DevcontainerConfig{}, nil + }) // Make sure the ticker function has been registered // before advancing the clock. @@ -1453,6 +1465,13 @@ func TestAPI(t *testing.T) { assert.Empty(t, args) return nil }) // Exec pwd. + testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) (agentcontainers.DevcontainerConfig, error) { + assert.Contains(t, envs, "CODER_AGENT_NAME=test-container") + assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") + assert.Contains(t, envs, "CODER_USER_NAME=test-user") + assert.Contains(t, envs, "CODER_DEPLOYMENT_URL=test-subagent-url") + return agentcontainers.DevcontainerConfig{}, nil + }) err = api.RefreshContainers(ctx) require.NoError(t, err, "refresh containers should not fail") @@ -1603,6 +1622,74 @@ func TestAPI(t *testing.T) { assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppPortForward) }, }, + { + name: "WithApps", + customization: []agentcontainers.CoderCustomization{ + { + Apps: []agentcontainers.SubAgentApp{ + { + Slug: "web-app", + DisplayName: ptr.Ref("Web Application"), + URL: ptr.Ref("http://localhost:8080"), + OpenIn: codersdk.WorkspaceAppOpenInTab, + Share: codersdk.WorkspaceAppSharingLevelOwner, + Icon: ptr.Ref("/icons/web.svg"), + Order: ptr.Ref(int32(1)), + }, + { + Slug: "api-server", + DisplayName: ptr.Ref("API Server"), + URL: ptr.Ref("http://localhost:3000"), + OpenIn: codersdk.WorkspaceAppOpenInSlimWindow, + Share: codersdk.WorkspaceAppSharingLevelAuthenticated, + Icon: ptr.Ref("/icons/api.svg"), + Order: ptr.Ref(int32(2)), + Hidden: ptr.Ref(true), + }, + { + Slug: "docs", + DisplayName: ptr.Ref("Documentation"), + URL: ptr.Ref("http://localhost:4000"), + OpenIn: codersdk.WorkspaceAppOpenInTab, + Share: codersdk.WorkspaceAppSharingLevelPublic, + Icon: ptr.Ref("/icons/book.svg"), + Order: ptr.Ref(int32(3)), + }, + }, + }, + }, + afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) { + require.Len(t, subAgent.Apps, 3) + + // Verify first app + assert.Equal(t, "web-app", subAgent.Apps[0].Slug) + assert.Equal(t, "Web Application", *subAgent.Apps[0].DisplayName) + assert.Equal(t, "http://localhost:8080", *subAgent.Apps[0].URL) + assert.Equal(t, codersdk.WorkspaceAppOpenInTab, subAgent.Apps[0].OpenIn) + assert.Equal(t, codersdk.WorkspaceAppSharingLevelOwner, subAgent.Apps[0].Share) + assert.Equal(t, "/icons/web.svg", *subAgent.Apps[0].Icon) + assert.Equal(t, int32(1), *subAgent.Apps[0].Order) + + // Verify second app + assert.Equal(t, "api-server", subAgent.Apps[1].Slug) + assert.Equal(t, "API Server", *subAgent.Apps[1].DisplayName) + assert.Equal(t, "http://localhost:3000", *subAgent.Apps[1].URL) + assert.Equal(t, codersdk.WorkspaceAppOpenInSlimWindow, subAgent.Apps[1].OpenIn) + assert.Equal(t, codersdk.WorkspaceAppSharingLevelAuthenticated, subAgent.Apps[1].Share) + assert.Equal(t, "/icons/api.svg", *subAgent.Apps[1].Icon) + assert.Equal(t, int32(2), *subAgent.Apps[1].Order) + assert.Equal(t, true, *subAgent.Apps[1].Hidden) + + // Verify third app + assert.Equal(t, "docs", subAgent.Apps[2].Slug) + assert.Equal(t, "Documentation", *subAgent.Apps[2].DisplayName) + assert.Equal(t, "http://localhost:4000", *subAgent.Apps[2].URL) + assert.Equal(t, codersdk.WorkspaceAppOpenInTab, subAgent.Apps[2].OpenIn) + assert.Equal(t, codersdk.WorkspaceAppSharingLevelPublic, subAgent.Apps[2].Share) + assert.Equal(t, "/icons/book.svg", *subAgent.Apps[2].Icon) + assert.Equal(t, int32(3), *subAgent.Apps[2].Order) + }, + }, } for _, tt := range tests { diff --git a/agent/agentcontainers/devcontainercli.go b/agent/agentcontainers/devcontainercli.go index 002858c70562e..a18c896073ee5 100644 --- a/agent/agentcontainers/devcontainercli.go +++ b/agent/agentcontainers/devcontainercli.go @@ -32,13 +32,14 @@ type DevcontainerCustomizations struct { type CoderCustomization struct { DisplayApps map[codersdk.DisplayApp]bool `json:"displayApps,omitempty"` + Apps []SubAgentApp `json:"apps,omitempty"` } // DevcontainerCLI is an interface for the devcontainer CLI. type DevcontainerCLI interface { Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (id string, err error) Exec(ctx context.Context, workspaceFolder, configPath string, cmd string, cmdArgs []string, opts ...DevcontainerCLIExecOptions) error - ReadConfig(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIReadConfigOptions) (DevcontainerConfig, error) + ReadConfig(ctx context.Context, workspaceFolder, configPath string, env []string, opts ...DevcontainerCLIReadConfigOptions) (DevcontainerConfig, error) } // DevcontainerCLIUpOptions are options for the devcontainer CLI Up @@ -113,8 +114,8 @@ type devcontainerCLIReadConfigConfig struct { stderr io.Writer } -// WithExecOutput sets additional stdout and stderr writers for logs -// during Exec operations. +// WithReadConfigOutput sets additional stdout and stderr writers for logs +// during ReadConfig operations. func WithReadConfigOutput(stdout, stderr io.Writer) DevcontainerCLIReadConfigOptions { return func(o *devcontainerCLIReadConfigConfig) { o.stdout = stdout @@ -250,7 +251,7 @@ func (d *devcontainerCLI) Exec(ctx context.Context, workspaceFolder, configPath return nil } -func (d *devcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIReadConfigOptions) (DevcontainerConfig, error) { +func (d *devcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, configPath string, env []string, opts ...DevcontainerCLIReadConfigOptions) (DevcontainerConfig, error) { conf := applyDevcontainerCLIReadConfigOptions(opts) logger := d.logger.With(slog.F("workspace_folder", workspaceFolder), slog.F("config_path", configPath)) @@ -263,6 +264,7 @@ func (d *devcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, confi } c := d.execer.CommandContext(ctx, "devcontainer", args...) + c.Env = append(c.Env, env...) var stdoutBuf bytes.Buffer stdoutWriters := []io.Writer{&stdoutBuf, &devcontainerCLILogWriter{ctx: ctx, logger: logger.With(slog.F("stdout", true))}} diff --git a/agent/agentcontainers/devcontainercli_test.go b/agent/agentcontainers/devcontainercli_test.go index cffb3e12fd494..311ec440e357a 100644 --- a/agent/agentcontainers/devcontainercli_test.go +++ b/agent/agentcontainers/devcontainercli_test.go @@ -316,7 +316,7 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) { } dccli := agentcontainers.NewDevcontainerCLI(logger, testExecer) - config, err := dccli.ReadConfig(ctx, tt.workspaceFolder, tt.configPath, tt.opts...) + config, err := dccli.ReadConfig(ctx, tt.workspaceFolder, tt.configPath, []string{}, tt.opts...) if tt.wantError { assert.Error(t, err, "want error") assert.Equal(t, agentcontainers.DevcontainerConfig{}, config, "expected empty config on error") diff --git a/agent/agentcontainers/subagent.go b/agent/agentcontainers/subagent.go index ea527f8c46e37..6a4af8f2094dc 100644 --- a/agent/agentcontainers/subagent.go +++ b/agent/agentcontainers/subagent.go @@ -21,6 +21,7 @@ type SubAgent struct { Directory string Architecture string OperatingSystem string + Apps []SubAgentApp DisplayApps []codersdk.DisplayApp } @@ -41,7 +42,30 @@ func (s SubAgent) EqualConfig(other SubAgent) bool { s.Directory == other.Directory && s.Architecture == other.Architecture && s.OperatingSystem == other.OperatingSystem && - slices.Equal(s.DisplayApps, other.DisplayApps) + slices.Equal(s.DisplayApps, other.DisplayApps) && + slices.Equal(s.Apps, other.Apps) +} + +type SubAgentApp struct { + Slug string `json:"slug"` + Command *string `json:"command"` + DisplayName *string `json:"displayName"` + External *bool `json:"external"` + Group *string `json:"group"` + HealthCheck *SubAgentHealthCheck `json:"healthCheck"` + Hidden *bool `json:"hidden"` + Icon *string `json:"icon"` + OpenIn codersdk.WorkspaceAppOpenIn `json:"openIn"` + Order *int32 `json:"order"` + Share codersdk.WorkspaceAppSharingLevel `json:"share"` + Subdomain *bool `json:"subdomain"` + URL *string `json:"url"` +} + +type SubAgentHealthCheck struct { + Interval int32 `json:"interval"` + Threshold int32 `json:"threshold"` + URL string `json:"url"` } // SubAgentClient is an interface for managing sub agents and allows @@ -125,12 +149,63 @@ func (a *subAgentAPIClient) Create(ctx context.Context, agent SubAgent) (SubAgen displayApps = append(displayApps, app) } + apps := make([]*agentproto.CreateSubAgentRequest_App, 0, len(agent.Apps)) + for _, app := range agent.Apps { + var healthCheck *agentproto.CreateSubAgentRequest_App_Healthcheck + if app.HealthCheck != nil { + healthCheck = &agentproto.CreateSubAgentRequest_App_Healthcheck{ + Interval: app.HealthCheck.Interval, + Threshold: app.HealthCheck.Threshold, + Url: app.HealthCheck.URL, + } + } + + var openIn *agentproto.CreateSubAgentRequest_App_OpenIn + switch app.OpenIn { + case codersdk.WorkspaceAppOpenInSlimWindow: + openIn = agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum() + case codersdk.WorkspaceAppOpenInTab: + openIn = agentproto.CreateSubAgentRequest_App_TAB.Enum() + default: + return SubAgent{}, xerrors.Errorf("unexpected codersdk.WorkspaceAppOpenIn: %#v", app.OpenIn) + } + + var share *agentproto.CreateSubAgentRequest_App_SharingLevel + switch app.Share { + case codersdk.WorkspaceAppSharingLevelAuthenticated: + share = agentproto.CreateSubAgentRequest_App_AUTHENTICATED.Enum() + case codersdk.WorkspaceAppSharingLevelOwner: + share = agentproto.CreateSubAgentRequest_App_OWNER.Enum() + case codersdk.WorkspaceAppSharingLevelPublic: + share = agentproto.CreateSubAgentRequest_App_PUBLIC.Enum() + default: + return SubAgent{}, xerrors.Errorf("unexpected codersdk.WorkspaceAppSharingLevel: %#v", app.Share) + } + + apps = append(apps, &agentproto.CreateSubAgentRequest_App{ + Slug: app.Slug, + Command: app.Command, + DisplayName: app.DisplayName, + External: app.External, + Group: app.Group, + Healthcheck: healthCheck, + Hidden: app.Hidden, + Icon: app.Icon, + OpenIn: openIn, + Order: app.Order, + Share: share, + Subdomain: app.Subdomain, + Url: app.URL, + }) + } + resp, err := a.api.CreateSubAgent(ctx, &agentproto.CreateSubAgentRequest{ Name: agent.Name, Directory: agent.Directory, Architecture: agent.Architecture, OperatingSystem: agent.OperatingSystem, DisplayApps: displayApps, + Apps: apps, }) if err != nil { return SubAgent{}, err diff --git a/agent/agentcontainers/subagent_test.go b/agent/agentcontainers/subagent_test.go index 4b805d7549fce..984730fb36ae8 100644 --- a/agent/agentcontainers/subagent_test.go +++ b/agent/agentcontainers/subagent_test.go @@ -4,11 +4,13 @@ import ( "testing" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/agent/agentcontainers" "github.com/coder/coder/v2/agent/agenttest" agentproto "github.com/coder/coder/v2/agent/proto" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" "github.com/coder/coder/v2/tailnet" @@ -102,4 +104,232 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { }) } }) + + t.Run("CreateWithApps", func(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + apps []agentcontainers.SubAgentApp + expectedApps []*agentproto.CreateSubAgentRequest_App + }{ + { + name: "single app with minimal fields", + apps: []agentcontainers.SubAgentApp{ + { + Slug: "code-server", + OpenIn: codersdk.WorkspaceAppOpenInSlimWindow, + Share: codersdk.WorkspaceAppSharingLevelOwner, + }, + }, + expectedApps: []*agentproto.CreateSubAgentRequest_App{ + { + Slug: "code-server", + OpenIn: agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum(), + Share: agentproto.CreateSubAgentRequest_App_OWNER.Enum(), + }, + }, + }, + { + name: "single app with all fields", + apps: []agentcontainers.SubAgentApp{ + { + Slug: "jupyter", + Command: ptr.Ref("jupyter lab --port=8888"), + DisplayName: ptr.Ref("Jupyter Lab"), + External: ptr.Ref(false), + Group: ptr.Ref("Development"), + HealthCheck: &agentcontainers.SubAgentHealthCheck{ + Interval: 30, + Threshold: 3, + URL: "http://localhost:8888/api", + }, + Hidden: ptr.Ref(false), + Icon: ptr.Ref("/icon/jupyter.svg"), + OpenIn: codersdk.WorkspaceAppOpenInTab, + Order: ptr.Ref(int32(1)), + Share: codersdk.WorkspaceAppSharingLevelAuthenticated, + Subdomain: ptr.Ref(true), + URL: ptr.Ref("http://localhost:8888"), + }, + }, + expectedApps: []*agentproto.CreateSubAgentRequest_App{ + { + Slug: "jupyter", + Command: ptr.Ref("jupyter lab --port=8888"), + DisplayName: ptr.Ref("Jupyter Lab"), + External: ptr.Ref(false), + Group: ptr.Ref("Development"), + Healthcheck: &agentproto.CreateSubAgentRequest_App_Healthcheck{ + Interval: 30, + Threshold: 3, + Url: "http://localhost:8888/api", + }, + Hidden: ptr.Ref(false), + Icon: ptr.Ref("/icon/jupyter.svg"), + OpenIn: agentproto.CreateSubAgentRequest_App_TAB.Enum(), + Order: ptr.Ref(int32(1)), + Share: agentproto.CreateSubAgentRequest_App_AUTHENTICATED.Enum(), + Subdomain: ptr.Ref(true), + Url: ptr.Ref("http://localhost:8888"), + }, + }, + }, + { + name: "multiple apps with different sharing levels", + apps: []agentcontainers.SubAgentApp{ + { + Slug: "owner-app", + OpenIn: codersdk.WorkspaceAppOpenInSlimWindow, + Share: codersdk.WorkspaceAppSharingLevelOwner, + }, + { + Slug: "authenticated-app", + OpenIn: codersdk.WorkspaceAppOpenInTab, + Share: codersdk.WorkspaceAppSharingLevelAuthenticated, + }, + { + Slug: "public-app", + OpenIn: codersdk.WorkspaceAppOpenInSlimWindow, + Share: codersdk.WorkspaceAppSharingLevelPublic, + }, + }, + expectedApps: []*agentproto.CreateSubAgentRequest_App{ + { + Slug: "owner-app", + OpenIn: agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum(), + Share: agentproto.CreateSubAgentRequest_App_OWNER.Enum(), + }, + { + Slug: "authenticated-app", + OpenIn: agentproto.CreateSubAgentRequest_App_TAB.Enum(), + Share: agentproto.CreateSubAgentRequest_App_AUTHENTICATED.Enum(), + }, + { + Slug: "public-app", + OpenIn: agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum(), + Share: agentproto.CreateSubAgentRequest_App_PUBLIC.Enum(), + }, + }, + }, + { + name: "app with health check", + apps: []agentcontainers.SubAgentApp{ + { + Slug: "health-app", + HealthCheck: &agentcontainers.SubAgentHealthCheck{ + Interval: 60, + Threshold: 5, + URL: "http://localhost:3000/health", + }, + OpenIn: codersdk.WorkspaceAppOpenInSlimWindow, + Share: codersdk.WorkspaceAppSharingLevelOwner, + }, + }, + expectedApps: []*agentproto.CreateSubAgentRequest_App{ + { + Slug: "health-app", + Healthcheck: &agentproto.CreateSubAgentRequest_App_Healthcheck{ + Interval: 60, + Threshold: 5, + Url: "http://localhost:3000/health", + }, + OpenIn: agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum(), + Share: agentproto.CreateSubAgentRequest_App_OWNER.Enum(), + }, + }, + }, + { + name: "app without health check", + apps: []agentcontainers.SubAgentApp{ + { + Slug: "no-health-app", + OpenIn: codersdk.WorkspaceAppOpenInTab, + Share: codersdk.WorkspaceAppSharingLevelOwner, + }, + }, + expectedApps: []*agentproto.CreateSubAgentRequest_App{ + { + Slug: "no-health-app", + OpenIn: agentproto.CreateSubAgentRequest_App_TAB.Enum(), + Share: agentproto.CreateSubAgentRequest_App_OWNER.Enum(), + }, + }, + }, + { + name: "no apps", + apps: []agentcontainers.SubAgentApp{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + logger := testutil.Logger(t) + statsCh := make(chan *agentproto.Stats) + + agentAPI := agenttest.NewClient(t, logger, uuid.New(), agentsdk.Manifest{}, statsCh, tailnet.NewCoordinator(logger)) + + agentClient, _, err := agentAPI.ConnectRPC26(ctx) + require.NoError(t, err) + + subAgentClient := agentcontainers.NewSubAgentClientFromAPI(logger, agentClient) + + // When: We create a sub agent with display apps. + subAgent, err := subAgentClient.Create(ctx, agentcontainers.SubAgent{ + Name: "sub-agent-" + tt.name, + Directory: "/workspaces/coder", + Architecture: "amd64", + OperatingSystem: "linux", + Apps: tt.apps, + }) + require.NoError(t, err) + + apps, err := agentAPI.GetSubAgentApps(subAgent.ID) + require.NoError(t, err) + + // Then: We expect the apps to be created. + require.Len(t, apps, len(tt.expectedApps)) + for i, expectedApp := range tt.expectedApps { + actualApp := apps[i] + + assert.Equal(t, expectedApp.Slug, actualApp.Slug) + assert.Equal(t, expectedApp.Command, actualApp.Command) + assert.Equal(t, expectedApp.DisplayName, actualApp.DisplayName) + assert.Equal(t, expectedApp.External, actualApp.External) + assert.Equal(t, expectedApp.Group, actualApp.Group) + assert.Equal(t, expectedApp.Hidden, actualApp.Hidden) + assert.Equal(t, expectedApp.Icon, actualApp.Icon) + assert.Equal(t, expectedApp.Order, actualApp.Order) + assert.Equal(t, expectedApp.Subdomain, actualApp.Subdomain) + assert.Equal(t, expectedApp.Url, actualApp.Url) + + if expectedApp.OpenIn != nil { + require.NotNil(t, actualApp.OpenIn) + assert.Equal(t, *expectedApp.OpenIn, *actualApp.OpenIn) + } else { + assert.Equal(t, expectedApp.OpenIn, actualApp.OpenIn) + } + + if expectedApp.Share != nil { + require.NotNil(t, actualApp.Share) + assert.Equal(t, *expectedApp.Share, *actualApp.Share) + } else { + assert.Equal(t, expectedApp.Share, actualApp.Share) + } + + if expectedApp.Healthcheck != nil { + require.NotNil(t, expectedApp.Healthcheck) + assert.Equal(t, expectedApp.Healthcheck.Interval, actualApp.Healthcheck.Interval) + assert.Equal(t, expectedApp.Healthcheck.Threshold, actualApp.Healthcheck.Threshold) + assert.Equal(t, expectedApp.Healthcheck.Url, actualApp.Healthcheck.Url) + } else { + assert.Equal(t, expectedApp.Healthcheck, actualApp.Healthcheck) + } + } + }) + } + }) } diff --git a/agent/agenttest/client.go b/agent/agenttest/client.go index 0fc8a38af80b6..5d78dfe697c93 100644 --- a/agent/agenttest/client.go +++ b/agent/agenttest/client.go @@ -175,6 +175,10 @@ func (c *Client) GetSubAgentDisplayApps(id uuid.UUID) ([]agentproto.CreateSubAge return c.fakeAgentAPI.GetSubAgentDisplayApps(id) } +func (c *Client) GetSubAgentApps(id uuid.UUID) ([]*agentproto.CreateSubAgentRequest_App, error) { + return c.fakeAgentAPI.GetSubAgentApps(id) +} + type FakeAgentAPI struct { sync.Mutex t testing.TB @@ -192,6 +196,7 @@ type FakeAgentAPI struct { subAgents map[uuid.UUID]*agentproto.SubAgent subAgentDirs map[uuid.UUID]string subAgentDisplayApps map[uuid.UUID][]agentproto.CreateSubAgentRequest_DisplayApp + subAgentApps map[uuid.UUID][]*agentproto.CreateSubAgentRequest_App getAnnouncementBannersFunc func() ([]codersdk.BannerConfig, error) getResourcesMonitoringConfigurationFunc func() (*agentproto.GetResourcesMonitoringConfigurationResponse, error) @@ -410,6 +415,10 @@ func (f *FakeAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Creat f.subAgentDisplayApps = make(map[uuid.UUID][]agentproto.CreateSubAgentRequest_DisplayApp) } f.subAgentDisplayApps[subAgentID] = req.GetDisplayApps() + if f.subAgentApps == nil { + f.subAgentApps = make(map[uuid.UUID][]*agentproto.CreateSubAgentRequest_App) + } + f.subAgentApps[subAgentID] = req.GetApps() // For a fake implementation, we don't create workspace apps. // Real implementations would handle req.Apps here. @@ -502,6 +511,22 @@ func (f *FakeAgentAPI) GetSubAgentDisplayApps(id uuid.UUID) ([]agentproto.Create return displayApps, nil } +func (f *FakeAgentAPI) GetSubAgentApps(id uuid.UUID) ([]*agentproto.CreateSubAgentRequest_App, error) { + f.Lock() + defer f.Unlock() + + if f.subAgentApps == nil { + return nil, xerrors.New("no sub-agent apps available") + } + + apps, ok := f.subAgentApps[id] + if !ok { + return nil, xerrors.New("sub-agent apps not found") + } + + return apps, nil +} + func NewFakeAgentAPI(t testing.TB, logger slog.Logger, manifest *agentproto.Manifest, statsCh chan *agentproto.Stats) *FakeAgentAPI { return &FakeAgentAPI{ t: t, From 0dc5bb2056f210946bbb120388800a7bb2e89f6b Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 12 Jun 2025 22:56:39 +0000 Subject: [PATCH 02/16] chore: copilot feedback --- agent/agentcontainers/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 41d112788f684..70fc80456f23c 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -163,7 +163,7 @@ func WithWorkspaceName(name string) Option { } } -// WithUserName sets the workspace name for the sub-agent. +// WithUserName sets the user name for the sub-agent. func WithUserName(name string) Option { return func(api *API) { api.userName = name From bcd6689225719a36c7d22e55e34c9d9d14e6022a Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 13 Jun 2025 10:06:43 +0000 Subject: [PATCH 03/16] chore: more changes --- agent/agentcontainers/devcontainercli.go | 2 +- agent/api.go | 13 ++++++++++--- site/src/pages/WorkspacePage/Workspace.tsx | 2 ++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/agent/agentcontainers/devcontainercli.go b/agent/agentcontainers/devcontainercli.go index a18c896073ee5..f82d210f47bec 100644 --- a/agent/agentcontainers/devcontainercli.go +++ b/agent/agentcontainers/devcontainercli.go @@ -264,7 +264,7 @@ func (d *devcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, confi } c := d.execer.CommandContext(ctx, "devcontainer", args...) - c.Env = append(c.Env, env...) + // c.Env = append(c.Env, env...) var stdoutBuf bytes.Buffer stdoutWriters := []io.Writer{&stdoutBuf, &devcontainerCLILogWriter{ctx: ctx, logger: logger.With(slog.F("stdout", true))}} diff --git a/agent/api.go b/agent/api.go index 1c9a707fbb338..b869cedb6cb32 100644 --- a/agent/api.go +++ b/agent/api.go @@ -50,10 +50,17 @@ func (a *agent) apiHandler(aAPI proto.DRPCAgentClient26) (http.Handler, func() e } manifest := a.manifest.Load() if manifest != nil && len(manifest.Devcontainers) > 0 { - containerAPIOpts = append( - containerAPIOpts, - agentcontainers.WithDevcontainers(manifest.Devcontainers, manifest.Scripts), + containerAPIOpts = append(containerAPIOpts, + agentcontainers.WithUserName(manifest.OwnerName), + agentcontainers.WithWorkspaceName(manifest.WorkspaceName), ) + + if len(manifest.Devcontainers) > 0 { + containerAPIOpts = append( + containerAPIOpts, + agentcontainers.WithDevcontainers(manifest.Devcontainers, manifest.Scripts), + ) + } } // Append after to allow the agent options to override the default options. diff --git a/site/src/pages/WorkspacePage/Workspace.tsx b/site/src/pages/WorkspacePage/Workspace.tsx index 5c032c04efbdf..d01a304fa395b 100644 --- a/site/src/pages/WorkspacePage/Workspace.tsx +++ b/site/src/pages/WorkspacePage/Workspace.tsx @@ -99,6 +99,8 @@ export const Workspace: FC = ({ const shouldShowProvisionerAlert = workspacePending && !haveBuildLogs && !provisionersHealthy && !isRestarting; + console.log(selectedResource?.agents); + return (
Date: Fri, 13 Jun 2025 14:55:53 +0000 Subject: [PATCH 04/16] chore: pass env variables + remove accidental commit --- agent/agentcontainers/api.go | 14 ++++++++------ agent/agentcontainers/devcontainercli.go | 3 ++- site/src/pages/WorkspacePage/Workspace.tsx | 2 -- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 70fc80456f23c..29a7d953a6682 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -1146,12 +1146,14 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c var apps []SubAgentApp - if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath, []string{ - fmt.Sprintf("CODER_AGENT_NAME=%s", dc.Name), - fmt.Sprintf("CODER_USER_NAME=%s", api.userName), - fmt.Sprintf("CODER_WORKSPACE_NAME=%s", api.workspaceName), - fmt.Sprintf("CODER_DEPLOYMENT_URL=%s", api.subAgentURL), - }); err != nil { + if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath, + []string{ + fmt.Sprintf("CODER_WORKSPACE_AGENT_NAME=%s", dc.Name), + fmt.Sprintf("CODER_WORKSPACE_OWNER_NAME=%s", api.userName), + fmt.Sprintf("CODER_WORKSPACE_NAME=%s", api.workspaceName), + fmt.Sprintf("CODER_DEPLOYMENT_URL=%s", api.subAgentURL), + }, + ); err != nil { api.logger.Error(ctx, "unable to read devcontainer config", slog.Error(err)) } else { coderCustomization := config.MergedConfiguration.Customizations.Coder diff --git a/agent/agentcontainers/devcontainercli.go b/agent/agentcontainers/devcontainercli.go index f82d210f47bec..61477733d46cf 100644 --- a/agent/agentcontainers/devcontainercli.go +++ b/agent/agentcontainers/devcontainercli.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "io" + "os" "golang.org/x/xerrors" @@ -264,7 +265,7 @@ func (d *devcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, confi } c := d.execer.CommandContext(ctx, "devcontainer", args...) - // c.Env = append(c.Env, env...) + c.Env = append(os.Environ(), env...) var stdoutBuf bytes.Buffer stdoutWriters := []io.Writer{&stdoutBuf, &devcontainerCLILogWriter{ctx: ctx, logger: logger.With(slog.F("stdout", true))}} diff --git a/site/src/pages/WorkspacePage/Workspace.tsx b/site/src/pages/WorkspacePage/Workspace.tsx index d01a304fa395b..5c032c04efbdf 100644 --- a/site/src/pages/WorkspacePage/Workspace.tsx +++ b/site/src/pages/WorkspacePage/Workspace.tsx @@ -99,8 +99,6 @@ export const Workspace: FC = ({ const shouldShowProvisionerAlert = workspacePending && !haveBuildLogs && !provisionersHealthy && !isRestarting; - console.log(selectedResource?.agents); - return (
Date: Fri, 13 Jun 2025 15:36:29 +0000 Subject: [PATCH 05/16] chore: fix some tests --- agent/agentcontainers/api_test.go | 32 +++++----- agent/agentcontainers/devcontainercli.go | 3 +- agent/agentcontainers/subagent.go | 62 +++++++++++--------- agent/agentcontainers/subagent_test.go | 75 +++++++----------------- 4 files changed, 71 insertions(+), 101 deletions(-) diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index bb803ffc4c689..480642156a797 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -1319,9 +1319,9 @@ func TestAPI(t *testing.T) { return nil }) // Exec pwd. testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) (agentcontainers.DevcontainerConfig, error) { - assert.Contains(t, envs, "CODER_AGENT_NAME=test-container") + assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container") assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") - assert.Contains(t, envs, "CODER_USER_NAME=test-user") + assert.Contains(t, envs, "CODER_WORKSPACE_OWNER_NAME=test-user") assert.Contains(t, envs, "CODER_DEPLOYMENT_URL=test-subagent-url") return agentcontainers.DevcontainerConfig{}, nil }) @@ -1466,9 +1466,9 @@ func TestAPI(t *testing.T) { return nil }) // Exec pwd. testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) (agentcontainers.DevcontainerConfig, error) { - assert.Contains(t, envs, "CODER_AGENT_NAME=test-container") + assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container") assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") - assert.Contains(t, envs, "CODER_USER_NAME=test-user") + assert.Contains(t, envs, "CODER_WORKSPACE_OWNER_NAME=test-user") assert.Contains(t, envs, "CODER_DEPLOYMENT_URL=test-subagent-url") return agentcontainers.DevcontainerConfig{}, nil }) @@ -1631,8 +1631,8 @@ func TestAPI(t *testing.T) { Slug: "web-app", DisplayName: ptr.Ref("Web Application"), URL: ptr.Ref("http://localhost:8080"), - OpenIn: codersdk.WorkspaceAppOpenInTab, - Share: codersdk.WorkspaceAppSharingLevelOwner, + OpenIn: ptr.Ref(codersdk.WorkspaceAppOpenInTab), + Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelOwner), Icon: ptr.Ref("/icons/web.svg"), Order: ptr.Ref(int32(1)), }, @@ -1640,8 +1640,8 @@ func TestAPI(t *testing.T) { Slug: "api-server", DisplayName: ptr.Ref("API Server"), URL: ptr.Ref("http://localhost:3000"), - OpenIn: codersdk.WorkspaceAppOpenInSlimWindow, - Share: codersdk.WorkspaceAppSharingLevelAuthenticated, + OpenIn: ptr.Ref(codersdk.WorkspaceAppOpenInSlimWindow), + Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelAuthenticated), Icon: ptr.Ref("/icons/api.svg"), Order: ptr.Ref(int32(2)), Hidden: ptr.Ref(true), @@ -1650,8 +1650,8 @@ func TestAPI(t *testing.T) { Slug: "docs", DisplayName: ptr.Ref("Documentation"), URL: ptr.Ref("http://localhost:4000"), - OpenIn: codersdk.WorkspaceAppOpenInTab, - Share: codersdk.WorkspaceAppSharingLevelPublic, + OpenIn: ptr.Ref(codersdk.WorkspaceAppOpenInTab), + Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelPublic), Icon: ptr.Ref("/icons/book.svg"), Order: ptr.Ref(int32(3)), }, @@ -1665,8 +1665,8 @@ func TestAPI(t *testing.T) { assert.Equal(t, "web-app", subAgent.Apps[0].Slug) assert.Equal(t, "Web Application", *subAgent.Apps[0].DisplayName) assert.Equal(t, "http://localhost:8080", *subAgent.Apps[0].URL) - assert.Equal(t, codersdk.WorkspaceAppOpenInTab, subAgent.Apps[0].OpenIn) - assert.Equal(t, codersdk.WorkspaceAppSharingLevelOwner, subAgent.Apps[0].Share) + assert.Equal(t, codersdk.WorkspaceAppOpenInTab, *subAgent.Apps[0].OpenIn) + assert.Equal(t, codersdk.WorkspaceAppSharingLevelOwner, *subAgent.Apps[0].Share) assert.Equal(t, "/icons/web.svg", *subAgent.Apps[0].Icon) assert.Equal(t, int32(1), *subAgent.Apps[0].Order) @@ -1674,8 +1674,8 @@ func TestAPI(t *testing.T) { assert.Equal(t, "api-server", subAgent.Apps[1].Slug) assert.Equal(t, "API Server", *subAgent.Apps[1].DisplayName) assert.Equal(t, "http://localhost:3000", *subAgent.Apps[1].URL) - assert.Equal(t, codersdk.WorkspaceAppOpenInSlimWindow, subAgent.Apps[1].OpenIn) - assert.Equal(t, codersdk.WorkspaceAppSharingLevelAuthenticated, subAgent.Apps[1].Share) + assert.Equal(t, codersdk.WorkspaceAppOpenInSlimWindow, *subAgent.Apps[1].OpenIn) + assert.Equal(t, codersdk.WorkspaceAppSharingLevelAuthenticated, *subAgent.Apps[1].Share) assert.Equal(t, "/icons/api.svg", *subAgent.Apps[1].Icon) assert.Equal(t, int32(2), *subAgent.Apps[1].Order) assert.Equal(t, true, *subAgent.Apps[1].Hidden) @@ -1684,8 +1684,8 @@ func TestAPI(t *testing.T) { assert.Equal(t, "docs", subAgent.Apps[2].Slug) assert.Equal(t, "Documentation", *subAgent.Apps[2].DisplayName) assert.Equal(t, "http://localhost:4000", *subAgent.Apps[2].URL) - assert.Equal(t, codersdk.WorkspaceAppOpenInTab, subAgent.Apps[2].OpenIn) - assert.Equal(t, codersdk.WorkspaceAppSharingLevelPublic, subAgent.Apps[2].Share) + assert.Equal(t, codersdk.WorkspaceAppOpenInTab, *subAgent.Apps[2].OpenIn) + assert.Equal(t, codersdk.WorkspaceAppSharingLevelPublic, *subAgent.Apps[2].Share) assert.Equal(t, "/icons/book.svg", *subAgent.Apps[2].Icon) assert.Equal(t, int32(3), *subAgent.Apps[2].Order) }, diff --git a/agent/agentcontainers/devcontainercli.go b/agent/agentcontainers/devcontainercli.go index 61477733d46cf..cac210b5b56c2 100644 --- a/agent/agentcontainers/devcontainercli.go +++ b/agent/agentcontainers/devcontainercli.go @@ -265,7 +265,8 @@ func (d *devcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, confi } c := d.execer.CommandContext(ctx, "devcontainer", args...) - c.Env = append(os.Environ(), env...) + c.Env = append(c.Env, os.Environ()...) + c.Env = append(c.Env, env...) var stdoutBuf bytes.Buffer stdoutWriters := []io.Writer{&stdoutBuf, &devcontainerCLILogWriter{ctx: ctx, logger: logger.With(slog.F("stdout", true))}} diff --git a/agent/agentcontainers/subagent.go b/agent/agentcontainers/subagent.go index 6a4af8f2094dc..13956eab3d6df 100644 --- a/agent/agentcontainers/subagent.go +++ b/agent/agentcontainers/subagent.go @@ -47,19 +47,19 @@ func (s SubAgent) EqualConfig(other SubAgent) bool { } type SubAgentApp struct { - Slug string `json:"slug"` - Command *string `json:"command"` - DisplayName *string `json:"displayName"` - External *bool `json:"external"` - Group *string `json:"group"` - HealthCheck *SubAgentHealthCheck `json:"healthCheck"` - Hidden *bool `json:"hidden"` - Icon *string `json:"icon"` - OpenIn codersdk.WorkspaceAppOpenIn `json:"openIn"` - Order *int32 `json:"order"` - Share codersdk.WorkspaceAppSharingLevel `json:"share"` - Subdomain *bool `json:"subdomain"` - URL *string `json:"url"` + Slug string `json:"slug"` + Command *string `json:"command"` + DisplayName *string `json:"displayName"` + External *bool `json:"external"` + Group *string `json:"group"` + HealthCheck *SubAgentHealthCheck `json:"healthCheck"` + Hidden *bool `json:"hidden"` + Icon *string `json:"icon"` + OpenIn *codersdk.WorkspaceAppOpenIn `json:"openIn"` + Order *int32 `json:"order"` + Share *codersdk.WorkspaceAppSharingLevel `json:"share"` + Subdomain *bool `json:"subdomain"` + URL *string `json:"url"` } type SubAgentHealthCheck struct { @@ -161,25 +161,29 @@ func (a *subAgentAPIClient) Create(ctx context.Context, agent SubAgent) (SubAgen } var openIn *agentproto.CreateSubAgentRequest_App_OpenIn - switch app.OpenIn { - case codersdk.WorkspaceAppOpenInSlimWindow: - openIn = agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum() - case codersdk.WorkspaceAppOpenInTab: - openIn = agentproto.CreateSubAgentRequest_App_TAB.Enum() - default: - return SubAgent{}, xerrors.Errorf("unexpected codersdk.WorkspaceAppOpenIn: %#v", app.OpenIn) + if app.OpenIn != nil { + switch *app.OpenIn { + case codersdk.WorkspaceAppOpenInSlimWindow: + openIn = agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum() + case codersdk.WorkspaceAppOpenInTab: + openIn = agentproto.CreateSubAgentRequest_App_TAB.Enum() + default: + return SubAgent{}, xerrors.Errorf("unexpected codersdk.WorkspaceAppOpenIn: %#v", app.OpenIn) + } } var share *agentproto.CreateSubAgentRequest_App_SharingLevel - switch app.Share { - case codersdk.WorkspaceAppSharingLevelAuthenticated: - share = agentproto.CreateSubAgentRequest_App_AUTHENTICATED.Enum() - case codersdk.WorkspaceAppSharingLevelOwner: - share = agentproto.CreateSubAgentRequest_App_OWNER.Enum() - case codersdk.WorkspaceAppSharingLevelPublic: - share = agentproto.CreateSubAgentRequest_App_PUBLIC.Enum() - default: - return SubAgent{}, xerrors.Errorf("unexpected codersdk.WorkspaceAppSharingLevel: %#v", app.Share) + if app.Share != nil { + switch *app.Share { + case codersdk.WorkspaceAppSharingLevelAuthenticated: + share = agentproto.CreateSubAgentRequest_App_AUTHENTICATED.Enum() + case codersdk.WorkspaceAppSharingLevelOwner: + share = agentproto.CreateSubAgentRequest_App_OWNER.Enum() + case codersdk.WorkspaceAppSharingLevelPublic: + share = agentproto.CreateSubAgentRequest_App_PUBLIC.Enum() + default: + return SubAgent{}, xerrors.Errorf("unexpected codersdk.WorkspaceAppSharingLevel: %#v", app.Share) + } } apps = append(apps, &agentproto.CreateSubAgentRequest_App{ diff --git a/agent/agentcontainers/subagent_test.go b/agent/agentcontainers/subagent_test.go index 984730fb36ae8..a135383c4b92a 100644 --- a/agent/agentcontainers/subagent_test.go +++ b/agent/agentcontainers/subagent_test.go @@ -114,24 +114,20 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { expectedApps []*agentproto.CreateSubAgentRequest_App }{ { - name: "single app with minimal fields", + name: "SlugOnly", apps: []agentcontainers.SubAgentApp{ { - Slug: "code-server", - OpenIn: codersdk.WorkspaceAppOpenInSlimWindow, - Share: codersdk.WorkspaceAppSharingLevelOwner, + Slug: "code-server", }, }, expectedApps: []*agentproto.CreateSubAgentRequest_App{ { - Slug: "code-server", - OpenIn: agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum(), - Share: agentproto.CreateSubAgentRequest_App_OWNER.Enum(), + Slug: "code-server", }, }, }, { - name: "single app with all fields", + name: "AllFields", apps: []agentcontainers.SubAgentApp{ { Slug: "jupyter", @@ -146,9 +142,9 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { }, Hidden: ptr.Ref(false), Icon: ptr.Ref("/icon/jupyter.svg"), - OpenIn: codersdk.WorkspaceAppOpenInTab, + OpenIn: ptr.Ref(codersdk.WorkspaceAppOpenInTab), Order: ptr.Ref(int32(1)), - Share: codersdk.WorkspaceAppSharingLevelAuthenticated, + Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelAuthenticated), Subdomain: ptr.Ref(true), URL: ptr.Ref("http://localhost:8888"), }, @@ -176,44 +172,38 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { }, }, { - name: "multiple apps with different sharing levels", + name: "AllSharingLevels", apps: []agentcontainers.SubAgentApp{ { - Slug: "owner-app", - OpenIn: codersdk.WorkspaceAppOpenInSlimWindow, - Share: codersdk.WorkspaceAppSharingLevelOwner, + Slug: "owner-app", + Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelOwner), }, { - Slug: "authenticated-app", - OpenIn: codersdk.WorkspaceAppOpenInTab, - Share: codersdk.WorkspaceAppSharingLevelAuthenticated, + Slug: "authenticated-app", + Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelAuthenticated), }, { - Slug: "public-app", - OpenIn: codersdk.WorkspaceAppOpenInSlimWindow, - Share: codersdk.WorkspaceAppSharingLevelPublic, + Slug: "public-app", + Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelPublic), }, }, expectedApps: []*agentproto.CreateSubAgentRequest_App{ { - Slug: "owner-app", - OpenIn: agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum(), - Share: agentproto.CreateSubAgentRequest_App_OWNER.Enum(), + Slug: "owner-app", + Share: agentproto.CreateSubAgentRequest_App_OWNER.Enum(), }, { - Slug: "authenticated-app", - OpenIn: agentproto.CreateSubAgentRequest_App_TAB.Enum(), - Share: agentproto.CreateSubAgentRequest_App_AUTHENTICATED.Enum(), + Slug: "authenticated-app", + Share: agentproto.CreateSubAgentRequest_App_AUTHENTICATED.Enum(), }, { - Slug: "public-app", - OpenIn: agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum(), - Share: agentproto.CreateSubAgentRequest_App_PUBLIC.Enum(), + Slug: "public-app", + Share: agentproto.CreateSubAgentRequest_App_PUBLIC.Enum(), }, }, }, { - name: "app with health check", + name: "WithHealthCheck", apps: []agentcontainers.SubAgentApp{ { Slug: "health-app", @@ -222,8 +212,6 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { Threshold: 5, URL: "http://localhost:3000/health", }, - OpenIn: codersdk.WorkspaceAppOpenInSlimWindow, - Share: codersdk.WorkspaceAppSharingLevelOwner, }, }, expectedApps: []*agentproto.CreateSubAgentRequest_App{ @@ -234,32 +222,9 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { Threshold: 5, Url: "http://localhost:3000/health", }, - OpenIn: agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum(), - Share: agentproto.CreateSubAgentRequest_App_OWNER.Enum(), }, }, }, - { - name: "app without health check", - apps: []agentcontainers.SubAgentApp{ - { - Slug: "no-health-app", - OpenIn: codersdk.WorkspaceAppOpenInTab, - Share: codersdk.WorkspaceAppSharingLevelOwner, - }, - }, - expectedApps: []*agentproto.CreateSubAgentRequest_App{ - { - Slug: "no-health-app", - OpenIn: agentproto.CreateSubAgentRequest_App_TAB.Enum(), - Share: agentproto.CreateSubAgentRequest_App_OWNER.Enum(), - }, - }, - }, - { - name: "no apps", - apps: []agentcontainers.SubAgentApp{}, - }, } for _, tt := range tests { From 8698100bbdd809026607a0520f9841246706c29e Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 17 Jun 2025 15:57:55 +0000 Subject: [PATCH 06/16] chore: fix issues --- agent/agentcontainers/api.go | 1 + agent/agentcontainers/subagent.go | 2 ++ 2 files changed, 3 insertions(+) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 29a7d953a6682..dc233ea378505 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -1183,6 +1183,7 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c slices.Sort(displayApps) subAgentConfig.DisplayApps = displayApps + subAgentConfig.Apps = apps } deleteSubAgent := proc.agent.ID != uuid.Nil && maybeRecreateSubAgent && !proc.agent.EqualConfig(subAgentConfig) diff --git a/agent/agentcontainers/subagent.go b/agent/agentcontainers/subagent.go index 13956eab3d6df..1cbf416c10aa9 100644 --- a/agent/agentcontainers/subagent.go +++ b/agent/agentcontainers/subagent.go @@ -181,6 +181,8 @@ func (a *subAgentAPIClient) Create(ctx context.Context, agent SubAgent) (SubAgen share = agentproto.CreateSubAgentRequest_App_OWNER.Enum() case codersdk.WorkspaceAppSharingLevelPublic: share = agentproto.CreateSubAgentRequest_App_PUBLIC.Enum() + case codersdk.WorkspaceAppSharingLevelOrganization: + share = agentproto.CreateSubAgentRequest_App_ORGANIZATION.Enum() default: return SubAgent{}, xerrors.Errorf("unexpected codersdk.WorkspaceAppSharingLevel: %#v", app.Share) } From 8ec61b6b0e29547a2f462fe2fa45726a014622e0 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 17 Jun 2025 17:50:49 +0000 Subject: [PATCH 07/16] chore: fix and update test --- agent/agentcontainers/api_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 480642156a797..3bbebbb895de8 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -69,7 +69,7 @@ type fakeDevcontainerCLI struct { execErrC chan func(cmd string, args ...string) error // If set, send fn to return err, nil or close to return execErr. readConfig agentcontainers.DevcontainerConfig readConfigErr error - readConfigErrC chan func(envs []string) (agentcontainers.DevcontainerConfig, error) + readConfigErrC chan func(envs []string) error } func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) { @@ -107,7 +107,7 @@ func (f *fakeDevcontainerCLI) ReadConfig(ctx context.Context, _, _ string, envs return agentcontainers.DevcontainerConfig{}, ctx.Err() case fn, ok := <-f.readConfigErrC: if ok { - return fn(envs) + return f.readConfig, fn(envs) } } } @@ -1255,7 +1255,7 @@ func TestAPI(t *testing.T) { } fakeDCCLI = &fakeDevcontainerCLI{ execErrC: make(chan func(cmd string, args ...string) error, 1), - readConfigErrC: make(chan func(envs []string) (agentcontainers.DevcontainerConfig, error), 1), + readConfigErrC: make(chan func(envs []string) error, 1), } testContainer = codersdk.WorkspaceAgentContainer{ @@ -1304,7 +1304,7 @@ func TestAPI(t *testing.T) { close(fakeSAC.createErrC) close(fakeSAC.deleteErrC) close(fakeDCCLI.execErrC) - defer close(fakeDCCLI.readConfigErrC) + close(fakeDCCLI.readConfigErrC) _ = api.Close() }) @@ -1318,12 +1318,12 @@ func TestAPI(t *testing.T) { assert.Empty(t, args) return nil }) // Exec pwd. - testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) (agentcontainers.DevcontainerConfig, error) { + testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) error { assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container") assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") assert.Contains(t, envs, "CODER_WORKSPACE_OWNER_NAME=test-user") assert.Contains(t, envs, "CODER_DEPLOYMENT_URL=test-subagent-url") - return agentcontainers.DevcontainerConfig{}, nil + return nil }) // Make sure the ticker function has been registered @@ -1465,12 +1465,12 @@ func TestAPI(t *testing.T) { assert.Empty(t, args) return nil }) // Exec pwd. - testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) (agentcontainers.DevcontainerConfig, error) { + testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) error { assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container") assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") assert.Contains(t, envs, "CODER_WORKSPACE_OWNER_NAME=test-user") assert.Contains(t, envs, "CODER_DEPLOYMENT_URL=test-subagent-url") - return agentcontainers.DevcontainerConfig{}, nil + return nil }) err = api.RefreshContainers(ctx) From ab2e82e648299a8d8bc058f6bd2c844f7bc34041 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 18 Jun 2025 10:16:40 +0000 Subject: [PATCH 08/16] chore: fix oopsie --- agent/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/api.go b/agent/api.go index b869cedb6cb32..15f6617e4a3cd 100644 --- a/agent/api.go +++ b/agent/api.go @@ -49,7 +49,7 @@ func (a *agent) apiHandler(aAPI proto.DRPCAgentClient26) (http.Handler, func() e agentcontainers.WithSubAgentClient(agentcontainers.NewSubAgentClientFromAPI(a.logger, aAPI)), } manifest := a.manifest.Load() - if manifest != nil && len(manifest.Devcontainers) > 0 { + if manifest != nil { containerAPIOpts = append(containerAPIOpts, agentcontainers.WithUserName(manifest.OwnerName), agentcontainers.WithWorkspaceName(manifest.WorkspaceName), From 4db77afa15041ddee7c04b9112c4f3b84453104d Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 18 Jun 2025 10:29:56 +0000 Subject: [PATCH 09/16] test: add organization sharing level --- agent/agentcontainers/subagent_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/agent/agentcontainers/subagent_test.go b/agent/agentcontainers/subagent_test.go index a135383c4b92a..3f40598615877 100644 --- a/agent/agentcontainers/subagent_test.go +++ b/agent/agentcontainers/subagent_test.go @@ -186,6 +186,10 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { Slug: "public-app", Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelPublic), }, + { + Slug: "organization-app", + Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelOrganization), + }, }, expectedApps: []*agentproto.CreateSubAgentRequest_App{ { @@ -200,6 +204,10 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { Slug: "public-app", Share: agentproto.CreateSubAgentRequest_App_PUBLIC.Enum(), }, + { + Slug: "organization-app", + Share: agentproto.CreateSubAgentRequest_App_ORGANIZATION.Enum(), + }, }, }, { From d3a25831c0fcba2125bb63189cf5f44c5e33dde2 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 18 Jun 2025 11:31:53 +0000 Subject: [PATCH 10/16] chore: CODER_DEPLOYMENT_URL -> CODER_URL --- agent/agentcontainers/api.go | 2 +- agent/agentcontainers/api_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index dc233ea378505..9c88d04c602ff 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -1151,7 +1151,7 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c fmt.Sprintf("CODER_WORKSPACE_AGENT_NAME=%s", dc.Name), fmt.Sprintf("CODER_WORKSPACE_OWNER_NAME=%s", api.userName), fmt.Sprintf("CODER_WORKSPACE_NAME=%s", api.workspaceName), - fmt.Sprintf("CODER_DEPLOYMENT_URL=%s", api.subAgentURL), + fmt.Sprintf("CODER_URL=%s", api.subAgentURL), }, ); err != nil { api.logger.Error(ctx, "unable to read devcontainer config", slog.Error(err)) diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 3bbebbb895de8..5dddac80c3578 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -1322,7 +1322,7 @@ func TestAPI(t *testing.T) { assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container") assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") assert.Contains(t, envs, "CODER_WORKSPACE_OWNER_NAME=test-user") - assert.Contains(t, envs, "CODER_DEPLOYMENT_URL=test-subagent-url") + assert.Contains(t, envs, "CODER_URL=test-subagent-url") return nil }) @@ -1469,7 +1469,7 @@ func TestAPI(t *testing.T) { assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container") assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace") assert.Contains(t, envs, "CODER_WORKSPACE_OWNER_NAME=test-user") - assert.Contains(t, envs, "CODER_DEPLOYMENT_URL=test-subagent-url") + assert.Contains(t, envs, "CODER_URL=test-subagent-url") return nil }) From 20e832a91620a6e41eb66f22cc536f9c69bf9758 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 18 Jun 2025 11:32:05 +0000 Subject: [PATCH 11/16] chore: remove all the pointers --- agent/agentcontainers/api_test.go | 77 +++++++------- agent/agentcontainers/subagent.go | 142 ++++++++++++++----------- agent/agentcontainers/subagent_test.go | 42 ++++---- 3 files changed, 138 insertions(+), 123 deletions(-) diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 5dddac80c3578..c0cd0f2ab2145 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -26,7 +26,6 @@ import ( "github.com/coder/coder/v2/agent/agentcontainers" "github.com/coder/coder/v2/agent/agentcontainers/acmock" "github.com/coder/coder/v2/agent/agentcontainers/watcher" - "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" "github.com/coder/quartz" @@ -1629,31 +1628,31 @@ func TestAPI(t *testing.T) { Apps: []agentcontainers.SubAgentApp{ { Slug: "web-app", - DisplayName: ptr.Ref("Web Application"), - URL: ptr.Ref("http://localhost:8080"), - OpenIn: ptr.Ref(codersdk.WorkspaceAppOpenInTab), - Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelOwner), - Icon: ptr.Ref("/icons/web.svg"), - Order: ptr.Ref(int32(1)), + DisplayName: "Web Application", + URL: "http://localhost:8080", + OpenIn: codersdk.WorkspaceAppOpenInTab, + Share: codersdk.WorkspaceAppSharingLevelOwner, + Icon: "/icons/web.svg", + Order: int32(1), }, { Slug: "api-server", - DisplayName: ptr.Ref("API Server"), - URL: ptr.Ref("http://localhost:3000"), - OpenIn: ptr.Ref(codersdk.WorkspaceAppOpenInSlimWindow), - Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelAuthenticated), - Icon: ptr.Ref("/icons/api.svg"), - Order: ptr.Ref(int32(2)), - Hidden: ptr.Ref(true), + DisplayName: "API Server", + URL: "http://localhost:3000", + OpenIn: codersdk.WorkspaceAppOpenInSlimWindow, + Share: codersdk.WorkspaceAppSharingLevelAuthenticated, + Icon: "/icons/api.svg", + Order: int32(2), + Hidden: true, }, { Slug: "docs", - DisplayName: ptr.Ref("Documentation"), - URL: ptr.Ref("http://localhost:4000"), - OpenIn: ptr.Ref(codersdk.WorkspaceAppOpenInTab), - Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelPublic), - Icon: ptr.Ref("/icons/book.svg"), - Order: ptr.Ref(int32(3)), + DisplayName: "Documentation", + URL: "http://localhost:4000", + OpenIn: codersdk.WorkspaceAppOpenInTab, + Share: codersdk.WorkspaceAppSharingLevelPublic, + Icon: "/icons/book.svg", + Order: int32(3), }, }, }, @@ -1663,31 +1662,31 @@ func TestAPI(t *testing.T) { // Verify first app assert.Equal(t, "web-app", subAgent.Apps[0].Slug) - assert.Equal(t, "Web Application", *subAgent.Apps[0].DisplayName) - assert.Equal(t, "http://localhost:8080", *subAgent.Apps[0].URL) - assert.Equal(t, codersdk.WorkspaceAppOpenInTab, *subAgent.Apps[0].OpenIn) - assert.Equal(t, codersdk.WorkspaceAppSharingLevelOwner, *subAgent.Apps[0].Share) - assert.Equal(t, "/icons/web.svg", *subAgent.Apps[0].Icon) - assert.Equal(t, int32(1), *subAgent.Apps[0].Order) + assert.Equal(t, "Web Application", subAgent.Apps[0].DisplayName) + assert.Equal(t, "http://localhost:8080", subAgent.Apps[0].URL) + assert.Equal(t, codersdk.WorkspaceAppOpenInTab, subAgent.Apps[0].OpenIn) + assert.Equal(t, codersdk.WorkspaceAppSharingLevelOwner, subAgent.Apps[0].Share) + assert.Equal(t, "/icons/web.svg", subAgent.Apps[0].Icon) + assert.Equal(t, int32(1), subAgent.Apps[0].Order) // Verify second app assert.Equal(t, "api-server", subAgent.Apps[1].Slug) - assert.Equal(t, "API Server", *subAgent.Apps[1].DisplayName) - assert.Equal(t, "http://localhost:3000", *subAgent.Apps[1].URL) - assert.Equal(t, codersdk.WorkspaceAppOpenInSlimWindow, *subAgent.Apps[1].OpenIn) - assert.Equal(t, codersdk.WorkspaceAppSharingLevelAuthenticated, *subAgent.Apps[1].Share) - assert.Equal(t, "/icons/api.svg", *subAgent.Apps[1].Icon) - assert.Equal(t, int32(2), *subAgent.Apps[1].Order) - assert.Equal(t, true, *subAgent.Apps[1].Hidden) + assert.Equal(t, "API Server", subAgent.Apps[1].DisplayName) + assert.Equal(t, "http://localhost:3000", subAgent.Apps[1].URL) + assert.Equal(t, codersdk.WorkspaceAppOpenInSlimWindow, subAgent.Apps[1].OpenIn) + assert.Equal(t, codersdk.WorkspaceAppSharingLevelAuthenticated, subAgent.Apps[1].Share) + assert.Equal(t, "/icons/api.svg", subAgent.Apps[1].Icon) + assert.Equal(t, int32(2), subAgent.Apps[1].Order) + assert.Equal(t, true, subAgent.Apps[1].Hidden) // Verify third app assert.Equal(t, "docs", subAgent.Apps[2].Slug) - assert.Equal(t, "Documentation", *subAgent.Apps[2].DisplayName) - assert.Equal(t, "http://localhost:4000", *subAgent.Apps[2].URL) - assert.Equal(t, codersdk.WorkspaceAppOpenInTab, *subAgent.Apps[2].OpenIn) - assert.Equal(t, codersdk.WorkspaceAppSharingLevelPublic, *subAgent.Apps[2].Share) - assert.Equal(t, "/icons/book.svg", *subAgent.Apps[2].Icon) - assert.Equal(t, int32(3), *subAgent.Apps[2].Order) + assert.Equal(t, "Documentation", subAgent.Apps[2].DisplayName) + assert.Equal(t, "http://localhost:4000", subAgent.Apps[2].URL) + assert.Equal(t, codersdk.WorkspaceAppOpenInTab, subAgent.Apps[2].OpenIn) + assert.Equal(t, codersdk.WorkspaceAppSharingLevelPublic, subAgent.Apps[2].Share) + assert.Equal(t, "/icons/book.svg", subAgent.Apps[2].Icon) + assert.Equal(t, int32(3), subAgent.Apps[2].Order) }, }, } diff --git a/agent/agentcontainers/subagent.go b/agent/agentcontainers/subagent.go index 1cbf416c10aa9..b3fe12a99d9a1 100644 --- a/agent/agentcontainers/subagent.go +++ b/agent/agentcontainers/subagent.go @@ -47,19 +47,81 @@ func (s SubAgent) EqualConfig(other SubAgent) bool { } type SubAgentApp struct { - Slug string `json:"slug"` - Command *string `json:"command"` - DisplayName *string `json:"displayName"` - External *bool `json:"external"` - Group *string `json:"group"` - HealthCheck *SubAgentHealthCheck `json:"healthCheck"` - Hidden *bool `json:"hidden"` - Icon *string `json:"icon"` - OpenIn *codersdk.WorkspaceAppOpenIn `json:"openIn"` - Order *int32 `json:"order"` - Share *codersdk.WorkspaceAppSharingLevel `json:"share"` - Subdomain *bool `json:"subdomain"` - URL *string `json:"url"` + Slug string `json:"slug"` + Command string `json:"command"` + DisplayName string `json:"displayName"` + External bool `json:"external"` + Group string `json:"group"` + HealthCheck SubAgentHealthCheck `json:"healthCheck"` + Hidden bool `json:"hidden"` + Icon string `json:"icon"` + OpenIn codersdk.WorkspaceAppOpenIn `json:"openIn"` + Order int32 `json:"order"` + Share codersdk.WorkspaceAppSharingLevel `json:"share"` + Subdomain bool `json:"subdomain"` + URL string `json:"url"` +} + +func (app SubAgentApp) ToProtoApp() (*agentproto.CreateSubAgentRequest_App, error) { + proto := agentproto.CreateSubAgentRequest_App{ + Slug: app.Slug, + External: &app.External, + Hidden: &app.Hidden, + Order: &app.Order, + Subdomain: &app.Subdomain, + } + + if app.Command != "" { + proto.Command = &app.Command + } + if app.DisplayName != "" { + proto.DisplayName = &app.DisplayName + } + if app.Group != "" { + proto.Group = &app.Group + } + if app.Icon != "" { + proto.Icon = &app.Icon + } + if app.URL != "" { + proto.Url = &app.URL + } + + if app.HealthCheck.URL != "" { + proto.Healthcheck = &agentproto.CreateSubAgentRequest_App_Healthcheck{ + Interval: app.HealthCheck.Interval, + Threshold: app.HealthCheck.Threshold, + Url: app.HealthCheck.URL, + } + } + + if app.OpenIn != "" { + switch app.OpenIn { + case codersdk.WorkspaceAppOpenInSlimWindow: + proto.OpenIn = agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum() + case codersdk.WorkspaceAppOpenInTab: + proto.OpenIn = agentproto.CreateSubAgentRequest_App_TAB.Enum() + default: + return nil, xerrors.Errorf("unexpected codersdk.WorkspaceAppOpenIn: %#v", app.OpenIn) + } + } + + if app.Share != "" { + switch app.Share { + case codersdk.WorkspaceAppSharingLevelAuthenticated: + proto.Share = agentproto.CreateSubAgentRequest_App_AUTHENTICATED.Enum() + case codersdk.WorkspaceAppSharingLevelOwner: + proto.Share = agentproto.CreateSubAgentRequest_App_OWNER.Enum() + case codersdk.WorkspaceAppSharingLevelPublic: + proto.Share = agentproto.CreateSubAgentRequest_App_PUBLIC.Enum() + case codersdk.WorkspaceAppSharingLevelOrganization: + proto.Share = agentproto.CreateSubAgentRequest_App_ORGANIZATION.Enum() + default: + return nil, xerrors.Errorf("unexpected codersdk.WorkspaceAppSharingLevel: %#v", app.Share) + } + } + + return &proto, nil } type SubAgentHealthCheck struct { @@ -151,58 +213,12 @@ func (a *subAgentAPIClient) Create(ctx context.Context, agent SubAgent) (SubAgen apps := make([]*agentproto.CreateSubAgentRequest_App, 0, len(agent.Apps)) for _, app := range agent.Apps { - var healthCheck *agentproto.CreateSubAgentRequest_App_Healthcheck - if app.HealthCheck != nil { - healthCheck = &agentproto.CreateSubAgentRequest_App_Healthcheck{ - Interval: app.HealthCheck.Interval, - Threshold: app.HealthCheck.Threshold, - Url: app.HealthCheck.URL, - } - } - - var openIn *agentproto.CreateSubAgentRequest_App_OpenIn - if app.OpenIn != nil { - switch *app.OpenIn { - case codersdk.WorkspaceAppOpenInSlimWindow: - openIn = agentproto.CreateSubAgentRequest_App_SLIM_WINDOW.Enum() - case codersdk.WorkspaceAppOpenInTab: - openIn = agentproto.CreateSubAgentRequest_App_TAB.Enum() - default: - return SubAgent{}, xerrors.Errorf("unexpected codersdk.WorkspaceAppOpenIn: %#v", app.OpenIn) - } - } - - var share *agentproto.CreateSubAgentRequest_App_SharingLevel - if app.Share != nil { - switch *app.Share { - case codersdk.WorkspaceAppSharingLevelAuthenticated: - share = agentproto.CreateSubAgentRequest_App_AUTHENTICATED.Enum() - case codersdk.WorkspaceAppSharingLevelOwner: - share = agentproto.CreateSubAgentRequest_App_OWNER.Enum() - case codersdk.WorkspaceAppSharingLevelPublic: - share = agentproto.CreateSubAgentRequest_App_PUBLIC.Enum() - case codersdk.WorkspaceAppSharingLevelOrganization: - share = agentproto.CreateSubAgentRequest_App_ORGANIZATION.Enum() - default: - return SubAgent{}, xerrors.Errorf("unexpected codersdk.WorkspaceAppSharingLevel: %#v", app.Share) - } + protoApp, err := app.ToProtoApp() + if err != nil { + return SubAgent{}, xerrors.Errorf("convert app: %w", err) } - apps = append(apps, &agentproto.CreateSubAgentRequest_App{ - Slug: app.Slug, - Command: app.Command, - DisplayName: app.DisplayName, - External: app.External, - Group: app.Group, - Healthcheck: healthCheck, - Hidden: app.Hidden, - Icon: app.Icon, - OpenIn: openIn, - Order: app.Order, - Share: share, - Subdomain: app.Subdomain, - Url: app.URL, - }) + apps = append(apps, protoApp) } resp, err := a.api.CreateSubAgent(ctx, &agentproto.CreateSubAgentRequest{ diff --git a/agent/agentcontainers/subagent_test.go b/agent/agentcontainers/subagent_test.go index 3f40598615877..ad3040e12bc13 100644 --- a/agent/agentcontainers/subagent_test.go +++ b/agent/agentcontainers/subagent_test.go @@ -131,22 +131,22 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { apps: []agentcontainers.SubAgentApp{ { Slug: "jupyter", - Command: ptr.Ref("jupyter lab --port=8888"), - DisplayName: ptr.Ref("Jupyter Lab"), - External: ptr.Ref(false), - Group: ptr.Ref("Development"), - HealthCheck: &agentcontainers.SubAgentHealthCheck{ + Command: "jupyter lab --port=8888", + DisplayName: "Jupyter Lab", + External: false, + Group: "Development", + HealthCheck: agentcontainers.SubAgentHealthCheck{ Interval: 30, Threshold: 3, URL: "http://localhost:8888/api", }, - Hidden: ptr.Ref(false), - Icon: ptr.Ref("/icon/jupyter.svg"), - OpenIn: ptr.Ref(codersdk.WorkspaceAppOpenInTab), - Order: ptr.Ref(int32(1)), - Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelAuthenticated), - Subdomain: ptr.Ref(true), - URL: ptr.Ref("http://localhost:8888"), + Hidden: false, + Icon: "/icon/jupyter.svg", + OpenIn: codersdk.WorkspaceAppOpenInTab, + Order: int32(1), + Share: codersdk.WorkspaceAppSharingLevelAuthenticated, + Subdomain: true, + URL: "http://localhost:8888", }, }, expectedApps: []*agentproto.CreateSubAgentRequest_App{ @@ -176,19 +176,19 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { apps: []agentcontainers.SubAgentApp{ { Slug: "owner-app", - Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelOwner), + Share: codersdk.WorkspaceAppSharingLevelOwner, }, { Slug: "authenticated-app", - Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelAuthenticated), + Share: codersdk.WorkspaceAppSharingLevelAuthenticated, }, { Slug: "public-app", - Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelPublic), + Share: codersdk.WorkspaceAppSharingLevelPublic, }, { Slug: "organization-app", - Share: ptr.Ref(codersdk.WorkspaceAppSharingLevelOrganization), + Share: codersdk.WorkspaceAppSharingLevelOrganization, }, }, expectedApps: []*agentproto.CreateSubAgentRequest_App{ @@ -215,7 +215,7 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { apps: []agentcontainers.SubAgentApp{ { Slug: "health-app", - HealthCheck: &agentcontainers.SubAgentHealthCheck{ + HealthCheck: agentcontainers.SubAgentHealthCheck{ Interval: 60, Threshold: 5, URL: "http://localhost:3000/health", @@ -271,12 +271,12 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { assert.Equal(t, expectedApp.Slug, actualApp.Slug) assert.Equal(t, expectedApp.Command, actualApp.Command) assert.Equal(t, expectedApp.DisplayName, actualApp.DisplayName) - assert.Equal(t, expectedApp.External, actualApp.External) + assert.Equal(t, ptr.NilToEmpty(expectedApp.External), ptr.NilToEmpty(actualApp.External)) assert.Equal(t, expectedApp.Group, actualApp.Group) - assert.Equal(t, expectedApp.Hidden, actualApp.Hidden) + assert.Equal(t, ptr.NilToEmpty(expectedApp.Hidden), ptr.NilToEmpty(actualApp.Hidden)) assert.Equal(t, expectedApp.Icon, actualApp.Icon) - assert.Equal(t, expectedApp.Order, actualApp.Order) - assert.Equal(t, expectedApp.Subdomain, actualApp.Subdomain) + assert.Equal(t, ptr.NilToEmpty(expectedApp.Order), ptr.NilToEmpty(actualApp.Order)) + assert.Equal(t, ptr.NilToEmpty(expectedApp.Subdomain), ptr.NilToEmpty(actualApp.Subdomain)) assert.Equal(t, expectedApp.Url, actualApp.Url) if expectedApp.OpenIn != nil { From 336997b5dc0a754209d389400ce4f5487cae65c5 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 18 Jun 2025 11:32:34 +0000 Subject: [PATCH 12/16] chore: handle in CloneConfig as well --- agent/agentcontainers/subagent.go | 1 + 1 file changed, 1 insertion(+) diff --git a/agent/agentcontainers/subagent.go b/agent/agentcontainers/subagent.go index b3fe12a99d9a1..b8e87707b3058 100644 --- a/agent/agentcontainers/subagent.go +++ b/agent/agentcontainers/subagent.go @@ -34,6 +34,7 @@ func (s SubAgent) CloneConfig(dc codersdk.WorkspaceAgentDevcontainer) SubAgent { Architecture: s.Architecture, OperatingSystem: s.OperatingSystem, DisplayApps: slices.Clone(s.DisplayApps), + Apps: slices.Clone(s.Apps), } } From 0ae2616364edee07254c3a05800e72bce9dc772d Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 18 Jun 2025 11:35:33 +0000 Subject: [PATCH 13/16] chore: rename WithUserName and WithWorkspaceName to WithManifestInfo --- agent/agentcontainers/api.go | 19 +++++++------------ agent/agentcontainers/api_test.go | 3 +-- agent/api.go | 3 +-- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 9c88d04c602ff..b1770d024f695 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -64,7 +64,7 @@ type API struct { subAgentURL string subAgentEnv []string - userName string + ownerName string workspaceName string mu sync.RWMutex @@ -156,17 +156,12 @@ func WithSubAgentEnv(env ...string) Option { } } -// WithWorkspaceName sets the workspace name for the sub-agent. -func WithWorkspaceName(name string) Option { +// WithManifestInfo sets the owner name, and workspace name +// for the sub-agent. +func WithManifestInfo(owner, workspace string) Option { return func(api *API) { - api.workspaceName = name - } -} - -// WithUserName sets the user name for the sub-agent. -func WithUserName(name string) Option { - return func(api *API) { - api.userName = name + api.ownerName = owner + api.workspaceName = workspace } } @@ -1149,7 +1144,7 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath, []string{ fmt.Sprintf("CODER_WORKSPACE_AGENT_NAME=%s", dc.Name), - fmt.Sprintf("CODER_WORKSPACE_OWNER_NAME=%s", api.userName), + fmt.Sprintf("CODER_WORKSPACE_OWNER_NAME=%s", api.ownerName), fmt.Sprintf("CODER_WORKSPACE_NAME=%s", api.workspaceName), fmt.Sprintf("CODER_URL=%s", api.subAgentURL), }, diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index c0cd0f2ab2145..3ce7fbb72d170 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -1294,8 +1294,7 @@ func TestAPI(t *testing.T) { agentcontainers.WithSubAgentClient(fakeSAC), agentcontainers.WithSubAgentURL("test-subagent-url"), agentcontainers.WithDevcontainerCLI(fakeDCCLI), - agentcontainers.WithUserName("test-user"), - agentcontainers.WithWorkspaceName("test-workspace"), + agentcontainers.WithManifestInfo("test-user", "test-workspace"), ) apiClose := func() { closeOnce.Do(func() { diff --git a/agent/api.go b/agent/api.go index 15f6617e4a3cd..464c5fc5dab30 100644 --- a/agent/api.go +++ b/agent/api.go @@ -51,8 +51,7 @@ func (a *agent) apiHandler(aAPI proto.DRPCAgentClient26) (http.Handler, func() e manifest := a.manifest.Load() if manifest != nil { containerAPIOpts = append(containerAPIOpts, - agentcontainers.WithUserName(manifest.OwnerName), - agentcontainers.WithWorkspaceName(manifest.WorkspaceName), + agentcontainers.WithManifestInfo(manifest.OwnerName, manifest.WorkspaceName), ) if len(manifest.Devcontainers) > 0 { From e17b2284d05daafe2a9ab571224a54e03c423193 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 18 Jun 2025 11:50:45 +0000 Subject: [PATCH 14/16] chore: just use PATH --- agent/agentcontainers/devcontainercli.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agentcontainers/devcontainercli.go b/agent/agentcontainers/devcontainercli.go index cac210b5b56c2..335be53648c2d 100644 --- a/agent/agentcontainers/devcontainercli.go +++ b/agent/agentcontainers/devcontainercli.go @@ -265,7 +265,7 @@ func (d *devcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, confi } c := d.execer.CommandContext(ctx, "devcontainer", args...) - c.Env = append(c.Env, os.Environ()...) + c.Env = append(c.Env, "PATH="+os.Getenv("PATH")) c.Env = append(c.Env, env...) var stdoutBuf bytes.Buffer From d1da3a1cbd31f47fc006f22c7ae8506132ea532e Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 18 Jun 2025 12:06:04 +0000 Subject: [PATCH 15/16] chore: app deduplication --- agent/agentcontainers/api.go | 23 +++++++++++++++-- agent/agentcontainers/api_test.go | 42 +++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index b1770d024f695..3e42a737463c4 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -1139,7 +1139,7 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c codersdk.DisplayAppPortForward: true, } - var apps []SubAgentApp + var appsWithPossibleDuplicates []SubAgentApp if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath, []string{ @@ -1165,7 +1165,7 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c displayAppsMap[app] = enabled } - apps = append(apps, customization.Apps...) + appsWithPossibleDuplicates = append(appsWithPossibleDuplicates, customization.Apps...) } } @@ -1177,6 +1177,25 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c } slices.Sort(displayApps) + appSlugs := make(map[string]struct{}) + apps := make([]SubAgentApp, 0, len(appsWithPossibleDuplicates)) + + // We want to deduplicate the apps based on their slugs here. + // As we want to prioritize later apps, we will walk through this + // backwards. + for _, app := range slices.Backward(appsWithPossibleDuplicates) { + if _, slugAlreadyExists := appSlugs[app.Slug]; slugAlreadyExists { + continue + } + + appSlugs[app.Slug] = struct{}{} + apps = append(apps, app) + } + + // Apps is currently in reverse order here, so by reversing it we restore + // it to the original order. + slices.Reverse(apps) + subAgentConfig.DisplayApps = displayApps subAgentConfig.Apps = apps } diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 3ce7fbb72d170..5922567716584 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -1688,6 +1688,48 @@ func TestAPI(t *testing.T) { assert.Equal(t, int32(3), subAgent.Apps[2].Order) }, }, + { + name: "AppDeduplication", + customization: []agentcontainers.CoderCustomization{ + { + Apps: []agentcontainers.SubAgentApp{ + { + Slug: "foo-app", + Hidden: true, + Order: 1, + }, + { + Slug: "bar-app", + }, + }, + }, + { + Apps: []agentcontainers.SubAgentApp{ + { + Slug: "foo-app", + Order: 2, + }, + { + Slug: "baz-app", + }, + }, + }, + }, + afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) { + require.Len(t, subAgent.Apps, 3) + + // As the original "foo-app" gets overriden by the later "foo-app", + // we expect "bar-app" to be first in the order. + assert.Equal(t, "bar-app", subAgent.Apps[0].Slug) + assert.Equal(t, "foo-app", subAgent.Apps[1].Slug) + assert.Equal(t, "baz-app", subAgent.Apps[2].Slug) + + // We do not expect the properties from the original "foo-app" to be + // carried over. + assert.Equal(t, false, subAgent.Apps[1].Hidden) + assert.Equal(t, int32(2), subAgent.Apps[1].Order) + }, + }, } for _, tt := range tests { From c6652a15f27872a53d28bee64b20e2eba7e7098e Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 18 Jun 2025 12:46:57 +0000 Subject: [PATCH 16/16] chore: appease linter --- agent/agentcontainers/api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 5922567716584..526c7432c3790 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -1718,7 +1718,7 @@ func TestAPI(t *testing.T) { afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) { require.Len(t, subAgent.Apps, 3) - // As the original "foo-app" gets overriden by the later "foo-app", + // As the original "foo-app" gets overridden by the later "foo-app", // we expect "bar-app" to be first in the order. assert.Equal(t, "bar-app", subAgent.Apps[0].Slug) assert.Equal(t, "foo-app", subAgent.Apps[1].Slug)