Merge pull request #18376 from ewbankkit/td-add-semgrep-rule-calling-…

…SetId-with-empty-string-in-resource-create

Add Semgrep rule to warn on calling 'd.SetId()' in a resource create or delete function

.semgrep.yml

@@ -649,3 +649,37 @@ rules:
           regex: (\d+)
     severity: ERROR
     fix: "aws.Int64($X)"
+
+  - id: calling-SetId-with-empty-string-in-resource-create
+    languages: [go]
+    message: Do not call `d.SetId("")` inside a resource create function
+    paths:
+      include:
+        - internal/service/
+    patterns:
+      - pattern: |
+          func $FUNC(...) {
+            ...
+            d.SetId("")
+          }
+      - metavariable-regex:
+          metavariable: "$FUNC"
+          regex: "^resource\\w*(Create|Put|Set|Upsert|Enable)$"
+    severity: WARNING
+
+  - id: calling-SetId-in-resource-delete
+    languages: [go]
+    message: Do not call `d.SetId(...)` inside a resource delete function
+    paths:
+      include:
+        - internal/service/
+    patterns:
+      - pattern: |
+          func $FUNC(...) {
+            ...
+            d.SetId(...)
+          }
+      - metavariable-regex:
+          metavariable: "$FUNC"
+          regex: "^resource\\w*(Delete|Disable)$"
+    severity: WARNING

internal/service/cloudformation/stack.go

@@ -128,10 +128,12 @@ func resourceStackCreate(d *schema.ResourceData, meta interface{}) error {
 	tags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{})))
 
 	requestToken := resource.UniqueId()
