Update resource_aws_sns_topic_subscription to create the subscription…

[carlos-zaragoza]

… attributes simulatenously with the general subscription creation

Previously the attributes were created after the subscription was created. This left a few seconds in time where messages from the topic could make it through to the endpoint without adhering to things like the FilterPolicy and the RawMessageDelivery flag.

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #10495

Release note for CHANGELOG:

SNS Subscription attributes (`raw_message_delivery`, `filter_policy`, and `delivery_policy`) will now be created at the same time the subscription is created rather than afterwards.

Output from acceptance testing:

When I run the tests one at a time, they pass successfully. The tests also pass when run concurrently on the automated build server.

I have issues running the tests concurrently on my machine but the issue happens both on this branch and on master so I believe this issue is a problem with my local environment.

For completeness' sake, I've included the test results of running both concurrently and running one at a time. They are listed separately below.

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSNSTopicSubscription'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 40 -run=TestAccAWSSNSTopicSubscription -timeout 120m
=== RUN   TestAccAWSSNSTopicSubscription_basic
=== PAUSE TestAccAWSSNSTopicSubscription_basic
=== RUN   TestAccAWSSNSTopicSubscription_filterPolicy
=== PAUSE TestAccAWSSNSTopicSubscription_filterPolicy
=== RUN   TestAccAWSSNSTopicSubscription_deliveryPolicy
=== PAUSE TestAccAWSSNSTopicSubscription_deliveryPolicy
=== RUN   TestAccAWSSNSTopicSubscription_rawMessageDelivery
=== PAUSE TestAccAWSSNSTopicSubscription_rawMessageDelivery
=== RUN   TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint
=== PAUSE TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint
=== RUN   TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint
=== PAUSE TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint
=== CONT  TestAccAWSSNSTopicSubscription_basic
=== CONT  TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint
=== CONT  TestAccAWSSNSTopicSubscription_deliveryPolicy
=== CONT  TestAccAWSSNSTopicSubscription_rawMessageDelivery
=== CONT  TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint
=== CONT  TestAccAWSSNSTopicSubscription_filterPolicy
--- FAIL: TestAccAWSSNSTopicSubscription_filterPolicy (14.31s)
    testing.go:569: Step 0 error: Check failed: Check 2/2 error: aws_sns_topic_subscription.test_subscription: Attribute 'filter_policy' expected "{\"key1\": [\"val1\"], \"key2\": [\"val2\"]}", got ""
    testing.go:630: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: errors during apply: AWS.SimpleQueueService.NonExistentQueue: The specified queue does not exist for this wsdl version.
                status code: 400, request id: 832a200f-a5f1-58b1-b128-76122e3a8a2e

        State: aws_sqs_queue.test_queue:
          ID = https://sqs.us-west-2.amazonaws.com/072662658510/terraform-subscription-test-queue-9103664681323863048
          provider = provider.aws
          arn = arn:aws:sqs:us-west-2:072662658510:terraform-subscription-test-queue-9103664681323863048
          content_based_deduplication = false
          delay_seconds = 0
          fifo_queue = false
          kms_data_key_reuse_period_seconds = 300
          kms_master_key_id =
          max_message_size = 262144
          message_retention_seconds = 345600
          name = terraform-subscription-test-queue-9103664681323863048
          policy =
          receive_wait_time_seconds = 0
          redrive_policy =
          tags.% = 0
          visibility_timeout_seconds = 30
--- FAIL: TestAccAWSSNSTopicSubscription_rawMessageDelivery (14.41s)
    testing.go:569: Step 0 error: Check failed: Check 2/2 error: aws_sns_topic_subscription.test_subscription: Attribute 'raw_message_delivery' expected "true", got "false"
--- FAIL: TestAccAWSSNSTopicSubscription_basic (14.41s)
    testing.go:569: Step 0 error: Check failed: Check 3/8 error: aws_sns_topic_subscription.test_subscription: Attribute 'delivery_policy' expected "", got "{\"healthyRetryPolicy\":{\"minDelayTarget\":5,\"maxDelayTarget\":20,\"numRetries\":5,\"numMaxDelayRetries\":null,\"numNoDelayRetries\":null,\"numMinDelayRetries\":null,\"backoffFunction\":null},\"sicklyRetryPolicy\":null,\"throttlePolicy\":null,\"guaranteed\":false}"
--- FAIL: TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint (19.37s)
    testing.go:569: Step 0 error: errors during apply:

        Error: Error creating IAM Role tf-acc-test-sns-9103664681323863048: EntityAlreadyExists: Role with name tf-acc-test-sns-9103664681323863048 already exists.
                status code: 409, request id: 7083aa00-689b-4fb8-94cd-3857002a45a0

          on C:\Users\socce\AppData\Local\Temp\tf-test576338621\main.tf line 51:
          (source code not available)


--- FAIL: TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint (51.90s)
    testing.go:569: Step 0 error: errors during apply:

        Error: Error creating SNS topic: NotFound: Topic does not exist
                status code: 404, request id: b9a48349-709f-5d2b-af55-d554fb452784

          on C:\Users\socce\AppData\Local\Temp\tf-test233225962\main.tf line 113:
          (source code not available)


