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

Added support for http/https endpoints that auto confirms SNS topic subscription. #4711

Merged
merged 6 commits into from
Feb 14, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
86 changes: 85 additions & 1 deletion builtin/providers/aws/resource_aws_sns_topic_subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/sns"
"time"
)

const awsSNSPendingConfirmationMessage = "pending confirmation"
Expand All @@ -27,7 +28,7 @@ func resourceAwsSnsTopicSubscription() *schema.Resource {
ForceNew: false,
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
value := v.(string)
forbidden := []string{"email", "sms", "http"}
forbidden := []string{"email", "sms"}
for _, f := range forbidden {
if strings.Contains(value, f) {
errors = append(
Expand All @@ -44,6 +45,24 @@ func resourceAwsSnsTopicSubscription() *schema.Resource {
Required: true,
ForceNew: false,
},
"endpoint_auto_confirms": &schema.Schema{
Type: schema.TypeBool,
Optional: true,
ForceNew: false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe ForceNew: false is a default behaviour so we shouldn't need to specify it here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which applies to the other 2 fields below too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point but almost all the schema elements have ForceNew: false, I guess they made a bad copy :). I will fix it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not everything you see elsewhere in the codebase is worth copying, yet many things do! 😃

Default: false,
},
"max_fetch_retries": &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,
Expand Down Expand Up @@ -178,6 +197,13 @@ func subscribeToSNSTopic(d *schema.ResourceData, snsconn *sns.SNS) (output *sns.
protocol := d.Get("protocol").(string)
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))

if strings.Contains(protocol, "http") && !endpoint_auto_confirms {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like to move this to validateFunc but I need a function which can give me whole map. I need help on this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I see your point. This will be eventually possible after merging #3641 but I wouldn't want to block this PR on that.

The only suggestion I have regarding this is that we should rather be comparing protocols as IDs (i.e. (protocol == "http" || protocol == "https")), not arbitrary strings.

I almost thought on first sight you forgot about https.
Also there might be a new service in the future which will actually contain http in the name and that would make things even more confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree that there might be newer protocol which has .http. or .email. etc in it but then I saw logic around line 33 and thought that I should follow on the similar footsteps just to be consistent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the logic around line 33 should be changed too then... if you want to do it, go ahead, but I won't push back on this PR if you won't. It would be nice to get it consistent (and only use equal sign).

return nil, fmt.Errorf("Protocol http/https is only supported for endpoints which auto confirms!")
}

log.Printf("[DEBUG] SNS create topic subscription: %s (%s) @ '%s'", endpoint, protocol, topic_arn)

Expand All @@ -192,6 +218,64 @@ func subscribeToSNSTopic(d *schema.ResourceData, snsconn *sns.SNS) (output *sns.
return nil, fmt.Errorf("Error creating SNS topic: %s", err)
}

if strings.Contains(protocol, "http") && (output.SubscriptionArn == nil || *output.SubscriptionArn == awsSNSPendingConfirmationMessage) {

log.Printf("[DEBUG] SNS create topic subscritpion is pending so fetching the subscription list for topic : %s (%s) @ '%s'", endpoint, protocol, topic_arn)

for i := 0; i < max_fetch_retries && output.SubscriptionArn != nil && *output.SubscriptionArn == awsSNSPendingConfirmationMessage; i++ {

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
}

time.Sleep(time.Second * fetch_retry_delay)
}

if output.SubscriptionArn == nil || *output.SubscriptionArn == awsSNSPendingConfirmationMessage {
return nil, fmt.Errorf("Endpoint (%s) did not autoconfirm the subscription for topic %s", endpoint, topic_arn)
}
}

log.Printf("[DEBUG] Created new subscription!")
return output, nil
}

// finds a subscription using protocol, endpoint and topic_arn (which is a key in sns subscription)
func findSubscriptionByNonID(d *schema.ResourceData, snsconn *sns.SNS) (*sns.Subscription, error) {
protocol := d.Get("protocol").(string)
endpoint := d.Get("endpoint").(string)
topic_arn := d.Get("topic_arn").(string)

req := &sns.ListSubscriptionsByTopicInput{
TopicArn: aws.String(topic_arn),
}

for {

res, err := snsconn.ListSubscriptionsByTopic(req)

if err != nil {
return nil, fmt.Errorf("Error fetching subscripitions for topic %s : %s", topic_arn, err)
}

for _, subscription := range res.Subscriptions {
if *subscription.Endpoint == endpoint && *subscription.Protocol == protocol && *subscription.TopicArn == topic_arn {
return subscription, nil
}
}

// if there are more than 100 subscriptions then go to the next 100 otherwise return nil
if res.NextToken != nil {
req.NextToken = res.NextToken
} else {
return nil, nil
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,11 @@ resource "aws_sns_topic_subscription" "user_updates_sqs_target" {
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`, or `application`. (`email`, `http`, `https`, `sms`, are options but unsupported, see below)
* `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 (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.
* `fetch_retry_delay` - (Optional) Integer indicating number of seconds to sleep before re-fetching the subscription list for the topic.
* `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
Expand All @@ -61,12 +64,15 @@ Supported SNS protocols include:
* `sqs` -- delivery of JSON-encoded message to an Amazon SQS queue
* `application` -- delivery of JSON-encoded message to an EndpointArn for a mobile app and device

Partially supported SNS protocols include:

* `http` -- delivery of JSON-encoded messages via HTTP. Supported only for the end points that auto confirms the subscription.
* `https` -- delivery of JSON-encoded messages via HTTPS. Supported only for the end points that auto confirms the subscription.

Unsupported protocols include the following:

* `email` -- delivery of message via SMTP
* `email-json` -- delivery of JSON-encoded message via SMTP
* `http` -- delivery via HTTP
* `http(s)` -- delivery via HTTPS
* `sms` -- delivery text message

These are unsupported because the endpoint needs to be authorized and does not
Expand Down