-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
provider/aws: Limit SNS Topic Subscription protocols #4639
Conversation
output, err := subscribeToSNSTopic(d, snsconn) | ||
|
||
if err != nil { | ||
return err | ||
} | ||
|
||
if output.SubscriptionArn != nil && *output.SubscriptionArn == "pending configuration" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this a typo s/configuration/confirmation/
- maybe pull the string out into a constant?
👍 |
- update the ARN with the new ID
c7972d4
to
ac60d6b
Compare
provider/aws: Limit SNS Topic Subscription protocols
…ubscription. http and https SNS topic subscription endpoints require confirmation to set a valid arn otherwise arn would be set to "pending confirmation". If the endpoints auto confirm then arn is set asynchronously but if we try to create another subscription with same parameters then api returns "pending subscription" as arn but does not create another a duplicate subscription. In order to solve this we should be fetching the subscription list for the topic and identify the subscription with same parameters i.e., protocol, topic_arn, endpoint and extract the subscription arn. Following changes were made to support the http/https endpoints that auto confirms 1. Added 3 extra parameters i.e., 1. endpoint_auto_confirms -> boolean indicates if end points auto confirms 2. max_fetch_retries -> number of times to fetch subscription list for the topic to get the subscription arn 3. fetch_retry_delay -> delay b/w fetch subscription list call as the confirmation is done asynchronously. With these parameters help added support http and https protocol based endpoints that auto confirm. 2. Update website doc appropriately
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Some SNS Topic Subscription protocols require an external confirmation before we can get an ARN from the service, and as such they don't fit well with Terraform. Email was already blocked, and this PR expands that to
http(s)
andsms
, as well.Fixes #2776