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

Deploy fails after removing service and upgrading to new version of serverless framework because an ApiGatewayLogGroup exists #6906

Open
fernando-mc opened this issue Oct 30, 2019 · 9 comments

Comments

@fernando-mc
Copy link
Contributor

Bug Report

Description

  1. What did you do?
    Ran serverless deploy for a recently removed service.

  2. What happened?
    The deployment failed because a resource was left over from a previous sls remove

  3. What should've happened?
    The service should deploy.

  4. What's the content of your serverless.yml file?

org: fernando
app: enterprise
service: demo-email-form

frameworkVersion: '>=1.38.0 <2.0.0'

provider:
  name: aws
  runtime: nodejs10.x
  iamRoleStatements:
    - Effect: 'Allow'
      Action:
        - 'sns:createTopic'
        - 'sns:deleteTopic'
        - 'sns:publish'
        - 'dynamodb:putItem'
      Resource:
        - '*'
  environment: 
    PHONE_NUMBER: ${secrets:ALERT_NUMBER}

functions:
  formSubmit:
    handler: index.submit
    events:
      - http:
          path: submit
          method: post
          cors: true
  1. What's the output you get when you use the SLS_DEBUG=* environment variable (e.g. SLS_DEBUG=* serverless deploy)

From serverless deploy --verbose

Serverless: Packaging service...
Serverless: Excluding development dependencies...
Serverless: Installing dependencies for custom CloudFormation resources...
Serverless: Safeguards Processing...
Serverless: Safeguards Results:

   Summary --------------------------------------------------

   passed - allowed-runtimes
   warned - no-wild-iam-role-statements
   passed - no-secret-env-vars

   Details --------------------------------------------------

   1) Warned - iamRoleStatement granting Resource='*'. Wildcard resources in iamRoleStatements are not permitted.
      details: http://slss.io/sg-no-wild-iam-role
      Prevent "*" permissions being used in AWS IAM Roles by checking for wildcards on Actions and Resources in grant statements.


Serverless: Safeguards Summary: 2 passed, 1 warnings, 0 errors
Serverless: Uploading CloudFormation file to S3...
Serverless: Uploading artifacts...
Serverless: Uploading service demo-email-form.zip file to S3 (77.86 KB)...
Serverless: Uploading custom CloudFormation resources...
Serverless: Validating template...
Serverless: Updating Stack...
Serverless: Checking Stack update progress...
CloudFormation - UPDATE_IN_PROGRESS - AWS::CloudFormation::Stack - demo-email-form-dev
CloudFormation - CREATE_IN_PROGRESS - AWS::Logs::LogGroup - FormSubmitLogGroup
CloudFormation - CREATE_IN_PROGRESS - AWS::Logs::LogGroup - ApiGatewayLogGroup
CloudFormation - CREATE_IN_PROGRESS - AWS::IAM::Role - IamRoleLambdaExecution
CloudFormation - CREATE_IN_PROGRESS - AWS::IAM::Role - IamRoleCustomResourcesLambdaExecution
CloudFormation - CREATE_IN_PROGRESS - AWS::ApiGateway::RestApi - ApiGatewayRestApi
CloudFormation - CREATE_IN_PROGRESS - AWS::Logs::LogGroup - FormSubmitLogGroup
CloudFormation - CREATE_IN_PROGRESS - AWS::IAM::Role - IamRoleCustomResourcesLambdaExecution
CloudFormation - CREATE_IN_PROGRESS - AWS::IAM::Role - IamRoleLambdaExecution
CloudFormation - CREATE_FAILED - AWS::Logs::LogGroup - ApiGatewayLogGroup
CloudFormation - CREATE_IN_PROGRESS - AWS::ApiGateway::RestApi - ApiGatewayRestApi
CloudFormation - CREATE_COMPLETE - AWS::Logs::LogGroup - FormSubmitLogGroup
CloudFormation - CREATE_COMPLETE - AWS::ApiGateway::RestApi - ApiGatewayRestApi
CloudFormation - CREATE_FAILED - AWS::IAM::Role - IamRoleCustomResourcesLambdaExecution
CloudFormation - CREATE_FAILED - AWS::IAM::Role - IamRoleLambdaExecution
CloudFormation - UPDATE_ROLLBACK_IN_PROGRESS - AWS::CloudFormation::Stack - demo-email-form-dev
CloudFormation - UPDATE_ROLLBACK_COMPLETE_CLEANUP_IN_PROGRESS - AWS::CloudFormation::Stack - demo-email-form-dev
CloudFormation - DELETE_IN_PROGRESS - AWS::Logs::LogGroup - FormSubmitLogGroup
CloudFormation - DELETE_IN_PROGRESS - AWS::ApiGateway::RestApi - ApiGatewayRestApi
CloudFormation - DELETE_IN_PROGRESS - AWS::IAM::Role - IamRoleLambdaExecution
CloudFormation - DELETE_IN_PROGRESS - AWS::IAM::Role - IamRoleCustomResourcesLambdaExecution
CloudFormation - DELETE_COMPLETE - AWS::Logs::LogGroup - ApiGatewayLogGroup
CloudFormation - DELETE_COMPLETE - AWS::Logs::LogGroup - FormSubmitLogGroup
CloudFormation - DELETE_COMPLETE - AWS::ApiGateway::RestApi - ApiGatewayRestApi
CloudFormation - DELETE_COMPLETE - AWS::IAM::Role - IamRoleCustomResourcesLambdaExecution
CloudFormation - DELETE_COMPLETE - AWS::IAM::Role - IamRoleLambdaExecution
CloudFormation - UPDATE_ROLLBACK_COMPLETE - AWS::CloudFormation::Stack - demo-email-form-dev
Serverless: Operation failed!
Serverless: View the full error output: https://us-east-1.console.aws.amazon.com/cloudformation/home?region=us-east-1#/stack/detail?stackId=arn%3Aaws%3Acloudformation%3Aus-east-1%3A757370802528%3Astack%2Fdemo-email-form-dev%2F0d782920-fb68-11e9-889e-0a743b4c9590
Serverless: Publishing service to the Serverless Dashboard...
Serverless: Successfully published your service to the Serverless Dashboard: https://dashboard.serverless.com/tenants/fernando/applications/enterprise/services/demo-email-form/stage/dev/region/us-east-1
 
  Serverless Error ---------------------------------------
 
  An error occurred: ApiGatewayLogGroup - /aws/api-gateway/demo-email-form-dev already exists.
 
  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Issues:        forum.serverless.com
 
  Your Environment Information ---------------------------
     Operating System:          darwin
     Node Version:              6.10.3
     Framework Version:         1.55.1
     Plugin Version:            3.2.1
     SDK Version:               2.1.2

Similar or dependent issues:

@fernando-mc fernando-mc changed the title serverless deploy fails for a recently removed service after upgrading to new version - An error occurred: ApiGatewayLogGroup - /aws/api-gateway/demo-email-form-dev already exists. Deploy fails after removing service and upgrading to new version of serverless framework because an ApiGatewayLogGroup exists Oct 30, 2019
@Komsomol
Copy link

I have the same issue.

@medikoo
Copy link
Contributor

medikoo commented Mar 4, 2020

This bug can happen when following deploy flow happens:

  1. Service with API Gateway logging on (provider.logs.restApi: true) is deployed
  2. Same service is redeployed with provider.logs.restApi removed. This removes the log group, but logging is not turned off (it's due to limitations on how this setting is handled in Framework, framework has no state history, and doesn't know that for previous deployment it was true)
    In result after following Rest API requests (to be logged) log group is automatically recreated by AWS
  3. Same service is again deployed with logging on (provider.logs.restApi: true). This crashes as there's an attempt to create log group which apparently exists.

Workaround is to at step 2, redeploy service with logs explicitly turned off (via provider.logs.restApi: false), that should not produce log writes between deployments.

This will be effectively fixed once current handling of API Gateway stage settings is moved to custom resources (or CloudFormation when modifications to it will be supported). I have that on my todo list, but due to other priorities release window is uncertain.

@ChrisMcHarvey
Copy link

Hi medikoo,

I'm experiencing this same issue but can't see how to deploy the service with logging on or off. Where would I be adding or removing this parameter?

Thanks

@ChrisMcHarvey
Copy link

For reference, this is my serverless.yml file:

org: chrismcharvey
app: ucann-rest
service: ucann-rest

provider:
name: aws
runtime: nodejs12.x
stage: dev
region: us-west-2
logs:
restApi: false

functions:
api:
handler: handler.handler
events:
- http:
path: "{proxy+}"
method: ANY
cors: true
- https:
path: "{proxy+}"
method: ANY
cors: true

@medikoo
Copy link
Contributor

medikoo commented Apr 6, 2020

@pgrzesik
Copy link
Contributor

pgrzesik commented Feb 4, 2021

Hello 👋 There's another instance of that happening reported here: #8841

I investigated that issue a bit and I was wondering if defaulting provider.logs.restApi to false if it's not defined could solve the issue. I believe that would force stage update via SDK, which will properly turn of logging on API Gateway level. The only thing I'm not sure about is if this is going to have some other implications that would prevent it from being a viable solution. @medikoo what do you think?

@medikoo
Copy link
Contributor

medikoo commented Feb 4, 2021

The only thing I'm not sure about is if this is going to have some other implications that would prevent it from being a viable solution

At the beginning it was working like that, and there was a lot of issues, still they were implied by also by other bugs (e.g. we were disabling logs on externally setup API Gateway, defined by id in a service, or we were doing it even if there's no API Gateway configuration at all - that raised IAM error issues).

Currently I'm still not sure. This is the setting that cannot be reliably set via CF, and users in general might have that setting set manually on their own, if we suddenly start to disable it (because this change will introduce such action), I can imagine we may receive a reports that we started to break the behavior.

I think bulletproof approach would be to (1) Introduce state persistence with locking (thing we discussed once), I've patched the initial proposal here: #8896 and having that (2) react purely on changes to that setting (so remove only when provider.logs.restApi: true was removed, but not at all times when it's not present.

What do you think?

@pgrzesik
Copy link
Contributor

pgrzesik commented Feb 4, 2021

Thanks for the clarification @medikoo - I was exactly worried about potentially introducing even more bugs while trying to fix this using the current approach. I agree that the best approach moving forward would be to introduce a persistent state that could be used to track such settings.

@adam-greaves
Copy link

Seems a way around this is by changing your service name to something new to work around the error of the existing log.

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

No branches or pull requests

7 participants