From d9fc9fa7d5342504653128e9790a511dd5e7ab40 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 24 Jul 2020 19:54:33 -0400 Subject: [PATCH] resource/aws_sns_topic_subscription: Use paginated ListSubscriptionsByTopic and return immediately on errors (#14262) * tests/resource/aws_sns_topic_subscription: Fix recurring and unrelated test configuration error Previously: ``` --- FAIL: TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint (63.28s) testing.go:684: Step 0 error: After applying this step, the plan was not empty: DIFF: UPDATE: aws_api_gateway_authorizer.test ... authorizer_result_ttl_in_seconds: "300" => "0" ``` Output from acceptance testing: ``` --- PASS: TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint (91.18s) ``` * resource/aws_sns_topic_subscription: Use paginated ListSubscriptionsByTopic and return immediately on errors Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/13409 Output from acceptance testing: ``` --- PASS: TestAccAWSSNSTopicSubscription_basic (13.47s) --- PASS: TestAccAWSSNSTopicSubscription_rawMessageDelivery (27.13s) --- PASS: TestAccAWSSNSTopicSubscription_filterPolicy (28.12s) --- PASS: TestAccAWSSNSTopicSubscription_deliveryPolicy (28.38s) --- PASS: TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint (48.31s) --- PASS: TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint (91.18s) ``` --- aws/resource_aws_sns_topic_subscription.go | 69 +++++++++++-------- ...esource_aws_sns_topic_subscription_test.go | 1 - 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/aws/resource_aws_sns_topic_subscription.go b/aws/resource_aws_sns_topic_subscription.go index 2f6654486750..313d9e1b04f0 100644 --- a/aws/resource_aws_sns_topic_subscription.go +++ b/aws/resource_aws_sns_topic_subscription.go @@ -235,18 +235,16 @@ func subscribeToSNSTopic(d *schema.ResourceData, snsconn *sns.SNS) (output *sns. subscription, err := findSubscriptionByNonID(d, snsconn) - if subscription != nil { - output.SubscriptionArn = subscription.SubscriptionArn - return nil + if err != nil { + return resource.NonRetryableError(err) } - if err != nil { - return resource.RetryableError( - fmt.Errorf("Error fetching subscriptions for SNS topic %s: %s", topic_arn, err)) + if subscription == nil { + return resource.RetryableError(fmt.Errorf("Endpoint (%s) did not autoconfirm the subscription for topic %s", endpoint, topic_arn)) } - return resource.RetryableError( - fmt.Errorf("Endpoint (%s) did not autoconfirm the subscription for topic %s", endpoint, topic_arn)) + output.SubscriptionArn = subscription.SubscriptionArn + return nil }) if isResourceTimeoutError(err) { @@ -268,37 +266,52 @@ func subscribeToSNSTopic(d *schema.ResourceData, snsconn *sns.SNS) (output *sns. } // 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) { +func findSubscriptionByNonID(d *schema.ResourceData, conn *sns.SNS) (*sns.Subscription, error) { protocol := d.Get("protocol").(string) endpoint := d.Get("endpoint").(string) - topic_arn := d.Get("topic_arn").(string) + topicARN := d.Get("topic_arn").(string) obfuscatedEndpoint := obfuscateEndpoint(endpoint) - req := &sns.ListSubscriptionsByTopicInput{ - TopicArn: aws.String(topic_arn), + input := &sns.ListSubscriptionsByTopicInput{ + TopicArn: aws.String(topicARN), } + var result *sns.Subscription - for { - res, err := snsconn.ListSubscriptionsByTopic(req) - - if err != nil { - return nil, fmt.Errorf("Error fetching subscriptions for topic %s : %s", topic_arn, err) + err := conn.ListSubscriptionsByTopicPages(input, func(page *sns.ListSubscriptionsByTopicOutput, lastPage bool) bool { + if page == nil { + return !lastPage } - for _, subscription := range res.Subscriptions { - log.Printf("[DEBUG] check subscription with Subscription EndPoint %s (local: %s), Protocol %s, topicARN %s and SubscriptionARN %s", *subscription.Endpoint, obfuscatedEndpoint, *subscription.Protocol, *subscription.TopicArn, *subscription.SubscriptionArn) - if *subscription.Endpoint == obfuscatedEndpoint && *subscription.Protocol == protocol && *subscription.TopicArn == topic_arn && !subscriptionHasPendingConfirmation(subscription.SubscriptionArn) { - return subscription, nil + for _, subscription := range page.Subscriptions { + if subscription == nil { + continue + } + + if aws.StringValue(subscription.Endpoint) != obfuscatedEndpoint { + continue } - } - // if there are more than 100 subscriptions then go to the next 100 otherwise return an error - if res.NextToken != nil { - req.NextToken = res.NextToken - } else { - return nil, fmt.Errorf("Error finding subscription for topic %s with endpoint %s and protocol %s", topic_arn, endpoint, protocol) + if aws.StringValue(subscription.Protocol) != protocol { + continue + } + + if aws.StringValue(subscription.TopicArn) != topicARN { + continue + } + + if subscriptionHasPendingConfirmation(subscription.SubscriptionArn) { + continue + } + + result = subscription + + return false } - } + + return !lastPage + }) + + return result, err } // returns true if arn is nil or has both pending and confirmation words in the arn diff --git a/aws/resource_aws_sns_topic_subscription_test.go b/aws/resource_aws_sns_topic_subscription_test.go index 55c5887c6e04..0f19aa1def7d 100644 --- a/aws/resource_aws_sns_topic_subscription_test.go +++ b/aws/resource_aws_sns_topic_subscription_test.go @@ -749,7 +749,6 @@ resource "aws_api_gateway_authorizer" "test" { name = "tf-acc-test-api-gw-authorizer-%d" rest_api_id = "${aws_api_gateway_rest_api.test.id}" authorizer_uri = "${aws_lambda_function.authorizer.invoke_arn}" - authorizer_result_ttl_in_seconds = "0" authorizer_credentials = "${aws_iam_role.invocation_role.arn}" }