shishantbiswas/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-06-02 20:48:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(provisioner): make coder_env and coder_script iteration deterministic (#22706)

Browse Source 
## Description

Fixes https://github.com/coder/coder/issues/21885

When multiple `coder_env` resources define the same key for a single
agent, the final environment variable value was non-deterministic
because Go maps have random iteration order. The `ConvertState` function
iterated over `tfResourcesByLabel` (a map) to associate `coder_env`
resources with agents, making the order of `ExtraEnvs` unpredictable
across builds.

## Changes

- Added `sortedResourcesByType()` helper in `resources.go` that collects
resources of a given type from the label map and sorts them by Terraform
address before processing
- Replaced map iteration for `coder_env` and `coder_script` association
with sorted iteration, ensuring deterministic ordering
- Added `duplicate-env-keys` test case and fixture verifying that when
two `coder_env` resources define the same key, the result is
deterministic (sorted by address)

## How it works

When duplicate keys exist, the last one by sorted Terraform address
wins. For example, `coder_env.path_a` is processed before
`coder_env.path_b`, so `path_b`'s value will be the final one in
`ExtraEnvs`. Since `provisionerdserver.go` merges `ExtraEnvs` into a map
(last wins), this produces stable, predictable results.
This commit is contained in:
Kacper Sawicki
2026-03-11 15:33:54 +01:00
committed by GitHub
parent ac791e5bd3
commit 9d2643d3aa
9 changed files with 577 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
provisioner/terraform/resources.go
+77 -61
View File
@@ -1,9 +1,11 @@
package terraform
import (
"cmp"
"context"
"fmt"
"math"
"slices"
"strings"
"github.com/awalterschulze/gographviz"
@@ -634,81 +636,77 @@ func ConvertState(ctx context.Context, modules []*tfjson.StateModule, rawGraph s
}
// Associate envs with agents.
for _, resources := range tfResourcesByLabel {
for _, resource := range resources {
if resource.Type != "coder_env" {
continue
}
var attrs agentEnvAttributes
err = mapstructure.Decode(resource.AttributeValues, &attrs)
if err != nil {
return nil, xerrors.Errorf("decode env attributes: %w", err)
}
// Collect and sort env resources by address for deterministic ordering.
// When multiple coder_env resources define the same key, the last one
// by sorted address wins, ensuring stable behavior across builds.
sortedEnvResources := sortedResourcesByType(tfResourcesByLabel, "coder_env")
for _, resource := range sortedEnvResources {
var attrs agentEnvAttributes
err = mapstructure.Decode(resource.AttributeValues, &attrs)
if err != nil {
return nil, xerrors.Errorf("decode env attributes: %w", err)
}
env := &proto.Env{
Name: attrs.Name,
Value: attrs.Value,
}
env := &proto.Env{
Name: attrs.Name,
Value: attrs.Value,
}
envAgentLoop:
for _, agents := range resourceAgents {
for _, agent := range agents {
// Find agents with the matching ID and associate them!
if dependsOnAgent(graph, agent, attrs.AgentID, resource) {
agent.ExtraEnvs = append(agent.ExtraEnvs, env)
envAgentLoop:
for _, agents := range resourceAgents {
for _, agent := range agents {
// Find agents with the matching ID and associate them!
if dependsOnAgent(graph, agent, attrs.AgentID, resource) {
agent.ExtraEnvs = append(agent.ExtraEnvs, env)
break envAgentLoop
}
for _, dc := range agent.GetDevcontainers() {
if dependsOnDevcontainer(graph, dc, attrs.AgentID, resource) {
dc.Envs = append(dc.Envs, env)
break envAgentLoop
}
for _, dc := range agent.GetDevcontainers() {
if dependsOnDevcontainer(graph, dc, attrs.AgentID, resource) {
dc.Envs = append(dc.Envs, env)
break envAgentLoop
}
}
}
}
}
}
// Associate scripts with agents.
for _, resources := range tfResourcesByLabel {
for _, resource := range resources {
if resource.Type != "coder_script" {
continue
}
var attrs agentScriptAttributes
err = mapstructure.Decode(resource.AttributeValues, &attrs)
if err != nil {
return nil, xerrors.Errorf("decode script attributes: %w", err)
}
// Sort for deterministic ordering, same as envs above.
sortedScriptResources := sortedResourcesByType(tfResourcesByLabel, "coder_script")
for _, resource := range sortedScriptResources {
var attrs agentScriptAttributes
err = mapstructure.Decode(resource.AttributeValues, &attrs)
if err != nil {
return nil, xerrors.Errorf("decode script attributes: %w", err)
}
script := &proto.Script{
DisplayName: attrs.DisplayName,
Icon: attrs.Icon,
Script: attrs.Script,
Cron: attrs.Cron,
LogPath: attrs.LogPath,
StartBlocksLogin: attrs.StartBlocksLogin,
RunOnStart: attrs.RunOnStart,
RunOnStop: attrs.RunOnStop,
TimeoutSeconds: attrs.TimeoutSeconds,
}
script := &proto.Script{
DisplayName: attrs.DisplayName,
Icon: attrs.Icon,
Script: attrs.Script,
Cron: attrs.Cron,
LogPath: attrs.LogPath,
StartBlocksLogin: attrs.StartBlocksLogin,
RunOnStart: attrs.RunOnStart,
RunOnStop: attrs.RunOnStop,
TimeoutSeconds: attrs.TimeoutSeconds,
}
scriptAgentLoop:
for _, agents := range resourceAgents {
for _, agent := range agents {
// Find agents with the matching ID and associate them!
if dependsOnAgent(graph, agent, attrs.AgentID, resource) {
agent.Scripts = append(agent.Scripts, script)
scriptAgentLoop:
for _, agents := range resourceAgents {
for _, agent := range agents {
// Find agents with the matching ID and associate them!
if dependsOnAgent(graph, agent, attrs.AgentID, resource) {
agent.Scripts = append(agent.Scripts, script)
break scriptAgentLoop
}
for _, dc := range agent.GetDevcontainers() {
if dependsOnDevcontainer(graph, dc, attrs.AgentID, resource) {
dc.Scripts = append(dc.Scripts, script)
break scriptAgentLoop
}
for _, dc := range agent.GetDevcontainers() {
if dependsOnDevcontainer(graph, dc, attrs.AgentID, resource) {
dc.Scripts = append(dc.Scripts, script)
break scriptAgentLoop
}
}
}
}
}
@@ -1147,6 +1145,24 @@ func PtrInt32(number int) *int32 {
return &n
}
// sortedResourcesByType collects all resources of the given type from the
// label map and returns them sorted by address. This ensures deterministic
// iteration order when processing resources that are stored in Go maps.
func sortedResourcesByType(tfResourcesByLabel map[string]map[string]*tfjson.StateResource, resourceType string) []*tfjson.StateResource {
var result []*tfjson.StateResource
for _, resources := range tfResourcesByLabel {
for _, resource := range resources {
if resource.Type == resourceType {
result = append(result, resource)
}
}
}
slices.SortFunc(result, func(a, b *tfjson.StateResource) int {
return cmp.Compare(a.Address, b.Address)
})
return result
}
// convertAddressToLabel returns the Terraform address without the count
// specifier.
// eg. "module.ec2_dev.ec2_instance.dev[0]" becomes "module.ec2_dev.ec2_instance.dev"
provisioner/terraform/resources_test.go
+43
View File
@@ -368,6 +368,49 @@ func TestConvertResources(t *testing.T) {
Type: "coder_env",
}},
},
// Verifies that when multiple coder_env resources define the
// same key, the ordering is deterministic (sorted by Terraform
// address). This prevents a race condition where Go map
// iteration order could cause non-deterministic env values.
"duplicate-env-keys": {
resources: []*proto.Resource{{
Name: "dev",
Type: "null_resource",
Agents: []*proto.Agent{{
Name: "dev",
OperatingSystem: "linux",
Architecture: "amd64",
ExtraEnvs: []*proto.Env{
{
Name: "PATH",
Value: "/a/bin",
},
{
Name: "PATH",
Value: "/b/bin",
},
{
Name: "UNIQUE",
Value: "unique_value",
},
},
Auth: &proto.Agent_Token{},
ApiKeyScope: "all",
ConnectionTimeoutSeconds: 120,
DisplayApps: &displayApps,
ResourcesMonitoring: &proto.ResourcesMonitoring{},
}},
}, {
Name: "path_a",
Type: "coder_env",
}, {
Name: "path_b",
Type: "coder_env",
}, {
Name: "unique_env",
Type: "coder_env",
}},
},
"multiple-agents-multiple-monitors": {
resources: []*proto.Resource{{
Name: "dev",
provisioner/terraform/testdata/resources/duplicate-env-keys/converted_state.plan.golden
Vendored
+60
View File
@@ -0,0 +1,60 @@
{
"Resources": [
{
"name": "dev",
"type": "null_resource",
"agents": [
{
"id": "aaaaaaaa-1111-2222-3333-444444444444",
"name": "dev",
"operating_system": "linux",
"architecture": "amd64",
"Auth": {
"Token": "11111111-2222-3333-4444-555555555555"
},
"connection_timeout_seconds": 120,
"display_apps": {
"vscode": true,
"web_terminal": true,
"ssh_helper": true,
"port_forwarding_helper": true
},
"extra_envs": [
{
"name": "PATH",
"value": "/a/bin"
},
{
"name": "PATH",
"value": "/b/bin"
},
{
"name": "UNIQUE",
"value": "unique_value"
}
],
"resources_monitoring": {},
"api_key_scope": "all"
}
]
},
{
"name": "path_a",
"type": "coder_env"
},
{
"name": "path_b",
"type": "coder_env"
},
{
"name": "unique_env",
"type": "coder_env"
}
],
"Parameters": [],
"Presets": [],
"ExternalAuthProviders": [],
"AITasks": [],
"HasAITasks": false,
"HasExternalAgents": false
}
provisioner/terraform/testdata/resources/duplicate-env-keys/converted_state.state.golden
Vendored
+60
View File
@@ -0,0 +1,60 @@
{
"Resources": [
{
"name": "dev",
"type": "null_resource",
"agents": [
{
"id": "aaaaaaaa-1111-2222-3333-444444444444",
"name": "dev",
"operating_system": "linux",
"architecture": "amd64",
"Auth": {
"Token": "11111111-2222-3333-4444-555555555555"
},
"connection_timeout_seconds": 120,
"display_apps": {
"vscode": true,
"web_terminal": true,
"ssh_helper": true,
"port_forwarding_helper": true
},
"extra_envs": [
{
"name": "PATH",
"value": "/a/bin"
},
{
"name": "PATH",
"value": "/b/bin"
},
{
"name": "UNIQUE",
"value": "unique_value"
}
],
"resources_monitoring": {},
"api_key_scope": "all"
}
]
},
{
"name": "path_a",
"type": "coder_env"
},
{
"name": "path_b",
"type": "coder_env"
},
{
"name": "unique_env",
"type": "coder_env"
}
],
"Parameters": [],
"Presets": [],
"ExternalAuthProviders": [],
"AITasks": [],
"HasAITasks": false,
"HasExternalAgents": false
}
provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tf
Vendored
+37
View File
@@ -0,0 +1,37 @@
terraform {
required_providers {
coder = {
source = "coder/coder"
version = ">=2.0.0"
}
}
}
resource "coder_agent" "dev" {
os = "linux"
arch = "amd64"
}
resource "coder_env" "path_b" {
agent_id = coder_agent.dev.id
name = "PATH"
value = "/b/bin"
}
resource "coder_env" "path_a" {
agent_id = coder_agent.dev.id
name = "PATH"
value = "/a/bin"
}
resource "coder_env" "unique_env" {
agent_id = coder_agent.dev.id
name = "UNIQUE"
value = "unique_value"
}
resource "null_resource" "dev" {
depends_on = [
coder_agent.dev
]
}
provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfplan.dot
Generated Vendored
+25
View File
@@ -0,0 +1,25 @@
digraph {
compound = "true"
newrank = "true"
subgraph "root" {
"[root] coder_agent.dev (expand)" [label = "coder_agent.dev", shape = "box"]
"[root] coder_env.path_a (expand)" [label = "coder_env.path_a", shape = "box"]
"[root] coder_env.path_b (expand)" [label = "coder_env.path_b", shape = "box"]
"[root] coder_env.unique_env (expand)" [label = "coder_env.unique_env", shape = "box"]
"[root] null_resource.dev (expand)" [label = "null_resource.dev", shape = "box"]
"[root] provider[\"registry.terraform.io/coder/coder\"]" [label = "provider[\"registry.terraform.io/coder/coder\"]", shape = "diamond"]
"[root] provider[\"registry.terraform.io/hashicorp/null\"]" [label = "provider[\"registry.terraform.io/hashicorp/null\"]", shape = "diamond"]
"[root] coder_agent.dev (expand)" -> "[root] provider[\"registry.terraform.io/coder/coder\"]"
"[root] coder_env.path_a (expand)" -> "[root] coder_agent.dev (expand)"
"[root] coder_env.path_b (expand)" -> "[root] coder_agent.dev (expand)"
"[root] coder_env.unique_env (expand)" -> "[root] coder_agent.dev (expand)"
"[root] null_resource.dev (expand)" -> "[root] coder_agent.dev (expand)"
"[root] null_resource.dev (expand)" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"]"
"[root] provider[\"registry.terraform.io/coder/coder\"] (close)" -> "[root] coder_env.path_a (expand)"
"[root] provider[\"registry.terraform.io/coder/coder\"] (close)" -> "[root] coder_env.path_b (expand)"
"[root] provider[\"registry.terraform.io/coder/coder\"] (close)" -> "[root] coder_env.unique_env (expand)"
"[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" -> "[root] null_resource.dev (expand)"
"[root] root" -> "[root] provider[\"registry.terraform.io/coder/coder\"] (close)"
"[root] root" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)"
}
}
provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfplan.json
Generated Vendored
+125
View File
@@ -0,0 +1,125 @@
{
"format_version": "1.2",
"terraform_version": "1.11.0",
"planned_values": {
"root_module": {
"resources": [
{
"address": "coder_agent.dev",
"mode": "managed",
"type": "coder_agent",
"name": "dev",
"provider_name": "registry.terraform.io/coder/coder",
"schema_version": 1,
"values": {
"api_key_scope": "all",
"arch": "amd64",
"auth": "token",
"connection_timeout": 120,
"dir": null,
"display_apps": [
{
"port_forwarding_helper": true,
"ssh_helper": true,
"vscode": true,
"vscode_insiders": false,
"web_terminal": true
}
],
"env": null,
"id": "aaaaaaaa-1111-2222-3333-444444444444",
"init_script": "",
"metadata": [],
"motd_file": null,
"order": null,
"os": "linux",
"resources_monitoring": [],
"shutdown_script": null,
"startup_script": null,
"startup_script_behavior": "non-blocking",
"token": "11111111-2222-3333-4444-555555555555",
"troubleshooting_url": null
},
"sensitive_values": {
"display_apps": [
{}
],
"metadata": [],
"resources_monitoring": [],
"token": true
}
},
{
"address": "coder_env.path_a",
"mode": "managed",
"type": "coder_env",
"name": "path_a",
"provider_name": "registry.terraform.io/coder/coder",
"schema_version": 1,
"values": {
"agent_id": "aaaaaaaa-1111-2222-3333-444444444444",
"id": "bbbbbbbb-1111-2222-3333-444444444444",
"name": "PATH",
"value": "/a/bin"
},
"sensitive_values": {},
"depends_on": [
"coder_agent.dev"
]
},
{
"address": "coder_env.path_b",
"mode": "managed",
"type": "coder_env",
"name": "path_b",
"provider_name": "registry.terraform.io/coder/coder",
"schema_version": 1,
"values": {
"agent_id": "aaaaaaaa-1111-2222-3333-444444444444",
"id": "cccccccc-1111-2222-3333-444444444444",
"name": "PATH",
"value": "/b/bin"
},
"sensitive_values": {},
"depends_on": [
"coder_agent.dev"
]
},
{
"address": "coder_env.unique_env",
"mode": "managed",
"type": "coder_env",
"name": "unique_env",
"provider_name": "registry.terraform.io/coder/coder",
"schema_version": 1,
"values": {
"agent_id": "aaaaaaaa-1111-2222-3333-444444444444",
"id": "dddddddd-1111-2222-3333-444444444444",
"name": "UNIQUE",
"value": "unique_value"
},
"sensitive_values": {},
"depends_on": [
"coder_agent.dev"
]
},
{
"address": "null_resource.dev",
"mode": "managed",
"type": "null_resource",
"name": "dev",
"provider_name": "registry.terraform.io/hashicorp/null",
"schema_version": 0,
"values": {
"id": "1234567890123456789",
"triggers": null
},
"sensitive_values": {},
"depends_on": [
"coder_agent.dev"
]
}
]
}
}
}
provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfstate.dot
Generated Vendored
+25
View File
@@ -0,0 +1,25 @@
digraph {
compound = "true"
newrank = "true"
subgraph "root" {
"[root] coder_agent.dev (expand)" [label = "coder_agent.dev", shape = "box"]
"[root] coder_env.path_a (expand)" [label = "coder_env.path_a", shape = "box"]
"[root] coder_env.path_b (expand)" [label = "coder_env.path_b", shape = "box"]
"[root] coder_env.unique_env (expand)" [label = "coder_env.unique_env", shape = "box"]
"[root] null_resource.dev (expand)" [label = "null_resource.dev", shape = "box"]
"[root] provider[\"registry.terraform.io/coder/coder\"]" [label = "provider[\"registry.terraform.io/coder/coder\"]", shape = "diamond"]
"[root] provider[\"registry.terraform.io/hashicorp/null\"]" [label = "provider[\"registry.terraform.io/hashicorp/null\"]", shape = "diamond"]
"[root] coder_agent.dev (expand)" -> "[root] provider[\"registry.terraform.io/coder/coder\"]"
"[root] coder_env.path_a (expand)" -> "[root] coder_agent.dev (expand)"
"[root] coder_env.path_b (expand)" -> "[root] coder_agent.dev (expand)"
"[root] coder_env.unique_env (expand)" -> "[root] coder_agent.dev (expand)"
"[root] null_resource.dev (expand)" -> "[root] coder_agent.dev (expand)"
"[root] null_resource.dev (expand)" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"]"
"[root] provider[\"registry.terraform.io/coder/coder\"] (close)" -> "[root] coder_env.path_a (expand)"
"[root] provider[\"registry.terraform.io/coder/coder\"] (close)" -> "[root] coder_env.path_b (expand)"
"[root] provider[\"registry.terraform.io/coder/coder\"] (close)" -> "[root] coder_env.unique_env (expand)"
"[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" -> "[root] null_resource.dev (expand)"
"[root] root" -> "[root] provider[\"registry.terraform.io/coder/coder\"] (close)"
"[root] root" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)"
}
}
provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfstate.json
Generated Vendored
+125
View File
@@ -0,0 +1,125 @@
{
"format_version": "1.0",
"terraform_version": "1.11.0",
"values": {
"root_module": {
"resources": [
{
"address": "coder_agent.dev",
"mode": "managed",
"type": "coder_agent",
"name": "dev",
"provider_name": "registry.terraform.io/coder/coder",
"schema_version": 1,
"values": {
"api_key_scope": "all",
"arch": "amd64",
"auth": "token",
"connection_timeout": 120,
"dir": null,
"display_apps": [
{
"port_forwarding_helper": true,
"ssh_helper": true,
"vscode": true,
"vscode_insiders": false,
"web_terminal": true
}
],
"env": null,
"id": "aaaaaaaa-1111-2222-3333-444444444444",
"init_script": "",
"metadata": [],
"motd_file": null,
"order": null,
"os": "linux",
"resources_monitoring": [],
"shutdown_script": null,
"startup_script": null,
"startup_script_behavior": "non-blocking",
"token": "11111111-2222-3333-4444-555555555555",
"troubleshooting_url": null
},
"sensitive_values": {
"display_apps": [
{}
],
"metadata": [],
"resources_monitoring": [],
"token": true
}
},
{
"address": "coder_env.path_a",
"mode": "managed",
"type": "coder_env",
"name": "path_a",
"provider_name": "registry.terraform.io/coder/coder",
"schema_version": 1,
"values": {
"agent_id": "aaaaaaaa-1111-2222-3333-444444444444",
"id": "bbbbbbbb-1111-2222-3333-444444444444",
"name": "PATH",
"value": "/a/bin"
},
"sensitive_values": {},
"depends_on": [
"coder_agent.dev"
]
},
{
"address": "coder_env.path_b",
"mode": "managed",
"type": "coder_env",
"name": "path_b",
"provider_name": "registry.terraform.io/coder/coder",
"schema_version": 1,
"values": {
"agent_id": "aaaaaaaa-1111-2222-3333-444444444444",
"id": "cccccccc-1111-2222-3333-444444444444",
"name": "PATH",
"value": "/b/bin"
},
"sensitive_values": {},
"depends_on": [
"coder_agent.dev"
]
},
{
"address": "coder_env.unique_env",
"mode": "managed",
"type": "coder_env",
"name": "unique_env",
"provider_name": "registry.terraform.io/coder/coder",
"schema_version": 1,
"values": {
"agent_id": "aaaaaaaa-1111-2222-3333-444444444444",
"id": "dddddddd-1111-2222-3333-444444444444",
"name": "UNIQUE",
"value": "unique_value"
},
"sensitive_values": {},
"depends_on": [
"coder_agent.dev"
]
},
{
"address": "null_resource.dev",
"mode": "managed",
"type": "null_resource",
"name": "dev",
"provider_name": "registry.terraform.io/hashicorp/null",
"schema_version": 0,
"values": {
"id": "1234567890123456789",
"triggers": null
},
"sensitive_values": {},
"depends_on": [
"coder_agent.dev"
]
}
]
}
}
}