From f21dc995c53a5c88263015f2547a1047d0fcb328 Mon Sep 17 00:00:00 2001 From: Srikalyan Swayampakula Date: Fri, 12 Feb 2016 12:21:52 -0800 Subject: [PATCH] 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. --- .../resource_aws_sns_topic_subscription.go | 43 +++++++------------ .../r/sns_topic_subscription.html.markdown | 5 +-- 2 files changed, 18 insertions(+), 30 deletions(-) diff --git a/builtin/providers/aws/resource_aws_sns_topic_subscription.go b/builtin/providers/aws/resource_aws_sns_topic_subscription.go index dee6210f0..75909fefb 100644 --- a/builtin/providers/aws/resource_aws_sns_topic_subscription.go +++ b/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) - fetch_retry_delay := time.Duration(d.Get("fetch_retry_delay").(int)) + confirmation_timeout_in_minutes := time.Duration(d.Get("confirmation_timeout_in_minutes").(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 nil, fmt.Errorf("Endpoint (%s) did not autoconfirm the subscription for topic %s", endpoint, topic_arn) + return 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 } diff --git a/website/source/docs/providers/aws/r/sns_topic_subscription.html.markdown b/website/source/docs/providers/aws/r/sns_topic_subscription.html.markdown index b8811aa86..ab4ffd594 100644 --- a/website/source/docs/providers/aws/r/sns_topic_subscription.html.markdown +++ b/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) -* `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). -* `fetch_retry_delay` - (Optional) Integer indicating number of seconds to sleep before re-fetching the subscription list for the topic (default is 1 second). +* `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) +* `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). * `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