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

AWS: Fix stage name validation timing and allow hyphen #5686

Merged
merged 8 commits into from Jan 14, 2019

Conversation

Projects
None yet
6 participants
@exoego
Copy link
Contributor

exoego commented Jan 11, 2019

What did you implement:

Closes #5655, #5674, #5683, #5684, #5693 and supersedes PR #5658

How did you implement it:

  • Move the validation to after ${variable} are resolved.
  • Allow hyphen since API Gateway allows it now.

How can we verify it:

  1. npm install -g exoego/serverless#validation-after-var-resolution

  2. Deploy below where a stage name is a variable and contains a hyphen after populated.

service:
  name: test-stack

custom:
  env: test-env

provider:
  name: aws
  stage: ${self:custom.env, 'hyphen-is_valid'}
  runtime: nodejs8.10

functions:
  tracerTest:
    memorySize: 512
    handler: tracer-test.handler
    events:
      - http:
          method: get
          path: whatever

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@exoego exoego referenced this pull request Jan 11, 2019

Closed

Allow hyphens in stage name #5658

7 of 7 tasks complete

@exoego exoego force-pushed the exoego:validation-after-var-resolution branch from 855a520 to 5ca3115 Jan 11, 2019

@exoego exoego referenced this pull request Jan 11, 2019

Merged

AWS: Add API Gateway stage name validation. #5639

7 of 7 tasks complete

exoego added some commits Jan 12, 2019

@efimk-lu

This comment has been minimized.

Copy link

efimk-lu commented Jan 12, 2019

@exoego can I help somehow to speed the release?

@exoego

This comment has been minimized.

Copy link
Contributor Author

exoego commented Jan 12, 2019

@exoego can I help somehow to speed the release?

I think a review and testing on your actual serverless.yml help a lot.

@MozhdeAmiri

This comment has been minimized.

Copy link

MozhdeAmiri commented Jan 12, 2019

Hi,
I have the same issue yet. It worked well 2 days ago.

I have a react-scripts project. Here is the serverless.yml.


provider:
  name: aws
  runtime: nodejs8.10
  stage: ${opt:stage, self:custom.defaultStage}
  apiKeys:
    - x-apikey-${opt:stage, self:custom.defaultStage}
  region: us-east-1
  environment:
    PROVIDER_STAGE: ${self:provider.stage}

plugins:
  - serverless-domain-manager
  - serverless-offline

custom:
  customDomain:
    domainName: ${self:custom.domains.${self:provider.stage}}
    basePath: ""
    stage: ${self:provider.stage}
    createRoute53Record: false
  defaultStage: dev

functions:
  app:
    handler: service.handler
    timeout: 30
    memory: 128
    events:
      - http: ANY /
      - http: ANY /static/js/
      - http: ANY /static/css/
      - http: 'ANY {proxy+}'```


Is this problem addressed? If not how can I help?
@efimk-lu

This comment has been minimized.

Copy link

efimk-lu commented Jan 12, 2019

@exoego, @dschep working on my side, please merge and release a hotfix as soon as possible, please

@exoego exoego changed the title AWS: Fix stage name is validated before variable resolution AWS: Fix stage name validation timing and allow hyphen Jan 13, 2019

@dschep

dschep approved these changes Jan 14, 2019

Copy link
Member

dschep left a comment

Love it @exoego, definitely makes more sense to run this validation evaluated variables

@dschep dschep merged commit 96906c1 into serverless:master Jan 14, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 90.847%
Details

@exoego exoego deleted the exoego:validation-after-var-resolution branch Jan 14, 2019

@shortjared shortjared added the bug label Jan 22, 2019

@shortjared shortjared modified the milestones: v1.36.3, v1.36.2 Jan 22, 2019

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