Serverless-application-model: RFC: Allow customizations on AWS::Serverless::Function API event types

[beck3905]

Swagger seems to be an all-or-nothing approach to AWS::Serverless::Api resources. If the swagger definition is provided in the API then the entire API needs to defined in swagger. I often have a single AWS::Serverless::Function that needs some customization in how it should be defined in the API, but in order to accomplish that I would need to provide boilderplate swagger for the entire API plus my customization.

It would be much more convenient and simple if I could just add my customizations to the Lambda API event and the translator would apply these to the API definition for that specific function.

[keetonian]

@beck3905 I like this idea. Like you posted, there are several features in https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-method.html that could be useful in API Events.

Some of these issues are:

• Authorization Scopes (#652)
• Request Models (#895)
• API Key Required (and usage plans) (#248)

Could you provide any sample syntax of how you might like to use some of these features? Or any other features that you think would be good to add to the API Event?

[beck3905]

@keetonian I recently had a use case where I had an API with caching enabled that requires an Authorization token. Because the caching only looks at the URL and not the headers the cache would return valid values for users with invalid tokens and would return data for other users. Obviously, this was a problem. I solved it by adding the Authorization header parameter to the Method Request and enabling caching on that parameter. Since Serverless::API is pretty much all or nothing with swagger, the only way to accomplish this was with swagger. However, if the https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-method.html#cfn-apigateway-method-requestparameters were available in the API event then I would've only had to add a small bit of configuration there to accomplish the same thing.

I suggest exposing the following properties in the API event source type:

• ApiKeyRequired
• AuthorizationScopes
• RequestModels (linked to Models defined in the Serverless::API resource)
• RequestParameters
• RequestValidatorId
• ResponseModels (mapping of status code to model and linked to Models defined in the Serverless::API resource)
• ResponseParameters (mapping of status code to parameters)

For example:

Resource:
  Api:
    Type: AWS::Serverless::Api
    Properties:
      Models: # use swagger format to define models. This gets passed through to the `definitions` section of swagger.
        User:   
          type: object
          required:
            - userName
          properties:
            userName:
              type: string
            firstName:
              type: string
            lastName:
              type: string
     ...

  Function:
    Type: AWS::Serverless::Function
    Properties:
      Events:
        ApiEvent:
          Type: Api
          Properties:
            RestApiId: !Ref Api
            RequestModel: User    # reference by string name
            RequestParameters:
              - method.request.header.Authorization:
                   required: true
                   caching: true
            ResponseModels: 
              200: User  # reference by string name
            ResponseParameters:
              200:
                - method.response.header.Access-Control-Allow-Origin: "'*'"
            Auth:
              ApiKeyRequired: true
              Authorizer: MyCognitoAuthorizer
              OauthScopes:
                - scope3

[beck3905]

@keetonian I've submitted a PR for my first stab at this. I implemented the RequestModel support in #948. Would you please take a look?

[beck3905]

@keetonian I've just submitted a second PR for RequestParameters support in #953. Would you please also take a look at that?

[MohammedAlSafwan]

@keetonian is there an estimate date on when will this be released ?

[praneetap]

@MohammedAlSafwan We're working on making our release process more transparent to external customers, but we're not quite where we want to be yet. For now, you can see when we cut release branches, which can give you some indication of when we begin working on a specific release.

[praneetap]

@beck3905 While writing tests for this feature during our internal QA process for Release V1.15, here's what i discovered -

1. I found that setting method parameters to required didn't fail my request if it didn't have the query parameters. This is (i think) because api GW requires you to define request validators to make the required setting apply. What was your use case for adding the required flag?
2. I tried to test caching but it didn't work for me, until I set the Cache ttl on the Api GW console. On digging deeper we need to set MethodSettings on the API in the SAM template, or the settings on the method don't get applied, and the cache ttl value gets overwritten to 0. Here is the template that (finally) worked for me -

Globals:
  Api:
    OpenApiVersion: '3.0.1'
    CacheClusterEnabled: true
    CacheClusterSize: '0.5'
    MethodSettings:
      - ResourcePath: /one
        HttpMethod: "GET"
        CachingEnabled: true # required to enable caching
        CacheTtlInSeconds: 15 # optional
Resources:
  ApiParameterFunction:
    Type: AWS::Serverless::Function
    Properties:
      InlineCode: |
        exports.handler = function(event, context, callback) {
            var returnVal = "undef";
            if (event.queryStringParameters.type === "time") {
                returnVal = "time " + Date.now();
            }

            if (event.queryStringParameters.type === "random") {
                returnVal = "Random " + Math.random();
            }

            callback(null, {
                "statusCode": 200,
                "body": returnVal
            });
        }
      Handler: index.handler
      Runtime: nodejs8.10
      Events:
        GetHtml:
          Type: Api
          Properties:
            Path: /one
            Method: get
            RequestParameters:
              - method.request.querystring.type:
                  Required: true
                  Caching: true
        AnotherGetHtml:
          Type: Api
          Properties:
            Path: /two
            Method: get

[beck3905]

@praneetap

1. Yes, I believe the behavior you described is the same when setting up required parameters in the API Gateway console. My use case for adding the required flag is for API Gateway to fail the request if the parameter is not present. Perhaps another property is need on the API event source object for RequestValidators.

2. I don't recall having to set the TTL or caching method settings. I just had to set the CacheClusterEnable: true and `CacheClusterSize' properties on the API. Maybe I missed the TTL when inspecting the result of the stack in the API Gateway console. Could this be handled by an update to documentation?

[praneetap]

@praneetap

1. Yes, I believe the behavior you described is the same when setting up required parameters in the API Gateway console. My use case for adding the required flag is for API Gateway to fail the request if the parameter is not present. Perhaps another property is need on the API event source object for RequestValidators.
2. I don't recall having to set the TTL or caching method settings. I just had to set the CacheClusterEnable: true and `CacheClusterSize' properties on the API. Maybe I missed the TTL when inspecting the result of the stack in the API Gateway console. Could this be handled by an update to documentation?

Thanks for getting back @beck3905

1. We will create a task and submit it for prioritization. Currently API GW will not fail if the parameter is not present, so the task would be to add x-amazon-apigateway-request-validators extension support in the swagger.

2. Doc update is absolutely necessary to the examples/ folder, but I also think having to specify MethodSettings for the api is awkward syntax. Ideally, setting caching on a parameter should do this for you. I am curious to hear your thoughts on it.

[beck3905]

@praneetap I agree. Does this mean that this feature is not releasable until those 2 things are accomplished? Or can this be released and then address the 2 things in a future release?

Thanks. Side-note: Will any of you be at RE:Invent this year? I would love to put faces to names as you all have been so much help in moving these issues and PRs forward and I hope to continue contributing to SAM.

[praneetap]

@beck3905

@praneetap I agree. Does this mean that this feature is not releasable until those 2 things are accomplished? Or can this be released and then address the 2 things in a future release?

We decided to move forward with releasing this anyway because it adds a lot of value as is. Updating it to add MethodSettings for you, and request-validators are two tasks we can address in the near future. Not sure if you have seen this already, but we now have a project board that shows you what stage a release version is in. Each release has a tracking issue that contains more information on what is being released and the issues found during internal QA.

Thanks. Side-note: Will any of you be at RE:Invent this year? I would love to put faces to names as you all have been so much help in moving these issues and PRs forward and I hope to continue contributing to SAM.

That just made my day! :) thank you. I won't be going unfortunately, but I am sure someone from my team will be there. More a question for @jlhood I think. We really look forward to meeting SAM contributors :)

[jlhood]

@beck3905 Yes, I'll be at re:Invent this year giving some talks and working at the serverless booth at the expo. Would love to meet up with you as well! Twitter is probably the best way to reach out to me (@jlhcoder). Message me as the date gets closer, and we'll try to find a time that works. 😊

[ShreyaGangishetty]

Closing this issue as v1.15.0 is released

[beck3905]

@ShreyaGangishetty I think there is more work to be done related to this issue. Only part of it was released in v1.15.0. Should this issue remain open to account for that remaining work?

[azarboon]

Adding RequestParameters to AWS::Serverless::Function, doesn't enforce it. Even though the query string is defined as required, but it's not enforced.

            RequestParameters: 
              - method.request.querystring.myString: 
                  Required: true

And in API GW console, under URL Query String Parameters, it shows this warning:

You have marked some query string parameter as required but thee request validator assigned to this method is not configured to validate parameters. To ensure that incoming HTTP requests include the required query string parameters, select an appropriate request validator.

This makes us to need to updated RequestParameters in template (in order to have updated Swagger specs) and to update Lambda business logic (to perform validation). So it's extra work and can lead to inconsistency.

[keetonian]

@praneetap mentioned this in https://github.com/awslabs/serverless-application-model/issues/931#issuecomment-534463087

This is an open feature request to add validators.

[tuler]

I tried to add caching to a method with a parameter /{id}, and the caching works but the parameter is ignored. (so /1 is cached for any other id request after that)

My setup is as follows:

  RestApi:
    Type: AWS::Serverless::Api
    Properties:
      StageName: Prod
      CacheClusterEnabled: true
      CacheClusterSize: '0.5'
      MethodSettings:
        - ResourcePath: /{id}
          HttpMethod: "GET"
          CachingEnabled: true
          CacheTtlInSeconds: 60

  GetFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: src/
      Handler: app.handler
      Runtime: nodejs12.x
      Events:
        Http:
          Type: Api
          Properties:
            Path: /{id}
            Method: get
            RequestParameters:
              method.request.path.id:
                Required: true
                Caching: true
            RestApiId: !Ref RestApi

The GET method at the Prod stage is marked as Override for this method, and Enable Method Cache is checked.

id is marked as cached as well. Screenshot below.

[tuler]

I think I just found my own mistake.

       Events:
         Http:
           Type: Api
           Properties:
             Path: /{id}
             Method: get
             RequestParameters:
(-)           method.request.path.id:
(-)             Required: true
(-)             Caching: true
(+)           - method.request.path.id:
(+)               Required: true
(+)               Caching: true

It's sad I don't get an error though.

[keetonian]

@tuler agree, it looks like SAM should add an error at that point for an incorrect configuration.

It looks like SAM reads it as a list, some digging is needed to see why an error wasn't thrown when you gave it a dictionary.

https://github.com/awslabs/serverless-application-model/blob/develop/samtranslator/model/eventsources/push.py#L485