SNS Support #332

uberblah · 2017-05-08T14:55:16Z

This PR adds support for SNS Topics and Subscriptions

Please see "topic_arns" in sns_topic.rb for an example of paging AWS results to ensure arbitrary numbers of resources can be imported.

dtan4 · 2017-05-08T14:55:23Z

lib/terraforming/resource/sns_topic_subscription.rb

+      def subscriptions
+        subscription_arns.map do |subscription_arn|
+          # check explicitly for an issue with some subscriptions that returns ARN=PendingConfirmation
+          if subscription_arn == "PendingConfirmation" then next end


[rubocop]

Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||. :ref

dtan4 · 2017-05-08T14:55:24Z

lib/terraforming/resource/sns_topic_subscription.rb

+          resp = @client.list_subscriptions(next_token: token)
+          arns += resp.subscriptions.map(&:subscription_arn).flatten
+          token = resp.next_token
+        end until token.nil?


[rubocop]

Use Kernel#loop with break rather than begin/end/until(or while). :ref

dtan4 · 2017-05-08T14:55:24Z

lib/terraforming/resource/sns_topic.rb

+          resp = @client.list_topics(next_token: token)
+          arns += resp.topics.map(&:topic_arn).flatten
+          token = resp.next_token
+        end until token.nil?


[rubocop]

Use Kernel#loop with break rather than begin/end/until(or while). :ref

dtan4 · 2017-05-08T14:55:24Z

spec/lib/terraforming/resource/sns_topic_subscription_spec.rb

+
+module Terraforming
+  module Resource
+    describe SNSTopicSubscription do


[rubocop]

Block has too many lines. [57/25]

dtan4 · 2017-05-08T14:55:25Z

spec/lib/terraforming/resource/sns_topic_spec.rb

+
+module Terraforming
+  module Resource
+    describe SNSTopic do


[rubocop]

Block has too many lines. [103/25]

dtan4 · 2017-05-08T14:55:25Z

spec/lib/terraforming/resource/sns_topic_spec.rb

+        client.stub_responses(:get_topic_attributes, attributes: attributes)
+      end
+
+      describe ".tf" do


[rubocop]

Block has too many lines. [56/25]

coveralls · 2017-05-08T15:00:52Z

Coverage remained the same at 100.0% when pulling 7ba1108 on uberblah:sns into 918540b on dtan4:master.

coveralls · 2017-05-08T15:06:36Z

Coverage remained the same at 100.0% when pulling 19506c0 on uberblah:sns into 918540b on dtan4:master.

uberblah · 2017-05-08T15:10:37Z

Let me know if these large block comments from Rubocop are really important, but for now I'm assuming they aren't a huge deal within tests, since tests for other resources have comparably-sized blocks

coveralls · 2017-05-08T15:17:01Z

Coverage remained the same at 100.0% when pulling 4ddb5e0 on uberblah:sns into 918540b on dtan4:master.

coveralls · 2017-05-08T22:21:27Z

Coverage remained the same at 100.0% when pulling 479c05f on uberblah:sns into 918540b on dtan4:master.

dtan4 · 2017-06-01T03:40:00Z

Sorry for late review 🙇

SNS Topics snst works well, but SNS Topic Subscriptions snss does not.

One of our SNS Topic Subscription uses email protocol. However, Terraform does not support email protocol.
See: https://www.terraform.io/docs/providers/aws/r/sns_topic_subscription.html#protocols-supported

resource "aws_sns_topic_subscription" "xxx" {
  topic_arn                       = "arn:aws:sns:ap-northeast-1:xxx:dev"
  protocol                        = "email"
  endpoint                        = "[email protected]"
  raw_message_delivery            = "false"
}

$ terraform plan
1 error(s) occurred:

* aws_sns_topic_subscription.xxx: Unsupported protocol (email) for SNS Topic

I have a suggestion 💡 if a SNS subscription uses email or another unsupported protocol...

do not generate tfstate for the resource
generate tf for the resource as comment

dtan4 · 2017-06-02T19:12:48Z

lib/terraforming/resource/sns_topic_subscription.rb

+      end
+
+      def tfstate
+        subscriptions.reject{|x| x["Protocol"].include?("email")}


[rubocop]

Space missing to the left of {.

Space between { and | missing.

Space missing inside }.

coveralls · 2017-06-02T19:16:01Z

Coverage remained the same at 100.0% when pulling 260db3d on uberblah:sns into 918540b on dtan4:master.

coveralls · 2017-06-02T19:20:20Z

Coverage remained the same at 100.0% when pulling 1d7fd3d on uberblah:sns into 918540b on dtan4:master.

coveralls · 2017-06-02T19:30:30Z

Coverage remained the same at 100.0% when pulling 30facaf on uberblah:sns into 918540b on dtan4:master.

coveralls · 2017-06-02T19:30:30Z

Coverage remained the same at 100.0% when pulling 30facaf on uberblah:sns into 918540b on dtan4:master.

dtan4 · 2017-06-02T19:34:13Z

spec/lib/terraforming/resource/sns_topic_subscription_spec.rb

+
+module Terraforming
+  module Resource
+    describe SNSTopicSubscription do


[rubocop]

Block has too many lines. [81/25]

coveralls · 2017-06-02T19:37:14Z

Coverage remained the same at 100.0% when pulling 23e1a32 on uberblah:sns into 918540b on dtan4:master.

uberblah · 2017-06-02T19:38:24Z

I can't see what's going on in the SideCI tests, so I can't tell why that's failing but everything on my end appears to work properly... Let me know if this update helped. Should now produce no tfstate and commented tf. Thanks for the advice! @dtan4

dtan4 · 2017-06-14T03:42:43Z

SideCI says the lines of method exceeds threshold... but it's not a important issue. You can ignore it.
(I'll remove this check asap...)

I checked the behavior again with the latest patch, and it suceeded!
LGTM 👍 Thanks ❗️

uberblah added 8 commits May 4, 2017 13:53

initial attempt at sns_topic support

728b80f

fixes, tests for sns topics

6ffdc81

sns subscriptions, bug fixes

3c06598

clarity, add bugfix to sns topics

975d380

ensure all results are collected

3e966ca

readability, remove unneeded bugfix

77d5df4

additional fields, some mandatory in tfstate

49340e4

style and formatting

7ba1108

dtan4 reviewed May 8, 2017

View reviewed changes

fix rubocop violations

19506c0

add SNS subscriptions to readme

4ddb5e0

mark a field as optional

479c05f

uberblah changed the title ~~SNS Support, suggestions for paged request handling~~ SNS Support May 14, 2017

filter out email subscriptions

260db3d

dtan4 reviewed Jun 2, 2017

View reviewed changes

minor template/test mismatch

1d7fd3d

style comments

30facaf

tests for commenting of esns email subscription tf

23e1a32

dtan4 reviewed Jun 2, 2017

View reviewed changes

dtan4 merged commit 8061079 into dtan4:master Jun 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SNS Support #332

SNS Support #332

uberblah commented May 8, 2017 •

edited

Loading

dtan4 May 8, 2017

dtan4 May 8, 2017

dtan4 May 8, 2017

dtan4 May 8, 2017

dtan4 May 8, 2017

dtan4 May 8, 2017

coveralls commented May 8, 2017 •

edited

Loading

coveralls commented May 8, 2017 •

edited

Loading

uberblah commented May 8, 2017

coveralls commented May 8, 2017 •

edited

Loading

coveralls commented May 8, 2017 •

edited

Loading

dtan4 commented Jun 1, 2017

dtan4 Jun 2, 2017

coveralls commented Jun 2, 2017 •

edited

Loading

coveralls commented Jun 2, 2017 •

edited

Loading

coveralls commented Jun 2, 2017 •

edited

Loading

coveralls commented Jun 2, 2017

dtan4 Jun 2, 2017

coveralls commented Jun 2, 2017 •

edited

Loading

uberblah commented Jun 2, 2017 •

edited

Loading

dtan4 commented Jun 14, 2017

SNS Support #332

SNS Support #332

Conversation

uberblah commented May 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 8, 2017 • edited Loading

coveralls commented May 8, 2017 • edited Loading

uberblah commented May 8, 2017

coveralls commented May 8, 2017 • edited Loading

coveralls commented May 8, 2017 • edited Loading

dtan4 commented Jun 1, 2017

Choose a reason for hiding this comment

coveralls commented Jun 2, 2017 • edited Loading

coveralls commented Jun 2, 2017 • edited Loading

coveralls commented Jun 2, 2017 • edited Loading

coveralls commented Jun 2, 2017

Choose a reason for hiding this comment

coveralls commented Jun 2, 2017 • edited Loading

uberblah commented Jun 2, 2017 • edited Loading

dtan4 commented Jun 14, 2017

uberblah commented May 8, 2017 •

edited

Loading

coveralls commented May 8, 2017 •

edited

Loading

coveralls commented May 8, 2017 •

edited

Loading

coveralls commented May 8, 2017 •

edited

Loading

coveralls commented May 8, 2017 •

edited

Loading

coveralls commented Jun 2, 2017 •

edited

Loading

coveralls commented Jun 2, 2017 •

edited

Loading

coveralls commented Jun 2, 2017 •

edited

Loading

coveralls commented Jun 2, 2017 •

edited

Loading

uberblah commented Jun 2, 2017 •

edited

Loading