serverless-heaven / serverless-aws-alias

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

Support Lambda@Edge functions #75

Closed sonofbjorn closed 7 years ago

sonofbjorn commented 7 years ago

Lambda@Edge functions do not allow Environment Variables. This change skips creation of the SERVERLESS_ALIAS env var for any functions listed as Lambda@Edge functions in cfg below.

I'm certainly open to other implementation approaches, but it'd be great to be able to use this plugin in combination with Lambda@Edge. If you agree to the PR, I'll add README updates as well.

custom:
  lambdaEdgeFunctions:
    - platform-spa-origin-request-lambda-edge
coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 74.419% when pulling 23c4e2e74fb782328859eb0e29e20bdb668799fa on sonofbjorn:feature/support-lambda-edge into e0215d2694b743567e58a48746129b33af13cc01 on HyperBrain:master.

HyperBrain commented 7 years ago

Hi @sonofbjorn ,

thank you for the PR. I agree that edge functions should be supported and highly appreciate a solution. I've read the AWS documentation and I assume that we do not need a separate custom array to declare the edge functions separately and introduce a new custom property that has to be documented.

According to http://docs.aws.amazon.com/lambda/latest/dg/API_CreateFunction.html#SSS-CreateFunction-request-Runtime we should be able to distinguish any edge function by it's Runtime value. If it is an edge function the runtime should be set to nodejs4.3-edge. So the best solution imo should be to check for

if (resource.Type === 'AWS::Lambda::Function' && !_.endsWith(resource.Runtime, '-edge')) { ... }

Checking for the -edge suffix should be more future proof than just checking for the exact available values.

What do you think and could you try if that works?

sonofbjorn commented 7 years ago

Absolutely, I'll give it a try today. Agree that this approach is much cleaner. Thanks for the quick response!

mikelax commented 7 years ago

Nice job on adding this capability. One potential issue I found, though it could be a typo in the AWS docs, it looks like the edge functions do support node 6.10. Here is the AWS docs for their tutorial and they say to specifically use node 6.10.

Maybe the API doc hasn't been fully updated yet, worth a shot to see if it supports nodejs6.10-edge as well.

HyperBrain commented 7 years ago

@mikelax Good catch. @sonofbjorn , could you do an experiment with nodejs6.10-edge too, to confirm that it is supported by AWS?

sonofbjorn commented 7 years ago

@HyperBrain I've done a bit of testing... Here's what I've found. I was able to create a lambda with the nodejs4.3-edge runtime, but it appears this runtime is deprecated (the error below appears in the Lambda console and NO RUNTIME is selected).

The Edge Node.js 4.3 runtime is no longer supported. We recommend you migrate your functions that use Edge Node.js 4.3 to a newer runtime as soon as possible.

When attempting to create a lambda with the nodejs6.10-edge runtime, I receive the following error during deploy (note that nodejs-4.3-edge doesn't appear in this list either, though it passes validation).

1 validation error detected: Value 'nodejs6.10-edge' at 'runtime' failed to satisfy constraint: Member must satisfy enum value set: [java8, nodejs, nodejs4.3, nodejs6.10, python2.7, python3.6, dotnetcore1.0]

As @mikelax points out from the AWS docs, they clearly state to select node 6.10 runtime when creating a Lambda@Edge function. I suspect that when these functions are replicated to the edge locations, they're using nodejs6.10-edge, but it doesn't appear possible to select that option when creating them.

At this point, I don't see a great way to distinguish a Lambda@Edge function from a standard Lambda, outside of additional configuration.

HyperBrain commented 7 years ago

I also dug a bit to find any information. It seems that there is currently no working CloudFormation support for Lambda@Edge. See: https://forums.aws.amazon.com/thread.jspa?threadID=262327

The plugin as workaround looks also quite fragile to me as it circumvents CF and leaves things out-of-control. Before taking any decision here we should try to get a clear statement, if or if not, it is supported by CloudFormation. I'm sure that CloudFormation (and maybe Serverless) must handle edge lambdas differently. Do edge lambdas work in Serverless without the alias plugin?

Regarding the validation error: Serverless uses the CloudFormation REST API call validateTemplate() to validate the generated CloudFormation template. It seems that this validation call fails here. To be sure we should check the Serverless code if there is some additional check for the runtime that might lead to a failure-

sonofbjorn commented 7 years ago

Agreed on all counts. Serverless does not support Lambda@Edge (L@E) out of the box currently, nor does CloudFormation. There have been a couple of issues raised regarding CF support for L@E, but I haven't seen any response from AWS on the topic. At this point, I'm leaning toward addressing this issue with a L@E plugin instead. You can decline this PR.

Just for reference, there's already an existing plugin, but it assumes the L@E function is in the same CloudFormation stack as the linked CloudFront distribution. In my use case the L@E function is standalone and can be triggered by many CloudFront distributions. Thanks you for the time and thoughtful response, and great work on this plugin!

HyperBrain commented 7 years ago

Closed as discussed