From 81dd288f7a5045057c298a459498154179a0f413 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 13 Jun 2025 10:22:39 +0000 Subject: [PATCH 1/3] fix(agent/agentcontainers): treat customizations as array This PR fixes a mistake from the previous PR https://github.com/coder/coder/pull/18342. Merged configuration results in the customization being an array not an object. This PR also moves `displayApps` from being an array to being an object, like the terraform provider has. --- agent/agentcontainers/api.go | 16 +++++++++-- agent/agentcontainers/api_test.go | 28 +++++++++++++------ agent/agentcontainers/devcontainercli.go | 4 +-- agent/agentcontainers/devcontainercli_test.go | 16 ++++++++--- .../read-config-with-coder-customization.log | 2 +- 5 files changed, 47 insertions(+), 19 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index ce252fe2909ab..bb521454176f9 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -1099,14 +1099,24 @@ func (api *API) injectSubAgentIntoContainerLocked(ctx context.Context, dc coders directory = DevcontainerDefaultContainerWorkspaceFolder } - var displayApps []codersdk.DisplayApp + displayAppsMap := make(map[codersdk.DisplayApp]bool) if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath); err != nil { api.logger.Error(ctx, "unable to read devcontainer config", slog.Error(err)) } else { coderCustomization := config.MergedConfiguration.Customizations.Coder - if coderCustomization != nil { - displayApps = coderCustomization.DisplayApps + + for _, customization := range coderCustomization { + for app, enabled := range customization.DisplayApps { + displayAppsMap[app] = enabled + } + } + } + + displayApps := make([]codersdk.DisplayApp, 0, len(displayAppsMap)) + for app, enabled := range displayAppsMap { + if enabled { + displayApps = append(displayApps, app) } } diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index d8e696e151db2..1f185c53a4fea 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -1450,7 +1450,7 @@ func TestAPI(t *testing.T) { tests := []struct { name string - customization *agentcontainers.CoderCustomization + customization []agentcontainers.CoderCustomization afterCreate func(t *testing.T, subAgent agentcontainers.SubAgent) }{ { @@ -1459,18 +1459,28 @@ func TestAPI(t *testing.T) { }, { name: "WithDisplayApps", - customization: &agentcontainers.CoderCustomization{ - DisplayApps: []codersdk.DisplayApp{ - codersdk.DisplayAppSSH, - codersdk.DisplayAppWebTerminal, - codersdk.DisplayAppVSCodeInsiders, + customization: []agentcontainers.CoderCustomization{ + { + DisplayApps: map[codersdk.DisplayApp]bool{ + codersdk.DisplayAppSSH: true, + codersdk.DisplayAppWebTerminal: false, + codersdk.DisplayAppVSCodeDesktop: true, + codersdk.DisplayAppPortForward: true, + }, + }, + { + DisplayApps: map[codersdk.DisplayApp]bool{ + codersdk.DisplayAppSSH: true, + codersdk.DisplayAppWebTerminal: true, + codersdk.DisplayAppPortForward: false, + }, }, }, afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) { require.Len(t, subAgent.DisplayApps, 3) - assert.Equal(t, codersdk.DisplayAppSSH, subAgent.DisplayApps[0]) - assert.Equal(t, codersdk.DisplayAppWebTerminal, subAgent.DisplayApps[1]) - assert.Equal(t, codersdk.DisplayAppVSCodeInsiders, subAgent.DisplayApps[2]) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppSSH) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppWebTerminal) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppVSCodeDesktop) }, }, } diff --git a/agent/agentcontainers/devcontainercli.go b/agent/agentcontainers/devcontainercli.go index 2fad8c6560067..002858c70562e 100644 --- a/agent/agentcontainers/devcontainercli.go +++ b/agent/agentcontainers/devcontainercli.go @@ -27,11 +27,11 @@ type DevcontainerConfiguration struct { } type DevcontainerCustomizations struct { - Coder *CoderCustomization `json:"coder,omitempty"` + Coder []CoderCustomization `json:"coder,omitempty"` } type CoderCustomization struct { - DisplayApps []codersdk.DisplayApp `json:"displayApps,omitempty"` + DisplayApps map[codersdk.DisplayApp]bool `json:"displayApps,omitempty"` } // DevcontainerCLI is an interface for the devcontainer CLI. diff --git a/agent/agentcontainers/devcontainercli_test.go b/agent/agentcontainers/devcontainercli_test.go index dfe390ff7e6df..cffb3e12fd494 100644 --- a/agent/agentcontainers/devcontainercli_test.go +++ b/agent/agentcontainers/devcontainercli_test.go @@ -258,10 +258,18 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) { wantConfig: agentcontainers.DevcontainerConfig{ MergedConfiguration: agentcontainers.DevcontainerConfiguration{ Customizations: agentcontainers.DevcontainerCustomizations{ - Coder: &agentcontainers.CoderCustomization{ - DisplayApps: []codersdk.DisplayApp{ - codersdk.DisplayAppVSCodeDesktop, - codersdk.DisplayAppWebTerminal, + Coder: []agentcontainers.CoderCustomization{ + { + DisplayApps: map[codersdk.DisplayApp]bool{ + codersdk.DisplayAppVSCodeDesktop: true, + codersdk.DisplayAppWebTerminal: true, + }, + }, + { + DisplayApps: map[codersdk.DisplayApp]bool{ + codersdk.DisplayAppVSCodeInsiders: true, + codersdk.DisplayAppWebTerminal: false, + }, }, }, }, diff --git a/agent/agentcontainers/testdata/devcontainercli/readconfig/read-config-with-coder-customization.log b/agent/agentcontainers/testdata/devcontainercli/readconfig/read-config-with-coder-customization.log index fd052c50662e9..d98eb5e056d0c 100644 --- a/agent/agentcontainers/testdata/devcontainercli/readconfig/read-config-with-coder-customization.log +++ b/agent/agentcontainers/testdata/devcontainercli/readconfig/read-config-with-coder-customization.log @@ -5,4 +5,4 @@ {"type":"stop","level":2,"timestamp":1749557820039,"text":"Run: docker ps -q -a --filter label=devcontainer.local_folder=/home/coder/coder --filter label=devcontainer.config_file=/home/coder/coder/.devcontainer/devcontainer.json","startTimestamp":1749557820023} {"type":"start","level":2,"timestamp":1749557820039,"text":"Run: docker ps -q -a --filter label=devcontainer.local_folder=/home/coder/coder"} {"type":"stop","level":2,"timestamp":1749557820054,"text":"Run: docker ps -q -a --filter label=devcontainer.local_folder=/home/coder/coder","startTimestamp":1749557820039} -{"mergedConfiguration":{"customizations":{"coder":{"displayApps":["vscode", "web_terminal"]}}}} +{"mergedConfiguration":{"customizations":{"coder":[{"displayApps":{"vscode":true,"web_terminal":true}},{"displayApps":{"vscode_insiders":true,"web_terminal":false}}]}}} From 2f278fa338b66cdbec2b9ab9dd33b35b28290b52 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 13 Jun 2025 12:57:59 +0000 Subject: [PATCH 2/3] chore: add terraform defaults --- agent/agentcontainers/api.go | 11 +++++- agent/agentcontainers/api_test.go | 61 +++++++++++++++++++++++++------ 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index bb521454176f9..1dddcc709848e 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -1099,7 +1099,16 @@ func (api *API) injectSubAgentIntoContainerLocked(ctx context.Context, dc coders directory = DevcontainerDefaultContainerWorkspaceFolder } - displayAppsMap := make(map[codersdk.DisplayApp]bool) + displayAppsMap := map[codersdk.DisplayApp]bool{ + // NOTE(DanielleMaywood): + // We use the same defaults here as set in terraform-provider-coder. + // https://github.com/coder/terraform-provider-coder/blob/c1c33f6d556532e75662c0ca373ed8fdea220eb5/provider/agent.go#L38-L51 + codersdk.DisplayAppVSCodeDesktop: true, + codersdk.DisplayAppVSCodeInsiders: false, + codersdk.DisplayAppWebTerminal: true, + codersdk.DisplayAppSSH: true, + codersdk.DisplayAppPortForward: true, + } if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath); 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 1f185c53a4fea..454a30fa5f1ce 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -1458,29 +1458,68 @@ func TestAPI(t *testing.T) { customization: nil, }, { - name: "WithDisplayApps", + name: "WithDefaultDisplayApps", + customization: []agentcontainers.CoderCustomization{}, + afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) { + require.Len(t, subAgent.DisplayApps, 4) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppVSCodeDesktop) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppWebTerminal) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppSSH) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppPortForward) + }, + }, + { + name: "WithAllDisplayApps", customization: []agentcontainers.CoderCustomization{ { DisplayApps: map[codersdk.DisplayApp]bool{ - codersdk.DisplayAppSSH: true, - codersdk.DisplayAppWebTerminal: false, - codersdk.DisplayAppVSCodeDesktop: true, - codersdk.DisplayAppPortForward: true, + codersdk.DisplayAppSSH: true, + codersdk.DisplayAppWebTerminal: true, + codersdk.DisplayAppVSCodeDesktop: true, + codersdk.DisplayAppVSCodeInsiders: true, + codersdk.DisplayAppPortForward: true, }, }, + }, + afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) { + require.Len(t, subAgent.DisplayApps, 5) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppSSH) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppWebTerminal) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppVSCodeDesktop) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppVSCodeInsiders) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppPortForward) + }, + }, + { + name: "WithSomeDisplayAppsDisabled", + customization: []agentcontainers.CoderCustomization{ { DisplayApps: map[codersdk.DisplayApp]bool{ - codersdk.DisplayAppSSH: true, - codersdk.DisplayAppWebTerminal: true, + codersdk.DisplayAppSSH: false, + codersdk.DisplayAppWebTerminal: false, + codersdk.DisplayAppVSCodeInsiders: false, + + // We'll enable vscode in this layer, and disable + // it in the next layer to ensure a layer can be + // disabled. + codersdk.DisplayAppVSCodeDesktop: true, + + // We disable port-forward in this layer, and + // then re-enable it in the next layer to ensure + // that behaviour works. codersdk.DisplayAppPortForward: false, }, }, + { + DisplayApps: map[codersdk.DisplayApp]bool{ + codersdk.DisplayAppVSCodeDesktop: false, + codersdk.DisplayAppPortForward: true, + }, + }, }, afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) { - require.Len(t, subAgent.DisplayApps, 3) - assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppSSH) - assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppWebTerminal) - assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppVSCodeDesktop) + require.Len(t, subAgent.DisplayApps, 1) + assert.Contains(t, subAgent.DisplayApps, codersdk.DisplayAppPortForward) }, }, } From 49fc8607611089d8bfce7ce4330f8ce4619c0c3e Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 13 Jun 2025 13:36:08 +0000 Subject: [PATCH 3/3] chore: oops, i used english english --- 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 454a30fa5f1ce..821117685b50e 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -1506,7 +1506,7 @@ func TestAPI(t *testing.T) { // We disable port-forward in this layer, and // then re-enable it in the next layer to ensure - // that behaviour works. + // that behavior works. codersdk.DisplayAppPortForward: false, }, },