Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support all intrinsic functions for SNS definition #3824

Closed
timsuchanek opened this issue Jun 21, 2017 · 28 comments · Fixed by #5443
Closed

Support all intrinsic functions for SNS definition #3824

timsuchanek opened this issue Jun 21, 2017 · 28 comments · Fixed by #5443

Comments

@timsuchanek
Copy link

This is a Bug Report

Description

The docs state, that you can use cloudformation intrinsic functions while defining the sns source for a function, however, when I take this example:

functions:
  dispatcher:
    handler: dispatcher.dispatch
    events:
      - sns:
          arn:
            Fn::Join:
              - ""
              - - "arn:aws:sns:"
                - Ref: "AWS::Region"
                - ":"
                - Ref: "AWS::AccountId"
                - ":MyCustomTopic"
          topicName: MyCustomTopic

and adjust it to ours needs

functions:
  dispatcher:
    handler: dispatcher.dispatch
    events:
      - sns:
          arn:
            Fn::Join:
              - ""
              - - "arn:aws:sns:"
                - Ref: "AWS::Region"
                - ":"
                - Ref: "AWS::AccountId"
                - ":"
                - { Fn::GetAtt: [ Infrastructure, Outputs.SNSTopic ] }
          topicName: { Fn::GetAtt: [ Infrastructure, Outputs.SNSTopic ] }

it returns the following error:

  Stack Trace --------------------------------------------

TypeError: event.sns.arn.indexOf is not a function
    at functionObj.events.forEach.event (/Users/tim/npm-global/lib/node_modules/serverless/lib/plugins/aws/package/compile/events/sns/index.js:36:44)
    at Array.forEach (native)
    at serverless.service.getAllFunctions.forEach (/Users/tim/npm-global/lib/node_modules/serverless/lib/plugins/aws/package/compile/events/sns/index.js:22:28)
    at Array.forEach (native)
    at AwsCompileSNSEvents.compileSNSEvents (/Users/tim/npm-global/lib/node_modules/serverless/lib/plugins/aws/package/compile/events/sns/index.js:18:47)
    at BbPromise.reduce (/Users/tim/npm-global/lib/node_modules/serverless/lib/classes/PluginManager.js:254:55)
From previous event:
    at PluginManager.invoke (/Users/tim/npm-global/lib/node_modules/serverless/lib/classes/PluginManager.js:254:22)
    at PluginManager.spawn (/Users/tim/npm-global/lib/node_modules/serverless/lib/classes/PluginManager.js:266:17)
    at Deploy.BbPromise.bind.then (/Users/tim/npm-global/lib/node_modules/serverless/lib/plugins/deploy/deploy.js:88:48)
From previous event:
    at Object.before:deploy:deploy [as hook] (/Users/tim/npm-global/lib/node_modules/serverless/lib/plugins/deploy/deploy.js:86:8)
    at BbPromise.reduce (/Users/tim/npm-global/lib/node_modules/serverless/lib/classes/PluginManager.js:254:55)
From previous event:
    at PluginManager.invoke (/Users/tim/npm-global/lib/node_modules/serverless/lib/classes/PluginManager.js:254:22)
    at PluginManager.run (/Users/tim/npm-global/lib/node_modules/serverless/lib/classes/PluginManager.js:273:17)
    at variables.populateService.then (/Users/tim/npm-global/lib/node_modules/serverless/lib/Serverless.js:105:33)
    at runCallback (timers.js:672:20)
    at tryOnImmediate (timers.js:645:5)
    at processImmediate [as _immediateCallback] (timers.js:617:5)
From previous event:
    at Serverless.run (/Users/tim/npm-global/lib/node_modules/serverless/lib/Serverless.js:92:74)
    at serverless.init.then (/Users/tim/npm-global/lib/node_modules/serverless/bin/serverless:30:50)

The example how it is in the docs works without problems, but adding the Fn::GetAtt makes it fail.

Related PR

Additional Data

Your Environment Information

  • OS: darwin
  • Node Version: 7.9.0
  • Serverless Version: 1.15.3
@pmuens
Copy link
Contributor

pmuens commented Jun 22, 2017

Thanks for opening @timsuchanek 👍

