fix!: enforce agent names be case-insensitive-unique per-workspace (#16614)

Relates to https://github.com/coder/coder-desktop-macos/issues/54

Currently, it's possible to have two agents within the same workspace whose names only differ in capitalization:
This leads to an ambiguity in two cases:
- For CoderVPN, we'd like to allow support to workspaces with a hostname of the form: `agent.workspace.username.coder`.
- Workspace apps (`coder_app`s) currently use subdomains of the form: `<app>--<agent>--<workspace>--<username>(--<suffix>)?`.

Of note is that DNS hosts must be strictly lower case, hence the ambiguity.

This fix is technically a breaking change, but only for the incredibly rare use case where a user has:
- A workspace with two agents
- Those agent names differ only in capitalization.

Those templates & workspaces will now fail to build. This can be fixed by choosing wholly unique names for the agents.
This commit is contained in:
Ethan
2025-02-20 12:51:25 +11:00
committed by GitHub
parent 9f5ad23644
commit 3fddfef879
4 changed files with 67 additions and 4 deletions
@@ -1891,10 +1891,12 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid.
appSlugs = make(map[string]struct{})
)
for _, prAgent := range protoResource.Agents {
if _, ok := agentNames[prAgent.Name]; ok {
// Agent names must be case-insensitive-unique, to be unambiguous in
// `coder_app`s and CoderVPN DNS names.
if _, ok := agentNames[strings.ToLower(prAgent.Name)]; ok {
return xerrors.Errorf("duplicate agent name %q", prAgent.Name)
}
agentNames[prAgent.Name] = struct{}{}
agentNames[strings.ToLower(prAgent.Name)] = struct{}{}
var instanceID sql.NullString
if prAgent.GetInstanceId() != "" {
@@ -1905,6 +1905,32 @@ func TestInsertWorkspaceResource(t *testing.T) {
})
require.ErrorContains(t, err, "duplicate app slug")
})
t.Run("DuplicateAgentNames", func(t *testing.T) {
t.Parallel()
db := dbmem.New()
job := uuid.New()
// case-insensitive-unique
err := insert(db, job, &sdkproto.Resource{
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev",
}, {
Name: "Dev",
}},
})
require.ErrorContains(t, err, "duplicate agent name")
err = insert(db, job, &sdkproto.Resource{
Name: "something",
Type: "aws_instance",
Agents: []*sdkproto.Agent{{
Name: "dev",
}, {
Name: "dev",
}},
})
require.ErrorContains(t, err, "duplicate agent name")
})
t.Run("Success", func(t *testing.T) {
t.Parallel()
db := dbmem.New()
+4 -2
View File
@@ -215,10 +215,12 @@ func ConvertState(ctx context.Context, modules []*tfjson.StateModule, rawGraph s
return nil, xerrors.Errorf("decode agent attributes: %w", err)
}
if _, ok := agentNames[tfResource.Name]; ok {
// Agent names must be case-insensitive-unique, to be unambiguous in
// `coder_app`s and CoderVPN DNS names.
if _, ok := agentNames[strings.ToLower(tfResource.Name)]; ok {
return nil, xerrors.Errorf("duplicate agent name: %s", tfResource.Name)
}
agentNames[tfResource.Name] = struct{}{}
agentNames[strings.ToLower(tfResource.Name)] = struct{}{}
// Handling for deprecated attributes. login_before_ready was replaced
// by startup_script_behavior, but we still need to support it for
+33
View File
@@ -1026,6 +1026,39 @@ func TestAppSlugValidation(t *testing.T) {
require.ErrorContains(t, err, "duplicate app slug")
}
func TestAgentNameDuplicate(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)
// nolint:dogsled
_, filename, _, _ := runtime.Caller(0)
dir := filepath.Join(filepath.Dir(filename), "testdata", "multiple-agents")
tfPlanRaw, err := os.ReadFile(filepath.Join(dir, "multiple-agents.tfplan.json"))
require.NoError(t, err)
var tfPlan tfjson.Plan
err = json.Unmarshal(tfPlanRaw, &tfPlan)
require.NoError(t, err)
tfPlanGraph, err := os.ReadFile(filepath.Join(dir, "multiple-agents.tfplan.dot"))
require.NoError(t, err)
for _, resource := range tfPlan.PlannedValues.RootModule.Resources {
if resource.Type == "coder_agent" {
switch resource.Name {
case "dev1":
resource.Name = "dev"
case "dev2":
resource.Name = "Dev"
}
}
}
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph), logger)
require.Nil(t, state)
require.Error(t, err)
require.ErrorContains(t, err, "duplicate agent name")
}
func TestMetadataResourceDuplicate(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)