Skip to content

Commit 0660cba

Browse files
DanielleMaywoodDanielleMaywood
committed
feat(agent/agentcontainers): fall back to workspace folder name
This PR changes the logic for how we decide on an agent name. Previously it followed these steps: 1. Use a name from `customizations.coder.name` 2. Use a name from the terraform resource `coder_devcontainer` 3. Use the dev container's friendly name With this change it now does: 1. Use a name from `customizations.coder.name` 2. Use a name from the terraform resource `coder_devcontainer` 3. Use a name from the workspace folder 4. Use the dev container's friendly name We now attempt to construct a valid agent name from the workspace folder. Should we fail to construct a valid agent name from the workspace folder, we will fall back to the dev container's friendly name.
1 parent 32239b2 commit 0660cba

File tree

4 files changed

+69
-71
lines changed

4 files changed

+69
-71
lines changed

‎agent/agentcontainers/api.go

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,21 @@
11
package agentcontainers
22

33
import (
4-
"bytes"
54
"context"
65
"errors"
76
"fmt"
8-
"io"
97
"net/http"
108
"os"
119
"path"
1210
"path/filepath"
11+
"regexp"
1312
"runtime"
1413
"slices"
1514
"strings"
1615
"sync"
1716
"sync/atomic"
1817
"time"
18+
"unicode"
1919

2020
"github.com/fsnotify/fsnotify"
2121
"github.com/go-chi/chi/v5"
@@ -585,10 +585,10 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code
585585
if dc.Container != nil {
586586
if !api.devcontainerNames[dc.Name] {
587587
// If the devcontainer name wasn't set via terraform, we
588-
// use the containers friendly name as a fallback which
589-
// will keep changing as the devcontainer is recreated.
590-
// TODO(mafredri): Parse the container label (i.e. devcontainer.json) for customization.
591-
dc.Name = safeFriendlyName(dc.Container.FriendlyName)
588+
// will attempt to create an agent name based on the workspace
589+
// folder's name. If that is not possible, we will fall back
590+
// to using the container's friendly name.
591+
dc.Name = safeAgentName(filepath.Base(dc.WorkspaceFolder), dc.Container.FriendlyName)
592592
}
593593
}
594594

@@ -633,6 +633,39 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code
633633
api.containersErr = nil
634634
}
635635

