Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitb49e62f

Browse files
fix(agent/agentcontainers): ensure agent name env var is correct (#18457)
Previously, `CODER_WORKSPACE_AGENT_NAME` would always be passed as thedev container name.This is invalid for the following scenarios:- The dev container is specified in terraform- The dev container has a name customizationThis change now runs `ReadConfig` twice. The first read is to extract aname (if present), from the `devcontainer.json`. The second read willthen use the name we have stored for the dev container (so this could beeither the customization, terraform resource name, or container name).
1 parent9e7b7f2 commitb49e62f

File tree

2 files changed

+151
-28
lines changed

2 files changed

+151
-28
lines changed

‎agent/agentcontainers/api.go

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,18 +1147,49 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11471147
}
11481148

11491149
varappsWithPossibleDuplicates []SubAgentApp
1150-
varpossibleAgentNamestring
1151-
1152-
ifconfig,err:=api.dccli.ReadConfig(ctx,dc.WorkspaceFolder,dc.ConfigPath,
1153-
[]string{
1154-
fmt.Sprintf("CODER_WORKSPACE_AGENT_NAME=%s",dc.Name),
1155-
fmt.Sprintf("CODER_WORKSPACE_OWNER_NAME=%s",api.ownerName),
1156-
fmt.Sprintf("CODER_WORKSPACE_NAME=%s",api.workspaceName),
1157-
fmt.Sprintf("CODER_URL=%s",api.subAgentURL),
1158-
},
1159-
);err!=nil {
1160-
api.logger.Error(ctx,"unable to read devcontainer config",slog.Error(err))
1161-
}else {
1150+
1151+
iferr:=func()error {
1152+
var (
1153+
configDevcontainerConfig
1154+
configOutdatedbool
1155+
)
1156+
1157+
readConfig:=func() (DevcontainerConfig,error) {
1158+
returnapi.dccli.ReadConfig(ctx,dc.WorkspaceFolder,dc.ConfigPath, []string{
1159+
fmt.Sprintf("CODER_WORKSPACE_AGENT_NAME=%s",subAgentConfig.Name),
1160+
fmt.Sprintf("CODER_WORKSPACE_OWNER_NAME=%s",api.ownerName),
1161+
fmt.Sprintf("CODER_WORKSPACE_NAME=%s",api.workspaceName),
1162+
fmt.Sprintf("CODER_URL=%s",api.subAgentURL),
1163+
})
1164+
}
1165+
1166+
ifconfig,err=readConfig();err!=nil {
1167+
returnerr
1168+
}
1169+
1170+
// NOTE(DanielleMaywood):
1171+
// We only want to take an agent name specified in the root customization layer.
1172+
// This restricts the ability for a feature to specify the agent name. We may revisit
1173+
// this in the future, but for now we want to restrict this behavior.
1174+
ifname:=config.Configuration.Customizations.Coder.Name;name!="" {
1175+
// We only want to pick this name if it is a valid name.
1176+
ifprovisioner.AgentNameRegex.Match([]byte(name)) {
1177+
subAgentConfig.Name=name
1178+
configOutdated=true
1179+
}else {
1180+
logger.Warn(ctx,"invalid name in devcontainer customization, ignoring",
1181+
slog.F("name",name),
1182+
slog.F("regex",provisioner.AgentNameRegex.String()),
1183+
)
1184+
}
1185+
}
1186+
1187+
ifconfigOutdated {
1188+
ifconfig,err=readConfig();err!=nil {
1189+
returnerr
1190+
}
1191+
}
1192+
11621193
coderCustomization:=config.MergedConfiguration.Customizations.Coder
11631194

11641195
for_,customization:=rangecoderCustomization {
@@ -1176,18 +1207,9 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11761207
appsWithPossibleDuplicates=append(appsWithPossibleDuplicates,customization.Apps...)
11771208
}
11781209

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-
ifname:=config.Configuration.Customizations.Coder.Name;name!="" {
1184-
// We only want to pick this name if it is a valid name.
1185-
ifprovisioner.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-
}
1210+
returnnil
1211+
}();err!=nil {
1212+
api.logger.Error(ctx,"unable to read devcontainer config",slog.Error(err))
11911213
}
11921214

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

12201242
subAgentConfig.DisplayApps=displayApps
12211243
subAgentConfig.Apps=apps
1222-
1223-
ifpossibleAgentName!="" {
1224-
subAgentConfig.Name=possibleAgentName
1225-
}
12261244
}
12271245

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

