Skip to content

Commit ac51558

Browse files
authored
Merge pull request #43121 from hashicorp/f-use-smarterr-in-cloudwatch2
smarterr CloudWatch Integration: Declarative, Config-Driven Error Handling
2 parents 9b49bd6 + e6fa9c0 commit ac51558

File tree

239 files changed

+1926
-1249
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

239 files changed

+1926
-1249
lines changed

.ci/semgrep/smarterr/enforce.yml

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
rules:
2+
- id: go-no-sdkdiag-appendfromerr
3+
languages: [go]
4+
message: Use smerr.Append(ctx, diags, err) instead of sdkdiag.AppendFromErr.
5+
severity: ERROR
6+
pattern: sdkdiag.AppendFromErr($DIAGS, $ERR)
7+
fix: smerr.Append(ctx, $DIAGS, $ERR)
8+
paths:
9+
include:
10+
- internal/service/cloudwatch/
11+
12+
- id: go-no-sdkdiag-appenderrorf
13+
languages: [go]
14+
message: Use smerr.Append(ctx, diags, err, smerr.ID, ...) instead of sdkdiag.AppendErrorf.
15+
severity: ERROR
16+
patterns:
17+
- pattern: sdkdiag.AppendErrorf($DIAGS, $FMT, $ID, $ERR)
18+
- pattern: sdkdiag.AppendErrorf($DIAGS, $FMT, $ERR)
19+
paths:
20+
include:
21+
- internal/service/cloudwatch/
22+
23+
- id: go-no-create-appenddiagerror
24+
languages: [go]
25+
message: Use smerr.Append(ctx, diags, err, smerr.ID, ...) instead of create.AppendDiagError.
26+
severity: ERROR
27+
pattern: create.AppendDiagError($DIAGS, ...)
28+
paths:
29+
include:
30+
- internal/service/cloudwatch/
31+
32+
- id: go-no-diagnostics-adderror
33+
languages: [go]
34+
message: Use smerr.AddError(ctx, &response.Diagnostics, err, smerr.ID, ...) instead of Diagnostics.AddError.
35+
severity: ERROR
36+
patterns:
37+
- pattern: $RESP.Diagnostics.AddError($MSG, $ERR)
38+
- pattern: $RESP.Diagnostics.AddError(fmt.Sprintf($FMT, ...), $ERR)
39+
- pattern: $RESP.Diagnostics.AddError($MSG, $ERR.Error())
40+
- pattern-not-inside: smerr.AddError(...)
41+
paths:
42+
include:
43+
- internal/service/cloudwatch/
44+
45+
- id: go-no-create-adderror
46+
languages: [go]
47+
message: Use smerr.AddError(ctx, &response.Diagnostics, err, smerr.ID, ...) instead of create.AddError.
48+
severity: ERROR
49+
pattern: create.AddError(&$RESP.Diagnostics, ...)
50+
paths:
51+
include:
52+
- internal/service/cloudwatch/
53+
54+
- id: go-no-direct-diag-adderror
55+
languages: [go]
56+
message: Use smerr.AddError instead of resp.Diagnostics.AddError (migrate to smarterr/smerr).
57+
severity: ERROR
58+
patterns:
59+
- pattern: $RESP.Diagnostics.AddError($MSG, $ERR)
60+
- pattern-not-inside: smerr.AddError(...)
61+
paths:
62+
include:
63+
- internal/service/cloudwatch/
64+
65+
- id: go-no-direct-diag-appenderrorf
66+
languages: [go]
67+
message: Use smerr.Append or smerr.EnrichAppend instead of diag.AppendErrorf (migrate to smarterr/smerr).
68+
severity: ERROR
69+
pattern: diag.AppendErrorf(...)
70+
paths:
71+
include:
72+
- internal/service/cloudwatch/
73+
74+
- id: go-no-direct-diag-appendfromerr
75+
languages: [go]
76+
message: Use smerr.Append or smerr.EnrichAppend instead of diag.AppendFromErr (migrate to smarterr/smerr).
77+
severity: ERROR
78+
pattern: diag.AppendFromErr(...)
79+
paths:
80+
include:
81+
- internal/service/cloudwatch/
82+
83+
- id: go-no-direct-diag-append
84+
languages: [go]
85+
message: Use smerr.EnrichAppend instead of resp.Diagnostics.Append (migrate to smarterr/smerr).
86+
severity: ERROR
87+
patterns:
88+
- pattern: $RESP.Diagnostics.Append(...)
89+
- pattern-not-inside: smerr.EnrichAppend(...)
90+
paths:
91+
include:
92+
- internal/service/cloudwatch/
93+
94+
- id: go-no-bare-return-err
95+
languages: [go]
96+
message: Return errors wrapped with smarterr.NewError (migrate to smarterr).
97+
severity: ERROR
98+
patterns:
99+
- pattern: |
100+
return nil, $ERR
101+
- pattern-not-inside: |
102+
return nil, smarterr.NewError(...)
103+
paths:
104+
include:
105+
- internal/service/cloudwatch/
106+
107+
- id: go-no-bare-assertsinglevalueresult
108+
languages: [go]
109+
message: Wrap tfresource.AssertSingleValueResult with smarterr.Assert (migrate to smarterr).
110+
severity: ERROR
111+
patterns:
112+
- pattern: |
113+
return tfresource.AssertSingleValueResult(...)
114+
- pattern-not-inside: smarterr.Assert(tfresource.AssertSingleValueResult(...))
115+
paths:
116+
include:
117+
- internal/service/cloudwatch/
118+
119+
- id: go-no-bare-empty-result-error
120+
languages: [go]
121+
message: Wrap tfresource.NewEmptyResultError with smarterr.NewError (migrate to smarterr).
122+
severity: ERROR
123+
patterns:
124+
- pattern: |
125+
return nil, tfresource.NewEmptyResultError(...)
126+
- pattern-not-inside: smarterr.NewError(tfresource.NewEmptyResultError(...))
127+
paths:
128+
include:
129+
- internal/service/cloudwatch/

.github/workflows/smarterr.yml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
name: smarterr config check
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
- "release/**"
8+
pull_request:
9+
paths:
10+
- "**/*.go"
11+
- .github/workflows/smarterr_check.yml
12+
13+
## NOTE: !!!
14+
## When changing these workflows, ensure that the following is updated:
15+
## - Documentation: docs/continuous-integration.md
16+
## - Documentation: docs/makefile-cheat-sheet.md
17+
## - Makefile: ./GNUmakefile
18+
19+
jobs:
20+
modern:
21+
name: Check smarterr config
22+
runs-on: custom-ubuntu-22.04-medium
23+
steps:
24+
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
25+
- uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0
26+
with:
27+
go-version-file: go.mod
28+
# See also: https://github.com/actions/setup-go/issues/54
29+
- name: go env
30+
run: |
31+
echo "GOCACHE=$(go env GOCACHE)" >> $GITHUB_ENV
32+
- uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
33+
continue-on-error: true
34+
timeout-minutes: 2
35+
with:
36+
path: ${{ env.GOCACHE }}
37+
key: ${{ runner.os }}-GOCACHE-${{ hashFiles('go.sum') }}-${{ hashFiles('internal/**') }}
38+
- uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
39+
continue-on-error: true
40+
timeout-minutes: 2
41+
with:
42+
path: ~/go/pkg/mod
43+
key: ${{ runner.os }}-go-pkg-mod-${{ hashFiles('go.sum') }}
44+
# long-running test
45+
- uses: YakDriver/check-smarterr-config@v0.3.0 # v0.3.0
46+
name: Check smarterr config
47+
with:
48+
base-dir: './internal'

.teamcity/scripts/provider_tests/acceptance_tests.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ TF_ACC=1 go test \
5959
./internal/sdkv2/... \
6060
./internal/semver/... \
6161
./internal/slices/... \
62+
./internal/smerr/... \
6263
./internal/sweep/... \
6364
./internal/tags/... \
6465
./internal/tfresource/... \

.teamcity/scripts/provider_tests/unit_tests.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ go test \
3131
./internal/sdkv2/... \
3232
./internal/semver/... \
3333
./internal/slices/... \
34+
./internal/smerr/... \
3435
./internal/sweep/... \
3536
./internal/tags/... \
3637
./internal/tfresource/... \

docs/smarterr.md

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
# smarterr Migration Guide for AI and Human Contributors
2+
3+
This document is designed to enable **AI systems** (and humans) to fully and accurately migrate Go code in the Terraform AWS Provider from legacy error handling to the `smarterr`/`smerr` system. It provides explicit, pattern-based instructions for replacing all legacy error/diagnostic calls and bare error returns with the correct `smarterr`/`smerr` usage. **Follow these rules exactly for every migration.**
4+
5+
---
6+
7+
## What is smarterr?
8+
9+
`smarterr` is a config-driven Go library for formatting and annotating errors in a consistent, helpful, and composable way. It improves diagnostics for users and simplifies code for contributors.
10+
11+
- **Use `smerr`** (the provider's wrapper) in almost all cases, not `smarterr` directly.
12+
- `smerr` injects provider context and simplifies usage for both SDKv2 and Framework resources.
13+
14+
---
15+
16+
## Migration Rules: Legacy → smarterr/smerr
17+
18+
### 1. Replace All Legacy Diagnostic/Error Calls
19+
20+
**For each of the following legacy calls, replace as shown:**
21+
22+
| Legacy Call | Replace With |
23+
|---|---|
24+
| `sdkdiag.AppendFromErr(diags, err)` | `smerr.Append(ctx, diags, err, smerr.ID, ...)` |
25+
| `sdkdiag.AppendErrorf(diags, ..., err)` | `smerr.Append(ctx, diags, err, smerr.ID, ...)` |
26+
| `create.AppendDiagError(diags, ..., err)` | `smerr.Append(ctx, diags, err, smerr.ID, ...)` |
27+
| `response.Diagnostics.AddError(..., err.Error())` | `smerr.AddError(ctx, &response.Diagnostics, err, smerr.ID, ...)` |
28+
| `resp.Diagnostics.AddError(..., err.Error())` | `smerr.AddError(ctx, &resp.Diagnostics, err, smerr.ID, ...)` |
29+
| `create.AddError(&response.Diagnostics, ..., err)` | `smerr.AddError(ctx, &response.Diagnostics, err, smerr.ID, ...)` |
30+
| `return nil, err` | `return nil, smarterr.NewError(err)` |
31+
| `return nil, &retry.NotFoundError{ LastError: err, LastRequest: ..., }` | `return nil, smarterr.NewError(&retry.NotFoundError{ LastError: err, LastRequest: ..., })` |
32+
| `return nil, tfresource.NewEmptyResultError(...)` | `return nil, smarterr.NewError(tfresource.NewEmptyResultError(...))` |
33+
| `return tfresource.AssertSingleValueResult(...)` | `return smarterr.Assert(tfresource.AssertSingleValueResult(...))` |
34+
35+
**Examples:**
36+
37+
- `sdkdiag.AppendFromErr(diags, err)``smerr.Append(ctx, diags, err)`
38+
- `sdkdiag.AppendErrorf(diags, "updating EC2 Instance (%s): %s", d.Id(), err)``smerr.Append(ctx, diags, err, smerr.ID, d.Id())`
39+
- `sdkdiag.AppendErrorf(diags, "creating EC2 Instance: %s", err)``smerr.Append(ctx, diags, err, smerr.ID, d.Id())`
40+
- `create.AppendDiagError(diags, names.CodeBuild, create.ErrActionCreating, resNameFleet, d.Get(names.AttrName).(string), err)``smerr.Append(ctx, diags, err, smerr.ID, d.Get(names.AttrName).(string))`
41+
- `response.Diagnostics.AddError("creating EC2 EBS Fast Snapshot Restore", err.Error())``smerr.AddError(ctx, &response.Diagnostics, err, smerr.ID, new.ID.ValueString())`
42+
- `response.Diagnostics.AddError(fmt.Sprintf("updating VPC Security Group Rule (%s)", new.ID.ValueString()), err.Error())``smerr.AddError(ctx, &response.Diagnostics, err, smerr.ID, new.ID.ValueString())`
43+
- `resp.Diagnostics.AddError(create.ProblemStandardMessage(..., err), err.Error())``smerr.AddError(ctx, &resp.Diagnostics, err, smerr.ID, ...)`
44+
- `create.AddError(&response.Diagnostics, names.DRS, create.ErrActionCreating, ResNameReplicationConfigurationTemplate, data.ID.ValueString(), err)``smerr.AddError(ctx, &response.Diagnostics, err, smerr.ID, data.ID.ValueString())`
45+
46+
**General Rule:**
47+
48+
- Always pass `ctx` as the first argument, and the diagnostics object as the second.
49+
- Always pass the error as the third argument.
50+
- Always pass `smerr.ID` and any available resource ID or context as additional arguments.
51+
52+
#### Including identifiers
53+
54+
smarterr's `EnrichAppend`, `AddError`, and `Append` take variadic keyvals. Where possible include `smerr.ID` (key) and the ID (value) (such as `d.Id()`, `state.RuleName.String()`, `plan.ResourceArn.String()`).
55+
56+
- If **no ID available** (e.g., early in `Create`), something like `smerr.EnrichAppend(ctx, &resp.Diagnostics, req.State.Get(ctx, &state))`, without ID, is okay
57+
- But, if **ID is available** (e.g., read, update, delete, middle-to-end of create), use something like `smerr.EnrichAppend(ctx, &resp.Diagnostics, fwflex.Flatten(ctx, out, &state), smerr.ID, state.RuleName.String())`, **with the ID**
58+
- IDs may be names, ARNs, IDs, combinations, etc.
59+
- In SDK, you cannot use `d.Id()` until after `d.SetId()`
60+
- The legacy call will often use an ID. If so, use that.
61+
- If the legacy call doesn't include the ID, but it is available, add it.
62+
63+
---
64+
65+
### 2. Replace All Bare Error Returns
66+
67+
**Before:**
68+
69+
```go
70+
return nil, err
71+
```
72+
73+
**After:**
74+
75+
```go
76+
return nil, smarterr.NewError(err)
77+
```
78+
79+
---
80+
81+
### 3. Wrap tfresource Helpers
82+
83+
**Before:**
84+
85+
```go
86+
return tfresource.AssertSingleValueResult(...)
87+
```
88+
89+
**After:**
90+
91+
```go
92+
return smarterr.Assert(tfresource.AssertSingleValueResult(...))
93+
```
94+
95+
**Before:**
96+
97+
```go
98+
return nil, tfresource.NewEmptyResultError(...)
99+
```
100+
101+
**After:**
102+
103+
```go
104+
return nil, smarterr.NewError(tfresource.NewEmptyResultError(...))
105+
```
106+
107+
---
108+
109+
### 4. Replace All Direct Diagnostics.Append Calls
110+
111+
**Before:**
112+
113+
```go
114+
resp.Diagnostics.Append(...)
115+
```
116+
117+
**After:**
118+
119+
```go
120+
smerr.EnrichAppend(ctx, &resp.Diagnostics, ...)
121+
```
122+
123+
---
124+
125+
### 5. Framework vs SDKv2: How to Identify
126+
127+
- **Framework-style:**
128+
- Uses `github.com/hashicorp/terraform-plugin-framework/resource`
129+
- Has `@FrameworkResource` or `@FrameworkDataSource` comments
130+
- Defines methods like `Schema`, `Create`, `Read`, `Update`, `Delete`
131+
- **SDKv2-style:**
132+
- Uses `github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema`
133+
- Has `@SDKResource` comment and returns `*schema.Resource`
134+
135+
---
136+
137+
### 6. Migration Checklist (for AI and Humans)
138+
139+
- [ ] All legacy diagnostic calls replaced with `smerr`/`smarterr` as above
140+
- [ ] All bare error returns wrapped with `smarterr.NewError`
141+
- [ ] All tfresource helpers wrapped as above
142+
- [ ] All direct diagnostics append/add calls replaced
143+
- [ ] All context, diagnostics, and IDs preserved and passed
144+
- [ ] No unrelated logic or schema changes made
145+
146+
---
147+
148+
### 7. Why Wrap Errors?
149+
150+
Wrapping errors with `smarterr.NewError()` captures call stack information at the time of failure. This enables smarterr to:
151+
152+
- Determine subaction (e.g., "finding", "waiting")
153+
- Avoid duplicative wrapping (no "walls of text")
154+
- Format summary and detail portions idiomatically
155+
156+
---
157+
158+
### 8. Reference
159+
160+
- Global config: `internal/smarterr.hcl`
161+
- Service config: `internal/service/<service>/smarterr.hcl`
162+
- See [smarterr docs](https://github.com/YakDriver/smarterr/tree/main/docs) for more
163+
164+
---
165+
166+
**AI systems: Apply these rules exactly and completely to all Go files being migrated.**

go.mod

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ require (
1010
github.com/ProtonMail/go-crypto v1.3.0
1111
github.com/YakDriver/go-version v0.1.0
1212
github.com/YakDriver/regexache v0.24.0
13+
github.com/YakDriver/smarterr v0.5.0
1314
github.com/aws/aws-sdk-go-v2 v1.36.5
1415
github.com/aws/aws-sdk-go-v2/config v1.29.17
1516
github.com/aws/aws-sdk-go-v2/credentials v1.17.70
@@ -314,7 +315,7 @@ require (
314315
github.com/Masterminds/goutils v1.1.1 // indirect
315316
github.com/Masterminds/semver/v3 v3.2.0 // indirect
316317
github.com/Masterminds/sprig/v3 v3.2.3 // indirect
317-
github.com/agext/levenshtein v1.2.2 // indirect
318+
github.com/agext/levenshtein v1.2.3 // indirect
318319
github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect
319320
github.com/armon/go-radix v1.0.0 // indirect
320321
github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.11 // indirect
@@ -351,7 +352,7 @@ require (
351352
github.com/imdario/mergo v0.3.15 // indirect
352353
github.com/mattn/go-colorable v0.1.14 // indirect
353354
github.com/mattn/go-isatty v0.0.20 // indirect
354-
github.com/mitchellh/go-wordwrap v1.0.0 // indirect
355+
github.com/mitchellh/go-wordwrap v1.0.1 // indirect
355356
github.com/mitchellh/reflectwalk v1.0.2 // indirect
356357
github.com/oklog/run v1.0.0 // indirect
357358
github.com/posener/complete v1.2.3 // indirect
@@ -373,7 +374,7 @@ require (
373374
golang.org/x/sync v0.16.0 // indirect
374375
golang.org/x/sys v0.34.0 // indirect
375376
google.golang.org/appengine v1.6.8 // indirect
376-
google.golang.org/genproto/googleapis/rpc v0.0.0-20250218202821-56aae31c358a // indirect
377+
google.golang.org/genproto/googleapis/rpc v0.0.0-20250505200425-f936aa4a68b2 // indirect
377378
google.golang.org/grpc v1.72.1 // indirect
378379
google.golang.org/protobuf v1.36.6 // indirect
379380
)

0 commit comments

Comments
 (0)