Skip to content

Commit a04268a

Browse files
DanielleMaywoodDanielleMaywood
authored
feat(agent/agentcontainers): support agent name in customization (#18451)
Relates to coder/internal#732 This PR supports specifying a name that will be used for the devcontainer agent in the customizations section of the devcontainer.json configuration file.
1 parent 884ad39 commit a04268a

File tree

4 files changed

+99
-19
lines changed

4 files changed

+99
-19
lines changed

‎agent/agentcontainers/api.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/coder/coder/v2/coderd/httpapi"
2929
"github.com/coder/coder/v2/codersdk"
3030
"github.com/coder/coder/v2/codersdk/agentsdk"
31+
"github.com/coder/coder/v2/provisioner"
3132
"github.com/coder/quartz"
3233
)
3334

@@ -1146,6 +1147,7 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11461147
}
11471148

11481149
var appsWithPossibleDuplicates []SubAgentApp
1150+
var possibleAgentName string
11491151

11501152
if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath,
11511153
[]string{
@@ -1173,6 +1175,19 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11731175

11741176
appsWithPossibleDuplicates = append(appsWithPossibleDuplicates, customization.Apps...)
11751177
}
1178+
1179+
// NOTE(DanielleMaywood):
1180+
// We only want to take an agent name specified in the root customization layer.
1181+
// This restricts the ability for a feature to specify the agent name. We may revisit
1182+
// this in the future, but for now we want to restrict this behavior.
1183+
if name := config.Configuration.Customizations.Coder.Name; name != "" {
1184+
// We only want to pick this name if it is a valid name.
1185+
if provisioner.AgentNameRegex.Match([]byte(name)) {
1186+
possibleAgentName = name
1187+
} else {
1188+
logger.Warn(ctx, "invalid agent name in devcontainer customization, ignoring", slog.F("name", name))
1189+
}
1190+
}
11761191
}
11771192

11781193
displayApps := make([]codersdk.DisplayApp, 0, len(displayAppsMap))
@@ -1204,6 +1219,10 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
12041219

12051220
subAgentConfig.DisplayApps = displayApps
12061221
subAgentConfig.Apps = apps
1222+
1223+
if possibleAgentName != "" {
1224+
subAgentConfig.Name = possibleAgentName
1225+
}
12071226
}
12081227

12091228
deleteSubAgent := proc.agent.ID != uuid.Nil && maybeRecreateSubAgent && !proc.agent.EqualConfig(subAgentConfig)

‎agent/agentcontainers/api_test.go

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1556,17 +1556,18 @@ func TestAPI(t *testing.T) {
15561556
}
15571557

