opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat(agent/agentcontainers): fall back to workspace folder name (#18466)

Browse Source 
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.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Danielle Maywood
2025-06-23 13:05:39 +01:00
committed by GitHub
co-authored by Copilot
parent 0a12ec5fd8
commit c3bc1e75ec
3 changed files with 255 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
agent/agentcontainers/api.go
+40 -4
View File
@@ -8,6 +8,7 @@ import (
"os"
"path"
"path/filepath"
"regexp"
"runtime"
"slices"
"strings"
@@ -39,6 +40,8 @@ const (
// by tmpfs or other mounts. This assumes the container root filesystem is
// read-write, which seems sensible for devcontainers.
coderPathInsideContainer = "/.coder-agent/coder"
maxAgentNameLength = 64
)
// API is responsible for container-related operations in the agent.
@@ -583,10 +586,11 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code
if dc.Container != nil {
if !api.devcontainerNames[dc.Name] {
// If the devcontainer name wasn't set via terraform, we
// use the containers friendly name as a fallback which
// will keep changing as the devcontainer is recreated.
// TODO(mafredri): Parse the container label (i.e. devcontainer.json) for customization.
dc.Name = safeFriendlyName(dc.Container.FriendlyName)
// will attempt to create an agent name based on the workspace
// folder's name. If it is not possible to generate a valid
// agent name based off of the folder name (i.e. no valid characters),
// we will instead fall back to using the container's friendly name.
dc.Name = safeAgentName(path.Base(filepath.ToSlash(dc.WorkspaceFolder)), dc.Container.FriendlyName)
}
}
@@ -631,6 +635,38 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code
api.containersErr = nil
}
var consecutiveHyphenRegex = regexp.MustCompile("-+")
// `safeAgentName` returns a safe agent name derived from a folder name,
// falling back to the container’s friendly name if needed.
func safeAgentName(name string, friendlyName string) string {
// Keep only ASCII letters and digits, replacing everything
// else with a hyphen.
var sb strings.Builder
for _, r := range strings.ToLower(name) {
if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') {
_, _ = sb.WriteRune(r)
} else {
_, _ = sb.WriteRune('-')
}
}
// Remove any consecutive hyphens, and then trim any leading
// and trailing hyphens.
name = consecutiveHyphenRegex.ReplaceAllString(sb.String(), "-")
name = strings.Trim(name, "-")
// Ensure the name of the agent doesn't exceed the maximum agent
// name length.
name = name[:min(len(name), maxAgentNameLength)]
if provisioner.AgentNameRegex.Match([]byte(name)) {
return name
}
return safeFriendlyName(friendlyName)
}
// safeFriendlyName returns a API safe version of the container's
// friendly name.
//
agent/agentcontainers/api_internal_test.go
+201
View File
@@ -0,0 +1,201 @@
package agentcontainers
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/coder/coder/v2/provisioner"
)
func TestSafeAgentName(t *testing.T) {
t.Parallel()
tests := []struct {
name string
folderName string
expected string
}{
// Basic valid names
{
folderName: "simple",
expected: "simple",
},
{
folderName: "with-hyphens",
expected: "with-hyphens",
},
{
folderName: "123numbers",
expected: "123numbers",
},
{
folderName: "mixed123",
expected: "mixed123",
},
// Names that need transformation
{
folderName: "With_Underscores",
expected: "with-underscores",
},
{
folderName: "With Spaces",
expected: "with-spaces",
},
{
folderName: "UPPERCASE",
expected: "uppercase",
},
{
folderName: "Mixed_Case-Name",
expected: "mixed-case-name",
},
// Names with special characters that get replaced
{
folderName: "special@#$chars",
expected: "special-chars",
},
{
folderName: "dots.and.more",
expected: "dots-and-more",
},
{
folderName: "multiple___underscores",
expected: "multiple-underscores",
},
{
folderName: "multiple---hyphens",
expected: "multiple-hyphens",
},
// Edge cases with leading/trailing special chars
{
folderName: "-leading-hyphen",
expected: "leading-hyphen",
},
{
folderName: "trailing-hyphen-",
expected: "trailing-hyphen",
},
{
folderName: "_leading_underscore",
expected: "leading-underscore",
},
{
folderName: "trailing_underscore_",
expected: "trailing-underscore",
},
{
folderName: "---multiple-leading",
expected: "multiple-leading",
},
{
folderName: "trailing-multiple---",
expected: "trailing-multiple",
},
// Complex transformation cases
{
folderName: "___very---complex@@@name___",
expected: "very-complex-name",
},
{
folderName: "my.project-folder_v2",
expected: "my-project-folder-v2",
},
// Empty and fallback cases - now correctly uses friendlyName fallback
{
folderName: "",
expected: "friendly-fallback",
},
{
folderName: "---",
expected: "friendly-fallback",
},
{
folderName: "___",
expected: "friendly-fallback",
},
{
folderName: "@#$",
expected: "friendly-fallback",
},
// Additional edge cases
{
folderName: "a",
expected: "a",
},
{
folderName: "1",
expected: "1",
},
{
folderName: "a1b2c3",
expected: "a1b2c3",
},
{
folderName: "CamelCase",
expected: "camelcase",
},
{
folderName: "snake_case_name",
expected: "snake-case-name",
},
{
folderName: "kebab-case-name",
expected: "kebab-case-name",
},
{
folderName: "mix3d_C4s3-N4m3",
expected: "mix3d-c4s3-n4m3",
},
{
folderName: "123-456-789",
expected: "123-456-789",
},
{
folderName: "abc123def456",
expected: "abc123def456",
},
{
folderName: " spaces everywhere ",
expected: "spaces-everywhere",
},
{
folderName: "unicode-café-naïve",
expected: "unicode-caf-na-ve",
},
{
folderName: "path/with/slashes",
expected: "path-with-slashes",
},
{
folderName: "file.tar.gz",
expected: "file-tar-gz",
},
{
folderName: "version-1.2.3-alpha",
expected: "version-1-2-3-alpha",
},
// Truncation test for names exceeding 64 characters
{
folderName: "this-is-a-very-long-folder-name-that-exceeds-sixty-four-characters-limit-and-should-be-truncated",
expected: "this-is-a-very-long-folder-name-that-exceeds-sixty-four-characte",
},
}
for _, tt := range tests {
t.Run(tt.folderName, func(t *testing.T) {
t.Parallel()
name := safeAgentName(tt.folderName, "friendly-fallback")
assert.Equal(t, tt.expected, name)
assert.True(t, provisioner.AgentNameRegex.Match([]byte(name)))
})
}
}
agent/agentcontainers/api_test.go
+14 -14
View File
@@ -897,8 +897,8 @@ func TestAPI(t *testing.T) {
FriendlyName: "project1-container",
Running: true,
Labels: map[string]string{
agentcontainers.DevcontainerLocalFolderLabel: "/workspace/project",
agentcontainers.DevcontainerConfigFileLabel: "/workspace/project/.devcontainer/devcontainer.json",
agentcontainers.DevcontainerLocalFolderLabel: "/workspace/project1",
agentcontainers.DevcontainerConfigFileLabel: "/workspace/project1/.devcontainer/devcontainer.json",
},
},
{
@@ -906,8 +906,8 @@ func TestAPI(t *testing.T) {
FriendlyName: "project2-container",
Running: true,
Labels: map[string]string{
agentcontainers.DevcontainerLocalFolderLabel: "/home/user/project",
agentcontainers.DevcontainerConfigFileLabel: "/home/user/project/.devcontainer/devcontainer.json",
agentcontainers.DevcontainerLocalFolderLabel: "/home/user/project2",
agentcontainers.DevcontainerConfigFileLabel: "/home/user/project2/.devcontainer/devcontainer.json",
},
},
{
@@ -915,8 +915,8 @@ func TestAPI(t *testing.T) {
FriendlyName: "project3-container",
Running: true,
Labels: map[string]string{
agentcontainers.DevcontainerLocalFolderLabel: "/var/lib/project",
agentcontainers.DevcontainerConfigFileLabel: "/var/lib/project/.devcontainer/devcontainer.json",
agentcontainers.DevcontainerLocalFolderLabel: "/var/lib/project3",
agentcontainers.DevcontainerConfigFileLabel: "/var/lib/project3/.devcontainer/devcontainer.json",
},
},
},
@@ -1326,7 +1326,7 @@ func TestAPI(t *testing.T) {
// Allow initial agent creation and injection to succeed.
testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil)
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_AGENT_NAME=coder")
assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace")
assert.Contains(t, envs, "CODER_WORKSPACE_OWNER_NAME=test-user")
assert.Contains(t, envs, "CODER_URL=test-subagent-url")
@@ -1349,7 +1349,7 @@ func TestAPI(t *testing.T) {
// Verify agent was created.
require.Len(t, fakeSAC.created, 1)
assert.Equal(t, "test-container", fakeSAC.created[0].Name)
assert.Equal(t, "coder", fakeSAC.created[0].Name)
assert.Equal(t, "/workspaces/coder", fakeSAC.created[0].Directory)
assert.Len(t, fakeSAC.deleted, 0)
@@ -1405,7 +1405,7 @@ func TestAPI(t *testing.T) {
WaitStartLoop:
for {
// Agent reinjection will succeed and we will not re-create the
// agent, nor re-probe pwd.
// agent.
mCCLI.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{testContainer},
}, nil).Times(1) // 1 update.
@@ -1468,7 +1468,7 @@ func TestAPI(t *testing.T) {
// Expect the agent to be recreated.
testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil)
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_AGENT_NAME=coder")
assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace")
assert.Contains(t, envs, "CODER_WORKSPACE_OWNER_NAME=test-user")
assert.Contains(t, envs, "CODER_URL=test-subagent-url")
@@ -1910,8 +1910,8 @@ func TestAPI(t *testing.T) {
Running: true,
CreatedAt: time.Now(),
Labels: map[string]string{
agentcontainers.DevcontainerLocalFolderLabel: "/workspaces",
agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json",
agentcontainers.DevcontainerLocalFolderLabel: "/workspaces/coder",
agentcontainers.DevcontainerConfigFileLabel: "/workspaces/coder/.devcontainer/devcontainer.json",
},
}
)
@@ -1953,13 +1953,13 @@ func TestAPI(t *testing.T) {
testutil.RequireSend(ctx, t, fSAC.createErrC, nil)
testutil.RequireSend(ctx, t, fDCCLI.readConfigErrC, func(env []string) error {
// We expect the wrong workspace agent name passed in first.
assert.Contains(t, env, "CODER_WORKSPACE_AGENT_NAME=test-container")
assert.Contains(t, env, "CODER_WORKSPACE_AGENT_NAME=coder")
return nil
})
testutil.RequireSend(ctx, t, fDCCLI.readConfigErrC, func(env []string) error {
// We then expect the agent name passed here to have been read from the config.
assert.Contains(t, env, "CODER_WORKSPACE_AGENT_NAME=custom-name")
assert.NotContains(t, env, "CODER_WORKSPACE_AGENT_NAME=test-container")
assert.NotContains(t, env, "CODER_WORKSPACE_AGENT_NAME=coder")
return nil
})