fix(sns): create subscription object under subscriber

Create the SNS Subscription object under the subscriber instead of the subscribee, avoiding cyclic stack dependencies if those objects would be in different stacks. Fixes #1643, fixes #1534.
aws · Jan 31, 2019 · 5c4a9e5 · 5c4a9e5
1 parent 8f1a5e8
commit 5c4a9e5
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 49 deletions.
diff --git a/packages/@aws-cdk/aws-sns/lib/topic-ref.ts b/packages/@aws-cdk/aws-sns/lib/topic-ref.ts
@@ -169,14 +169,18 @@ export abstract class TopicBase extends cdk.Construct implements ITopic {
    * @param queue The target queue
    */
   public subscribeQueue(queue: sqs.IQueue): Subscription {
-    const subscriptionName = queue.node.id + 'Subscription';
-    if (this.node.tryFindChild(subscriptionName)) {
+    if (!cdk.Construct.isConstruct(queue)) {
+      throw new Error(`The supplied Queue object must be an instance of Construct`);
+    }
+
+    const subscriptionName = this.node.id + 'Subscription';
+    if (queue.node.tryFindChild(subscriptionName)) {
       throw new Error(`A subscription between the topic ${this.node.id} and the queue ${queue.node.id} already exists`);
     }
 
     // we use the queue name as the subscription's. there's no meaning to subscribing
     // the same queue twice on the same topic.
-    const sub = new Subscription(this, subscriptionName, {
+    const sub = new Subscription(queue, subscriptionName, {
       topic: this,
       endpoint: queue.queueArn,
       protocol: SubscriptionProtocol.Sqs
@@ -203,13 +207,17 @@ export abstract class TopicBase extends cdk.Construct implements ITopic {
    * @param lambdaFunction The Lambda function to invoke
    */
   public subscribeLambda(lambdaFunction: lambda.IFunction): Subscription {
-    const subscriptionName = lambdaFunction.id + 'Subscription';
+    if (!cdk.Construct.isConstruct(lambdaFunction)) {
+      throw new Error(`The supplied lambda Function object must be an instance of Construct`);
+    }
+
+    const subscriptionName = this.node.id + 'Subscription';
 
-    if (this.node.tryFindChild(subscriptionName)) {
+    if (lambdaFunction.node.tryFindChild(subscriptionName)) {
       throw new Error(`A subscription between the topic ${this.node.id} and the lambda ${lambdaFunction.id} already exists`);
     }
 
-    const sub = new Subscription(this, subscriptionName, {
+    const sub = new Subscription(lambdaFunction, subscriptionName, {
       topic: this,
       endpoint: lambdaFunction.functionArn,
       protocol: SubscriptionProtocol.Lambda

diff --git a/packages/@aws-cdk/aws-sns/test/integ.sns-bucket-notifications.expected.json b/packages/@aws-cdk/aws-sns/test/integ.sns-bucket-notifications.expected.json
@@ -204,4 +204,4 @@
       }
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-sns/test/integ.sns-event-rule-target.expected.json b/packages/@aws-cdk/aws-sns/test/integ.sns-event-rule-target.expected.json
@@ -3,36 +3,21 @@
     "MyTopic86869434": {
       "Type": "AWS::SNS::Topic"
     },
-    "MyTopicMyQueueSubscription3245B11E": {
-      "Type": "AWS::SNS::Subscription",
-      "Properties": {
-        "Endpoint": {
-          "Fn::GetAtt": [
-            "MyQueueE6CA6235",
-            "Arn"
-          ]
-        },
-        "Protocol": "sqs",
-        "TopicArn": {
-          "Ref": "MyTopic86869434"
-        }
-      }
-    },
     "MyTopicPolicy12A5EC17": {
       "Type": "AWS::SNS::TopicPolicy",
       "Properties": {
         "PolicyDocument": {
           "Statement": [
             {
-              "Sid": "0",
               "Action": "sns:Publish",
               "Effect": "Allow",
               "Principal": {
                 "Service": "events.amazonaws.com"
               },
               "Resource": {
                 "Ref": "MyTopic86869434"
-              }
+              },
+              "Sid": "0"
             }
           ],
           "Version": "2012-10-17"
@@ -62,6 +47,21 @@
     "MyQueueE6CA6235": {
       "Type": "AWS::SQS::Queue"
     },
+    "MyQueueMyTopicSubscriptionEB66AD1B": {
+      "Type": "AWS::SNS::Subscription",
+      "Properties": {
+        "Endpoint": {
+          "Fn::GetAtt": [
+            "MyQueueE6CA6235",
+            "Arn"
+          ]
+        },
+        "Protocol": "sqs",
+        "TopicArn": {
+          "Ref": "MyTopic86869434"
+        }
+      }
+    },
     "MyQueuePolicy6BBEDDAC": {
       "Type": "AWS::SQS::QueuePolicy",
       "Properties": {

diff --git a/packages/@aws-cdk/aws-sns/test/integ.sns-lambda.expected.json b/packages/@aws-cdk/aws-sns/test/integ.sns-lambda.expected.json
@@ -3,21 +3,6 @@
     "MyTopic86869434": {
       "Type": "AWS::SNS::Topic"
     },
-    "MyTopicEchoSubscription021036AD": {
-      "Type": "AWS::SNS::Subscription",
-      "Properties": {
-        "Endpoint": {
-          "Fn::GetAtt": [
-            "Echo11F3FB29",
-            "Arn"
-          ]
-        },
-        "Protocol": "lambda",
-        "TopicArn": {
-          "Ref": "MyTopic86869434"
-        }
-      }
-    },
     "EchoServiceRoleBE28060B": {
       "Type": "AWS::IAM::Role",
       "Properties": {
@@ -34,7 +19,18 @@
           "Version": "2012-10-17"
         },
         "ManagedPolicyArns": [
-          {"Fn::Join":["",["arn:",{"Ref":"AWS::Partition"},":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"]]}
+          {
+            "Fn::Join": [
+              "",
+              [
+                "arn:",
+                {
+                  "Ref": "AWS::Partition"
+                },
+                ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"
+              ]
+            ]
+          }
         ]
       }
     },
@@ -57,6 +53,21 @@
         "EchoServiceRoleBE28060B"
       ]
     },
+    "EchoMyTopicSubscriptionA634C6D7": {
+      "Type": "AWS::SNS::Subscription",
+      "Properties": {
+        "Endpoint": {
+          "Fn::GetAtt": [
+            "Echo11F3FB29",
+            "Arn"
+          ]
+        },
+        "Protocol": "lambda",
+        "TopicArn": {
+          "Ref": "MyTopic86869434"
+        }
+      }
+    },
     "EchoMyTopicF6EBB45F": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
@@ -71,4 +82,4 @@
       }
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-sns/test/integ.sns-sqs.lit.expected.json b/packages/@aws-cdk/aws-sns/test/integ.sns-sqs.lit.expected.json
@@ -3,7 +3,10 @@
     "MyTopic86869434": {
       "Type": "AWS::SNS::Topic"
     },
-    "MyTopicMyQueueSubscription3245B11E": {
+    "MyQueueE6CA6235": {
+      "Type": "AWS::SQS::Queue"
+    },
+    "MyQueueMyTopicSubscriptionEB66AD1B": {
       "Type": "AWS::SNS::Subscription",
       "Properties": {
         "Endpoint": {
@@ -18,9 +21,6 @@
         }
       }
     },
-    "MyQueueE6CA6235": {
-      "Type": "AWS::SQS::Queue"
-    },
     "MyQueuePolicy6BBEDDAC": {
       "Type": "AWS::SQS::QueuePolicy",
       "Properties": {

diff --git a/packages/@aws-cdk/aws-sns/test/test.sns.ts b/packages/@aws-cdk/aws-sns/test/test.sns.ts
@@ -149,7 +149,7 @@ export = {
           "TopicName": "topicName"
           }
         },
-        "MyTopicMyQueueSubscription3245B11E": {
+        "MyQueueMyTopicSubscriptionEB66AD1B": {
           "Type": "AWS::SNS::Subscription",
           "Properties": {
           "Endpoint": {
@@ -233,7 +233,7 @@ export = {
           "TopicName": "topicName"
           }
         },
-        "MyTopicMyFuncSubscriptionEAF54A3F": {
+        "MyFuncMyTopicSubscription708A6535": {
           "Type": "AWS::SNS::Subscription",
           "Properties": {
           "Endpoint": {
@@ -368,7 +368,7 @@ export = {
               "TopicName": "topicName"
             }
           },
-          "MyTopicMyQueueSubscription3245B11E": {
+          "MyQueueMyTopicSubscriptionEB66AD1B": {
             "Type": "AWS::SNS::Subscription",
             "Properties": {
               "Endpoint": {
@@ -383,7 +383,7 @@ export = {
               }
             }
           },
-          "MyTopicMyFuncSubscriptionEAF54A3F": {
+          "MyFuncMyTopicSubscription708A6535": {
             "Type": "AWS::SNS::Subscription",
             "Properties": {
               "Endpoint": {
-Original file line number
+Diff line change
@@ Expand Up / @@ -204,4 +204,4 @@ @@
           }
         }
       }
-    }
+    }