636+
var consecutiveHyphenRegex = regexp.MustCompile("-+")
637+
638+
// safeAgentName returns an agent name safe version
639+
// of a folder name, falling back to a safe version
640+
// of the container name if unable to create a name.
641+
func safeAgentName(name string, friendlyName string) string {
642+
// Keep only letters and digits, replacing everything
643+
// else with a hyphen.
644+
var sb strings.Builder
645+
for _, r := range strings.ToLower(name) {
646+
if unicode.IsLetter(r) || unicode.IsDigit(r) {
647+
sb.WriteRune(r)
648+
} else {
649+
sb.WriteRune('-')
650+
}
651+
}
652+
653+
// Remove any consecutive hyphens, and then trim and leading
654+
// and trailing hyphens.
655+
name = consecutiveHyphenRegex.ReplaceAllString(sb.String(), "-")
656+
name = strings.Trim(name, "-")
657+
658+
name = strings.ToLower(name)
659+
name = strings.ReplaceAll(name, " ", "-")
660+
name = strings.ReplaceAll(name, "_", "-")
661+
662+
if name == "" {
663+
return safeFriendlyName(name)
664+
}
665+
666+
return name
667+
}
668+
636669
// safeFriendlyName returns a API safe version of the container's
637670
// friendly name.
638671
//
@@ -1114,27 +1147,6 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11141147
if proc.agent.ID == uuid.Nil || maybeRecreateSubAgent {
11151148
subAgentConfig.Architecture = arch
11161149

1117-
// Detect workspace folder by executing `pwd` in the container.
1118-
// NOTE(mafredri): This is a quick and dirty way to detect the
1119-
// workspace folder inside the container. In the future we will
1120-
// rely more on `devcontainer read-configuration`.
1121-
var pwdBuf bytes.Buffer
1122-
err = api.dccli.Exec(ctx, dc.WorkspaceFolder, dc.ConfigPath, "pwd", []string{},
1123-
WithExecOutput(&pwdBuf, io.Discard),
1124-
WithExecContainerID(container.ID),
1125-
)
1126-
if err != nil {
1127-
return xerrors.Errorf("check workspace folder in container: %w", err)
1128-
}
1129-
directory := strings.TrimSpace(pwdBuf.String())
1130-
if directory == "" {
1131-
logger.Warn(ctx, "detected workspace folder is empty, using default workspace folder",
1132-
slog.F("default_workspace_folder", DevcontainerDefaultContainerWorkspaceFolder),
1133-
)
1134-
directory = DevcontainerDefaultContainerWorkspaceFolder
1135-
}
1136-
subAgentConfig.Directory = directory
1137-
11381150
displayAppsMap := map[codersdk.DisplayApp]bool{
11391151
// NOTE(DanielleMaywood):
11401152
// We use the same defaults here as set in terraform-provider-coder.
@@ -1207,6 +1219,8 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
12071219
appsWithPossibleDuplicates = append(appsWithPossibleDuplicates, customization.Apps...)
12081220
}
12091221

1222+
subAgentConfig.Directory = config.Workspace.WorkspaceFolder
1223+
12101224
return nil
12111225
}(); err != nil {
12121226
api.logger.Error(ctx, "unable to read devcontainer config", slog.Error(err))

‎agent/agentcontainers/api_test.go

Lines changed: 23 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -897,26 +897,26 @@ func TestAPI(t *testing.T) {
897897
FriendlyName: "project1-container",
898898
Running: true,
899899
Labels: map[string]string{
900-
agentcontainers.DevcontainerLocalFolderLabel: "/workspace/project",
901-
agentcontainers.DevcontainerConfigFileLabel: "/workspace/project/.devcontainer/devcontainer.json",
900+
agentcontainers.DevcontainerLocalFolderLabel: "/workspace/project1",
901+
agentcontainers.DevcontainerConfigFileLabel: "/workspace/project1/.devcontainer/devcontainer.json",
902902
},
903903
},
904904
{
905905
ID: "project2-container",
906906
FriendlyName: "project2-container",
907907
Running: true,
908908
Labels: map[string]string{
909-
agentcontainers.DevcontainerLocalFolderLabel: "/home/user/project",
910-
agentcontainers.DevcontainerConfigFileLabel: "/home/user/project/.devcontainer/devcontainer.json",
909+
agentcontainers.DevcontainerLocalFolderLabel: "/home/user/project2",
910+
agentcontainers.DevcontainerConfigFileLabel: "/home/user/project2/.devcontainer/devcontainer.json",
911911
},
912912
},
913913
{
914914
ID: "project3-container",
915915
FriendlyName: "project3-container",
916916
Running: true,
917917
Labels: map[string]string{
918-
agentcontainers.DevcontainerLocalFolderLabel: "/var/lib/project",
919-
agentcontainers.DevcontainerConfigFileLabel: "/var/lib/project/.devcontainer/devcontainer.json",
918+
agentcontainers.DevcontainerLocalFolderLabel: "/var/lib/project3",
919+
agentcontainers.DevcontainerConfigFileLabel: "/var/lib/project3/.devcontainer/devcontainer.json",
920920
},
921921
},
922922
},
@@ -1262,7 +1262,12 @@ func TestAPI(t *testing.T) {
12621262
deleteErrC: make(chan error, 1),
12631263
}
12641264
fakeDCCLI = &fakeDevcontainerCLI{
1265-
execErrC: make(chan func(cmd string, args ...string) error, 1),
1265+
execErrC: make(chan func(cmd string, args ...string) error, 1),
1266+
readConfig: agentcontainers.DevcontainerConfig{
1267+
Workspace: agentcontainers.DevcontainerWorkspace{
1268+
WorkspaceFolder: "/workspaces/coder",
1269+
},
1270+
},
12661271
readConfigErrC: make(chan func(envs []string) error, 1),
12671272
}
12681273

@@ -1273,8 +1278,8 @@ func TestAPI(t *testing.T) {
12731278
Running: true,
12741279
CreatedAt: time.Now(),
12751280
Labels: map[string]string{
1276-
agentcontainers.DevcontainerLocalFolderLabel: "/workspaces",
1277-
agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json",
1281+
agentcontainers.DevcontainerLocalFolderLabel: "/workspaces/coder",
1282+
agentcontainers.DevcontainerConfigFileLabel: "/workspaces/coder/.devcontainer/devcontainer.json",
12781283
},
12791284
}
12801285
)
@@ -1320,13 +1325,8 @@ func TestAPI(t *testing.T) {
13201325

13211326
// Allow initial agent creation and injection to succeed.
13221327
testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil)
1323-
testutil.RequireSend(ctx, t, fakeDCCLI.execErrC, func(cmd string, args ...string) error {
1324-
assert.Equal(t, "pwd", cmd)
1325-
assert.Empty(t, args)
1326-
return nil
1327-
}) // Exec pwd.
13281328
testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) error {
1329-
assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container")
1329+
assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=coder")
13301330
assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace")
13311331
assert.Contains(t, envs, "CODER_WORKSPACE_OWNER_NAME=test-user")
13321332
assert.Contains(t, envs, "CODER_URL=test-subagent-url")
@@ -1349,8 +1349,8 @@ func TestAPI(t *testing.T) {
13491349

13501350
// Verify agent was created.
13511351
require.Len(t, fakeSAC.created, 1)
1352-
assert.Equal(t, "test-container", fakeSAC.created[0].Name)
1353-
assert.Equal(t, "/workspaces", fakeSAC.created[0].Directory)
1352+
assert.Equal(t, "coder", fakeSAC.created[0].Name)
1353+
assert.Equal(t, "/workspaces/coder", fakeSAC.created[0].Directory)
13541354
assert.Len(t, fakeSAC.deleted, 0)
13551355

13561356
t.Log("Agent injected successfully, now testing reinjection into the same container...")
@@ -1405,7 +1405,7 @@ func TestAPI(t *testing.T) {
14051405
WaitStartLoop:
14061406
for {
14071407
// Agent reinjection will succeed and we will not re-create the
1408-
// agent, nor re-probe pwd.
1408+
// agent.
14091409
mCCLI.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{
14101410
Containers: []codersdk.WorkspaceAgentContainer{testContainer},
14111411
}, nil).Times(1) // 1 update.
@@ -1467,13 +1467,8 @@ func TestAPI(t *testing.T) {
14671467
testutil.RequireSend(ctx, t, fakeSAC.deleteErrC, nil)
14681468
// Expect the agent to be recreated.
14691469
testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil)
1470-
testutil.RequireSend(ctx, t, fakeDCCLI.execErrC, func(cmd string, args ...string) error {
1471-
assert.Equal(t, "pwd", cmd)
1472-
assert.Empty(t, args)
1473-
return nil
1474-
}) // Exec pwd.
14751470
testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) error {
1476-
assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container")
1471+
assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=coder")
14771472
assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace")
14781473
assert.Contains(t, envs, "CODER_WORKSPACE_OWNER_NAME=test-user")
14791474
assert.Contains(t, envs, "CODER_URL=test-subagent-url")
@@ -1814,7 +1809,6 @@ func TestAPI(t *testing.T) {
18141809
},
18151810
},
18161811
},
1817-
execErrC: make(chan func(cmd string, args ...string) error, 1),
18181812
}
18191813

