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

Cannot activate API gateway logs for shared API gateway #7036

Closed
tinexw opened this issue Dec 2, 2019 · 23 comments · Fixed by #7039
Closed

Cannot activate API gateway logs for shared API gateway #7036

tinexw opened this issue Dec 2, 2019 · 23 comments · Fixed by #7039

Comments

@tinexw
Copy link
Contributor

tinexw commented Dec 2, 2019

Bug Report

Description

The API gateway log settings are not applied when using a shared API gateway as described here.

  1. What did you do?
  • Created serverless.yml my copying the example from the docs and added
logs:
    restApi: true
  • Ran sls deploy
  1. What happened?
  • API Gateway logs are not enabled.
  1. What should've happened?
  • API Gateway logs should have been enabled.
  1. What's the content of your serverless.yml file?
service: my-api
provider:
  name: aws
  runtime: nodejs12.x
  region: eu-central-1
  logs:
    restApi: true
resources:
  Resources:
    MyApiGW:
      Type: 'AWS::ApiGateway::RestApi'
      Properties:
        Name: MyApiGW
  Outputs:
    apiGatewayRestApiId:
      Value:
        Ref: MyApiGW
      Export:
        Name: MyApiGateway-restApiId
    apiGatewayRestApiRootResourceId:
      Value:
        'Fn::GetAtt':
        - MyApiGW
        - RootResourceId
      Export:
        Name: MyApiGateway-rootResourceId
  1. What's the output you get when you use the SLS_DEBUG=* environment variable (e.g. SLS_DEBUG=* serverless deploy)
    No difference. However, if I add a http function I can see that the AWS CLI is used to set the log settings. If no http function is defined, updateStage.js is not called.
    I tried to remove this line, but then I get
Serverless: [AWS apigateway 200 0.202s 0 retries] getRestApis({ position: undefined, limit: 500 })
 
  Serverless Error ---------------------------------------
 
  ServerlessError: Rest API id could not be resolved.
  This might be caused by a custom API Gateway configuration.
  
  In given setup stage specific options such as `tracing`, `logs` and `tags` are not supported.

I guess as a workaround I can define my own resources. However, I would gladly submit a PR to allow setting this with the log level properties.

Similar or dependent issues:

  • There are several issues on API gateway logs but I did not find the exact same one.
@tinexw tinexw changed the title Cannot activate API gateway logs if Cannot activate API gateway logs for shared API gateway Dec 2, 2019
@medikoo
Copy link
Contributor

medikoo commented Dec 3, 2019

Will be fixed with: #7034

Note, that for shared APIGW, logs need to be configured in place in which APIGW is configured (and not in services which refer to it)

@tinexw
Copy link
Contributor Author

tinexw commented Dec 3, 2019

Note, that for shared APIGW, logs need to be configured in place in which APIGW is configured (and not in services which refer to it)

Yes, that’s what I was trying, see the example above.

@medikoo
Copy link
Contributor

medikoo commented Dec 3, 2019

Yes, that’s what I was trying, see the example above.

Sorry (I was fast reading) indeed it's a different case, and for your case, bug is here:

resource => resource.Type === 'AWS::ApiGateway::Resource'

Let me prepare PR

@medikoo medikoo added this to the 1.59.0 milestone Dec 3, 2019
@medikoo medikoo removed this from the 1.59.0 milestone Dec 4, 2019
@tinexw
Copy link
Contributor Author

tinexw commented Dec 8, 2019

Hi @medikoo , thanks for supplying this fix so soon. However, it's still not working.

There is still the issue, that updateStage.js is not called unless I remove this line. I can submit a PR that removes the line but I'm not sure of the implications because it was explicitly added by this commit .(has probably been there since beginning).

Even after removing the line, it does not immediately work. The issue is that initially there is no stage - so the log settings cannot be applied. If I deploy the API gateway, then the first service and then the API gateway again, it works.

Do you prefer if I create a new issue for this or is it fine to track it in here?

@medikoo
Copy link
Contributor

medikoo commented Dec 10, 2019

@tinexw indeed, it shouldn't be like that. It's updateStage logic that should eventually discard changes if it finds no API GW configured and sees no http events.

PR to fix it, is very welcome

@lunascat
Copy link

lunascat commented Oct 7, 2020

Using following serverless.yml file, I still run into the same issue, that API gateway logs are disabled (for a shared API gateway):

service: nodejs-example

provider:
  name: aws
  runtime: nodejs12.x
  stage: ${opt:stage, 'dev'}
  region: eu-central-1
  logRetentionInDays: 30
  apiGateway:
    restApiId: !Ref RestApi
    restApiRootResourceId: !Ref RestApiRoot
    metrics: true
  tracing:
    apiGateway: true
    lambda: true
  logs:
    restApi: true
      level: INFO
      accessLogging: true
      format: 'requestId: $context.requestId'

functions:
  hello:
    handler: handler.hello
    description: This is a test lambda
    events:
      - http:
          path: hello
          method: get

resources:
  Resources:
    RestApi:
      Type: AWS::ApiGateway::RestApi
      Properties:
        Name: nodejs-example
        Description: API GW to test enabling logging, tracing and metrics

    RestApiRoot:
      Type: AWS::ApiGateway::Resource
      Properties:
        RestApiId: !Ref RestApi
        ParentId: !GetAtt RestApi.RootResourceId
        PathPart: "test"

Am I missing something here? If I delete the custom API GW and let serverless create the GW itself, the logging is enabled as expected.

@medikoo
Copy link
Contributor

medikoo commented Oct 8, 2020

@lunascat Logging should be set in context of stack with which API Gateway is created. In your case you're referencing an external stack, and in such case Framework does not attempt to tweak logging settings (as that's just not safe).

Issue is that Framework doesn't crash with meaningful error on such configuration. if you wish to see that, please open a new bug report

@lunascat
Copy link

lunascat commented Oct 8, 2020

@medikoo Thanks for the quick reply.

The API Gateway is created within the same stack (see Resources part at the end). For me, it looks like the same case as in the original example above, except for the http function part (and the restApi reference in provider.apiGateway). What is the difference from serverless' point of view?

@medikoo
Copy link
Contributor

medikoo commented Oct 8, 2020

The API Gateway is created within the same stack

Then please remove apiGateway.restApiId setting. This is intended to point the API's configured externally only

@lunascat
Copy link

lunascat commented Oct 8, 2020

So, if I remove apiGateway.restApiId and the http function and move it to a different stack, using the shared API Gateway, the log settings are still not enabled.
In fact our scenario looks a bit more complicated, with a shared authorizer:

shared api-gateway serverless.yml
service: nodejs-auth-example
provider:
  name: aws
  runtime: nodejs12.x
  stage: ${opt:stage, 'dev'}
  region: eu-central-1
  logRetentionInDays: 30
  apiGateway:
    metrics: true
  tracing:
    apiGateway: true
    lambda: true
  logs:
    restApi: true

functions:
  authorizer:
    handler: src/functions/authorize.handler
    description: Lambda authorizer for the api gateway.

resources:
  Resources:
    RestApi:
      Type: AWS::ApiGateway::RestApi
      Properties:
        Name: nodejs-auth-example
        Description: API GW with auth to test enabling logging, tracing and metrics

    RestApiRoot:
      Type: AWS::ApiGateway::Resource
      Properties:
        RestApiId: !Ref RestApi
        ParentId: !GetAtt RestApi.RootResourceId
        PathPart: "test"

    ApiAuthorizer:
      Type: AWS::ApiGateway::Authorizer
      Properties:
        AuthorizerResultTtlInSeconds: 3600
        IdentitySource: method.request.header.Authorization
        Name: authorizer
        RestApiId: !Ref RestApi
        Type: TOKEN
        AuthorizerUri: 
          !Join [ '', [ arn:aws:apigateway:eu-central-1:lambda:path/2015-03-31/functions/, !GetAtt AuthorizerLambdaFunction.Arn, /invocations ] ]
    
    ApiAuthorizerPermission:
      Type: AWS::Lambda::Permission
      Properties:
        FunctionName: !GetAtt AuthorizerLambdaFunction.Arn
        Action: lambda:InvokeFunction
        Principal: apigateway.amazonaws.com

  Outputs:
    RestApiId:
      Value: !Ref RestApi
      Export: { Name: "api-gw:RestApiId" }

    RestApiRoot:
      Value: !Ref RestApiRoot
      Export: { Name: "api-gw:RestApiRootId" }
                    
    AuthorizerId:
      Value: !Ref ApiAuthorizer
      Export: { Name: "api-gw:ApiAuthorizer"}
