Repository: ACloudGuru/serverless-plugin-aws-alerts (js)

Issue:
Using AWS Pseudo Parameters with SNS Topic declaration
# This is a Bug Report

## Description

I am trying to reference a SNS Topic that already exists using AWS Pseudo Parameters e.g.

```
custom:
  alerts:
    topics:
      alarm: 
        topic:
          Fn::Join:
            - ""
            - - "arn:aws:sns:"
              - Ref: "AWS::Region"
              - ":"
              - Ref: "AWS::AccountId"
              - ":CloudWatch-ErrorLogs"
    function:
      - name: SomeAlarm
        description: Some Alarm Description
        metric: SomeMetric
        treatMissingData: notBreaching
        threshold: 0
        statistic: Sum
        period: 60
        evaluationPeriods: 1
        comparisonOperator: GreaterThanThreshold
        pattern: '[ERROR]'
```
However this results in the following error;

>   Type Error ---------------------------------------------
> 
>   topic.indexOf is not a function
> 
>      For debugging logs, run again after setting the "SLS_DEBUG=*" environment variable.
> 
>   Get Support --------------------------------------------
>      Docs:          docs.serverless.com
>      Bugs:          github.com/serverless/serverless/issues
>      Forums:        forum.serverless.com
>      Chat:          gitter.im/serverless/serverless
> 
>   Your Environment Information -----------------------------
>      OS:                     linux
>      Node Version:           9.4.0
>      Serverless Version:     1.26.0

Does the plugin not support using AWS Pseudo Parameters or am I just doing something wrong?
Also, I haven't tried it yet but would CloudFormation References work (E.g. ${cf:x.y})?

## Additional Data

I also tried using variable resolution instead, so creating a custom variable which the topic then references;

```
custom:
  sns: 
    Fn::Join:
      - ""
      - - "arn:aws:sns:"
        - Ref: "AWS::Region"
        - ":"
        - Ref: "AWS::AccountId"
        - ":CloudWatch-ErrorLogs"  
  alerts:
    topics:
      alarm: 
        topic: ${self:custom.sns}
```

However that produced the same result.

* ***Serverless Framework Version: 1.26.0***:


Pull request:
Support CloudFormation in SNS topic definitions
## What did you implement:

Closes #54 and #68

Also includes functionality requested in #76

Currently only ARN or Fn::ImportValue are supported to specify an existing SNS topic for alarms. This PR extends support to any CloudFormation, e.g. Fn::Join and Ref.

Note: I already had a PR (#90) open for this fix, but it was closed due to incompatibility with newest version + inactivity.

## How can we verify it:

serverless.yml:

```yaml
custom: 
  alerts:
    topics:
      alarm:
        topic:
          Fn::ImportValue: ServiceMonitoring:monitoring-${opt:stage, 'dev'}
      ok:
        topic:
          Fn::Join:
            - ':'
            - - arn:aws:sns
              - Ref: AWS::Region
              - Ref: AWS::AccountId
              - example-ok-topic
      insufficientData:
        topic:
          Ref: ExampleInsufficientdataTopic
          

resources:
  Resources:
    ExampleInsufficientdataTopic:
      Type: AWS::SNS::Topic
      Properties:
        DisplayName: example-insufficientdata-topic
        Subscription:
          - Endpoint: me@example.com
            Protocol: EMAIL
```

Patch:
diff --git a/README.md b/README.md
index cf06002..b621cc9 100644
--- a/README.md
+++ b/README.md
@@ -172,7 +172,7 @@ custom:
 ```
 ## SNS Topics
 
-If topic name is specified, plugin assumes that topic does not exist and will create it. To use existing topics, specify ARNs or use Fn::ImportValue to use a topic exported with CloudFormation.
+If topic name is specified, plugin assumes that topic does not exist and will create it. To use existing topics, specify ARNs or use CloudFormation (e.g. Fn::ImportValue, Fn::Join and Ref) to refer to existing topics.
 
 #### ARN support
 
@@ -184,15 +184,37 @@ custom:
         topic: arn:aws:sns:${self:region}:${self::accountId}:monitoring-${opt:stage}
 ```
 
-#### Import support
+#### CloudFormation support
 
 ```yaml
-custom:
+custom: 
   alerts:
     topics:
       alarm:
         topic:
           Fn::ImportValue: ServiceMonitoring:monitoring-${opt:stage, 'dev'}
+      ok:
+        topic:
+          Fn::Join:
+            - ':'
+            - - arn:aws:sns
+              - Ref: AWS::Region
+              - Ref: AWS::AccountId
+              - example-ok-topic
+      insufficientData:
+        topic:
+          Ref: ExampleInsufficientdataTopic
+          
+
+resources:
+  Resources:
+    ExampleInsufficientdataTopic:
+      Type: AWS::SNS::Topic
+      Properties:
+        DisplayName: example-insufficientdata-topic
+        Subscription:
+          - Endpoint: me@example.com
+            Protocol: EMAIL
 ```
 
 ## SNS Notifications
diff --git a/src/index.js b/src/index.js
index b701cd3..8014123 100644
--- a/src/index.js
+++ b/src/index.js
@@ -237,13 +237,14 @@ class AlertsPlugin {
   _addAlertTopic(key, topics, alertTopics, customAlarmName) {
     const topicConfig = topics[key];
     const isTopicConfigAnObject = _.isObject(topicConfig);
-    const isTopicConfigAnImport = isTopicConfigAnObject && topicConfig.topic['Fn::ImportValue'];
 
     const topic = isTopicConfigAnObject ? topicConfig.topic : topicConfig;
+    const isTopicAnObject = _.isObject(topic);
+
     const notifications = isTopicConfigAnObject ? topicConfig.notifications : [];
 
     if (topic) {
-      if (isTopicConfigAnImport || topic.indexOf('arn:') === 0) {
+      if (isTopicAnObject || topic.indexOf('arn:') === 0) {
         if (customAlarmName) {
           alertTopics[customAlarmName] = alertTopics[customAlarmName] || {};
           alertTopics[customAlarmName][key] = topic;


Test patch:
diff --git a/src/index.test.js b/src/index.test.js
index 1ffd9cb..9c8170e 100644
--- a/src/index.test.js
+++ b/src/index.test.js
@@ -581,6 +581,58 @@ describe('#index', function () {
         }
       });
     });
+
+    it('should not create SNS topic when an object including Fn::Join is passed under "topic"', () => {
+      const topic = {
+        'Fn::Join': [
+          ':',
+          [
+            'arn:aws:sns',
+            '${self:provider.region}',
+            {Ref: 'AWS::AccountId'},
+            'ok-topic'
+          ]
+        ],
+      }
+      const plugin = pluginFactory({
+        topics: {
+          ok: {
+            topic
+          }
+        }
+      });
+
+      const config = plugin.getConfig();
+      const topics = plugin.compileAlertTopics(config);
+
+      expect(topics).toEqual({
+        ok: topic
+      });
+
+      expect(plugin.serverless.service.provider.compiledCloudFormationTemplate.Resources).toEqual({});
+    });
+
+    it('should not create SNS topic when an object including Ref is passed under "topic"', () => {
+      const topic = {
+        Ref: 'OkTopicLogicalId',
+      }
+      const plugin = pluginFactory({
+        topics: {
+          ok: {
+            topic
+          }
+        }
+      });
+
+      const config = plugin.getConfig();
+      const topics = plugin.compileAlertTopics(config);
+
+      expect(topics).toEqual({
+        ok: topic
+      });
+
+      expect(plugin.serverless.service.provider.compiledCloudFormationTemplate.Resources).toEqual({});
+    });
   });
 
   describe('#compileAlarms', () => {