-	input := cloudformation.CreateStackInput{
-		StackName:          aws.String(d.Get("name").(string)),
+	name := d.Get("name").(string)
+	input := &cloudformation.CreateStackInput{
 		ClientRequestToken: aws.String(requestToken),
+		StackName:          aws.String(name),
 	}
+
 	if v, ok := d.GetOk("template_body"); ok {
 		template, err := verify.NormalizeJSONOrYAMLString(v)
 		if err != nil {
@@ -179,27 +181,18 @@ func resourceStackCreate(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	log.Printf("[DEBUG] Creating CloudFormation Stack: %s", input)
-	resp, err := conn.CreateStack(&input)
+	output, err := conn.CreateStack(input)
+
 	if err != nil {
-		return fmt.Errorf("creating CloudFormation stack failed: %w", err)
+		return fmt.Errorf("error creating CloudFormation Stack (%s): %w", name, err)
 	}
 
-	d.SetId(aws.StringValue(resp.StackId))
+	d.SetId(aws.StringValue(output.StackId))
 
-	stack, err := WaitStackCreated(conn, d.Id(), requestToken, d.Timeout(schema.TimeoutCreate))
-	if err != nil {
-		if stack != nil {
-			status := aws.StringValue(stack.StackStatus)
-			if status == cloudformation.StackStatusDeleteComplete || status == cloudformation.StackStatusDeleteFailed {
-				// Need to validate if this is actually necessary
-				d.SetId("")
-			}
-		}
-		return fmt.Errorf("error waiting for CloudFormation Stack creation: %w", err)
+	if _, err := WaitStackCreated(conn, d.Id(), requestToken, d.Timeout(schema.TimeoutCreate)); err != nil {
+		return fmt.Errorf("error waiting for CloudFormation Stack (%s) create: %w", d.Id(), err)
 	}
 
-	log.Printf("[INFO] CloudFormation Stack %q created", d.Id())
-
 	return resourceStackRead(d, meta)
 }
 

internal/service/ec2/fleet.go

@@ -340,24 +340,24 @@ func resourceFleetCreate(d *schema.ResourceData, meta interface{}) error {
 		OnDemandOptions:                  expandEc2OnDemandOptionsRequest(d.Get("on_demand_options").([]interface{})),
 		ReplaceUnhealthyInstances:        aws.Bool(d.Get("replace_unhealthy_instances").(bool)),
 		SpotOptions:                      expandEc2SpotOptionsRequest(d.Get("spot_options").([]interface{})),
+		TagSpecifications:                ec2TagSpecificationsFromKeyValueTags(tags, ec2.ResourceTypeFleet),
 		TargetCapacitySpecification:      expandEc2TargetCapacitySpecificationRequest(d.Get("target_capacity_specification").([]interface{})),
 		TerminateInstancesWithExpiration: aws.Bool(d.Get("terminate_instances_with_expiration").(bool)),
-		TagSpecifications:                ec2TagSpecificationsFromKeyValueTags(tags, ec2.ResourceTypeFleet),
 		Type:                             aws.String(d.Get("type").(string)),
 	}
 
 	if d.Get("type").(string) != ec2.FleetTypeMaintain {
 		if input.SpotOptions.MaintenanceStrategies != nil {
 			log.Printf("[WARN] EC2 Fleet (%s) has an invalid configuration and can not be created. Capacity Rebalance maintenance strategies can only be specified for fleets of type maintain.", input)
-			d.SetId("")
 			return nil
 		}
 	}
 
 	log.Printf("[DEBUG] Creating EC2 Fleet: %s", input)
 	output, err := conn.CreateFleet(input)
+
 	if err != nil {
-		return fmt.Errorf("error creating EC2 Fleet: %s", err)
+		return fmt.Errorf("error creating EC2 Fleet: %w", err)
 	}
 
 	d.SetId(aws.StringValue(output.FleetId))

internal/service/ec2/spot_fleet_request.go

@@ -982,7 +982,6 @@ func resourceSpotFleetRequestCreate(d *schema.ResourceData, meta interface{}) er
 	if d.Get("fleet_type").(string) != ec2.FleetTypeMaintain {
 		if spotFleetConfig.SpotMaintenanceStrategies != nil {
 			log.Printf("[WARN] Spot Fleet (%s) has an invalid configuration and can not be requested. Capacity Rebalance maintenance strategies can only be specified for spot fleets of type maintain.", spotFleetConfig)
-			d.SetId("")
 			return nil
 		}
 	}
@@ -1048,19 +1047,19 @@ func resourceSpotFleetRequestCreate(d *schema.ResourceData, meta interface{}) er
 	}
 
 	// http://docs.aws.amazon.com/sdk-for-go/api/service/ec2.html#type-RequestSpotFleetInput
-	spotFleetOpts := &ec2.RequestSpotFleetInput{
+	input := &ec2.RequestSpotFleetInput{
 		SpotFleetRequestConfig: spotFleetConfig,
 		DryRun:                 aws.Bool(false),
 	}
 
-	log.Printf("[DEBUG] Requesting spot fleet with these opts: %+v", spotFleetOpts)
+	log.Printf("[DEBUG] Creating Spot Fleet Request: %s", input)
 
 	// Since IAM is eventually consistent, we retry creation as a newly created role may not
 	// take effect immediately, resulting in an InvalidSpotFleetRequestConfig error
-	var resp *ec2.RequestSpotFleetOutput
+	var output *ec2.RequestSpotFleetOutput
 	err := resource.Retry(tfiam.PropagationTimeout, func() *resource.RetryError {
 		var err error
-		resp, err = conn.RequestSpotFleet(spotFleetOpts)
+		output, err = conn.RequestSpotFleet(input)
 
 		if tfawserr.ErrMessageContains(err, "InvalidSpotFleetRequestConfig", "Parameter: SpotFleetRequestConfig.IamFleetRole is invalid") {
 			return resource.RetryableError(err)
@@ -1078,14 +1077,14 @@ func resourceSpotFleetRequestCreate(d *schema.ResourceData, meta interface{}) er
 	})
 
 	if tfresource.TimedOut(err) {
-		resp, err = conn.RequestSpotFleet(spotFleetOpts)
+		output, err = conn.RequestSpotFleet(input)
 	}
 
 	if err != nil {
 		return fmt.Errorf("Error requesting spot fleet: %w", err)
 	}
 
-	d.SetId(aws.StringValue(resp.SpotFleetRequestId))
+	d.SetId(aws.StringValue(output.SpotFleetRequestId))
 
 	log.Printf("[INFO] Spot Fleet Request ID: %s", d.Id())
 	log.Println("[INFO] Waiting for Spot Fleet Request to be active")

internal/service/neptune/event_subscription.go

@@ -333,17 +333,18 @@ func resourceEventSubscriptionUpdate(d *schema.ResourceData, meta interface{}) e
 
 func resourceEventSubscriptionDelete(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).NeptuneConn
-	deleteOpts := neptune.DeleteEventSubscriptionInput{
+
+	log.Printf("[DEBUG] Deleting Neptune Event Subscription: %s", d.Id())
+	_, err := conn.DeleteEventSubscription(&neptune.DeleteEventSubscriptionInput{
 		SubscriptionName: aws.String(d.Id()),
+	})
+
+	if tfawserr.ErrCodeEquals(err, neptune.ErrCodeSubscriptionNotFoundFault) {
+		return nil
 	}
 
-	if _, err := conn.DeleteEventSubscription(&deleteOpts); err != nil {
-		if tfawserr.ErrMessageContains(err, neptune.ErrCodeSubscriptionNotFoundFault, "") {
-			log.Printf("[WARN] Neptune Event Subscription %s not found, removing from state", d.Id())
-			d.SetId("")
-			return nil
-		}
-		return fmt.Errorf("Error deleting Neptune Event Subscription %s: %s", d.Id(), err)
+	if err != nil {
+		return fmt.Errorf("error deleting Neptune Event Subscription (%s): %w", d.Id(), err)
 	}
 
 	stateConf := &resource.StateChangeConf{
@@ -355,9 +356,10 @@ func resourceEventSubscriptionDelete(d *schema.ResourceData, meta interface{}) e
 		Delay:      30 * time.Second,
 	}
 
-	_, err := stateConf.WaitForState()
+	_, err = stateConf.WaitForState()
+
 	if err != nil {
-		return fmt.Errorf("Error deleting Neptune Event Subscription %s: %s", d.Id(), err)
+		return fmt.Errorf("error waiting for Neptune Event Subscription (%s) delete: %w", d.Id(), err)
 	}
 
 	return nil

internal/service/redshift/snapshot_schedule_association.go

@@ -128,22 +128,17 @@ func resourceSnapshotScheduleAssociationDelete(d *schema.ResourceData, meta inte
 		return fmt.Errorf("Error parse Redshift Cluster Snapshot Schedule Association ID %s: %s", d.Id(), err)
 	}
 
+	log.Printf("[DEBUG] Deleting Redshift Cluster Snapshot Schedule Association: %s", d.Id())
 	_, err = conn.ModifyClusterSnapshotSchedule(&redshift.ModifyClusterSnapshotScheduleInput{
 		ClusterIdentifier:    aws.String(clusterIdentifier),
 		ScheduleIdentifier:   aws.String(scheduleIdentifier),
 		DisassociateSchedule: aws.Bool(true),
 	})
 
-	if tfawserr.ErrMessageContains(err, redshift.ErrCodeClusterNotFoundFault, "") {
-		log.Printf("[WARN] Redshift Snapshot Cluster (%s) not found, removing from state", clusterIdentifier)
-		d.SetId("")
-		return nil
-	}
-	if tfawserr.ErrMessageContains(err, redshift.ErrCodeSnapshotScheduleNotFoundFault, "") {
-		log.Printf("[WARN] Redshift Snapshot Schedule (%s) not found, removing from state", scheduleIdentifier)
-		d.SetId("")
+	if tfawserr.ErrCodeEquals(err, redshift.ErrCodeClusterNotFoundFault, redshift.ErrCodeSnapshotScheduleNotFoundFault) {
 		return nil
 	}
+
 	if err != nil {
 		return fmt.Errorf("Error disassociate Redshift Cluster (%s) and Snapshot Schedule (%s) Association: %s", clusterIdentifier, scheduleIdentifier, err)
 	}

internal/service/sns/topic_subscription.go

@@ -240,7 +240,6 @@ func resourceTopicSubscriptionDelete(d *schema.ResourceData, meta interface{}) e
 	})
 
 	if tfawserr.ErrMessageContains(err, sns.ErrCodeInvalidParameterException, "Cannot unsubscribe a subscription that is pending confirmation") {
-		log.Printf("[WARN] Removing unconfirmed SNS Topic Subscription (%s) from Terraform state but failed to remove it from AWS!", d.Id())
 		return nil
 	}
 

internal/service/wafregional/regex_match_set.go

@@ -143,13 +143,13 @@ func resourceRegexMatchSetDelete(d *schema.ResourceData, meta interface{}) error
 	region := meta.(*conns.AWSClient).Region
 
 	err := DeleteRegexMatchSetResource(conn, region, "global", d.Id(), getRegexMatchTuplesFromResourceData(d))
+
 	if tfawserr.ErrCodeEquals(err, wafregional.ErrCodeWAFNonexistentItemException) {
-		log.Printf("[WARN] WAF Regional Regex Match Set (%s) not found, removing from state", d.Id())
-		d.SetId("")
 		return nil
 	}
+
 	if err != nil {
-		return err
+		return fmt.Errorf("error deleting WAF Regional Regex Match Set (%s): %w", d.Id(), err)
 	}
 
 	return nil