Just dropping a line that the global arn parser could help here: #3212.

@pmuens pmuens changed the title Support Functions for SNS definition Support all intrinsic functions for SNS definition Jun 22, 2017
@jaikdean
Copy link
Contributor

+1

2 similar comments
@gabrielkaputa
Copy link

+1

@itzaliza
Copy link

+1

@reidblomquist
Copy link

reidblomquist commented Apr 30, 2018

+1 poked around looks like the type checking in serverless/lib/plugins/aws/package/compile/events/sns/index.js is way too fuzzy and allows for indexOf to be called on objects - if trying to define TopicName as an object it skips the first check in typeof event.topicName === 'string' and tries to reference the indexOf on event.sns.arn inside of an if that checks typeof event.sns.arn === 'object' || 'string'... Ideally this would at least be patched for now with a more meaningful error until it can properly parse objects

@pmuens would you accept a PR to clean the type checking here up and throw a more meaningful error if a non string topicName is provided?

@crysislinux
Copy link

any updates? I have to hardcode those arn related things for now. Feels not flexible.

@astroanu
Copy link

Is this being investigated ??

@AJF4
Copy link

AJF4 commented Jun 26, 2018

+1 - any updates on this?

@DavidJFelix
Copy link

We're also running into this using Fn::ImportValue we have one serverless project which defines and exports the arn of the event from the outputs section of resources and the other attempts to import them and fails with the same error shown above.

 hello:
    handler: handleHelloTopic
    events:
      - sns: 
          arn: {Fn::ImportValue: 'HelloSNSTopicArn'}

@gaviniflix
Copy link

gaviniflix commented Jul 23, 2018

handler: src/handlers/on-demand-extensions/index.extensionCreated
events:
  - sns: 
      arn:
        Fn::GetAtt:
          - OnDemandExtensionsCreatedTopic
          - Arn
      topicName: 
        Fn::Gett:
          - OnDemandExtensionsCreatedTopic
          - TopicName

gives me:

TypeError: event.sns.arn.indexOf is not a function
    at functionObj.events.forEach.event (/Users/gavinvangent/.nvm/versions/node/v6.10.3/lib/node_modules/serverless/lib/plugins/aws/package/compile/events/sns/index.js:35:44)
    at Array.forEach (native)
    at serverless.service.getAllFunctions.forEach (/Users/gavinvangent/.nvm/versions/node/v6.10.3/lib/node_modules/serverless/lib/plugins/aws/package/compile/events/sns/index.js:22:28)

using arn and topicName based off this output:

  Serverless Error ---------------------------------------

  Missing or invalid topicName property for sns event in function "onDemandExtensionsExtensionCreated"
The correct syntax is: sns: topic-name-or-arn OR an object with  arn and topicName
OR topicName and displayName. Please check the docs for more info.

using serverless@1.28.0

@drexler
Copy link
Contributor

drexler commented Sep 2, 2018

+1 here. is there a workaround in the interim for this? Using serverless@1.30.1

@DavidJFelix
Copy link

@drexler if you're trying to use Fn::import, we're working around it by having the exporting project set the variable in SSM. The current limitation on cloudformation SSM is that secure variables aren't allowed, but since exports from cloudformation are public, this shouldn't be an issue. You can import the SSM variable in the environment section and use it throughout serverless.yml in the consuming project.

@drexler
Copy link
Contributor

drexler commented Sep 8, 2018

@DavidJFelix thx.

@gaviniflix
Copy link

Still no fix? 😢

@ghost
Copy link

ghost commented Oct 30, 2018

Any update on this?

@hmshwt
Copy link

hmshwt commented Nov 1, 2018

@gaviniflix I found a work around by using the custom section in serverless.yml. I import the arn value from another cloudformation stack there.

custom:
  snsTopic:
    Fn::ImportValue: 'other-cloudformation-stack-${self:provider.stage}:TopicName'

and later in the events section I substitute it:

    events:
      - sns:
          arn: ${self:custom.snsTopic}
          topicName: 'TopicName'

It works for me on SLS version 1.32.0, node 8.10.0 . I hope it is helpful for you too.

@yahehe
Copy link
Contributor

yahehe commented Nov 2, 2018

@hmshwt I'm using the same version of SLS, node 8.12.0, and that does not work for me.

EDIT: As I discovered below, this does in fact work, I was using a non-string value for topicName which turns out to be unnecessary as when an arn object is specified, topicName is only used to name the underlying cloudformation resources.

@hmshwt
Copy link

hmshwt commented Nov 3, 2018

@yahehe Could you please paste the error and the relevant section of the serverless.yml file.

@yahehe
Copy link
Contributor

yahehe commented Nov 3, 2018

Serverless:

custom:
  snsTopic:
    Fn::ImportValue: '${self:provider.stage}:other-stack:OtherStackTopicName'
...
    events:
      - sns:
          arn: ${self:custom.snsTopic}

Error:

Type Error ---------------------------------------------

  event.sns.arn.indexOf is not a function

     For debugging logs, run again after setting the "SLS_DEBUG=*" environment variable.

  Stack Trace --------------------------------------------

TypeError: event.sns.arn.indexOf is not a function
    at functionObj.events.forEach.event (/usr/lib/node_modules/serverless/lib/plugins/aws/package/compile/events/sns/index.js:35:44)
    at Array.forEach (<anonymous>)
    at serverless.service.getAllFunctions.forEach (/usr/lib/node_modules/serverless/lib/plugins/aws/package/compile/events/sns/index.js:22:28)
    at Array.forEach (<anonymous>)
    at AwsCompileSNSEvents.compileSNSEvents (/usr/lib/node_modules/serverless/lib/plugins/aws/package/compile/events/sns/index.js:18:47)
    at BbPromise.reduce (/usr/lib/node_modules/serverless/lib/classes/PluginManager.js:390:55)

@hmshwt
Copy link

hmshwt commented Nov 3, 2018

@yahehe Have you tried adding

topicName: 'TopicName'

after

arn: ${self:custom.snsTopic}

?

@yahehe
Copy link
Contributor

yahehe commented Nov 3, 2018

That won't do anything, the way the code is structured will have it fail before it looks at the topic name. I'm working on a fix now

@hmshwt
Copy link

hmshwt commented Nov 4, 2018

@yahehe I tried to remove the topicName property and I get the same error as you did

event.sns.arn.indexOf is not a function ...

Once I add it back, the deployment is successful.
Anyway, thank you for working on solving the root problem and I hope it is merged soon.

@yahehe
Copy link
Contributor

yahehe commented Nov 4, 2018

@hmshwt ah you know what, I think you might be right. Part of my issue is I was trying to have topicName be a function as well, as it wasn't clear that it's really only being used to name the cloudformation resources.
I updated my PR to reflect that it basically just throws a better error message. Your solution did work for me.

@chensjlv
Copy link

chensjlv commented Nov 7, 2018

Just figured out, by reading https://github.com/serverless/serverless/blob/master/lib/plugins/aws/package/compile/events/sns/index.js#L32

topicName has to be a string for this to work.

functions:
  ***:
    handler: ***/***
    events:
      - sns:
          arn:
            Ref: SnsTopic
          topicName: ${self:custom.snsTopic}

If topicName is from something like Fn::GetAtt, topicName will become an object and trigger https://github.com/serverless/serverless/blob/master/lib/plugins/aws/package/compile/events/sns/index.js#L35, which looks like a bug for me, because why cannot we have both arn and topic name to be object.

@johndoe2361289638165
Copy link

Any updates on this?

@nkhine
Copy link

nkhine commented Feb 28, 2019

i am also getting this error, here is my serverless.yml

  s3ToFtp:
    handler: bin/s3-to-ftp
    # files are stored in memory, Ram should be sufficient to accomdate the file
    memorySize: 256
    timeout: 120
    package:
      individually: true
      include:
        - ./bin/s3-to-ftp
    environment:
      FTP_HOST: ${file(./config.yml):${opt:stage}.FTP_HOST}
      FTP_USERNAME: ${file(./config.yml):${opt:stage}.FTP_USERNAME}
      FTP_PASSWORD: ${file(./config.yml):${opt:stage}.FTP_PASSWORD}
    events:
      - sns:
          arn:
            Fn::GetAtt:
              - OriginalFileUploadSNSTopic
              - Arn

and my resource, section:

      OriginalFileUploadSQSQueue:
        Type: AWS::SQS::Queue
        Properties:
          MessageRetentionPeriod:
            Ref: MessageRetentionPeriod
          # QueueName:
          #   Ref: QueueName
          VisibilityTimeout:
            Ref: VisibilityTimeout
      OriginalFileUploadSQSQueuePolicy:
        Type: AWS::SQS::QueuePolicy
        Properties:
          PolicyDocument:
            Version: '2008-10-17'
            Statement:
            - Effect: Allow
              Principal:
                AWS: "*"
              Action:
              - SQS:SendMessage
              Resource: "*"
              Condition:
                ArnEquals:
                  aws:SourceArn:
                    Ref: OriginalFileUploadSNSTopic
          Queues:
            - Ref: OriginalFileUploadSQSQueue
      OriginalFileUploadSNSTopic:
        Type: AWS::SNS::Topic
        Properties:
          Subscription:
          - Endpoint:
              Fn::GetAtt:
              - OriginalFileUploadSQSQueue
              - Arn
            Protocol: sqs

produces:

TypeError: event.sns.arn.indexOf is not a function
    at functionObj.events.forEach.event (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/plugins/aws/package/compile/events/sns/index.js:35:44)
    at Array.forEach (<anonymous>)
    at serverless.service.getAllFunctions.forEach (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/plugins/aws/package/compile/events/sns/index.js:22:28)
    at Array.forEach (<anonymous>)
    at AwsCompileSNSEvents.compileSNSEvents (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/plugins/aws/package/compile/events/sns/index.js:18:47)
    at BbPromise.reduce (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/classes/PluginManager.js:407:55)
From previous event:
    at PluginManager.invoke (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/classes/PluginManager.js:407:22)
    at PluginManager.spawn (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/classes/PluginManager.js:425:17)
    at Deploy.BbPromise.bind.then (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/plugins/deploy/deploy.js:117:50)
From previous event:
    at Object.before:deploy:deploy [as hook] (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/plugins/deploy/deploy.js:107:10)
    at BbPromise.reduce (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/classes/PluginManager.js:407:55)
From previous event:
    at PluginManager.invoke (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/classes/PluginManager.js:407:22)
    at PluginManager.run (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/classes/PluginManager.js:438:17)
    at variables.populateService.then.then (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/Serverless.js:114:33)
    at runCallback (timers.js:810:20)
    at tryOnImmediate (timers.js:768:5)
    at processImmediate [as _immediateCallback] (timers.js:745:5)
From previous event:
    at Serverless.run (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/lib/Serverless.js:101:6)
    at serverless.init.then (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/bin/serverless:43:28)
    at /home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/node_modules/graceful-fs/graceful-fs.js:111:16
    at /home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/node_modules/graceful-fs/graceful-fs.js:45:10
    at FSReqWrap.oncomplete (fs.js:135:15)
From previous event:
    at initializeErrorReporter.then (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/bin/serverless:43:6)
    at runCallback (timers.js:810:20)
    at tryOnImmediate (timers.js:768:5)
    at processImmediate [as _immediateCallback] (timers.js:745:5)
From previous event:
    at /home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/bin/serverless:28:46
    at Object.<anonymous> (/home/khine/.nvm/versions/node/v8.11.2/lib/node_modules/serverless/bin/serverless:67:4)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Function.Module.runMain (module.js:693:10)
    at startup (bootstrap_node.js:191:16)
    at bootstrap_node.js:612:3

@yahehe
Copy link
Contributor

yahehe commented Feb 28, 2019

@nkhine you need topicName as well, in order to name the underlying cloudformation resources. It can be any string you want as long as it doesn't conflict with anything in the same template. Read the second-to-last response before yours for more context

@MatejBalantic
Copy link

Thanks for the forensics on this, it saved us a lot of time. We import the ARN, so providing additional topicName seems pointless.

For now, we just set the topicName to ThisValueIsNotImportantAsLongAsPresent. Would be cool if this is handled by serverless falling back to using some default name, or made more explicit that this is for CloudFormation-only naming by adding a variable like internalTopicName

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.