fix!: enforce agent names be case-insensitive-unique per-workspace

ethanndickson · ethanndickson · commit 566e3de17f5a · 2025-02-19T05:49:40.000Z
diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go
@@ -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() != "" {
diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go
@@ -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()
diff --git a/provisioner/terraform/resources.go b/provisioner/terraform/resources.go
@@ -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
diff --git a/provisioner/terraform/resources_test.go b/provisioner/terraform/resources_test.go
@@ -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)

Original file line number	Diff line number	Diff line change
`@@ -1891,10 +1891,12 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid.`
`1891`	`1891`	`appSlugs = make(map[string]struct{})`
`1892`	`1892`	`)`
`1893`	`1893`	`for _, prAgent := range protoResource.Agents {`
`1894`		`- if _, ok := agentNames[prAgent.Name]; ok {`
	`1894`	`+ // Agent names must be case-insensitive-unique, to be unambiguous in`
	`1895`	+ // `coder_app`s and CoderVPN DNS names.
	`1896`	`+ if _, ok := agentNames[strings.ToLower(prAgent.Name)]; ok {`
`1895`	`1897`	`return xerrors.Errorf("duplicate agent name %q", prAgent.Name)`
`1896`	`1898`	`}`
`1897`		`- agentNames[prAgent.Name] = struct{}{}`
	`1899`	`+ agentNames[strings.ToLower(prAgent.Name)] = struct{}{}`
`1898`	`1900`
`1899`	`1901`	`var instanceID sql.NullString`
`1900`	`1902`	`if prAgent.GetInstanceId() != "" {`
Original file line number	Diff line number	Diff line change
`@@ -215,10 +215,12 @@ func ConvertState(ctx context.Context, modules []*tfjson.StateModule, rawGraph s`
`215`	`215`	`return nil, xerrors.Errorf("decode agent attributes: %w", err)`
`216`	`216`	`}`
`217`	`217`
`218`		`- if _, ok := agentNames[tfResource.Name]; ok {`
	`218`	`+ // Agent names must be case-insensitive-unique, to be unambiguous in`
	`219`	+ // `coder_app`s and CoderVPN DNS names.
	`220`	`+ if _, ok := agentNames[strings.ToLower(tfResource.Name)]; ok {`
`219`	`221`	`return nil, xerrors.Errorf("duplicate agent name: %s", tfResource.Name)`
`220`	`222`	`}`
`221`		`- agentNames[tfResource.Name] = struct{}{}`
	`223`	`+ agentNames[strings.ToLower(tfResource.Name)] = struct{}{}`
`222`	`224`
`223`	`225`	`// Handling for deprecated attributes. login_before_ready was replaced`
`224`	`226`	`// by startup_script_behavior, but we still need to support it for`