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

SNS event compiler tries to use an existing ARN as a topic name #2183

Closed
danielkoo opened this issue Sep 22, 2016 · 16 comments
Closed

SNS event compiler tries to use an existing ARN as a topic name #2183

danielkoo opened this issue Sep 22, 2016 · 16 comments

Comments

@danielkoo
Copy link
Contributor

danielkoo commented Sep 22, 2016

This is a Bug Report

Description

For bug reports:

says one way is to use the following format:

events:
      - sns: arn:xxx

However, I get the error "Invalid parameter: Topic Name" as Serverless thinks the ARN is a topic name and tries to create a new topic with it, instead of leaving it as an existing ARN.

  • What did you expect should have happened?
    If event.sns is a string and is a valid SNS ARN value, it should not create a new SNS topic using the supplied ARN as the topic name; rather, it should set up the event using the existing SNS ARN.

I believe this is the part of the code in lib/plugins/aws/deploy/compile/events/sns/index.js

} else if (typeof event.sns === 'string') {
              topicName = event.sns;
}

The current workaround is just to use the other format to explicitly specify the topic ARN:

- sns:
        topicArn: arn:aws:sns:xxxx:xxxx:xxxx
  • What was the config you used?
events:
      - sns: arn:aws:sns:xxxx:xxxx:xxxx
  • What stacktrace or error message from your provider did you see?
 $ sls deploy
Serverless: Creating Stack...
Serverless: Checking Stack create progress...
.....
Serverless: Stack create finished...
Serverless: Packaging service...
Serverless: Uploading CloudFormation file to S3...
Serverless: Uploading service .zip file to S3...
Serverless: Updating Stack...
Serverless: Checking Stack update progress...
....................................................Serverless: Deployment failed!

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

     An error occurred while provisioning your stack: SNSTopicArnawssnsxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
     - Invalid parameter: Topic Name.

  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues

Similar or dependent issues:
N/A

Additional Data

  • _Serverless Framework Version you're using_: 1.0.0-rc.1
  • _Operating System_: Debian Linux
  • _Stack Trace_:
    Please see above
  • _Provider Error messages_:
     - Invalid parameter: Topic Name.
@johncmckim
Copy link
Member

I'd like to submit a PR of this issue if it's up for grabs.

@johncmckim
Copy link
Member

@danielkoo the issue seems a bit larger than just that one line. It looks like support for an sns arn was removed in 1.0.0-rc.2. Specifically this commit

My guess on why it was removed is that Cloud Formation does not support subscribing SNS topics to events separately to creating the topic. The only thing that config could help with is to add that lambda permission.

That all said. It is possible to create a custom CF resource backed by a lambda function. See https://gist.github.com/martinssipenko/4d7b48a3d6a6751e7464 as an example.

@eahefnawy @flomotlik @pmuens what do you think of the Serverless framework supporting existing SNS topics via a custom CF resource?

@eahefnawy
Copy link
Member

Hmmm I don't remember explicitly discussing removing it, might have happened by mistake due to the big change. But generally it was't doing much other than adding just another Lambda::Permission resource.

I'm definitely onboard with the suggested custom template idea! It'd make the feature more valuable . And I think it could solve other issues we might have in the framework due to CF limits.

@jthomerson
Copy link
Contributor

@danielkoo FYI: I made a plugin that allows for "external" (or already existing) SNS topics as an event source for your Lambda function. It automatically handles subscribing and unsubscribing also. See https://github.com/silvermine/serverless-plugin-external-sns-events

@alexef
Copy link
Contributor

alexef commented Oct 21, 2016

@jthomerson the plugin is cool, but it only works for SNS topics within the same account. Can you extend it to a generic sns topic arn, like sls used to support?

@johncmckim
Copy link
Member

@jthomerson the implementation of it might be affected by #2473 if that idea is accepted. Did you consider using a custom CloudFormation resource? How does the plugin handle rollbacks i.e. a failed deploy?

@viz
Copy link

viz commented Nov 9, 2016

Looks like the work-around described by @danielkoo no longer works. I tried adding topicArn: ... and got the following error:

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

     Missing "topicName" property for sns event in function
     updateStatusSubmitted The correct syntax is: sns: topic-name
     OR an object with "topicName" AND "displayName" properties.
     Please check the docs for more info.

  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues

  Your Environment Information -----------------------------
     OS:                 darwin
     Node Version:       6.6.0
     Serverless Version: 1.1.0

@jschr
Copy link

jschr commented Nov 10, 2016

Running into the same issue trying to use an existing topic. I've tried the following but I get the same errors as above:

- sns: arn:xxx
- sns:
        - topicArn: arn:xxx
- sns:
        - topicName: name
        - topicArn: arn:xxx
- sns:
        - topicName: name
        - displayName: displayName
        - topicArn: arn:xxx

@gmetzker
Copy link

Getting similar issues with several types of syntax, similar to @jschr
Would be nice to see the standard sns syntax support pre-existing topics by wiring up subscriptions. I realize CloudFormation has limitations on subscriptions but perhaps something similar to @jthomerson s plugin could be built-in.

@dougmoscrop
Copy link
Contributor

@gmetzker #2796 fixes it as far as I know. what kind of limitations are there on CFN subscriptions?

@gmetzker
Copy link

gmetzker commented Dec 20, 2016

@dougmoscrop: When trying to update an existing CF stack that contained SNS subscriptions, CF used to throw out errors:

Update to resource type AWS::SNS::Topic is not supported
See Stackoverflow

I can't remember the specifics but if I recall correctly it seemed that if you defined an SNS subscription in a CF stack and later wanted to make changes or do an Update Stack you might run into this error and the update was not allowed.

AWS may have fixed the limitation. It used to be documented but I can't seem to find a reference.

I would be curious if your serverless generated CF stack contains SNS subscriptions and if you could roll subsequent updates to your stack. Perhaps this works if the SNS subscription was created outside of the template...?

If this works that would be great for my use case.

@dougmoscrop
Copy link
Contributor

I see - I think my approach avoids this problem because it's creating a Subscription as a separate CloudFormation resource - if anything, the 'concession' I made to modify the Topic for backwards compatibility might not be a good idea because it could run in to the problem you linked. Hmm.

@MatthewDaniels
Copy link

MatthewDaniels commented Feb 13, 2017

I know this ticket is closed, but I ran into this problem in version 1.6.1 and found this issue after a quick Google... so just to add a little more info to this ticket - I having this problem on a service that was deploying fine earlier today. I realised that I was in a different terminal window, which was using NodeJS version 7.0.0 vs NodeJS version 4.3.2.

Switching to the older version of nodejs (I use NVM to manage my node versions), immediately fixed the problem with no changes to the YML.

@pmuens
Copy link
Contributor

pmuens commented Feb 13, 2017

Thanks for the tip @MatthewDaniels 👍

@jaikdean
Copy link
Contributor

I'm also having this issue with Node 7.6.0, like @MatthewDaniels.

@uday4393
Copy link

Any Update regarding this?
Whats the Fix.
Is this Fixed in the new serverless version?

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

No branches or pull requests