18201814
testContainer = codersdk.WorkspaceAgentContainer{
@@ -1861,15 +1855,9 @@ func TestAPI(t *testing.T) {
18611855

18621856
// Close before api.Close() defer to avoid deadlock after test.
18631857
defer close(fSAC.createErrC)
1864-
defer close(fDCCLI.execErrC)
18651858

18661859
// Given: We allow agent creation and injection to succeed.
18671860
testutil.RequireSend(ctx, t, fSAC.createErrC, nil)
1868-
testutil.RequireSend(ctx, t, fDCCLI.execErrC, func(cmd string, args ...string) error {
1869-
assert.Equal(t, "pwd", cmd)
1870-
assert.Empty(t, args)
1871-
return nil
1872-
})
18731861

18741862
// Wait until the ticker has been registered.
18751863
tickerTrap.MustWait(ctx).MustRelease(ctx)
@@ -1913,7 +1901,6 @@ func TestAPI(t *testing.T) {
19131901
},
19141902
},
19151903
readConfigErrC: make(chan func(envs []string) error, 2),
1916-
execErrC: make(chan func(cmd string, args ...string) error, 1),
19171904
}
19181905

19191906
testContainer = codersdk.WorkspaceAgentContainer{
@@ -1923,8 +1910,8 @@ func TestAPI(t *testing.T) {
19231910
Running: true,
19241911
CreatedAt: time.Now(),
19251912
Labels: map[string]string{
1926-
agentcontainers.DevcontainerLocalFolderLabel: "/workspaces",
1927-
agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json",
1913+
agentcontainers.DevcontainerLocalFolderLabel: "/workspaces/coder",
1914+
agentcontainers.DevcontainerConfigFileLabel: "/workspaces/coder/.devcontainer/devcontainer.json",
19281915
},
19291916
}
19301917
)
@@ -1960,25 +1947,19 @@ func TestAPI(t *testing.T) {
19601947

19611948
// Close before api.Close() defer to avoid deadlock after test.
19621949
defer close(fSAC.createErrC)
1963-
defer close(fDCCLI.execErrC)
19641950
defer close(fDCCLI.readConfigErrC)
19651951

19661952
// Given: We allow agent creation and injection to succeed.
19671953
testutil.RequireSend(ctx, t, fSAC.createErrC, nil)
1968-
testutil.RequireSend(ctx, t, fDCCLI.execErrC, func(cmd string, args ...string) error {
1969-
assert.Equal(t, "pwd", cmd)
1970-
assert.Empty(t, args)
1971-
return nil
1972-
})
19731954
testutil.RequireSend(ctx, t, fDCCLI.readConfigErrC, func(env []string) error {
19741955
// We expect the wrong workspace agent name passed in first.
1975-
assert.Contains(t, env, "CODER_WORKSPACE_AGENT_NAME=test-container")
1956+
assert.Contains(t, env, "CODER_WORKSPACE_AGENT_NAME=coder")
19761957
return nil
19771958
})
19781959
testutil.RequireSend(ctx, t, fDCCLI.readConfigErrC, func(env []string) error {
19791960
// We then expect the agent name passed here to have been read from the config.
19801961
assert.Contains(t, env, "CODER_WORKSPACE_AGENT_NAME=custom-name")
1981-
assert.NotContains(t, env, "CODER_WORKSPACE_AGENT_NAME=test-container")
1962+
assert.NotContains(t, env, "CODER_WORKSPACE_AGENT_NAME=coder")
19821963
return nil
19831964
})
19841965

‎agent/agentcontainers/devcontainer.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ const (
1818
// DevcontainerConfigFileLabel is the label that contains the path to
1919
// the devcontainer.json configuration file.
2020
DevcontainerConfigFileLabel = "devcontainer.config_file"
21-
// The default workspace folder inside the devcontainer.
22-
DevcontainerDefaultContainerWorkspaceFolder = "/workspaces"
2321
)
2422

2523
const devcontainerUpScriptTemplate = `

‎agent/agentcontainers/devcontainercli.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ import (
2222
type DevcontainerConfig struct {
2323
MergedConfiguration DevcontainerMergedConfiguration `json:"mergedConfiguration"`
2424
Configuration DevcontainerConfiguration `json:"configuration"`
25+
Workspace DevcontainerWorkspace `json:"workspace"`
26+
}
27+
28+
type DevcontainerWorkspace struct {
29+
WorkspaceFolder string `json:"workspaceFolder"`
2530
}
2631

2732
type DevcontainerMergedConfiguration struct {

0 commit comments

Comments
 (0)