search

serverless-heaven / serverless-aws-alias

Alias support for Serverless 1.x
MIT License
189 stars 68 forks source link

Support alias in SNS Topic subscription #43

Closed vkkis93 closed 7 years ago

vkkis93 commented 7 years ago

Hello, When you create SNS Topic in AWS Console you can add Lambda subscription with alias. Now if you deploys different aliases - subscription will be always ${functionName}. Can we do like this ${functionName}:${alias} ? Is it possible? http://prntscr.com/fcbdfc First subscription - it's what I need Second subscription - current state.

Thank you.

HyperBrain commented 7 years ago

Yes. You're right. SNS subscriptions should be mapped to the alias correctly. Currently event sources are handled correctly - SNS subscriptions should be the same.

Maybe you could add some info on how exactly you configured the SNS topic subscription/event in your serverless.yml file.

vkkis93 commented 7 years ago

@HyperBrain yes, sure. Please look https://prnt.sc/fcbqgd

HyperBrain commented 7 years ago

Thanks. That's enough do reproduce the behavior and provide an implementation 👍

andrewkuzmych commented 7 years ago

+1

HyperBrain commented 7 years ago

I'm nearly through with the implementation, but I found one issue that should be discussed before:

If you create a SNS topic via a function's event array, the lambda function is set as subscription. If I now on deployment, change the subscription to point to the aliased version, I would have to assign the topic to the deployed alias. This is because the aliases and function versions live in the CF alias stack. If I would leave it in the main stage stack, I have no access to the aliased/versioned CF function resources.

One option would be to keep the topic in the stage stack and set the subscription target as arn string (including the alias). But this would add the need to implement some kind of topic resource merging, so that all deployed aliases, that include the subscription, will be attached to the same topic, and a message to the topic will trigger all subscribed aliases. In my opinion this is very dangerous, because the SNS topic payload structure could change in aliases (e.g. if you implement new features that have other data types and structures and want to test them).

I would propose the following second option

If a SNS subscription/topic is deployed to an alias, I'll add the alias name to the topic name (e.g. mytopic-${stage}), so that a topic is created per alias. This has no impact programmatically, as you can use ${process.env.SERVERLESS_ALIAS} to construct the topic name or arn, when you need to send a message there. This will enable real parallel development including structural changes.

@vkkis93 @andrewkuzmych Any comments on this?

vkkis93 commented 7 years ago

@HyperBrain great stuff. You absolutely right - attach aliases to the same sns topic really can be very dangerous I agree with you - the best approach will be create sns topic per alias. I like it.

HyperBrain commented 6 years ago

Hi @baipi , this is already reported in #94 and fixed in PR #95.

It would be great if you could test the PR version and report back into #95 if it works for you. I plan to release the fix with the next release.

The subscription should then automatically target the aliased version from the deployment. There is no need to set it manually.

baipi commented 6 years ago

Yes, I saw that just after posting my message (that why I deleted it), I'm trying it. Thanks