Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .semgrep.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
27 changes: 10 additions & 17 deletions internal/service/cloudformation/stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}

Expand Down
6 changes: 3 additions & 3 deletions internal/service/ec2/fleet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
13 changes: 6 additions & 7 deletions internal/service/ec2/spot_fleet_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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)
Expand All @@ -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")
Expand Down
22 changes: 12 additions & 10 deletions internal/service/neptune/event_subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
Expand Down
11 changes: 3 additions & 8 deletions internal/service/redshift/snapshot_schedule_association.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
1 change: 0 additions & 1 deletion internal/service/sns/topic_subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions internal/service/wafregional/regex_match_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down