--- PASS: TestAccAWSSNSTopicSubscription_deliveryPolicy (111.66s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-aws/aws       112.014s
FAIL
make: *** [GNUmakefile:24: testacc] Error 1

Results of running all the tests one at a time.

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSNSTopicSubscription_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSSNSTopicSubscription_basic -timeout 120m
=== RUN   TestAccAWSSNSTopicSubscription_basic
=== PAUSE TestAccAWSSNSTopicSubscription_basic
=== CONT  TestAccAWSSNSTopicSubscription_basic
--- PASS: TestAccAWSSNSTopicSubscription_basic (47.87s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       48.291s


$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSNSTopicSubscription_filterPolicy'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSSNSTopicSubscription_filterPolicy -timeout 120m
=== RUN   TestAccAWSSNSTopicSubscription_filterPolicy
=== PAUSE TestAccAWSSNSTopicSubscription_filterPolicy
=== CONT  TestAccAWSSNSTopicSubscription_filterPolicy
--- PASS: TestAccAWSSNSTopicSubscription_filterPolicy (59.27s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       59.608s


$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSNSTopicSubscription_deliveryPolicy'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSSNSTopicSubscription_deliveryPolicy -timeout 120m
=== RUN   TestAccAWSSNSTopicSubscription_deliveryPolicy
=== PAUSE TestAccAWSSNSTopicSubscription_deliveryPolicy
=== CONT  TestAccAWSSNSTopicSubscription_deliveryPolicy
--- PASS: TestAccAWSSNSTopicSubscription_deliveryPolicy (53.87s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       54.091s


$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSNSTopicSubscription_rawMessageDelivery'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSSNSTopicSubscription_rawMessageDelivery -timeout 120m
=== RUN   TestAccAWSSNSTopicSubscription_rawMessageDelivery
=== PAUSE TestAccAWSSNSTopicSubscription_rawMessageDelivery
=== CONT  TestAccAWSSNSTopicSubscription_rawMessageDelivery
--- PASS: TestAccAWSSNSTopicSubscription_rawMessageDelivery (61.36s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       61.681s


$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint -timeout 120m
=== RUN   TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint
=== PAUSE TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint
=== CONT  TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint
--- PASS: TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint (46.61s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       46.825s


$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint -timeout 120m
=== RUN   TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint
=== PAUSE TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint
=== CONT  TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint
--- PASS: TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint (111.10s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       111.425s

[carlos-zaragoza]

I believe the issues I faced running the tests in parallel were localized to my machine. The tests passed successfully on the automated build server.

[mjgpy3]

If I'm understand this PR correctly, the flow (before these changes) looked like this

So that's 4 separate requests to subscribe and create subscription attributes.

The idea then is that, in high traffic environments, messages might be delivered before the requests for limiting attributes (e.g. filter_policy) go through. Is that correct @carlos-zaragoza?

This PR just simplifies the creation-path to the following

which should subscribe and create the limiting attributes atomically.

Is there still an issue (with atomic updates) when just an update occurs since the update-path still makes multiple calls?

[mjgpy3]

Moreover, it's hard to say how long these requests actually take since all kinds of "eventually consistency" is at play here.

[carlos-zaragoza]

@mjgpy3 Yes, your explanation and diagrams are correct. It's all about doing the request once with exactly what the user wants rather than pieces of what they want until it finally comes together.

You bring up a good point about the update-path that I hadn't noticed. As it is currently, it could potentially cause problems if someone needed more than one attribute changed at the same time. That should probably be changed as well.

[carlos-zaragoza]

@mjgpy3 I got around to looking into the update-path question and it looks like it has to be done in multiple calls. The set-subscription-attributes command doesn't support updating multiple attributes at once.

So I think the changes as they are now is about as good as the current CLI can do.

[mjgpy3]

Thanks for looking into that, @carlos-zaragoza.

[mike-dazn]

Hi, Has there been any progress on getting this change in? We're having the same issues as #10495

[carlos-zaragoza]

@mike-dazn I've yet to hear from the maintainers presumably because the issue and the PR are too far down the list when sorting by community reactions (👍). Please make sure to leave a 👍 reaction on both to bump them higher in the priority list if this change would help you.

[carlos-zaragoza]

Hi can we get some sort of update on this getting through? It's been quite a while.

[AdamSlack]

Just want to add some more noise to this, would be great to see this make some progress! Would be very useful for us.

[bflad]

Apologies for the delayed review, @carlos-zaragoza, this looks good to me. 🚀

Output from acceptance testing in AWS Commercial:

--- PASS: TestAccAWSSNSTopicSubscription_basic (18.88s)
--- PASS: TestAccAWSSNSTopicSubscription_filterPolicy (42.59s)
--- PASS: TestAccAWSSNSTopicSubscription_rawMessageDelivery (43.36s)
--- PASS: TestAccAWSSNSTopicSubscription_deliveryPolicy (43.50s)
--- PASS: TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint (46.81s)
--- PASS: TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint (86.36s)

Output from acceptance testing in AWS GovCloud (US) (test failures unrelated due to API Gateway setup in testing configuration):

--- PASS: TestAccAWSSNSTopicSubscription_basic (17.55s)
--- FAIL: TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint (20.69s)
--- FAIL: TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint (27.48s)
--- PASS: TestAccAWSSNSTopicSubscription_filterPolicy (35.59s)
--- PASS: TestAccAWSSNSTopicSubscription_rawMessageDelivery (36.02s)
--- PASS: TestAccAWSSNSTopicSubscription_deliveryPolicy (36.23s)

[ghost]

This has been released in version 3.10.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

[ghost]

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!