diff --git a/provisioner/terraform/resources.go b/provisioner/terraform/resources.go index d9da73d82f..b1132adc40 100644 --- a/provisioner/terraform/resources.go +++ b/provisioner/terraform/resources.go @@ -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" diff --git a/provisioner/terraform/resources_test.go b/provisioner/terraform/resources_test.go index b396c99af9..e3f0034143 100644 --- a/provisioner/terraform/resources_test.go +++ b/provisioner/terraform/resources_test.go @@ -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", diff --git a/provisioner/terraform/testdata/resources/duplicate-env-keys/converted_state.plan.golden b/provisioner/terraform/testdata/resources/duplicate-env-keys/converted_state.plan.golden new file mode 100644 index 0000000000..9e956c13d7 --- /dev/null +++ b/provisioner/terraform/testdata/resources/duplicate-env-keys/converted_state.plan.golden @@ -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 +} diff --git a/provisioner/terraform/testdata/resources/duplicate-env-keys/converted_state.state.golden b/provisioner/terraform/testdata/resources/duplicate-env-keys/converted_state.state.golden new file mode 100644 index 0000000000..9e956c13d7 --- /dev/null +++ b/provisioner/terraform/testdata/resources/duplicate-env-keys/converted_state.state.golden @@ -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 +} diff --git a/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tf b/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tf new file mode 100644 index 0000000000..edd03856b8 --- /dev/null +++ b/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tf @@ -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 + ] +} diff --git a/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfplan.dot b/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfplan.dot new file mode 100644 index 0000000000..b47bca648f --- /dev/null +++ b/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfplan.dot @@ -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)" + } +} diff --git a/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfplan.json b/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfplan.json new file mode 100644 index 0000000000..f216d9258e --- /dev/null +++ b/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfplan.json @@ -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" + ] + } + ] + } + } +} diff --git a/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfstate.dot b/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfstate.dot new file mode 100644 index 0000000000..b47bca648f --- /dev/null +++ b/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfstate.dot @@ -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)" + } +} diff --git a/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfstate.json b/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfstate.json new file mode 100644 index 0000000000..48534e3cd7 --- /dev/null +++ b/provisioner/terraform/testdata/resources/duplicate-env-keys/duplicate-env-keys.tfstate.json @@ -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" + ] + } + ] + } + } +}