15581558
tests := []struct {
1559-
name string
1560-
customization []agentcontainers.CoderCustomization
1561-
afterCreate func(t *testing.T, subAgent agentcontainers.SubAgent)
1559+
name string
1560+
customization agentcontainers.CoderCustomization
1561+
mergedCustomizations []agentcontainers.CoderCustomization
1562+
afterCreate func(t *testing.T, subAgent agentcontainers.SubAgent)
15621563
}{
15631564
{
1564-
name: "WithoutCustomization",
1565-
customization: nil,
1565+
name: "WithoutCustomization",
1566+
mergedCustomizations: nil,
15661567
},
15671568
{
1568-
name: "WithDefaultDisplayApps",
1569-
customization: []agentcontainers.CoderCustomization{},
1569+
name: "WithDefaultDisplayApps",
1570+
mergedCustomizations: []agentcontainers.CoderCustomization{},
15701571
afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) {
15711572
require.Len(t, subAgent.DisplayApps, 4)
15721573
assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppVSCodeDesktop)
@@ -1577,7 +1578,7 @@ func TestAPI(t *testing.T) {
15771578
},
15781579
{
15791580
name: "WithAllDisplayApps",
1580-
customization: []agentcontainers.CoderCustomization{
1581+
mergedCustomizations: []agentcontainers.CoderCustomization{
15811582
{
15821583
DisplayApps: map[codersdk.DisplayApp]bool{
15831584
codersdk.DisplayAppSSH: true,
@@ -1599,7 +1600,7 @@ func TestAPI(t *testing.T) {
15991600
},
16001601
{
16011602
name: "WithSomeDisplayAppsDisabled",
1602-
customization: []agentcontainers.CoderCustomization{
1603+
mergedCustomizations: []agentcontainers.CoderCustomization{
16031604
{
16041605
DisplayApps: map[codersdk.DisplayApp]bool{
16051606
codersdk.DisplayAppSSH: false,
@@ -1631,7 +1632,7 @@ func TestAPI(t *testing.T) {
16311632
},
16321633
{
16331634
name: "WithApps",
1634-
customization: []agentcontainers.CoderCustomization{
1635+
mergedCustomizations: []agentcontainers.CoderCustomization{
16351636
{
16361637
Apps: []agentcontainers.SubAgentApp{
16371638
{
@@ -1699,7 +1700,7 @@ func TestAPI(t *testing.T) {
16991700
},
17001701
{
17011702
name: "AppDeduplication",
1702-
customization: []agentcontainers.CoderCustomization{
1703+
mergedCustomizations: []agentcontainers.CoderCustomization{
17031704
{
17041705
Apps: []agentcontainers.SubAgentApp{
17051706
{
@@ -1739,6 +1740,52 @@ func TestAPI(t *testing.T) {
17391740
assert.Equal(t, int32(2), subAgent.Apps[1].Order)
17401741
},
17411742
},
1743+
{
1744+
name: "Name",
1745+
customization: agentcontainers.CoderCustomization{
1746+
Name: "this-name",
1747+
},
1748+
mergedCustomizations: []agentcontainers.CoderCustomization{
1749+
{
1750+
Name: "not-this-name",
1751+
},
1752+
{
1753+
Name: "or-this-name",
1754+
},
1755+
},
1756+
afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) {
1757+
require.Equal(t, "this-name", subAgent.Name)
1758+
},
1759+
},
1760+
{
1761+
name: "NameIsOnlyUsedFromRoot",
1762+
mergedCustomizations: []agentcontainers.CoderCustomization{
1763+
{
1764+
Name: "custom-name",
1765+
},
1766+
},
1767+
afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) {
1768+
require.NotEqual(t, "custom-name", subAgent.Name)
1769+
},
1770+
},
1771+
{
1772+
name: "EmptyNameIsIgnored",
1773+
customization: agentcontainers.CoderCustomization{
1774+
Name: "",
1775+
},
1776+
afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) {
1777+
require.NotEmpty(t, subAgent.Name)
1778+
},
1779+
},
1780+
{
1781+
name: "InvalidNameIsIgnored",
1782+
customization: agentcontainers.CoderCustomization{
1783+
Name: "This--Is_An_Invalid--Name",
1784+
},
1785+
afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) {
1786+
require.NotEqual(t, "This--Is_An_Invalid--Name", subAgent.Name)
1787+
},
1788+
},
17421789
}
17431790

17441791
for _, tt := range tests {
@@ -1756,11 +1803,16 @@ func TestAPI(t *testing.T) {
17561803
}
17571804
fDCCLI = &fakeDevcontainerCLI{
17581805
readConfig: agentcontainers.DevcontainerConfig{
1759-
MergedConfiguration: agentcontainers.DevcontainerConfiguration{
1806+
Configuration: agentcontainers.DevcontainerConfiguration{
17601807
Customizations: agentcontainers.DevcontainerCustomizations{
17611808
Coder: tt.customization,
17621809
},
17631810
},
1811+
MergedConfiguration: agentcontainers.DevcontainerMergedConfiguration{
1812+
Customizations: agentcontainers.DevcontainerMergedCustomizations{
1813+
Coder: tt.mergedCustomizations,
1814+
},
1815+
},
17641816
},
17651817
execErrC: make(chan func(cmd string, args ...string) error, 1),
17661818
}
@@ -1825,7 +1877,6 @@ func TestAPI(t *testing.T) {
18251877

18261878
// Then: We expected it to succeed
18271879
require.Len(t, fSAC.created, 1)
1828-
assert.Equal(t, testContainer.FriendlyName, fSAC.created[0].Name)
18291880

18301881
if tt.afterCreate != nil {
18311882
tt.afterCreate(t, fSAC.created[0])

‎agent/agentcontainers/devcontainercli.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,30 @@ import (
2020
// Unfortunately we cannot make use of `dcspec` as the output doesn't appear to
2121
// match.
2222
type DevcontainerConfig struct {
23-
MergedConfiguration DevcontainerConfiguration `json:"mergedConfiguration"`
23+
MergedConfiguration DevcontainerMergedConfiguration `json:"mergedConfiguration"`
24+
Configuration DevcontainerConfiguration `json:"configuration"`
25+
}
26+
27+
type DevcontainerMergedConfiguration struct {
28+
Customizations DevcontainerMergedCustomizations `json:"customizations,omitempty"`
29+
}
30+
31+
type DevcontainerMergedCustomizations struct {
32+
Coder []CoderCustomization `json:"coder,omitempty"`
2433
}
2534

2635
type DevcontainerConfiguration struct {
2736
Customizations DevcontainerCustomizations `json:"customizations,omitempty"`
2837
}
2938

3039
type DevcontainerCustomizations struct {
31-
Coder []CoderCustomization `json:"coder,omitempty"`
40+
Coder CoderCustomization `json:"coder,omitempty"`
3241
}
3342

3443
type CoderCustomization struct {
3544
DisplayApps map[codersdk.DisplayApp]bool `json:"displayApps,omitempty"`
3645
Apps []SubAgentApp `json:"apps,omitempty"`
46+
Name string `json:"name,omitempty"`
3747
}
3848

3949
// DevcontainerCLI is an interface for the devcontainer CLI.

‎agent/agentcontainers/devcontainercli_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,8 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) {
256256
wantArgs: "read-configuration --include-merged-configuration --workspace-folder /test/workspace",
257257
wantError: false,
258258
wantConfig: agentcontainers.DevcontainerConfig{
259-
MergedConfiguration: agentcontainers.DevcontainerConfiguration{
260-
Customizations: agentcontainers.DevcontainerCustomizations{
259+
MergedConfiguration: agentcontainers.DevcontainerMergedConfiguration{
260+
Customizations: agentcontainers.DevcontainerMergedCustomizations{
261261
Coder: []agentcontainers.CoderCustomization{
262262
{
263263
DisplayApps: map[codersdk.DisplayApp]bool{
@@ -284,8 +284,8 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) {
284284
wantArgs: "read-configuration --include-merged-configuration --workspace-folder /test/workspace --config /test/config.json",
285285
wantError: false,
286286
wantConfig: agentcontainers.DevcontainerConfig{
287-
MergedConfiguration: agentcontainers.DevcontainerConfiguration{
288-
Customizations: agentcontainers.DevcontainerCustomizations{
287+
MergedConfiguration: agentcontainers.DevcontainerMergedConfiguration{
288+
Customizations: agentcontainers.DevcontainerMergedCustomizations{
289289
Coder: nil,
290290
},
291291
},

0 commit comments

Comments
 (0)