[Bug]: Fix unreturned sdkdiag.AppendErrorf function calls
#38854
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Community NoteVoting for Prioritization
For Submitters
|
github-actions
bot
added
service/s3
jar-b
force-pushed
the
td-appenderrorf-return
branch
from
b570811
to
d380208
Compare
jar-b
changed the title
sdkdiag.AppendErrorf function callssdkdiag.AppendErrorf function calls
ewbankkit
approved these changes
Aug 14, 2024
|
Thank you for your contribution! 🚀 A new usage of AWS SDK for Go V1 was detected. Please prefer AWS SDK for Go V2 for all net-new services. If this is an enhancement or bug fix to an existing AWS SDK Go V1 based resource, this comment can be safely ignored. For additional information refer to the AWS SDK for Go Versions page in the contributor guide. |
|
Added a new rule. Running against % semgrep --config .ci/.semgrep.yml ./internal/service/
<snip>
internal/service/s3/bucket_lifecycle_configuration.go
❯❯❱ ci.unreturned-sdkdiag-AppendErrorf
Calls to `sdkdiag.AppendErrorf()` should be returned or set to the `diags` variable
292┆ if err != nil {
293┆ sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket Lifecycle Configuration (%s) create:
%s", d.Id(), err)
294┆ }
⋮┆----------------------------------------
390┆ if err != nil {
391┆ sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket Lifecycle Configuration (%s) update:
%s", d.Id(), err)
392┆ }
┌──────────────┐
│ Scan Summary │
└──────────────┘
Some files were skipped or only partially analyzed.
Scan was limited to files tracked by git.
Partially scanned: 8 files only partially analyzed due to parsing or internal Semgrep errors
Ran 34 rules on 6254 files: 12 findings.The same rule run on this branch produces no findings. |
```console % make testacc PKG=acm TESTS=TestAccACMCertificateDataSource_ make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.22.6 test ./internal/service/acm/... -v -count 1 -parallel 20 -run='TestAccACMCertificateDataSource_' -timeout 360m --- PASS: TestAccACMCertificateDataSource_tags (17.25s) --- PASS: TestAccACMCertificateDataSource_keyTypes (18.50s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/acm 24.817s ```
```console % make testacc PKG=appstream TESTS=TestAccAppStreamStack_ make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.22.6 test ./internal/service/appstream/... -v -count 1 -parallel 20 -run='TestAccAppStreamStack_' -timeout 360m --- PASS: TestAccAppStreamStack_disappears (33.89s) --- PASS: TestAccAppStreamStack_applicationSettings_removeFromDisabled (41.20s) --- PASS: TestAccAppStreamStack_basic (54.97s) --- PASS: TestAccAppStreamStack_applicationSettings_removeFromEnabled (55.92s) --- PASS: TestAccAppStreamStack_withTags (56.27s) --- PASS: TestAccAppStreamStack_complete (56.69s) --- PASS: TestAccAppStreamStack_streamingExperienceSettings_basic (70.81s) --- PASS: TestAccAppStreamStack_applicationSettings_basic (71.70s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/appstream 81.569s ```
…rrorf` calls
```console
% make testacc PKG=ecs TESTS=TestAccECSClusterCapacityProviders_
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.6 test ./internal/service/ecs/... -v -count 1 -parallel 20 -run='TestAccECSClusterCapacityProviders_' -timeout 360m
=== NAME TestAccECSClusterCapacityProviders_destroy
cluster_capacity_providers_test.go:120: Step 1/2 error: Check failed: Check 2/2 error: RegisteredContainerInstancesCount = 0, want 2
--- PASS: TestAccECSClusterCapacityProviders_defaults (47.98s)
--- PASS: TestAccECSClusterCapacityProviders_basic (48.20s)
--- PASS: TestAccECSClusterCapacityProviders_disappears (56.22s)
--- PASS: TestAccECSClusterCapacityProviders_Update_defaultStrategy (113.11s)
--- PASS: TestAccECSClusterCapacityProviders_Update_capacityProviders (113.13s)
--- FAIL: TestAccECSClusterCapacityProviders_destroy (169.06s)
FAIL
FAIL github.com/hashicorp/terraform-provider-aws/internal/service/ecs 179.063s
```
Failure unrelated to this change.
```console % make testacc PKG=rolesanywhere TESTS=TestAccRolesAnywhereProfile_ make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.22.6 test ./internal/service/rolesanywhere/... -v -count 1 -parallel 20 -run='TestAccRolesAnywhereProfile_' -timeout 360m --- PASS: TestAccRolesAnywhereProfile_disappears (16.78s) --- PASS: TestAccRolesAnywhereProfile_enabled (46.73s) --- PASS: TestAccRolesAnywhereProfile_tags (51.98s) --- PASS: TestAccRolesAnywhereProfile_basic (60.46s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/rolesanywhere 66.542s ```
…ndErrorf` calls
```console
% make testacc PKG=s3 TESTS=TestAccS3BucketLifecycleConfiguration_
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.6 test ./internal/service/s3/... -v -count 1 -parallel 20 -run='TestAccS3BucketLifecycleConfiguration_' -timeout 360m
=== NAME TestAccS3BucketLifecycleConfiguration_directoryBucket
bucket_lifecycle_configuration_test.go:1056: Step 1/1, expected an error with pattern, no match on: Error running apply: exit status 1
Error: creating S3 Bucket (tf-acc-test-503241382809497100--use1-az6--x-s3) Lifecycle Configuration: operation error S3: PutBucketLifecycleConfiguration, https response error StatusCode: 0, RequestID: , HostID: , request send failed, Put "https://s3express-control.aws-global.amazonaws.com/tf-acc-test-503241382809497100--use1-az6--x-s3?lifecycle=": dial tcp: lookup s3express-control.aws-global.amazonaws.com: no such host
with aws_s3_bucket_lifecycle_configuration.test,
on terraform_plugin_test.tf line 35, in resource "aws_s3_bucket_lifecycle_configuration" "test":
35: resource "aws_s3_bucket_lifecycle_configuration" "test" {
--- FAIL: TestAccS3BucketLifecycleConfiguration_directoryBucket (13.50s)
=== CONT TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeGreaterThanZero
--- PASS: TestAccS3BucketLifecycleConfiguration_migrate_noChange (71.76s)
=== CONT TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeRange
--- PASS: TestAccS3BucketLifecycleConfiguration_migrate_withChange (71.80s)
=== CONT TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeLessThan
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionStorageClassOnly_intelligentTiering (76.54s)
=== CONT TestAccS3BucketLifecycleConfiguration_filterWithPrefix
--- PASS: TestAccS3BucketLifecycleConfiguration_EmptyFilter_NonCurrentVersions (86.73s)
=== CONT TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeGreaterThan
--- PASS: TestAccS3BucketLifecycleConfiguration_RuleExpiration_emptyBlock (87.53s)
=== CONT TestAccS3BucketLifecycleConfiguration_multipleRules
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeRangeAndPrefix (87.60s)
=== CONT TestAccS3BucketLifecycleConfiguration_disappears
--- PASS: TestAccS3BucketLifecycleConfiguration_prefix (87.60s)
=== CONT TestAccS3BucketLifecycleConfiguration_disableRule
--- PASS: TestAccS3BucketLifecycleConfiguration_basic (87.64s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_Tag (87.64s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeGreaterThanZero (74.16s)
--- PASS: TestAccS3BucketLifecycleConfiguration_nonCurrentVersionExpiration (87.69s)
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionDate_standardIa (87.72s)
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionZeroDays_intelligentTiering (87.74s)
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionDate_intelligentTiering (87.76s)
--- PASS: TestAccS3BucketLifecycleConfiguration_multipleRules_noFilterOrPrefix (87.78s)
--- PASS: TestAccS3BucketLifecycleConfiguration_nonCurrentVersionTransition (88.18s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Update_filterWithAndToFilterWithPrefix (130.30s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeRange (64.26s)
--- PASS: TestAccS3BucketLifecycleConfiguration_disappears (48.44s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeLessThan (64.29s)
--- PASS: TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeGreaterThan (65.50s)
--- PASS: TestAccS3BucketLifecycleConfiguration_multipleRules (66.14s)
--- PASS: TestAccS3BucketLifecycleConfiguration_ruleAbortIncompleteMultipartUpload (158.96s)
--- PASS: TestAccS3BucketLifecycleConfiguration_RuleExpiration_expireMarkerOnly (159.03s)
--- PASS: TestAccS3BucketLifecycleConfiguration_filterWithPrefix (136.76s)
--- PASS: TestAccS3BucketLifecycleConfiguration_TransitionUpdateBetweenDaysAndDate_intelligentTiering (214.88s)
--- PASS: TestAccS3BucketLifecycleConfiguration_disableRule (184.62s)
FAIL
FAIL github.com/hashicorp/terraform-provider-aws/internal/service/s3 278.401s
```
Failure unrelated to this change.
Running this rule against `main` (without the fixes from this branch):
```console
% semgrep --config .ci/.semgrep.yml ./internal/service/
<snip>
internal/service/s3/bucket_lifecycle_configuration.go
❯❯❱ ci.unreturned-sdkdiag-AppendErrorf
Calls to `sdkdiag.AppendErrorf()` should be returned or set to the `diags` variable
292┆ if err != nil {
293┆ sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket Lifecycle Configuration (%s) create:
%s", d.Id(), err)
294┆ }
⋮┆----------------------------------------
390┆ if err != nil {
391┆ sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket Lifecycle Configuration (%s) update:
%s", d.Id(), err)
392┆ }
┌──────────────┐
│ Scan Summary │
└──────────────┘
Some files were skipped or only partially analyzed.
Scan was limited to files tracked by git.
Partially scanned: 8 files only partially analyzed due to parsing or internal Semgrep errors
Ran 34 rules on 6254 files: 12 findings.
```
The same rule run on this branch produces no findings.
jar-b
force-pushed
the
td-appenderrorf-return
branch
from
34aa919
to
6ceb4aa
Compare
|
This functionality has been released in v5.63.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
github-actions
bot
removed
the
prioritized
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes various calls to the
sdkdiag.AppendErrorffunction which are neither returned nor re-assigned to thediagsvariable.Relations
Closes #38769
References
Output from Acceptance Testing
Failure unrelated to this change.
Failure unrelated to this change.