‎agent/agentcontainers/api_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1884,6 +1884,111 @@ func TestAPI(t *testing.T) {
18841884
})
18851885
}
18861886
})
1887+
1888+
t.Run("CreateReadsConfigTwice",func(t*testing.T) {
1889+
t.Parallel()
1890+
1891+
ifruntime.GOOS=="windows" {
1892+
t.Skip("Dev Container tests are not supported on Windows (this test uses mocks but fails due to Windows paths)")
1893+
}
1894+
1895+
var (
1896+
ctx=testutil.Context(t,testutil.WaitMedium)
1897+
logger=testutil.Logger(t)
1898+
mClock=quartz.NewMock(t)
1899+
mCCLI=acmock.NewMockContainerCLI(gomock.NewController(t))
1900+
fSAC=&fakeSubAgentClient{
1901+
logger:logger.Named("fakeSubAgentClient"),
1902+
createErrC:make(chanerror,1),
1903+
}
1904+
fDCCLI=&fakeDevcontainerCLI{
1905+
readConfig: agentcontainers.DevcontainerConfig{
1906+
Configuration: agentcontainers.DevcontainerConfiguration{
1907+
Customizations: agentcontainers.DevcontainerCustomizations{
1908+
Coder: agentcontainers.CoderCustomization{
1909+
// We want to specify a custom name for this agent.
1910+
Name:"custom-name",
1911+
},
1912+
},
1913+
},
1914+
},
1915+
readConfigErrC:make(chanfunc(envs []string)error,2),
1916+
execErrC:make(chanfunc(cmdstring,args...string)error,1),
1917+
}
1918+
1919+
testContainer= codersdk.WorkspaceAgentContainer{
1920+
ID:"test-container-id",
1921+
FriendlyName:"test-container",
1922+
Image:"test-image",
1923+
Running:true,
1924+
CreatedAt:time.Now(),
1925+
Labels:map[string]string{
1926+
agentcontainers.DevcontainerLocalFolderLabel:"/workspaces",
1927+
agentcontainers.DevcontainerConfigFileLabel:"/workspace/.devcontainer/devcontainer.json",
1928+
},
1929+
}
1930+
)
1931+
1932+
coderBin,err:=os.Executable()
1933+
require.NoError(t,err)
1934+
1935+
// Mock the `List` function to always return out test container.
1936+
mCCLI.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{
1937+
Containers: []codersdk.WorkspaceAgentContainer{testContainer},
1938+
},nil).AnyTimes()
1939+
1940+
// Mock the steps used for injecting the coder agent.
1941+
gomock.InOrder(
1942+
mCCLI.EXPECT().DetectArchitecture(gomock.Any(),testContainer.ID).Return(runtime.GOARCH,nil),
1943+
mCCLI.EXPECT().ExecAs(gomock.Any(),testContainer.ID,"root","mkdir","-p","/.coder-agent").Return(nil,nil),
1944+
mCCLI.EXPECT().Copy(gomock.Any(),testContainer.ID,coderBin,"/.coder-agent/coder").Return(nil),
1945+
mCCLI.EXPECT().ExecAs(gomock.Any(),testContainer.ID,"root","chmod","0755","/.coder-agent","/.coder-agent/coder").Return(nil,nil),
1946+
)
1947+
1948+
mClock.Set(time.Now()).MustWait(ctx)
1949+
tickerTrap:=mClock.Trap().TickerFunc("updaterLoop")
1950+
1951+
api:=agentcontainers.NewAPI(logger,
1952+
agentcontainers.WithClock(mClock),
1953+
agentcontainers.WithContainerCLI(mCCLI),
1954+
agentcontainers.WithDevcontainerCLI(fDCCLI),
1955+
agentcontainers.WithSubAgentClient(fSAC),
1956+
agentcontainers.WithSubAgentURL("test-subagent-url"),
1957+
agentcontainers.WithWatcher(watcher.NewNoop()),
1958+
)
1959+
deferapi.Close()
1960+
1961+
// Close before api.Close() defer to avoid deadlock after test.
1962+
deferclose(fSAC.createErrC)
1963+
deferclose(fDCCLI.execErrC)
1964+
deferclose(fDCCLI.readConfigErrC)
1965+
1966+
// Given: We allow agent creation and injection to succeed.
1967+
testutil.RequireSend(ctx,t,fSAC.createErrC,nil)
1968+
testutil.RequireSend(ctx,t,fDCCLI.execErrC,func(cmdstring,args...string)error {
1969+
assert.Equal(t,"pwd",cmd)
1970+
assert.Empty(t,args)
1971+
returnnil
1972+
})
1973+
testutil.RequireSend(ctx,t,fDCCLI.readConfigErrC,func(env []string)error {
1974+
// We expect the wrong workspace agent name passed in first.
1975+
assert.Contains(t,env,"CODER_WORKSPACE_AGENT_NAME=test-container")
1976+
returnnil
1977+
})
1978+
testutil.RequireSend(ctx,t,fDCCLI.readConfigErrC,func(env []string)error {
1979+
// We then expect the agent name passed here to have been read from the config.
1980+
assert.Contains(t,env,"CODER_WORKSPACE_AGENT_NAME=custom-name")
1981+
assert.NotContains(t,env,"CODER_WORKSPACE_AGENT_NAME=test-container")
1982+
returnnil
1983+
})
1984+
1985+
// Wait until the ticker has been registered.
1986+
tickerTrap.MustWait(ctx).MustRelease(ctx)
1987+
tickerTrap.Close()
1988+
1989+
// Then: We expected it to succeed
1990+
require.Len(t,fSAC.created,1)
1991+
})
18871992
}
18881993

18891994
// mustFindDevcontainerByPath returns the devcontainer with the given workspace

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp