Update code based on the review suggestions.

1. Used resource.Retry instead of custom solution
2. Removed unnecessary variables and added required variable to resource.Retry.

builtin/providers/aws/resource_aws_sns_topic_subscription.go

@@ -9,10 +9,12 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/sns"
+	"github.com/hashicorp/terraform/helper/resource"
 	"time"
 )
 
 const awsSNSPendingConfirmationMessage = "pending confirmation"
+const awsSNSPendingConfirmationMessageWithoutSpaces = "pendingconfirmation"
 
 func resourceAwsSnsTopicSubscription() *schema.Resource {
 	return &schema.Resource{
@@ -43,40 +45,28 @@ func resourceAwsSnsTopicSubscription() *schema.Resource {
 			"endpoint": &schema.Schema{
 				Type:     schema.TypeString,
 				Required: true,
-				ForceNew: false,
 			},
 			"endpoint_auto_confirms": &schema.Schema{
 				Type:     schema.TypeBool,
 				Optional: true,
-				ForceNew: false,
 				Default:  false,
 			},
-			"max_fetch_retries": &schema.Schema{
+			"confirmation_timeout_in_minutes": &schema.Schema{
 				Type:     schema.TypeInt,
 				Optional: true,
-				ForceNew: false,
-				Default:  3,
-			},
-			"fetch_retry_delay": &schema.Schema{
-				Type:     schema.TypeInt,
-				Optional: true,
-				ForceNew: false,
 				Default:  1,
 			},
 			"topic_arn": &schema.Schema{
 				Type:     schema.TypeString,
 				Required: true,
-				ForceNew: false,
 			},
 			"delivery_policy": &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
-				ForceNew: false,
 			},
 			"raw_message_delivery": &schema.Schema{
 				Type:     schema.TypeBool,
 				Optional: true,
-				ForceNew: false,
 				Default:  false,
 			},
 			"arn": &schema.Schema{
@@ -198,8 +188,7 @@ func subscribeToSNSTopic(d *schema.ResourceData, snsconn *sns.SNS) (output *sns.
 	endpoint := d.Get("endpoint").(string)
 	topic_arn := d.Get("topic_arn").(string)
 	endpoint_auto_confirms := d.Get("endpoint_auto_confirms").(bool)
-	max_fetch_retries := d.Get("max_fetch_retries").(int)
+	confirmation_timeout_in_minutes := time.Duration(d.Get("confirmation_timeout_in_minutes").(int))
-	fetch_retry_delay := time.Duration(d.Get("fetch_retry_delay").(int))
 
 	if strings.Contains(protocol, "http") && !endpoint_auto_confirms {
 		return nil, fmt.Errorf("Protocol http/https is only supported for endpoints which auto confirms!")
@@ -222,26 +211,26 @@ func subscribeToSNSTopic(d *schema.ResourceData, snsconn *sns.SNS) (output *sns.
 
 	if strings.Contains(protocol, "http") && subscriptionHasPendingConfirmation(output.SubscriptionArn) {
 
-		log.Printf("[DEBUG] SNS create topic subscritpion is pending so fetching the subscription list for topic : %s (%s) @ '%s'", endpoint, protocol, topic_arn)
+		log.Printf("[DEBUG] SNS create topic subscription is pending so fetching the subscription list for topic : %s (%s) @ '%s'", endpoint, protocol, topic_arn)
 
-		for i := 0; i < max_fetch_retries && subscriptionHasPendingConfirmation(output.SubscriptionArn); i++ {
+		err = resource.Retry(time.Minute*confirmation_timeout_in_minutes, func() error {
 
 			subscription, err := findSubscriptionByNonID(d, snsconn)
 
-			if err != nil {
-				return nil, fmt.Errorf("Error fetching subscriptions for SNS topic %s: %s", topic_arn, err)
-			}
-
 			if subscription != nil {
 				output.SubscriptionArn = subscription.SubscriptionArn
-				break
+				return nil
 			}
 
-			time.Sleep(time.Second * fetch_retry_delay)
+			if err != nil {
+				return fmt.Errorf("Error fetching subscriptions for SNS topic %s: %s", topic_arn, err)
 			}
 
-		if subscriptionHasPendingConfirmation(output.SubscriptionArn) {
+			return fmt.Errorf("Endpoint (%s) did not autoconfirm the subscription for topic %s", endpoint, topic_arn)
-			return nil, fmt.Errorf("Endpoint (%s) did not autoconfirm the subscription for topic %s", endpoint, topic_arn)
+		})
+
+		if err != nil {
+			return nil, err
 		}
 	}
 
@@ -285,7 +274,7 @@ func findSubscriptionByNonID(d *schema.ResourceData, snsconn *sns.SNS) (*sns.Sub
 
 // returns true if arn is nil or has both pending and confirmation words in the arn
 func subscriptionHasPendingConfirmation(arn *string) bool {
-	if arn != nil && !strings.Contains(strings.ToLower(*arn), "pending") && !strings.Contains(strings.ToLower(*arn), "confirmation") {
+	if arn != nil && !strings.Contains(strings.Replace(strings.ToLower(*arn), " ", "", -1), awsSNSPendingConfirmationMessageWithoutSpaces) {
 		return false
 	}
 

website/source/docs/providers/aws/r/sns_topic_subscription.html.markdown

@@ -51,9 +51,8 @@ The following arguments are supported:
 * `topic_arn` - (Required) The ARN of the SNS topic to subscribe to
 * `protocol` - (Required) The protocol to use. The possible values for this are: `sqs`,  `lambda`, `application`. (`http` or `https` are partially supported, see below) (`email`, `sms`, are options but unsupported, see below).
 * `endpoint` - (Required) The endpoint to send data to, the contents will vary with the protocol. (see below for more information)
-* `endpoint_auto_confirms` - (Optional) Boolean indicating whether the end point is capable of [auto confirming subscription](http://docs.aws.amazon.com/sns/latest/dg/SendMessageToHttp.html#SendMessageToHttp.prepare) (default is false)
+* `endpoint_auto_confirms` - (Optional) Boolean indicating whether the end point is capable of [auto confirming subscription](http://docs.aws.amazon.com/sns/latest/dg/SendMessageToHttp.html#SendMessageToHttp.prepare) e.g., PagerDuty (default is false)
-* `max_fetch_retries` - (Optional) Integer indicating number of times the subscriptions list for a topic to be fetched to get the subscription arn for auto confirming end points (default is 3 times).
+* `confirmation_timeout_in_minutes` - (Optional) Integer indicating number of minutes to wait in retying mode for fetching subscription arn before marking it as failure. Only applicable for http and https protocols (default is 1 minute).
-* `fetch_retry_delay` - (Optional) Integer indicating number of seconds to sleep before re-fetching the subscription list for the topic (default is 1 second).
 * `raw_message_delivery` - (Optional) Boolean indicating whether or not to enable raw message delivery (the original message is directly passed, not wrapped in JSON with the original message in the message property).
 
 ### Protocols supported