serverless / serverless

⚡ Serverless Framework – Effortlessly build apps that auto-scale, incur zero costs when idle, and require minimal maintenance using AWS Lambda and other managed cloud services.
https://serverless.com
MIT License
46.36k stars 5.69k forks source link

Logical ID naming is not following the convention with `-` or `_` #4476

Open kayhide opened 6 years ago

kayhide commented 6 years ago

This is a Bug Report

Logical ID naming is not following the convention if lambda function name contains - or _.

Description

With this settings:

service: dash

provider:
  name: aws
  runtime: nodejs6.10

functions:
  photo-resize:
    handler: handler.hello
  photo_recolor:
    handler: handler.hi

sls package produced cloudformation-template-update-stack.json including folloings:

Expected is like:

as the document says:

https://github.com/serverless/serverless/blob/master/docs/providers/aws/guide/resources.md#override-aws-cloudformation-resource

This cause trouble when trying to hook S3 event, because we need to overwrite some auto generated resource.

https://github.com/serverless/serverless/blob/master/docs/providers/aws/events/s3.md#custom-bucket-configuration

Related to:

Additional Data

  Your Environment Information -----------------------------
     OS:                     darwin
     Node Version:           6.10.3
     Serverless Version:     1.23.0
HyperBrain commented 6 years ago

Hey @kayhide - good find 👍

Normally, I would propose to fix the issue and do it right. But in this case it would break all existing deployments with resource names that include dashes or underscores if we fix the generation of the logical ids and lead to unusable stacks, or deployment failures.

I would go for a documentation change at the moment that explicitly states that Dash and Underscore will not camelcase the next text in the normalized name.

How does that sound?

kayhide commented 6 years ago

Hi @HyperBrain, thank you for your quick reply.

Yeah, I understand the situation. I am fine with leaving it as it is, and updating the document.

Just I hope the naming itself is fixed some day 😉

kayhide commented 6 years ago

About this documentation, I have a suggestion. https://github.com/serverless/serverless/blob/master/docs/providers/aws/events/s3.md#custom-bucket-configuration

functions:
  resize:
    handler: resize.handler
    events:
      - s3: photos

How about making it clear of the rule of bucket, like:



Actually, I hit the third case and caused a problem. Bacause there is no support to change logical id of existing resource by AWS officially. https://forums.aws.amazon.com/thread.jspa?threadID=231788

So I needed some extra work to make its logical id to follow the serverless naming convention.

kayhide commented 6 years ago

Or more specifically, if we could write this way, it would be nicer.

functions:
  resize:
    handler: resize.handler
    events:
      - s3:
          logical_id: S3BucketPhotos

In this way, we get no need to explain the complicated meaning of bucket, and support any logical id. Just an idea.

HyperBrain commented 6 years ago

@kayhide Thank you for the suggestions.

@serverless/framework-maintainers What is your opinion on this?

hassankhan commented 6 years ago

Perhaps we could add this issue to the v2 milestone?

horike37 commented 6 years ago

It may add this to v2 milestone if we would change the convention.