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

moved stage to its own resource #3232

Closed
wants to merge 1 commit into from

Conversation

rochdev
Copy link

@rochdev rochdev commented Feb 10, 2017

What did you implement:

Closes #1918 #2665 #1704

Make API Gateway stage configuration available, including but not limited to logging, throttling and stage variables.

How did you implement it:

By adding an actual AWS::ApiGateway::Stage resource instead of simply using the StageNameproperty of AWS::Apigateway::Deployment.

I have also moved the service endpoint output from the deployment to the stage, since that is where it logically belongs.

The reason this is a breaking change is that using StageName from AWS::ApiGateway::Deployment implicitly creates a stage which conflicts with AWS::ApiGateway::Stage when using the same name, so I had to remove it. This makes updating existing deployments that were created using previous versions of serverless impossible (a workaround would be to create an intermediary stage, manually delete the stage implicitly created by the previous deployment, and then recreate the original stage).

How can we verify it:

To verify that everything works as before, simply deploy a Lambda function with an http event as usual.

To verify that it is now possible to configure the stage, add a ApiGatewayStage resource with some configuration (see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-stage.html for available options).

For example, to enable logging and add a stage variable:

resources:
  Resources:
    ApiGatewayStage:
      Type: "AWS::ApiGateway::Stage"
      Properties:
        MethodSettings:
          - DataTraceEnabled: true
            HttpMethod: "*"
            LoggingLevel: ERROR
            ResourcePath: "/*"
            MetricsEnabled: true
        Variables:
          foo: "bar"

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?: YES

@eahefnawy
Copy link
Member

This makes updating existing deployments that were created using previous versions of serverless impossible

This is a deal breaker 😒 ... while I understand your use case and I think it's completely valid, imo I personally don't believe it's worth breaking all of our current users projects. Probably 95% of them don't really care about the stage.

It should be also noted that we don't really use APIG stages for anything and it has nothing to do with Serverless stages, because when you deploy your service to a new stage, an entirely new stack is created. So we add the APIG stage only because we have to in the Deployment resource.

@rochdev is there anyway we can make that change in a backward compatible way?

@HyperBrain
Copy link
Member

HyperBrain commented Mar 10, 2017

@rochdev If you want to deploy multiple stages in APIG and also have the benefits of using aliased lambda functions and much more, you can try my alias plugin (https://github.com/HyperBrain/serverless-aws-alias).
It creates an APIG stage per alias - quite similar to the behavior of the old Serverless 0.5.

BTW: Even in this context of the plugin I came to the conclusion that a Stage resource is superfluous, as the alias API deployment resource controls and owns the stage.

@rochdev
Copy link
Author

rochdev commented Mar 10, 2017

@eahefnawy It would be possible to add a configuration flag to enable stage configuration, but it would need to be very clear in the documentation that this flag cannot be changed once deployed. The best way however would definitely be if this could be transparently handled in code, but I haven't figured out a way to do it.

@HyperBrain This PR's goal is to enable configuration of the stage implicitly created by Serverless, not deploy to multiple stages. That should be made possible by the framework without requiring an external plugin, but is currently made impossible by using CloudFormation in a way that prevents it.

@eahefnawy
Copy link
Member

@rochdev We really appreciate your efforts, but I'm afraid we can't merge this PR for the following reasons:

  • It's a massive breaking change. At the same time, providing a flag is not a pleasing idea as we want to keep serverless.yml simple. We used to do that for our breaking changes, but we stopped as the config file was getting complicated.
  • It increases CF template size for all users, which is something we always try to avoid so that we wouldn't hit the CF size limits as we did before.

I suggest you follow @HyperBrain advice for your use case, or you can overwrite our core CF resources using custom resources. This way you can overwrite our deployment resource and add your own stage resource.

@eahefnawy eahefnawy closed this Mar 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants