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

Race condition with AWS API Gateway Method & Lambda Permission #9525

Closed
richardowen opened this issue May 27, 2021 · 5 comments · Fixed by #9609
Closed

Race condition with AWS API Gateway Method & Lambda Permission #9525

richardowen opened this issue May 27, 2021 · 5 comments · Fixed by #9609

Comments

@richardowen
Copy link
Contributor

When creating an API gateway method using the lambda proxy integration, the method isn't set to depend on the lambda permission in the CloudFormation code. When you first deploy the API, this isn't a problem, because the API can be deployed and the lambda permission created in parallel, and once they're both done the API works.

However, if you perform an update which replaces the lambda function, this is what happens:

  1. New lambda function is created
  2. New lambda permission is created
  3. API gateway method is updated & API gateway deployment is created

Eventually, the API works. However during the update, users can receive errors from the API due to missing permissions. This is because step 2 & 3 above run in parallel, creating a race condition. The API gateway can be deployed before the Lambda permission has been created. Until the permission has been created, users will receive errors.

I think the fix for this should be fairly simple. The AWS::ApiGateway::Method resource in the CloudFormation stack needs a DependsOn attribute referencing the AWS::Lambda::Permission resource.

serverless.yml (for create)
service: serverless-apigateway-permission-test
provider:
  name: aws
  runtime: nodejs14.x
  region: eu-west-1
  lambdaHashingVersion: 20201221

functions:
  hello:
    handler: handler.hello
    events:
      - http: GET /hello
serverless.yml (for update)
service: serverless-apigateway-permission-test
provider:
  name: aws
  runtime: nodejs14.x
  region: eu-west-1
  lambdaHashingVersion: 20201221

functions:
  hello2:
    handler: handler.hello
    events:
      - http: GET /hello
SLS_DEBUG=* serverless deploy output
Serverless: To ensure safe major version upgrades ensure "frameworkVersion" setting in service configuration (recommended setup: "frameworkVersion: ^2.43.1")

Serverless: Load command interactiveCli
Serverless: Load command config
Serverless: Load command config:credentials
Serverless: Load command config:tabcompletion
Serverless: Load command config:tabcompletion:install
Serverless: Load command config:tabcompletion:uninstall
Serverless: Load command create
Serverless: Load command install
Serverless: Load command package
Serverless: Load command deploy
Serverless: Load command deploy:function
Serverless: Load command deploy:list
Serverless: Load command deploy:list:functions
Serverless: Load command invoke
Serverless: Load command invoke:local
Serverless: Load command info
Serverless: Load command logs
Serverless: Load command metrics
Serverless: Load command print
Serverless: Load command remove
Serverless: Load command rollback
Serverless: Load command rollback:function
Serverless: Load command slstats
Serverless: Load command plugin
Serverless: Load command plugin
Serverless: Load command plugin:install
Serverless: Load command plugin
Serverless: Load command plugin:uninstall
Serverless: Load command plugin
Serverless: Load command plugin:list
Serverless: Load command plugin
Serverless: Load command plugin:search
Serverless: Load command config
Serverless: Load command config:credentials
Serverless: Load command upgrade
Serverless: Load command uninstall
Serverless: Load command login
Serverless: Load command logout
Serverless: Load command generate-event
Serverless: Load command test
Serverless: Load command dashboard
Serverless: Load command output
Serverless: Load command output:get
Serverless: Load command output:list
Serverless: Load command param
Serverless: Load command param:get
Serverless: Load command param:list
Serverless: Load command studio
Serverless: Skipping variables resolution with old resolver (new resolver reported no more variables to resolve)
Serverless: Invoke deploy
Serverless: Invoke package
Serverless: Invoke aws:common:validate
Serverless: Invoke aws:common:cleanupTempDir
Serverless: Packaging service...
Serverless: Excluding development dependencies...
Serverless: Invoke aws:package:finalize
Serverless: Invoke aws:common:moveArtifactsToPackage
Serverless: Invoke aws:common:validate
Serverless: Invoke aws:deploy:deploy
Serverless: Uploading CloudFormation file to S3...
Serverless: Uploading artifacts...
Serverless: Uploading service serverless-apigateway-permission-test.zip file to S3 (36.04 MB)...
Serverless: Validating template...
Serverless: Updating Stack...
Serverless: Checking Stack update progress...
.............................
Serverless: Stack update finished...
Serverless: Invoke aws:info
Service Information
service: serverless-apigateway-permission-test
stage: dev
region: eu-west-1
stack: serverless-apigateway-permission-test-dev
resources: 11
api keys:
  None
endpoints:
  GET - https://b7urq35xz6.execute-api.eu-west-1.amazonaws.com/dev/hello
functions:
  hello2: serverless-apigateway-permission-test-dev-hello2
layers:
  None
Serverless: Invoke aws:deploy:finalize

Installed version

Framework Core: 2.43.1 (local)
Plugin: 5.1.3
SDK: 4.2.2
Components: 3.10.1
@pgrzesik
Copy link
Contributor

Hello @richardowen, thanks a lot for reporting and for spotting this bug. We'd be more than happy to accept a community PR that addresses this problem 🙇

@ParadoxInfinite
Copy link

@pgrzesik I'd like to take this up, but since I'm not aware of the codebase whatsoever, I'll be going through it first before I start working on it, hope that's okay!?

If anyone else in the mean time wants to pick it up, I'll be happy to contribute towards that PR or just even learning from their PR!

@ParadoxInfinite
Copy link

So, from what I can gather from the issue is that the template on line 23 in lib/plugins/aws/package/compile/events/apiGateway/lib/method/index.js needs to be changed?

Code being referenced here
      const template = {
        Type: 'AWS::ApiGateway::Method',
        Properties: {
          HttpMethod: event.http.method.toUpperCase(),
          RequestParameters: requestParameters,
          ResourceId: resourceId,
          RestApiId: this.provider.getApiGatewayRestApiId(),
          OperationName: event.http.operationId,
        },
        DependsOn: ['AWS::Lambda::Permission'] // Not sure which instance of Lambda Permission to reference here 
      };

I'm not sure which AWS::Lambda::Permission resource I need to add to the DependsOn attribute.

Any help is appreciated and if I'm on the wrong track, let me know and I'll go through it again.

@nyamba
Copy link
Contributor

nyamba commented Jun 7, 2021

Hello @ParadoxInfinite,
I'm also new to the project. In my opinion, the place you are pointing, it looks right place to add the DependsOn. However it should take resource name, not type. Following code help you take related resource name.

...
const dpOn = this.provider.naming.getLambdaApiGatewayPermissionLogicalId(event.functionName);
...
DependsOn: [dpOn]
...

And did you get a way to reproduce the issue?

@nyamba
Copy link
Contributor

nyamba commented Jun 16, 2021

@pgrzesik please check PR for it, #9609

nyamba added a commit to nyamba/serverless that referenced this issue Jun 21, 2021
nyamba added a commit to nyamba/serverless that referenced this issue Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants