fix(sns): create subscriptions in consumer scope

Since in most cases the consumer needs to reference the topic to permit it to send them messages (e.g. invoke a lambda function or send messages to the queue), it makes much more send to create the SNS subscription resource on the consumer's scope/stack instead of the topic's. This change adds an optional `scope` field to `TopicSubscriptionConfig` which is respected by `topic.addSubscription`. If `scope` is not defined, the topic's scope will be used. We also changed `subscriberId` to be optional, since in the case where `scope` is specified, the natural ID for the subscription construct would be the topic's unique ID, which is now the default. A runtime error will be emitted if both `scope` and `subscriberId` are not provided. Fixes #3064
aws · Jun 25, 2019 · 3d8cf59 · 3d8cf59
1 parent 6d38487
commit 3d8cf59
Show file tree

Hide file tree

Showing 8 changed files with 357 additions and 240 deletions.
diff --git a/packages/@aws-cdk/aws-sns-subscriptions/lib/lambda.ts b/packages/@aws-cdk/aws-sns-subscriptions/lib/lambda.ts
@@ -24,13 +24,14 @@ export class LambdaSubscription implements sns.ITopicSubscription {
       throw new Error(`The supplied lambda Function object must be an instance of Construct`);
     }
 
-    this.fn.addPermission(topic.node.id, {
+    this.fn.addPermission(`AllowInvoke:${topic.node.uniqueId}`, {
       sourceArn: topic.topicArn,
       principal: new iam.ServicePrincipal('sns.amazonaws.com'),
     });
 
     return {
-      subscriberId: this.fn.node.id,
+      scope: this.fn,
+      subscriberId: topic.node.id,
       endpoint: this.fn.functionArn,
       protocol: sns.SubscriptionProtocol.LAMBDA,
       filterPolicy: this.props.filterPolicy,

diff --git a/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts b/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts
@@ -44,7 +44,8 @@ export class SqsSubscription implements sns.ITopicSubscription {
     }));
 
     return {
-      subscriberId: this.queue.node.id,
+      scope: this.queue,
+      subscriberId: topic.node.uniqueId,
       endpoint: this.queue.queueArn,
       protocol: sns.SubscriptionProtocol.SQS,
       rawMessageDelivery: this.props.rawMessageDelivery,

diff --git a/packages/@aws-cdk/aws-sns-subscriptions/test/integ.sns-lambda.expected.json b/packages/@aws-cdk/aws-sns-subscriptions/test/integ.sns-lambda.expected.json
@@ -3,65 +3,6 @@
     "MyTopic86869434": {
       "Type": "AWS::SNS::Topic"
     },
-    "MyTopicEchoD1E0EE5C": {
-      "Type": "AWS::SNS::Subscription",
-      "Properties": {
-        "Protocol": "lambda",
-        "TopicArn": {
-          "Ref": "MyTopic86869434"
-        },
-        "Endpoint": {
-          "Fn::GetAtt": [
-            "Echo11F3FB29",
-            "Arn"
-          ]
-        }
-      }
-    },
-    "MyTopicFiltered55457D11": {
-      "Type": "AWS::SNS::Subscription",
-      "Properties": {
-        "Protocol": "lambda",
-        "TopicArn": {
-          "Ref": "MyTopic86869434"
-        },
-        "Endpoint": {
-          "Fn::GetAtt": [
-            "Filtered186C0D0A",
-            "Arn"
-          ]
-        },
-        "FilterPolicy": {
-          "color": [
-            "red",
-            {
-              "prefix": "bl"
-            },
-            {
-              "prefix": "ye"
-            }
-          ],
-          "size": [
-            {
-              "anything-but": [
-                "small",
-                "medium"
-              ]
-            }
-          ],
-          "price": [
-            {
-              "numeric": [
-                ">=",
-                100,
-                "<=",
-                200
-              ]
-            }
-          ]
-        }
-      }
-    },
     "EchoServiceRoleBE28060B": {
       "Type": "AWS::IAM::Role",
       "Properties": {
@@ -122,7 +63,7 @@
         "EchoServiceRoleBE28060B"
       ]
     },
-    "EchoMyTopicF6EBB45F": {
+    "EchoAllowInvokeawscdksnslambdaMyTopic6C62AB907F727CDA": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -138,6 +79,21 @@
         }
       }
     },
+    "EchoMyTopic4CB8819E": {
+      "Type": "AWS::SNS::Subscription",
+      "Properties": {
+        "Protocol": "lambda",
+        "TopicArn": {
+          "Ref": "MyTopic86869434"
+        },
+        "Endpoint": {
+          "Fn::GetAtt": [
+            "Echo11F3FB29",
+            "Arn"
+          ]
+        }
+      }
+    },
     "FilteredServiceRole16D9DDC1": {
       "Type": "AWS::IAM::Role",
       "Properties": {
@@ -198,7 +154,7 @@
         "FilteredServiceRole16D9DDC1"
       ]
     },
-    "FilteredMyTopic804BCBC3": {
+    "FilteredAllowInvokeawscdksnslambdaMyTopic6C62AB90A2EA1666": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -213,6 +169,50 @@
           "Ref": "MyTopic86869434"
         }
       }
+    },
+    "FilteredMyTopicC8395C27": {
+      "Type": "AWS::SNS::Subscription",
+      "Properties": {
+        "Protocol": "lambda",
+        "TopicArn": {
+          "Ref": "MyTopic86869434"
+        },
+        "Endpoint": {
+          "Fn::GetAtt": [
+            "Filtered186C0D0A",
+            "Arn"
+          ]
+        },
+        "FilterPolicy": {
+          "color": [
+            "red",
+            {
+              "prefix": "bl"
+            },
+            {
+              "prefix": "ye"
+            }
+          ],
+          "size": [
+            {
+              "anything-but": [
+                "small",
+                "medium"
+              ]
+            }
+          ],
+          "price": [
+            {
+              "numeric": [
+                ">=",
+                100,
+                "<=",
+                200
+              ]
+            }
+          ]
+        }
+      }
     }
   }
 }
diff --git a/packages/@aws-cdk/aws-sns-subscriptions/test/integ.sns-sqs.lit.expected.json b/packages/@aws-cdk/aws-sns-subscriptions/test/integ.sns-sqs.lit.expected.json
@@ -3,21 +3,6 @@
     "MyTopic86869434": {
       "Type": "AWS::SNS::Topic"
     },
-    "MyTopicMyQueueFA241964": {
-      "Type": "AWS::SNS::Subscription",
-      "Properties": {
-        "Protocol": "sqs",
-        "TopicArn": {
-          "Ref": "MyTopic86869434"
-        },
-        "Endpoint": {
-          "Fn::GetAtt": [
-            "MyQueueE6CA6235",
-            "Arn"
-          ]
-        }
-      }
-    },
     "MyQueueE6CA6235": {
       "Type": "AWS::SQS::Queue"
     },
@@ -55,6 +40,21 @@
           }
         ]
       }
+    },
+    "MyQueueawscdksnssqsMyTopic9361DEA223429051": {
+      "Type": "AWS::SNS::Subscription",
+      "Properties": {
+        "Protocol": "sqs",
+        "TopicArn": {
+          "Ref": "MyTopic86869434"
+        },
+        "Endpoint": {
+          "Fn::GetAtt": [
+            "MyQueueE6CA6235",
+            "Arn"
+          ]
+        }
+      }
     }
   }
 }