Skip to content

Commit 3b54de6

Browse files
aknyshclaudeautofix-ci[bot]
authored
Serialize Atmos library calls to prevent concurrent ReadDataSource crash (#523)
* Add local goreleaser config to exclude unsupported windows/arm target Go 1.24+ dropped support for the windows/arm (32-bit) GOOS/GOARCH pair. The shared org-wide goreleaser config includes it, causing release builds to fail after the Go 1.23 -> 1.26 upgrade in #522. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update fix doc to reflect org-wide goreleaser fix Document the broader scope: the windows/arm ignore rule is applied in the shared config (cloudposse/.github), the provider's local override (temporary), and needs to be added to atmos's local config. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Serialize Atmos library calls to fix concurrent ReadDataSource crash The Atmos library uses package-level mutable state (mergedConfigFiles in pkg/config/load.go) that is explicitly not safe for concurrent use. When Terraform executes multiple ReadDataSource calls in parallel (e.g., 5 concurrent utils_component_config reads), they race on the shared state, corrupt it, and trigger CheckErrorPrintAndExit → os.Exit(1), killing the gRPC plugin process ("Plugin did not respond"). Add a sync.Mutex to serialize all Atmos library calls across the five data sources that use it: component_config, describe_stacks, stack_config_yaml, spacelift_stack_config, and aws_eks_update_kubeconfig. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update fix doc: remove consumer-specific references Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address PR review: use defer for mutex unlock, update goreleaser for v2 - Refactor describe_stacks and stack_config_yaml to use anonymous functions with defer atmosMu.Unlock() instead of manual unlock on each error path - Add version: 2 header to .goreleaser.yml for GoReleaser v2 compliance - Replace deprecated .Commit with .FullCommit in goreleaser ldflags Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [autofix.ci] apply automated fixes --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent d668a0c commit 3b54de6

8 files changed

Lines changed: 277 additions & 45 deletions

.goreleaser.yml

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
version: 2
2+
3+
# Local goreleaser config — overrides the org-wide config at cloudposse/.github/.github/goreleaser.yml
4+
# Needed because Go 1.24+ dropped support for windows/arm (32-bit), which the shared config includes.
5+
6+
builds:
7+
- env:
8+
- CGO_ENABLED=0
9+
mod_timestamp: '{{ .CommitTimestamp }}'
10+
flags:
11+
- -trimpath
12+
ldflags:
13+
- '-s -w -X main.version={{.Version}} -X main.commit={{.FullCommit}}'
14+
goos:
15+
- freebsd
16+
- windows
17+
- linux
18+
- darwin
19+
goarch:
20+
- amd64
21+
- '386'
22+
- arm
23+
- arm64
24+
ignore:
25+
- goos: windows
26+
goarch: arm
27+
binary: '{{ .ProjectName }}'
28+
29+
archives:
30+
- format: "{{ .Env.ARCHIVES_FORMAT }}"
31+
name_template: '{{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }}'
32+
33+
checksum:
34+
name_template: '{{ .ProjectName }}_{{ .Version }}_SHA256SUMS'
35+
algorithm: sha256
36+
37+
signs:
38+
- artifacts: checksum
39+
args:
40+
- "--batch"
41+
- "--local-user"
42+
- "{{ .Env.GPG_FINGERPRINT }}"
43+
- "--output"
44+
- "${signature}"
45+
- "--detach-sign"
46+
- "${artifact}"
47+
48+
release:
49+
draft: true
50+
replace_existing_draft: true
51+
replace_existing_artifacts: true
52+
mode: keep-existing
53+
make_latest: false
54+
name_template: '{{.Tag}}'
55+
target_commitish: '{{ if index .Env "GO_RELEASER_TARGET_COMMITISH" }}{{ .Env.GO_RELEASER_TARGET_COMMITISH }}{{ else }}{{ .Branch }}{{ end }}'
56+
prerelease: auto
57+
58+
changelog:
59+
skip: true
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
# Provider `ReadDataSource` Crash — `os.Exit(1)` from Atmos Library
2+
3+
**Affected Versions:** `cloudposse/utils` provider v2.0.0 (Atmos v1.207.0 embedded)
4+
5+
**Severity:** Critical — provider process exits with code 1 during `ReadDataSource`
6+
calls, producing "Plugin did not respond" errors in Terraform
7+
8+
## Symptoms
9+
10+
Components with multiple `data "utils_component_config"` data sources crash the provider during
11+
`terraform plan`:
12+
13+
```text
14+
Error: Plugin did not respond
15+
16+
with module.iam_roles.module.account_map.data.utils_component_config.config[0],
17+
on .terraform/modules/iam_roles.account_map/modules/remote-state/main.tf line 1,
18+
in data "utils_component_config" "config":
19+
1: data "utils_component_config" "config" {
20+
```
21+
22+
`TF_LOG=TRACE` reveals the provider exits with **exit status 1** (not a panic or signal):
23+
24+
```text
25+
provider: plugin process exited: path=...terraform-provider-utils pid=3934 error="exit status 1"
26+
```
27+
28+
## Root Causes
29+
30+
Investigation revealed **two independent issues** that can each trigger `os.Exit(1)` inside the
31+
Atmos library, killing the provider's gRPC plugin process. Both are addressed in this fix.
32+
33+
### Issue 1: `os.Exit(1)` on errors in the Atmos library
34+
35+
The Atmos library uses `CheckErrorPrintAndExit()` in many code paths within `internal/exec`.
36+
This function is designed for CLI usage — it prints an error and exits the process. Inside a
37+
Terraform provider (gRPC plugin), calling `os.Exit(1)` terminates the plugin without returning
38+
a diagnostic error to Terraform.
39+
40+
Any error that reaches `CheckErrorPrintAndExit` silently crashes the provider. Code paths that
41+
use it include:
42+
43+
- `utils.go:664-673` — duplicate component config detection
44+
- `utils.go:753,765` — template processing errors
45+
- `yaml_func_store.go:29,89,99,107``!store` YAML tag errors
46+
- `yaml_func_store_get.go:52,90,101,116``!store.get` YAML tag errors
47+
- `describe_stacks.go:489,743,982` — describe stacks errors
48+
49+
**Any** error reaching these paths will crash the provider via `os.Exit(1)`. The provider cannot
50+
intercept `os.Exit` — the Atmos library kills the process before the provider can return a
51+
diagnostic to Terraform.
52+
53+
### Issue 2: Thread-unsafe global state (LATENT)
54+
55+
The Atmos library was designed as a single-threaded CLI tool. It has **package-level mutable
56+
state** that is explicitly documented as not thread-safe:
57+
58+
```go
59+
// pkg/config/load.go:51-54
60+
// NOTE: This package-level state assumes sequential (non-concurrent) calls to LoadConfig.
61+
// LoadConfig is NOT safe for concurrent use.
62+
var mergedConfigFiles []string
63+
```
64+
65+
Additional thread-unsafe global state includes:
66+
67+
- `errors/error_funcs.go:31``var atmosConfig *schema.AtmosConfiguration`
68+
- `errors/error_funcs.go:28``var render *markdown.Renderer`
69+
- `errors/error_funcs.go:34``var verboseFlag`
70+
71+
Terraform invokes `ReadDataSource` concurrently (one goroutine per data source instance). Each
72+
call enters `ProcessComponentInStack``InitCliConfig``LoadConfig`, which resets and writes
73+
to the shared `mergedConfigFiles` slice. Concurrent goroutines can corrupt the slice through
74+
interleaved reads/writes, causing downstream errors that hit `CheckErrorPrintAndExit`
75+
`os.Exit(1)`.
76+
77+
This is a **latent** issue — it is a real data race that could cause unpredictable failures
78+
under concurrent load.
79+
80+
### Debug log timeline (provider v2.0.0, pid=3934)
81+
82+
| Timestamp | Event |
83+
|----------------|--------------------------------------------------------------|
84+
| `19:24:15.825` | Provider starts, configures mTLS |
85+
| `19:24:15.941` | GetProviderSchema — success |
86+
| `19:24:15.945` | Configure — success |
87+
| `19:24:16.117` | ValidateDataSourceConfig — success |
88+
| `19:24:16.122` | **ReadDataSource #1** — "Calling downstream" (never returns) |
89+
| `19:24:16.152` | **ReadDataSource #2** — "Calling downstream" (never returns) |
90+
| `19:24:16.276` | **ReadDataSource #3** — "Calling downstream" (never returns) |
91+
| `19:24:16.817` | **Plugin process exited: exit status 1** |
92+
| `19:24:16.817` | gRPC: "connection reset by peer" |
93+
94+
## Fix
95+
96+
### Provider-side fix
97+
98+
Add a package-level `sync.Mutex` in the provider to serialize all calls into the Atmos library.
99+
This addresses **Issue 2** by preventing concurrent goroutines from accessing the thread-unsafe
100+
global state simultaneously. It does not prevent `os.Exit(1)` from stack config errors (Issue 1),
101+
but it eliminates the data race as a potential trigger.
102+
103+
**New file: `internal/provider/atmos_lock.go`**
104+
105+
```go
106+
package provider
107+
108+
import "sync"
109+
110+
// atmosMu serializes all calls into the Atmos library.
111+
// The Atmos library uses package-level mutable state (e.g., mergedConfigFiles in
112+
// pkg/config/load.go) that is explicitly documented as not safe for concurrent use.
113+
// Terraform invokes ReadDataSource concurrently for independent data sources, so
114+
// without this mutex, concurrent calls corrupt shared state and trigger os.Exit(1)
115+
// via CheckErrorPrintAndExit, killing the gRPC plugin process.
116+
var atmosMu sync.Mutex
117+
```
118+
119+
**Modified data source files:**
120+
121+
Each `ReadContext` function wraps its Atmos library calls with `atmosMu.Lock()` /
122+
`atmosMu.Unlock()`:
123+
124+
- `data_source_component_config.go` — wraps `ProcessComponentInStack` / `ProcessComponentFromContext`
125+
- `data_source_describe_stacks.go` — wraps `InitCliConfig` + `ExecuteDescribeStacks`
126+
- `data_source_stack_config_yaml.go` — wraps `InitCliConfig` + `ProcessYAMLConfigFiles`
127+
- `data_source_spacelift_stack_config.go` — wraps `CreateSpaceliftStacks`
128+
- `data_source_aws_eks_update_kubeconfig.go` — wraps `ExecuteAwsEksUpdateKubeconfig`
129+
130+
### Long-term fix (Atmos library)
131+
132+
The Atmos library should be refactored to:
133+
134+
1. Replace `CheckErrorPrintAndExit` / `os.Exit` calls in library code paths with proper error
135+
returns, so embedded consumers (like this provider) can handle errors gracefully — this is
136+
the most critical fix, as it would convert silent crashes into visible Terraform diagnostics
137+
2. Eliminate package-level mutable state in `pkg/config` and `errors`
138+
3. Pass configuration through context or options structs instead of global variables
139+
140+
## References
141+
142+
- Atmos `LoadConfig` thread-safety comment: `pkg/config/load.go:51-54`
143+
- `CheckErrorPrintAndExit` implementation: `errors/error_funcs.go:324-366`

internal/provider/atmos_lock.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package provider
2+
3+
import "sync"
4+
5+
// atmosMu serializes all calls into the Atmos library.
6+
//
7+
// The Atmos library uses package-level mutable state (e.g., mergedConfigFiles in
8+
// pkg/config/load.go) that is explicitly documented as not safe for concurrent use.
9+
// Terraform invokes ReadDataSource concurrently for independent data sources, so
10+
// without this mutex, concurrent calls corrupt shared state and trigger os.Exit(1)
11+
// via CheckErrorPrintAndExit, killing the gRPC plugin process.
12+
var atmosMu sync.Mutex

internal/provider/data_source_aws_eks_update_kubeconfig.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,10 @@ func dataSourceAwsEksUpdateKubeconfigRead(ctx context.Context, d *schema.Resourc
138138
return diag.FromErr(err)
139139
}
140140

141+
atmosMu.Lock()
141142
err = a.ExecuteAwsEksUpdateKubeconfig(kubeconfigContext)
143+
atmosMu.Unlock()
144+
142145
if err != nil {
143146
return diag.FromErr(err)
144147
}

internal/provider/data_source_component_config.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,9 @@ func dataSourceComponentConfigRead(ctx context.Context, d *schema.ResourceData,
114114
}
115115
}
116116

117+
atmosMu.Lock()
117118
if len(stack) > 0 {
118119
result, err = p.ProcessComponentInStack(component, stack, atmosCliConfigPath, atmosBasePath)
119-
if err != nil && !ignoreErrors {
120-
return diag.FromErr(err)
121-
}
122120
} else {
123121
result, err = p.ProcessComponentFromContext(&p.ComponentFromContextParams{
124122
Component: component,
@@ -129,9 +127,11 @@ func dataSourceComponentConfigRead(ctx context.Context, d *schema.ResourceData,
129127
AtmosCliConfigPath: atmosCliConfigPath,
130128
AtmosBasePath: atmosBasePath,
131129
})
132-
if err != nil && !ignoreErrors {
133-
return diag.FromErr(err)
134-
}
130+
}
131+
atmosMu.Unlock()
132+
133+
if err != nil && !ignoreErrors {
134+
return diag.FromErr(err)
135135
}
136136

137137
if err != nil {

internal/provider/data_source_describe_stacks.go

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -151,31 +151,37 @@ func dataSourceDescribeStacksRead(ctx context.Context, d *schema.ResourceData, m
151151
AtmosCliConfigPath: atmosCliConfigPath,
152152
}
153153

154-
cliConfig, err := cfg.InitCliConfig(info, true)
155-
if err != nil {
156-
return diag.FromErr(err)
157-
}
154+
result, err = func() (map[string]any, error) {
155+
atmosMu.Lock()
156+
defer atmosMu.Unlock()
158157

159-
var filterByStack string
160-
161-
if stack != "" {
162-
filterByStack = stack
163-
} else if namespace != "" || tenant != "" || environment != "" || stage != "" {
164-
filterByStack, err = cfg.GetStackNameFromContextAndStackNamePattern(namespace, tenant, environment, stage, cliConfig.Stacks.NamePattern)
158+
cliConfig, err := cfg.InitCliConfig(info, true)
165159
if err != nil {
166-
return diag.FromErr(err)
160+
return nil, err
167161
}
168-
}
169162

170-
result, err = describe.ExecuteDescribeStacks(
171-
cliConfig,
172-
filterByStack,
173-
componentsList,
174-
componentTypesList,
175-
sectionsList,
176-
false,
177-
true,
178-
)
163+
var filterByStack string
164+
165+
if stack != "" {
166+
filterByStack = stack
167+
} else if namespace != "" || tenant != "" || environment != "" || stage != "" {
168+
filterByStack, err = cfg.GetStackNameFromContextAndStackNamePattern(namespace, tenant, environment, stage, cliConfig.Stacks.NamePattern)
169+
if err != nil {
170+
return nil, err
171+
}
172+
}
173+
174+
return describe.ExecuteDescribeStacks(
175+
cliConfig,
176+
filterByStack,
177+
componentsList,
178+
componentTypesList,
179+
sectionsList,
180+
false,
181+
true,
182+
)
183+
}()
184+
179185
if err != nil && !ignoreErrors {
180186
return diag.FromErr(err)
181187
}

internal/provider/data_source_spacelift_stack_config.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ func dataSourceSpaceliftStackConfigRead(ctx context.Context, d *schema.ResourceD
9393
return diag.FromErr(err)
9494
}
9595

96+
atmosMu.Lock()
9697
spaceliftStacks, err := s.CreateSpaceliftStacks(
9798
stacksBasePath.(string),
9899
"",
@@ -104,6 +105,8 @@ func dataSourceSpaceliftStackConfigRead(ctx context.Context, d *schema.ResourceD
104105
processComponentDeps.(bool),
105106
processImports.(bool),
106107
stackConfigPathTemplate.(string))
108+
atmosMu.Unlock()
109+
107110
if err != nil {
108111
return diag.FromErr(err)
109112
}

internal/provider/data_source_stack_config_yaml.go

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -66,18 +66,13 @@ func dataSourceStackConfigYAML() *schema.Resource {
6666
}
6767

6868
func dataSourceStackConfigYAMLRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
69-
cliConfig, err := cfg.InitCliConfig(atmosSchema.ConfigAndStacksInfo{}, true)
70-
if err != nil {
71-
return diag.FromErr(err)
72-
}
73-
7469
input := d.Get("input")
7570
processStackDeps := d.Get("process_stack_deps")
7671
processComponentDeps := d.Get("process_component_deps")
7772
stacksBasePath := d.Get("base_path")
7873
env := d.Get("env").(map[string]any)
7974

80-
err = setEnv(env)
75+
err := setEnv(env)
8176
if err != nil {
8277
return diag.FromErr(err)
8378
}
@@ -87,18 +82,29 @@ func dataSourceStackConfigYAMLRead(ctx context.Context, d *schema.ResourceData,
8782
return diag.FromErr(err)
8883
}
8984

90-
result, _, _, err := s.ProcessYAMLConfigFiles(
91-
&cliConfig,
92-
stacksBasePath.(string),
93-
"",
94-
"",
95-
"",
96-
"",
97-
paths,
98-
processStackDeps.(bool),
99-
processComponentDeps.(bool),
100-
false,
101-
)
85+
var result []string
86+
result, _, _, err = func() ([]string, map[string]any, map[string]map[string]any, error) {
87+
atmosMu.Lock()
88+
defer atmosMu.Unlock()
89+
90+
cliConfig, err := cfg.InitCliConfig(atmosSchema.ConfigAndStacksInfo{}, true)
91+
if err != nil {
92+
return nil, nil, nil, err
93+
}
94+
95+
return s.ProcessYAMLConfigFiles(
96+
&cliConfig,
97+
stacksBasePath.(string),
98+
"",
99+
"",
100+
"",
101+
"",
102+
paths,
103+
processStackDeps.(bool),
104+
processComponentDeps.(bool),
105+
false,
106+
)
107+
}()
102108
if err != nil {
103109
return diag.FromErr(err)
104110
}

0 commit comments

Comments
 (0)