service serverless.yml
service: hello
provider:
  name: aws
  runtime: nodejs12.x
  stage: ${opt:stage, 'dev'}
  region: eu-central-1
  apiGateway:
    restApiId: ${self:custom.restApiId}
    restApiRootResourceId: ${self:custom.restApiRootId}

custom:
  restApiId: !ImportValue api-gw:RestApiId
  restApiRootId: !ImportValue api-gw:RestApiRootId
  authorizerId: !ImportValue api-gw:ApiAuthorizer

functions:
  hello:
    handler: src/functions/handler.hello
    description: This is a test lambda to test secured access to API Gateway methods.
    events:
      - http:
          path: /hello
          method: get
          authorizer: 
            type: CUSTOM 
            authorizerId: ${self:custom.authorizerId}

Are we supposed to define a custom log ressource in this case, because it's a custom API Gateway, not managed by serverless? On the other hand, the deployment with the defined stage is made by serverless and the stage contains the log settings, so serverless should also apply them.

@medikoo
Copy link
Contributor

medikoo commented Oct 8, 2020

@lunascat If I see correctly you miss AWS::ApiGateway::Deployment resource configuration in first stack. Once it's defined, then logging should be turned on.

Note that logging can only be configured on a deployed API Gateway stage (we need a deployment resource there)

@lunascat
Copy link

lunascat commented Oct 8, 2020

@medikoo Thanks, that works like a charm!

For reference, here's the Deployment ressource definition:

plugins:
  - serverless-plugin-bind-deployment-id

resources:
  Resources:
    RestApi:
      ...

    RestApiRoot:
      ...

    __deployment__:
      Type: AWS::ApiGateway::Deployment
      Properties:
        RestApiId: !Ref RestApi

@RichardBradley
Copy link

I have hit the same issue.
I am creating a REST API (API Gateway) in Terraform, then using Serverless to deploy a Stage inside it, by passing a "restApiId".
Serverless then ignores my "logs: restApi: accessLogging: true" settings.

I will attempt to move the logging config to Terraform, as you suggest. It is not straightforward as the log settings are per Stage and Serverless creates the Stage.

It would be much better if Serverless raised an error if it is asked to configure an externally provided restApi, if it refuses to do so, rather than silently ignoring these settings.

@medikoo
Copy link
Contributor

medikoo commented Mar 17, 2021

Serverless then ignores my "logs: restApi: accessLogging: true" settings.

That's expected. Logs are turned only, if REST API is created with a stack, and not when you reference an externally setup API (in this case, we do not mess with global settings of it, as all its setting should be set by entity which created it)

@RichardBradley
Copy link

a) I didn't expect it, as the docs don't mention that caveat at all.
See e.g. https://www.serverless.com/framework/docs/providers/aws/guide/serverless.yml/ : " Optional configuration which specifies if API Gateway logs are used. This can either be set to true to use defaults, or configured via subproperties."

b) as mentioned above, I can see that this is a reasonable thing to do, but I think serverless should raise a warning or error if it is going to ignore this setting

@medikoo
Copy link
Contributor

medikoo commented Mar 18, 2021

@RichardBradley that's a very good points

See e.g. https://www.serverless.com/framework/docs/providers/aws/guide/serverless.yml/ :

it's reference of all configuration options. So this configuration as a whole cannot be used. Still I agree that it could be stated here, that setting applies only to API Gateway as created with a stack

but I think serverless should raise a warning or error if it is going to ignore this setting

I fully agree, I've created a request to improve that: #9139

@RichardBradley
Copy link

it's reference of all configuration options. So this configuration as a whole cannot be used

Understood. I couldn't find a doc page about the "logs: restApi:" feature in isolation, just the release announcement.
If there is separate documentation for this feature then it should be updated as well

I've created a request to improve that: #9139

Thanks

@paulalex
Copy link

paulalex commented Nov 3, 2021

I am a bit late but have hit this myself, same with the xray tracing.

Can someone explain to me why exactly sls considers it unsafe to set logging options? The API gateway is created externally but the logging and tracing options are on a stage, the stage is deployed as part of the sls cloudformation stack, it would seem that anyone creating an api gw externally and then using sls to attach functions would not be attaching them in say, terraform.

What is the reasoning behind the decision? It seems for me its actually impossible to get the logging working because the stages are never created by terraform, but the api gateway is.

@medikoo
Copy link
Contributor

medikoo commented Nov 3, 2021

The API gateway is created externally but the logging and tracing options are on a stage, the stage is deployed as part of the sls cloudformation stack, it would seem that anyone creating an api gw externally and then using sls to attach functions would not be attaching them in say, terraform.

@paulalex In Framework, we do not assume that service owns the stage when shared API is used, but that it owns only configured endpoints (and other endpoints for same API and stage may be configured elsewhere).

Allowing stage wide settings in such context creates conflict, imagine two different services defining different endpoints, deploying to same stage, but with different logging setup.

@paulalex
Copy link

paulalex commented Nov 3, 2021

Ok fair point, I didnt realise you could define the stage outside serverless and then deploy functions into it, its not something I considered but would actually get around my issue, thinking about it I could always define the stage inside terraform and knowing I can then attach functions to a stage instead of just an API gateway is great to know!

Could you point me at a sample config which demonstrates this by any chance? Right now I pass the API gw details into SSM and then read these into the SLS at deployment time, but I could easily build the stage in terraform and then pass the stage details into SSM.

@RichardBradley
Copy link

I will attempt to move the logging config to Terraform, as you suggest. It is not straightforward as the log settings are per Stage and Serverless creates the Stage.

I moved my logging config to Terraform and outside of Serverless, but now terraform thinks it owns the "stage" and the "deployment" (these two are linked), so Terraform will roll back the AWS::ApiGateway::Deployment created by Serverless each time it runs.

Is there any way to have Serverless use a provided APIG but also to configure logging on the APIG?

@benrhine
Copy link

benrhine commented Jun 5, 2023

I know this has been closed for awhile but I had a similar error and you should check which was which Aws API type you're using, as how logs are enabled are slightly different between if you're using REST API or HTTP Api. In my case I was using the rest syntax when I needed to be using the http syntax.

logs:                 # Additional logging configuration - This will create an additional log file in cloudwatch
    httpApi: true       # Enables logs for Aws HTTP Api Gateway - Ref: https://www.serverless.com/framework/docs/providers/aws/events/http-api#access-logs
#    restApi: true      # Enables logs for Aws REST Api Gateway - Ref: https://www.serverless.com/framework/docs/providers/aws/events/apigateway#logs

This is posted as reference that there is a difference and may be worth checking if someone is struggling

@mishabruml
Copy link
Contributor

mishabruml commented Aug 9, 2023

Is there any way to have Serverless use a provided APIG but also to configure logging on the APIG?

Hey there, running into the exact same issue, would love if there was a good solution for this? I suppose you could maybe use some lifecycle hooks to prevent terraform from overwriting the sls deployment? Hopefully? Not a pretty solution, even if it did work 😬

Edit: yes i think https://developer.hashicorp.com/terraform/language/meta-arguments/lifecycle#ignore_changes would do it

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.

8 participants