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 ${cf.REGION} syntax causes deployment in wrong region #5650

Merged
merged 2 commits into from Jan 7, 2019

Conversation

Projects
None yet
3 participants
@exoego
Copy link
Contributor

exoego commented Jan 4, 2019

What did you implement:

Closes #5606

How did you implement it:

${cf.REGION} syntax changes region field of the credentials object returnd by awsProvider.getCredentials(), to get stack info in different region.
However, getCredentias() returns a reference to mutable credentials.
So the modified region are accidentally used for later AWS request, then a stack is deployed to wrong region which is specified by ${cf.REGION} syntax.

This PR fixes the issue by add deep-cloning the credentials before changing region.

How can we verify it:

  1. Install custom build of serverless from this PR
    npm i -g exoego/serverless#fix-cf-region-syntax

  2. Create a service to be referenced: sls create -t aws-nodejs --path my-service

  3. Update serverless.yml of my-service

service: my-service
provider:
  name: aws
  runtime: nodejs8.10
  region: ap-northeast-1
  memorySize: 512
functions:
  hello:
    name: ${self:custom.functionPrefix}hello
    handler: handler.hello
custom:
  functionPrefix: "issue5154-"
resources:
  Outputs:
    functionPrefix:
      Value: ${self:custom.functionPrefix}
      Export:
        Name: functionPrefix
    memorySize:
      Value: ${self:provider.memorySize}
      Export:
        Name: memorySize
  1. Deploy my-service: sls deploy
  2. Create another service: sls create -t aws-nodejs --path another
  3. Update serverless.yml of another
service: another
provider:
  name: aws
  region: us-east-1
  runtime: nodejs8.10
  memorySize: ${cf.ap-northeast-1:myService-dev.memorySize}
functions:
  hello:
    name: ${cf.ap-northeast-1:myService-dev.functionPrefix}hello-usa
    handler: handler.hello
  1. Deploy another service that reference my-service

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 changed the title AWS: Fix ${cf.REGION} syntax AWS: Fix ${cf.REGION} syntax causes deployment in wrong region Jan 4, 2019

@horike37
Copy link
Member

horike37 left a comment

@exoego
Looks good. Thank you for fixing 😄

@horike37 horike37 merged commit d4e36b7 into serverless:master Jan 7, 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.829%
Details

@exoego exoego deleted the exoego:fix-cf-region-syntax branch Jan 7, 2019

@dschep dschep referenced this pull request Jan 9, 2019

Merged

v1.36.0 release! #5670

@shortjared shortjared added this to the 1.36.0 milestone Jan 9, 2019

@piohhmy piohhmy referenced this pull request Jan 23, 2019

Open

Fix broken profiles #5739

0 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment