Skip to content

Commit 0a72ef6

Browse files
gdavisongdavison
authored
Merge pull request #34526 from hashicorp/td-return-diags
tech debt: Collect diags instead of returning directly - `a` services
2 parents 173cf34 + c6acf37 commit 0a72ef6

Some content is hidden

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

49 files changed

+812
-503
lines changed

‎.ci/semgrep/pluginsdk/diags.yml

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ rules:
55
message: Prefer `sdkdiag.AppendFromErr` to `diag.FromErr`
66
paths:
77
exclude:
8-
- internal/service
8+
- internal/service/[b-z]*
99
patterns:
1010
- pattern: diag.FromErr($ERR)
1111
severity: WARNING
@@ -18,7 +18,46 @@ rules:
1818
message: Prefer `sdkdiag.AppendErrorf` to `diag.Errorf`
1919
paths:
2020
exclude:
21-
- internal/service
21+
- internal/service/[b-z]*
2222
patterns:
2323
- pattern: diag.Errorf(...)
2424
severity: WARNING
25+
26+
- id: append-Read-to-diags
27+
fix-regex:
28+
regex: return (resource\w+Read\(.*\))
29+
replacement: return append(diags, \1...)
30+
languages: [go]
31+
message: Append results of $READFN to diags instead of returning directly
32+
paths:
33+
exclude:
34+
- internal/service/[b-z]*
35+
patterns:
36+
- pattern: return $READFN(...)
37+
- metavariable-regex:
38+
metavariable: "$READFN"
39+
regex: resource\w+Read
40+
severity: WARNING
41+
42+
- id: return-diags-not-nil
43+
fix-regex:
44+
regex: return nil
45+
replacement: return diags
46+
languages: [go]
47+
message: Return diags instead of nil
48+
paths:
49+
include:
50+
- internal/service
51+
exclude:
52+
- internal/service/[b-z]*
53+
patterns:
54+
- pattern: return nil
55+
- pattern-not-inside: |
56+
func(...) {
57+
...
58+
}
59+
- pattern-inside: |
60+
func $F(...) diag.Diagnostics {
61+
...
62+
}
63+
severity: WARNING

‎docs/error-handling.md

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -257,21 +257,25 @@ To prevent this type of Terraform CLI error, the resource implementation should
257257
In the Terraform AWS Provider, an initial fix for the Terraform CLI error will typically look like:
258258
259259
```go
260-
func resourceServiceThingCreate(d *schema.ResourceData, meta interface{}) error {
261-
/* ... */
260+
func resourceServiceThingCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
261+
var diags diag.Diagnostics
262+
263+
/* ... */
262264
263-
return resourceServiceThingRead(d, meta)
265+
return append(diags, resourceServiceThingRead(ctx, d, meta)...)
264266
}
265267
266-
func resourceServiceThingRead(d *schema.ResourceData, meta interface{}) error {
267-
/* ... */
268+
func resourceServiceThingRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
269+
var diags diag.Diagnostics
270+
271+
/* ... */
268272
269273
output, err := conn.DescribeServiceThing(input)
270274
271275
if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, "ResourceNotFoundException") {
272276
log.Printf("[WARN] {Service} {Thing} (%s) not found, removing from state", d.Id())
273277
d.SetId("")
274-
return nil
278+
return diags
275279
}
276280
277281
if err != nil {

‎docs/retries-and-waiters.md

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -439,12 +439,15 @@ const (
439439
```go
440440
// internal/service/{service}/{thing}.go
441441

442-
function ExampleThingCreate(d *schema.ResourceData, meta interface{}) error {
442+
function ExampleThingCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
443+
var diags diag.Diagnostics
443444
// ...
444-
return ExampleThingRead(d, meta)
445+
return append(diags, ExampleThingRead(ctx, d, meta)...)
445446
}
446447

447-
function ExampleThingRead(d *schema.ResourceData, meta interface{}) error {
448+
function ExampleThingRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
449+
var diags diag.Diagnostics
450+
448451
conn := meta.(*AWSClient).ExampleConn()
449452

450453
input := &example.OperationInput{/* ... */}
@@ -476,16 +479,16 @@ const (
476479
if !d.IsNewResource() && tfawserr.ErrorCodeEquals(err, example.ErrCodeNoSuchEntityException) {
477480
log.Printf("[WARN] Example Thing (%s) not found, removing from state", d.Id())
478481
d.SetId("")
479-
return nil
482+
return diags
480483
}
481484

482485
if err != nil {
483-
return fmt.Errorf("reading Example Thing (%s): %w", d.Id(), err)
486+
return sdkdiag.AppendErrorf(diags, "reading Example Thing (%s): %w", d.Id(), err)
484487
}
485488

486489
// Prevent panics.
487490
if output == nil {
488-
return fmt.Errorf("reading Example Thing (%s): empty response", d.Id())
491+
return sdkdiag.AppendErrorf(diags, "reading Example Thing (%s): empty response", d.Id())
489492
}
490493

491494
// ... refresh Terraform state as normal ...
@@ -770,13 +773,15 @@ func waitThingDeleted(ctx context.Context, conn *example.Example, id string, tim
770773
=== "Terraform Plugin SDK V2"
771774
```go
772775
func resourceThingCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
776+
var diags diag.Diagnostics
777+
773778
// ... AWS Go SDK logic to create resource ...
774779

775780
if _, err := waitThingCreated(ctx, conn, d.Id(), d.Timeout(schema.TimeoutCreate)) {
776781
return append(diags, create.DiagError(names.Example, create.ErrActionWaitingForCreation, ResNameThing, d.Id(), err)...)
777782
}
778783

779-
return ExampleThingRead(d, meta)
784+
return append(diags, ExampleThingRead(ctx, d, meta)...)
780785
}
781786

782787
func resourceThingDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {

‎internal/generate/tagresource/resource.tmpl

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
1111
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1212
"github.com/hashicorp/terraform-provider-aws/internal/conns"
13+
"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
1314
tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
1415
"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
1516
)
@@ -45,7 +46,7 @@ func ResourceTag() *schema.Resource {
4546
}
4647
}
4748

48-
func resourceTagCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { {{ if not (eq .ServicePackage "ec2") }}// nosemgrep:ci.semgrep.tags.calling-UpdateTags-in-resource-create{{- end }}
49+
func resourceTagCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) (diags diag.Diagnostics) { {{ if not (eq .ServicePackage "ec2") }}// nosemgrep:ci.semgrep.tags.calling-UpdateTags-in-resource-create{{- end }}
4950
conn := meta.(*conns.AWSClient).{{ .AWSServiceUpper }}Conn(ctx)
5051

5152
identifier := d.Get("{{ .IDAttribName }}").(string)
@@ -57,67 +58,67 @@ func resourceTagCreate(ctx context.Context, d *schema.ResourceData, meta interfa
5758
{{- else }}
5859
if err := {{ .UpdateTagsFunc }}(ctx, conn, identifier, nil, map[string]string{key: value}); err != nil {
5960
{{- end }}
60-
return diag.Errorf("creating %s resource (%s) tag (%s): %s", {{ .ServicePackage }}.ServiceID, identifier, key, err)
61+
return sdkdiag.AppendErrorf(diags, "creating %s resource (%s) tag (%s): %s", {{ .ServicePackage }}.ServiceID, identifier, key, err)
6162
}
6263

6364
d.SetId(tftags.SetResourceID(identifier, key))
6465

65-
return resourceTagRead(ctx, d, meta)
66+
return append(diags, resourceTagRead(ctx, d, meta)...)
6667
}
6768

68-
func resourceTagRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
69+
func resourceTagRead(ctx context.Context, d *schema.ResourceData, meta interface{}) (diags diag.Diagnostics) {
6970
conn := meta.(*conns.AWSClient).{{ .AWSServiceUpper }}Conn(ctx)
7071
identifier, key, err := tftags.GetResourceID(d.Id())
7172

7273
if err != nil {
73-
return diag.FromErr(err)
74+
return sdkdiag.AppendFromErr(diags, err)
7475
}
7576

7677
value, err := {{ .GetTagFunc }}(ctx, conn, identifier, key)
7778

7879
if !d.IsNewResource() && tfresource.NotFound(err) {
7980
log.Printf("[WARN] %s resource (%s) tag (%s) not found, removing from state", {{ .ServicePackage }}.ServiceID, identifier, key)
8081
d.SetId("")
81-
return nil
82+
return diags
8283
}
8384

8485
if err != nil {
85-
return diag.Errorf("reading %s resource (%s) tag (%s): %s", {{ .ServicePackage }}.ServiceID, identifier, key, err)
86+
return sdkdiag.AppendErrorf(diags, "reading %s resource (%s) tag (%s): %s", {{ .ServicePackage }}.ServiceID, identifier, key, err)
8687
}
8788

8889
d.Set("{{ .IDAttribName }}", identifier)
8990
d.Set("key", key)
9091
d.Set("value", value)
9192

92-
return nil
93+
return diags
9394
}
9495

95-
func resourceTagUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
96+
func resourceTagUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) (diags diag.Diagnostics) {
9697
conn := meta.(*conns.AWSClient).{{ .AWSServiceUpper }}Conn(ctx)
9798
identifier, key, err := tftags.GetResourceID(d.Id())
9899

99100
if err != nil {
100-
return diag.FromErr(err)
101+
return sdkdiag.AppendFromErr(diags, err)
101102
}
102103

103104
if err := {{ .UpdateTagsFunc }}(ctx, conn, identifier, nil, map[string]string{key: d.Get("value").(string)}); err != nil {
104-
return diag.Errorf("updating %s resource (%s) tag (%s): %s", {{ .ServicePackage }}.ServiceID, identifier, key, err)
105+
return sdkdiag.AppendErrorf(diags, "updating %s resource (%s) tag (%s): %s", {{ .ServicePackage }}.ServiceID, identifier, key, err)
105106
}
106107

107-
return resourceTagRead(ctx, d, meta)
108+
return append(diags, resourceTagRead(ctx, d, meta)...)
108109
}
109110

110-
func resourceTagDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
111+
func resourceTagDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) (diags diag.Diagnostics) {
111112
conn := meta.(*conns.AWSClient).{{ .AWSServiceUpper }}Conn(ctx)
112113
identifier, key, err := tftags.GetResourceID(d.Id())
113114

114115
if err != nil {
115-
return diag.FromErr(err)
116+
return sdkdiag.AppendFromErr(diags, err)
116117
}
117118

118119
if err := {{ .UpdateTagsFunc }}(ctx, conn, identifier, map[string]string{key: d.Get("value").(string)}, nil); err != nil {
119-
return diag.Errorf("deleting %s resource (%s) tag (%s): %s", {{ .ServicePackage }}.ServiceID, identifier, key, err)
120+
return sdkdiag.AppendErrorf(diags, "deleting %s resource (%s) tag (%s): %s", {{ .ServicePackage }}.ServiceID, identifier, key, err)
120121
}
121122

122-
return nil
123+
return diags
123124
}

‎internal/service/accessanalyzer/archive_rule.go

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
2020
"github.com/hashicorp/terraform-provider-aws/internal/conns"
2121
"github.com/hashicorp/terraform-provider-aws/internal/errs"
22+
"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
2223
"github.com/hashicorp/terraform-provider-aws/internal/flex"
2324
"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
2425
"github.com/hashicorp/terraform-provider-aws/internal/types/nullable"
@@ -88,6 +89,8 @@ func resourceArchiveRule() *schema.Resource {
8889
}
8990

9091
func resourceArchiveRuleCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
92+
var diags diag.Diagnostics
93+
9194
conn := meta.(*conns.AWSClient).AccessAnalyzerClient(ctx)
9295

9396
analyzerName := d.Get("analyzer_name").(string)
@@ -106,49 +109,53 @@ func resourceArchiveRuleCreate(ctx context.Context, d *schema.ResourceData, meta
106109
_, err := conn.CreateArchiveRule(ctx, input)
107110

108111
if err != nil {
109-
return diag.Errorf("creating IAM Access Analyzer Archive Rule (%s): %s", id, err)
112+
return sdkdiag.AppendErrorf(diags, "creating IAM Access Analyzer Archive Rule (%s): %s", id, err)
110113
}
111114

112115
d.SetId(id)
113116

114-
return resourceArchiveRuleRead(ctx, d, meta)
117+
return append(diags, resourceArchiveRuleRead(ctx, d, meta)...)
115118
}
116119

117120
func resourceArchiveRuleRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
121+
var diags diag.Diagnostics
122+
118123
conn := meta.(*conns.AWSClient).AccessAnalyzerClient(ctx)
119124

120125
analyzerName, ruleName, err := archiveRuleParseResourceID(d.Id())
121126

122127
if err != nil {
123-
return diag.FromErr(err)
128+
return sdkdiag.AppendFromErr(diags, err)
124129
}
125130

126131
archiveRule, err := findArchiveRuleByTwoPartKey(ctx, conn, analyzerName, ruleName)
127132

128133
if !d.IsNewResource() && tfresource.NotFound(err) {
129134
log.Printf("[WARN] IAM Access Analyzer Archive Rule (%s) not found, removing from state", d.Id())
130135
d.SetId("")
131-
return nil
136+
return diags
132137
}
133138

134139
if err != nil {
135-
return diag.Errorf("reading IAM Access Analyzer Archive Rule (%s): %s", d.Id(), err)
140+
return sdkdiag.AppendErrorf(diags, "reading IAM Access Analyzer Archive Rule (%s): %s", d.Id(), err)
136141
}
137142

138143
d.Set("analyzer_name", analyzerName)
139144
d.Set("filter", flattenFilter(archiveRule.Filter))
140145
d.Set("rule_name", archiveRule.RuleName)
141146

142-
return nil
147+
return diags
143148
}
144149

145150
func resourceArchiveRuleUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
151+
var diags diag.Diagnostics
152+
146153
conn := meta.(*conns.AWSClient).AccessAnalyzerClient(ctx)
147154

148155
analyzerName, ruleName, err := archiveRuleParseResourceID(d.Id())
149156

150157
if err != nil {
151-
return diag.FromErr(err)
158+
return sdkdiag.AppendFromErr(diags, err)
152159
}
153160

154161
input := &accessanalyzer.UpdateArchiveRuleInput{
@@ -164,19 +171,21 @@ func resourceArchiveRuleUpdate(ctx context.Context, d *schema.ResourceData, meta
164171
_, err = conn.UpdateArchiveRule(ctx, input)
165172

166173
if err != nil {
167-
return diag.Errorf("updating AWS IAM Access Analyzer Archive Rule (%s): %s", d.Id(), err)
174+
return sdkdiag.AppendErrorf(diags, "updating AWS IAM Access Analyzer Archive Rule (%s): %s", d.Id(), err)
168175
}
169176

170-
return resourceArchiveRuleRead(ctx, d, meta)
177+
return append(diags, resourceArchiveRuleRead(ctx, d, meta)...)
171178
}
172179

173180
func resourceArchiveRuleDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
181+
var diags diag.Diagnostics
182+
174183
conn := meta.(*conns.AWSClient).AccessAnalyzerClient(ctx)
175184

176185
analyzerName, ruleName, err := archiveRuleParseResourceID(d.Id())
177186

178187
if err != nil {
179-
return diag.FromErr(err)
188+
return sdkdiag.AppendFromErr(diags, err)
180189
}
181190

182191
log.Printf("[INFO] Deleting IAM Access Analyzer Archive Rule: %s", d.Id())
@@ -187,14 +196,14 @@ func resourceArchiveRuleDelete(ctx context.Context, d *schema.ResourceData, meta
187196
})
188197

189198
if errs.IsA[*types.ResourceNotFoundException](err) {
190-
return nil
199+
return diags
191200
}
192201

193202
if err != nil {
194-
return diag.Errorf("deleting IAM Access Analyzer Archive Rule (%s): %s", d.Id(), err)
203+
return sdkdiag.AppendErrorf(diags, "deleting IAM Access Analyzer Archive Rule (%s): %s", d.Id(), err)
195204
}
196205

197-
return nil
206+
return diags
198207
}
199208

200209
func findArchiveRuleByTwoPartKey(ctx context.Context, conn *accessanalyzer.Client, analyzerName, ruleName string) (*types.ArchiveRuleSummary, error) {

0 commit comments

Comments
 (0)