Merge pull request #38854 from hashicorp/td-appenderrorf-return

[Bug]: Fix unreturned `sdkdiag.AppendErrorf` function calls
hashicorp · Aug 15, 2024 · 7b440b8 · 7b440b8
2 parents eda6d6a + 6ceb4aa
commit 7b440b8
Show file tree

Hide file tree

Showing 10 changed files with 63 additions and 14 deletions.
diff --git a/.changelog/38854.txt b/.changelog/38854.txt
@@ -0,0 +1,24 @@
+```release-note:bug
+resource/aws_rolesanywhere_trust_anchor: Fix unreturned `sdkdiags.AppendErrorf` function calls
+```
+```release-note:bug
+resource/aws_rolesanywhere_profile: Fix unreturned `sdkdiags.AppendErrorf` function calls
+```
+```release-note:bug
+resource/aws_s3_bucket_lifecycle_configuration: Fix unreturned `sdkdiags.AppendErrorf` function calls
+```
+```release-note:bug
+resource/aws_ecs_cluster_capacity_providers: Fix unreturned `sdkdiags.AppendErrorf` function calls
+```
+```release-note:bug
+resource/aws_fms_policy: Fix unreturned `sdkdiags.AppendErrorf` function calls
+```
+```release-note:bug
+resource/aws_appstream_stack: Fix unreturned `sdkdiags.AppendErrorf` function calls
+```
+```release-note:bug
+resource/aws_controltower_landing_zone: Fix unreturned `sdkdiags.AppendErrorf` function calls
+```
+```release-note:bug
+data-source/aws_acm_certificate: Fix unreturned `sdkdiags.AppendErrorf` function calls
+```
diff --git a/.ci/.semgrep.yml b/.ci/.semgrep.yml
@@ -634,3 +634,28 @@ rules:
     patterns:
       - pattern-regex: "(Create|Read|Update|Delete)Context:"
     severity: ERROR
+
+  - id: unreturned-sdkdiag-AppendErrorf
+    languages: [go]
+    message: Calls to `sdkdiag.AppendErrorf()` should be returned or set to the `diags` variable
+    paths:
+      include:
+        - internal/
+    patterns:
+      - pattern: |
+          if err != nil {
+            sdkdiag.AppendErrorf($DIAGS, ...)
+          }
+      - pattern-not: |
+          if err != nil {
+            return sdkdiag.AppendErrorf($DIAGS, ...)
+          }
+      - pattern-not: |
+          if err != nil {
+            return ..., sdkdiag.AppendErrorf($DIAGS, ...)
+          }
+      - pattern-not: |
+          if err != nil {
+            $DIAGS = sdkdiag.AppendErrorf($DIAGS, ...)
+          }
+    severity: ERROR
diff --git a/internal/service/acm/certificate_data_source.go b/internal/service/acm/certificate_data_source.go
@@ -105,7 +105,7 @@ func dataSourceCertificateRead(ctx context.Context, d *schema.ResourceData, meta
 		},
 	)
 	if tfresource.NotFound(err) {
-		sdkdiag.AppendErrorf(diags, "XXX no ACM Certificate matching domain (%s)", domain)
+		return sdkdiag.AppendErrorf(diags, "XXX no ACM Certificate matching domain (%s)", domain)
 	} else if err != nil {
 		return sdkdiag.AppendErrorf(diags, "reading ACM Certificates: %s", err)
 	}

diff --git a/internal/service/appstream/stack.go b/internal/service/appstream/stack.go
@@ -409,7 +409,7 @@ func resourceStackUpdate(ctx context.Context, d *schema.ResourceData, meta inter
 		_, err := conn.UpdateStack(ctx, input)
 
 		if err != nil {
-			sdkdiag.AppendErrorf(diags, "updating Appstream Stack (%s): %s", d.Id(), err)
+			return sdkdiag.AppendErrorf(diags, "updating Appstream Stack (%s): %s", d.Id(), err)
 		}
 	}
 

diff --git a/internal/service/controltower/landing_zone.go b/internal/service/controltower/landing_zone.go
@@ -214,7 +214,7 @@ func resourceLandingZoneDelete(ctx context.Context, d *schema.ResourceData, meta
 	}
 
 	if _, err := waitLandingZoneOperationSucceeded(ctx, conn, aws.ToString(output.OperationIdentifier), d.Timeout(schema.TimeoutDelete)); err != nil {
-		sdkdiag.AppendErrorf(diags, "waiting for ControlTower Landing Zone (%s) delete: %s", d.Id(), err)
+		return sdkdiag.AppendErrorf(diags, "waiting for ControlTower Landing Zone (%s) delete: %s", d.Id(), err)
 	}
 
 	return diags

