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

Normalize logical id of alias part in Lambda roles #77

Merged
merged 1 commit into from
Nov 22, 2017

Conversation

HyperBrain
Copy link
Member

Fixes #68

Aliases can now include characters matching [a-zA-Z0-9\-+_]+.
To generate a valid role logical id for the alias, the alias name is normalized by using the following table:

- => Dash
+ => Plus
_ => Uscore

This should work as it is highly unlikely that someone uses one of the replacement strings in real alias names.

@HyperBrain
Copy link
Member Author

Locally ESLint as well as the testsuite passes. Will have to check why Travis fails.

@HyperBrain
Copy link
Member Author

Had to upgrade eslint and the testsuite. However unit tests for the normalized roles are yet missing.

@HyperBrain
Copy link
Member Author

@mbruning24 @arabold - Could you do a test with HyperBrain/serverless-aws-alias#normalize-role-name and check if aliases that include dashes work now?

@serverless-heaven serverless-heaven deleted a comment from coveralls Oct 29, 2017
@serverless-heaven serverless-heaven deleted a comment from coveralls Oct 29, 2017
@mbruning24
Copy link

mbruning24 commented Nov 2, 2017

@HyperBrain sorry it's been a hectic couple weeks. I tested this out. Error looks great BTW!

However, I found a HUGE bummer. API Gateway doesn't allow hyphens in the stage name! What a joke! I mean, come on AWS, it's a URL for crying out loud... The regex for that is /a-zA-Z0-9_/, so underscores are allowed by APIG, but ironically not by the regex! Maybe just switch that bad boy from hyphen to underscore and you'll have really done all you can. Hopefully AWS lets hyphens in stage names sometime in the future.

Edit: Here's my sls output:

$ sls deploy --alias GTWY-123
Serverless: Packaging service...
Serverless: Excluding development dependencies...
Serverless: Preparing alias ...
Serverless: Creating Alias Stack 'GTWY-123' ...
Serverless: Checking Stack create progress...
.....
Serverless: Stack create finished...
Serverless: Uploading CloudFormation file to S3...
Serverless: Uploading artifacts...
Serverless: Uploading service .zip file to S3 (165.39 KB)...
Serverless: Uploading CloudFormation alias file to S3...
Serverless: Validating template...
Serverless: Updating Stack...
Serverless: Checking Stack update progress...
........
Serverless: Stack update finished...
Serverless: Updating alias stack...
Serverless: Checking Stack update progress...
.............
Serverless: Operation failed!

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

  An error occurred: ApiGatewayStage - Stage name only allows a-zA-Z0-9_.

  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Forums:        forum.serverless.com
     Chat:          gitter.im/serverless/serverless

  Your Environment Information -----------------------------
     OS:                     win32
     Node Version:           6.11.3
     Serverless Version:     1.23.0

Edit 2: Found this error when trying to use an underscore

$ sls deploy --alias GTWY_123
Serverless: Packaging service...
Serverless: Excluding development dependencies...
Serverless: Preparing alias ...

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

  The stack alias name "ens-alias-dev-GTWY_123" is not valid. A service name should only contain alphanumeric (case sensitive) and hyphens. It should start with an alphabetic character and shouldn't exceed 128 characters.

  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Forums:        forum.serverless.com
     Chat:          gitter.im/serverless/serverless

  Your Environment Information -----------------------------
     OS:                     win32
     Node Version:           6.11.3
     Serverless Version:     1.23.0

@HyperBrain
Copy link
Member Author

HyperBrain commented Nov 3, 2017

@mbruning24 That's indeed ridiculous. I'll add a light normalization to the stage name creation which only will transform the hyphen as you proposed.
However I have to add that to the docs, because having both aliases my-alias and my_alias would lead to a clash then. Maybe we should even emit a warning in case - or _ are used in an alias.

The hyphen issue on AWS side would imo justify to open a issue/case on their side. Stage names should comply with the official URL rules.

@mbruning24
Copy link

@HyperBrain I've opened a support case with AWS in our company account. For now, I'd be happy with just underscore though, if you wouldn't mind changing that.

@HyperBrain
Copy link
Member Author

HyperBrain commented Nov 3, 2017 via email

@HyperBrain
Copy link
Member Author

@mbruning24 Sorry for the delay. Now, for stage names the hyphens are replaced by underscores.

Did you get any valuable feedback from AWS yet?

@serverless-heaven serverless-heaven deleted a comment from coveralls Nov 21, 2017
@serverless-heaven serverless-heaven deleted a comment from coveralls Nov 22, 2017
Turn hyphens to underscores in stage names

Added unit tests for normalization

Fixed regex.

Updated dependencies and test framework.

Fixed ESLint

Normalize logical id of alias part in Lambda roles
@HyperBrain HyperBrain merged commit 2b9a0b5 into master Nov 22, 2017
@HyperBrain HyperBrain deleted the normalize-role-name branch November 22, 2017 10:53
defnotrobbie pushed a commit to 1stdibs/serverless-aws-alias that referenced this pull request Nov 14, 2022
…e-name

Normalize logical id of alias part in Lambda roles
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants