Commits on Feb 6, 2020

1. helper/resource: Require explicit usage of NonRetryableError and Retr… …

   …yableError
   
   Reference: hashicorp/terraform#17220
   Reference: hashicorp/terraform-provider-aws#9779 (comment)
   Reference: hashicorp/terraform-provider-aws#9812 (comment)
   
   It is currently possible to introduce subtle bugs or crashes when using `resource.RetryableError` and `resource.NonRetryableError` and allowing a `nil` error input. This PR proposes behavior that requires providers to be explicit with their usage of these errors, otherwise returns a bug reporting message to the operator.
   
   For example (https://github.com/terraform-providers/terraform-provider-aws/blob/4c0387645f982ddcd51b3ffe2cc8992c06fb9c2c/aws/resource_aws_elastic_beanstalk_application.go#L105):
   
   ```go
   	var app *elasticbeanstalk.ApplicationDescription
   	err := resource.Retry(30*time.Second, func() *resource.RetryError {
   		var err error
   		app, err = getBeanstalkApplication(d.Id(), conn)
   		if err != nil {
   			return resource.NonRetryableError(err)
   		}
   
   		if app == nil {
   			if d.IsNewResource() {
   				return resource.RetryableError(fmt.Errorf("Elastic Beanstalk Application %q not found", d.Id()))
   			}
   			// err is nil here
   			return resource.NonRetryableError(err)
   		}
   		return nil
   	})
   	if err != nil {
   		return err
   	}
   	// app can be nil here, so this can crash Terraform
   	d.Set("name", app.ApplicationName)
   ```
   
   Another example (https://github.com/terraform-providers/terraform-provider-alicloud/blob/927a8e82386e0b32718aa5eef254255fdc36b070/alicloud/resource_alicloud_disk_attachment.go#L112):
   
   ```go
   	return resource.Retry(5*time.Minute, func() *resource.RetryError {
   		err := conn.DetachDisk(instanceID, diskID)
   		if err != nil {
   			if IsExceptedError(err, DiskIncorrectStatus) || IsExceptedError(err, InstanceLockedForSecurity) ||
   				IsExceptedError(err, DiskInvalidOperation) {
   				return resource.RetryableError(fmt.Errorf("Detach Disk timeout and got an error: %#v", err))
   			}
   		}
   
   		disks, _, descErr := conn.DescribeDisks(&ecs.DescribeDisksArgs{
   			RegionId: getRegion(d, meta),
   			DiskIds:  []string{diskID},
   		})
   
   		if descErr != nil {
   			log.Printf("[ERROR] Disk %s is not detached.", diskID)
   			// err can be nil here
   			return resource.NonRetryableError(err)
   		}
   
   		for _, disk := range disks {
   			if disk.Status != ecs.DiskStatusAvailable {
   				return resource.RetryableError(fmt.Errorf("Detach Disk timeout and got an error: %#v", err))
   			}
   		}
   		return nil
   	})
   ```
   
   Another example (https://github.com/terraform-providers/terraform-provider-aws/blob/75c32b375140813b7994f8031e74b8588a08035a/aws/resource_aws_kms_alias.go#L176-L190):
   
   ```go
   func retryFindKmsAliasByName(conn *kms.KMS, name string) (*kms.AliasListEntry, error) {
   	var resp *kms.AliasListEntry
   	err := resource.Retry(1*time.Minute, func() *resource.RetryError {
   		var err error
   		resp, err = findKmsAliasByName(conn, name, nil)
   		if err != nil {
   			return resource.NonRetryableError(err)
   		}
   		if resp == nil {
   			// err is nil here, so returns nil
   			return resource.RetryableError(err)
   		}
   		return nil
   	})
   	return resp, err
   }
   ```
   
   Another example (https://github.com/terraform-providers/terraform-provider-aws/blob/00909998d919faf5494ab8f6b38241deb1957d99/aws/resource_aws_internet_gateway.go#L55-L65):
   
   ```go
   	err = resource.Retry(5*time.Minute, func() *resource.RetryError {
   		igRaw, _, err := IGStateRefreshFunc(conn, d.Id())()
   		if igRaw != nil {
   			return nil
   		}
   		if err == nil {
   			// err is always nil here, so it will never retry
   			return resource.RetryableError(err)
   		} else {
   			return resource.NonRetryableError(err)
   		}
   	})
   ```

   

   bflad authored and appilon committed Feb 6, 2020
   Configuration menu

   • View commit details
   • Copy full SHA for 4f58f0e
   • Browse repository at this point

   Copy the full SHA

   4f58f0e View commit details

   Browse the repository at this point in the history