diff --git a/internal/service/ecs/cluster_capacity_providers.go b/internal/service/ecs/cluster_capacity_providers.go
@@ -115,7 +115,7 @@ func resourceClusterCapacityProvidersRead(ctx context.Context, d *schema.Resourc
 	cluster, err := findClusterByNameOrARN(ctx, conn, d.Id())
 
 	if !d.IsNewResource() && tfresource.NotFound(err) {
-		sdkdiag.AppendErrorf(diags, "[WARN] ECS Cluster Capacity Providers (%s) not found, removing from state", d.Id())
+		log.Printf("[WARN] ECS Cluster Capacity Providers (%s) not found, removing from state", d.Id())
 		d.SetId("")
 		return diags
 	}

diff --git a/internal/service/fms/policy.go b/internal/service/fms/policy.go
@@ -269,17 +269,17 @@ func resourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta interf
 	d.Set("delete_unused_fm_managed_resources", policy.DeleteUnusedFMManagedResources)
 	d.Set(names.AttrDescription, policy.PolicyDescription)
 	if err := d.Set("exclude_map", flattenPolicyMap(policy.ExcludeMap)); err != nil {
-		sdkdiag.AppendErrorf(diags, "setting exclude_map: %s", err)
+		diags = sdkdiag.AppendErrorf(diags, "setting exclude_map: %s", err)
 	}
 	d.Set("exclude_resource_tags", policy.ExcludeResourceTags)
 	if err := d.Set("include_map", flattenPolicyMap(policy.IncludeMap)); err != nil {
-		sdkdiag.AppendErrorf(diags, "setting include_map: %s", err)
+		diags = sdkdiag.AppendErrorf(diags, "setting include_map: %s", err)
 	}
 	d.Set(names.AttrName, policy.PolicyName)
 	d.Set("policy_update_token", policy.PolicyUpdateToken)
 	d.Set("remediation_enabled", policy.RemediationEnabled)
 	if err := d.Set(names.AttrResourceTags, flattenResourceTags(policy.ResourceTags)); err != nil {
-		sdkdiag.AppendErrorf(diags, "setting resource_tags: %s", err)
+		diags = sdkdiag.AppendErrorf(diags, "setting resource_tags: %s", err)
 	}
 	d.Set(names.AttrResourceType, policy.ResourceType)
 	d.Set("resource_type_list", policy.ResourceTypeList)
@@ -290,7 +290,7 @@ func resourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta interf
 		"policy_option":        flattenPolicyOption(policy.SecurityServicePolicyData.PolicyOption),
 	}}
 	if err := d.Set("security_service_policy_data", securityServicePolicy); err != nil {
-		sdkdiag.AppendErrorf(diags, "setting security_service_policy_data: %s", err)
+		diags = sdkdiag.AppendErrorf(diags, "setting security_service_policy_data: %s", err)
 	}
 
 	return diags

diff --git a/internal/service/rolesanywhere/profile.go b/internal/service/rolesanywhere/profile.go
@@ -192,12 +192,12 @@ func resourceProfileUpdate(ctx context.Context, d *schema.ResourceData, meta int
 		if n == true {
 			err := enableProfile(ctx, d.Id(), meta)
 			if err != nil {
-				sdkdiag.AppendErrorf(diags, "enabling RolesAnywhere Profile (%s): %s", d.Id(), err)
+				return sdkdiag.AppendErrorf(diags, "enabling RolesAnywhere Profile (%s): %s", d.Id(), err)
 			}
 		} else {
 			err := disableProfile(ctx, d.Id(), meta)
 			if err != nil {
-				sdkdiag.AppendErrorf(diags, "disabling RolesAnywhere Profile (%s): %s", d.Id(), err)
+				return sdkdiag.AppendErrorf(diags, "disabling RolesAnywhere Profile (%s): %s", d.Id(), err)
 			}
 		}
 	}

diff --git a/internal/service/rolesanywhere/trust_anchor.go b/internal/service/rolesanywhere/trust_anchor.go
@@ -164,11 +164,11 @@ func resourceTrustAnchorUpdate(ctx context.Context, d *schema.ResourceData, meta
 			_, n := d.GetChange(names.AttrEnabled)
 			if n == true {
 				if err := enableTrustAnchor(ctx, d.Id(), meta); err != nil {
-					sdkdiag.AppendErrorf(diags, "enabling RolesAnywhere Trust Anchor (%s): %s", d.Id(), err)
+					return sdkdiag.AppendErrorf(diags, "enabling RolesAnywhere Trust Anchor (%s): %s", d.Id(), err)
 				}
 			} else {
 				if err := disableTrustAnchor(ctx, d.Id(), meta); err != nil {
-					sdkdiag.AppendErrorf(diags, "disabling RolesAnywhere Trust Anchor (%s): %s", d.Id(), err)
+					return sdkdiag.AppendErrorf(diags, "disabling RolesAnywhere Trust Anchor (%s): %s", d.Id(), err)
 				}
 			}
 		}

diff --git a/internal/service/s3/bucket_lifecycle_configuration.go b/internal/service/s3/bucket_lifecycle_configuration.go
@@ -290,7 +290,7 @@ func resourceBucketLifecycleConfigurationCreate(ctx context.Context, d *schema.R
 	_, err = waitLifecycleRulesEquals(ctx, conn, bucket, expectedBucketOwner, rules, d.Timeout(schema.TimeoutCreate))
 
 	if err != nil {
-		sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket Lifecycle Configuration (%s) create: %s", d.Id(), err)
+		return sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket Lifecycle Configuration (%s) create: %s", d.Id(), err)
 	}
 
 	return append(diags, resourceBucketLifecycleConfigurationRead(ctx, d, meta)...)
@@ -388,7 +388,7 @@ func resourceBucketLifecycleConfigurationUpdate(ctx context.Context, d *schema.R
 	_, err = waitLifecycleRulesEquals(ctx, conn, bucket, expectedBucketOwner, rules, d.Timeout(schema.TimeoutUpdate))
 
 	if err != nil {
-		sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket Lifecycle Configuration (%s) update: %s", d.Id(), err)
+		return sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket Lifecycle Configuration (%s) update: %s", d.Id(), err)
 	}
 
 	return append(diags, resourceBucketLifecycleConfigurationRead(ctx, d, meta)...)