Skip to content

Commit

Permalink
resource/aws_sns_topic_subscription: Handle read-after-create eventua…
Browse files Browse the repository at this point in the history
…l consistency, enforce lowercase protocol argument validation

Reference: #10225
Reference: #11737
Reference: #12692
Reference: #16695
Reference: #16796

The `protocol` validation update is to catch where the API accepts uppercase values such as `HTTPS`, but prevents proper handling when the API canonicalizes it to lowercase. The API documentation and existing Terraform documentation solely use lowercase.

Output from acceptance testing in AWS Commercial:

```
--- PASS: TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint (95.60s)
--- PASS: TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint (137.43s)
--- PASS: TestAccAWSSNSTopicSubscription_basic (66.20s)
--- PASS: TestAccAWSSNSTopicSubscription_deliveryPolicy (69.55s)
--- PASS: TestAccAWSSNSTopicSubscription_disappears (74.02s)
--- PASS: TestAccAWSSNSTopicSubscription_disappears_topic (75.13s)
--- PASS: TestAccAWSSNSTopicSubscription_email (16.78s)
--- PASS: TestAccAWSSNSTopicSubscription_filterPolicy (71.62s)
--- PASS: TestAccAWSSNSTopicSubscription_firehose (140.29s)
--- PASS: TestAccAWSSNSTopicSubscription_rawMessageDelivery (69.77s)
--- PASS: TestAccAWSSNSTopicSubscription_redrivePolicy (64.88s)
```

Output from acceptance testing in AWS GovCloud (US):

```
--- PASS: TestAccAWSSNSTopicSubscription_basic (70.13s)
--- PASS: TestAccAWSSNSTopicSubscription_deliveryPolicy (106.45s)
--- PASS: TestAccAWSSNSTopicSubscription_disappears (82.09s)
--- PASS: TestAccAWSSNSTopicSubscription_disappears_topic (68.14s)
--- PASS: TestAccAWSSNSTopicSubscription_email (20.04s)
--- PASS: TestAccAWSSNSTopicSubscription_filterPolicy (106.32s)
--- PASS: TestAccAWSSNSTopicSubscription_rawMessageDelivery (95.36s)
--- PASS: TestAccAWSSNSTopicSubscription_redrivePolicy (110.75s)
--- SKIP: TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint (1.41s)
--- SKIP: TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint (1.41s)
--- SKIP: TestAccAWSSNSTopicSubscription_firehose (53.36s)
```
  • Loading branch information
bflad committed Mar 30, 2021
1 parent 2bf0416 commit df63e22
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 12 deletions.
7 changes: 7 additions & 0 deletions .changelog/pending.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:bug
resource/aws_sns_topic_subscription: Handle read-after-create eventual consistency
```

```release-note:bug
resource/aws_sns_topic_subscription: Enforce lowercase `protocol` argument validation to match API and prevent resource errors
```
1 change: 1 addition & 0 deletions aws/internal/service/sns/waiter/waiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
)

const (
SubscriptionCreateTimeout = 2 * time.Minute
SubscriptionPendingConfirmationTimeout = 2 * time.Minute
SubscriptionDeleteTimeout = 2 * time.Minute
)
Expand Down
39 changes: 27 additions & 12 deletions aws/resource_aws_sns_topic_subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import (
"github.com/aws/aws-sdk-go/aws/awsutil"
"github.com/aws/aws-sdk-go/service/sns"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/sns/finder"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/sns/waiter"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
)

func resourceAwsSnsTopicSubscription() *schema.Resource {
Expand Down Expand Up @@ -91,7 +93,7 @@ func resourceAwsSnsTopicSubscription() *schema.Resource {
"lambda",
"sms",
"sqs",
}, true),
}, false),
},
"raw_message_delivery": {
Type: schema.TypeBool,
Expand Down Expand Up @@ -169,27 +171,40 @@ func resourceAwsSnsTopicSubscriptionCreate(d *schema.ResourceData, meta interfac
func resourceAwsSnsTopicSubscriptionRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).snsconn

log.Printf("[DEBUG] Loading subscription %s", d.Id())
var output *sns.GetSubscriptionAttributesOutput

input := &sns.ListSubscriptionsByTopicInput{
TopicArn: aws.String(d.Get("topic_arn").(string)),
}
err := resource.Retry(waiter.SubscriptionCreateTimeout, func() *resource.RetryError {
var err error

_, err := conn.ListSubscriptionsByTopic(input)
output, err = finder.SubscriptionByARN(conn, d.Id())

if err != nil {
return resource.NonRetryableError(err)
}

if d.IsNewResource() && output == nil {
return resource.RetryableError(&resource.NotFoundError{
LastError: fmt.Errorf("SNS Topic Subscription Attributes (%s) not found", d.Id()),
})
}

if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, sns.ErrCodeNotFoundException) {
log.Printf("[WARN] SNS Topic Subscription (%s) not found, removing from state", d.Id())
d.SetId("")
return nil
})

if tfresource.TimedOut(err) {
output, err = finder.SubscriptionByARN(conn, d.Id())
}

output, err := finder.SubscriptionByARN(conn, d.Id())
if err != nil {
return fmt.Errorf("getting SNS subscription attributes (%s): %w", d.Id(), err)
return fmt.Errorf("error getting SNS Topic Subscription Attributes (%s): %w", d.Id(), err)
}

if output == nil {
log.Printf("[WARN] SNS subscription attributes (%s) not found, removing from state", d.Id())
if d.IsNewResource() {
return fmt.Errorf("error getting SNS Topic Subscription Attributes (%s): not found after creation", d.Id())
}

log.Printf("[WARN] SNS Topic Subscription Attributes (%s) not found, removing from state", d.Id())
d.SetId("")
return nil
}
Expand Down

0 comments on commit df63e22

Please sign in to comment.