Skip to content

Commit b738124

Browse files
DanielleMaywoodDanielleMaywood
committed
refactor: include configuration and merged configuration
We want to be able to pick customizations that are only specified in the top-level customizations stanza. That is, we do not want to accidentally pick something specified in a feature (such as the agent name). This refactor picks the agent name from configuration instead of the merged configuration to ensure a feature cannot influence it.
1 parent d094205 commit b738124

File tree

4 files changed

+47
-36
lines changed

4 files changed

+47
-36
lines changed

‎agent/agentcontainers/api.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,12 +1177,10 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11771177
}
11781178

11791179
// NOTE(DanielleMaywood):
1180-
// We only want to take an agent name specified in the very last customization layer.
1180+
// We only want to take an agent name specified in the root customization layer.
11811181
// This restricts the ability for a feature to specify the agent name. We may revisit
11821182
// this in the future, but for now we want to restrict this behavior.
1183-
if len(coderCustomization) > 0 {
1184-
name := coderCustomization[len(coderCustomization)-1].Name
1185-
1183+
if name := config.Configuration.Customizations.Coder.Name; name != "" {
11861184
// We only want to pick this name if it is a valid name.
11871185
if provisioner.AgentNameRegex.Match([]byte(name)) {
11881186
possibleAgentName = name

‎agent/agentcontainers/api_test.go

Lines changed: 30 additions & 26 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
{
@@ -1741,47 +1742,45 @@ func TestAPI(t *testing.T) {
17411742
},
17421743
{
17431744
name: "Name",
1744-
customization: []agentcontainers.CoderCustomization{
1745+
customization: agentcontainers.CoderCustomization{
1746+
Name: "this-name",
1747+
},
1748+
mergedCustomizations: []agentcontainers.CoderCustomization{
17451749
{
17461750
Name: "not-this-name",
17471751
},
17481752
{
1749-
Name: "custom-name",
1753+
Name: "or-this-name",
17501754
},
17511755
},
17521756
afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) {
1753-
require.Equal(t, "custom-name", subAgent.Name)
1757+
require.Equal(t, "this-name", subAgent.Name)
17541758
},
17551759
},
17561760
{
1757-
name: "NameIsOnlyUsedWhenInLastLayer",
1758-
customization: []agentcontainers.CoderCustomization{
1761+
name: "NameIsOnlyUsedFromRoot",
1762+
mergedCustomizations: []agentcontainers.CoderCustomization{
17591763
{
17601764
Name: "custom-name",
17611765
},
1762-
{},
17631766
},
17641767
afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) {
17651768
require.NotEqual(t, "custom-name", subAgent.Name)
17661769
},
17671770
},
17681771
{
17691772
name: "EmptyNameIsIgnored",
1770-
customization: []agentcontainers.CoderCustomization{
1771-
{
1772-
Name: "",
1773-
},
1773+
customization: agentcontainers.CoderCustomization{
1774+
Name: "",
17741775
},
17751776
afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) {
17761777
require.NotEmpty(t, subAgent.Name)
17771778
},
17781779
},
17791780
{
17801781
name: "InvalidNameIsIgnored",
1781-
customization: []agentcontainers.CoderCustomization{
1782-
{
1783-
Name: "This--Is_An_Invalid--Name",
1784-
},
1782+
customization: agentcontainers.CoderCustomization{
1783+
Name: "This--Is_An_Invalid--Name",
17851784
},
17861785
afterCreate: func(t *testing.T, subAgent agentcontainers.SubAgent) {
17871786
require.NotEqual(t, "This--Is_An_Invalid--Name", subAgent.Name)
@@ -1804,11 +1803,16 @@ func TestAPI(t *testing.T) {
18041803
}
18051804
fDCCLI = &fakeDevcontainerCLI{
18061805
readConfig: agentcontainers.DevcontainerConfig{
1807-
MergedConfiguration: agentcontainers.DevcontainerConfiguration{
1806+
Configuration: agentcontainers.DevcontainerConfiguration{
18081807
Customizations: agentcontainers.DevcontainerCustomizations{
18091808
Coder: tt.customization,
18101809
},
18111810
},
1811+
MergedConfiguration: agentcontainers.DevcontainerMergedConfiguration{
1812+
Customizations: agentcontainers.DevcontainerMergedCustomizations{
1813+
Coder: tt.mergedCustomizations,
1814+
},
1815+
},
18121816
},
18131817
execErrC: make(chan func(cmd string, args ...string) error, 1),
18141818
}

‎agent/agentcontainers/devcontainercli.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,24 @@ 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 {

‎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)