aws / serverless-java-container

A Java wrapper to run Spring, Spring Boot, Jersey, and other apps inside AWS Lambda.
https://aws.amazon.com/serverless/
Apache License 2.0
1.48k stars 551 forks source link

Spring Boot 2: StageName should be stripped from the path by default #401

Open ghost opened 3 years ago

ghost commented 3 years ago

Serverless Java Container version: 1.5.2

Implementations: Spring Boot 2

Framework version: spring-boot-starter-parent 2.3.8.RELEASE

Frontend service: AWS::Serverless::HttpApi (API Gateway HTTP)

Deployment method: SAM CLI

Scenario

Configure a simple HTTP API proxy to Lambda using Spring Boot 2, and be able to route requests that ignore the stage-name.

Expected behavior

Stage name (which is part of the path by default) is stripped before spring routes/maps to a controller.

Actual behavior

Getting 404's b/c spring is mapping <stage>/<path> vs <path>; requires my code to be 'stage-name aware'.

Steps to reproduce

event-source in sam template for AWS::Serverless::Function

      Events:
        BaseAdmin:
          Type: HttpApi
          Properties:
            ApiId: !Ref Api
            Method: any
            Path: /{proxy+}
            PayloadFormatVersion: "1.0"
            RouteSettings:
              ThrottlingBurstLimit: 100
              ThrottlingRateLimit: 100
            TimeoutInMillis: 29000

api def in sam template:

  Api:
    Type: AWS::Serverless::HttpApi
    Properties:
      FailOnWarnings: true
      StageName: dev

lambda handler

public class LambdaHandler implements RequestStreamHandler {
    private static final SpringBootLambdaContainerHandler<AwsProxyRequest, AwsProxyResponse> HANDLER;

    static {
        Application.envSetup();
        try {
            HANDLER = new SpringBootProxyHandlerBuilder<AwsProxyRequest>().defaultProxy()
                                                                          .asyncInit()
                                                                          .springBootApplication(Application.class)
                                                                          .buildAndInitialize();
        } catch (ContainerInitializationException e) {
            e.printStackTrace();
            throw new RuntimeException(e);
        }
        // Want to not need the line below uncommented to work!
        // HANDLER.stripBasePath(System.getenv("STRIP_BASE_PATH"));
    }

    @Override
    public void handleRequest(InputStream inputStream, OutputStream outputStream, Context context) throws IOException {
        HANDLER.proxyStream(inputStream, outputStream, context);
    }
}

Full log output

Paste the full log output from the Lambda function's CloudWatch logs dev is the stage name; the mapping is for /api/v1

DEBUG DispatcherServlet - GET "/dev/api/v1/some/resource", parameters={}
DEBUG DispatcherServlet - Exiting from "ERROR" dispatch, status 404

This isn't the worst thing ever...but it's definitely counter-intuitive to me. Would also appreciate any feedback on my solution; I did try some of the config options mentioned on the wiki page, but none of them seemed to do the trick until i found handler.stripBasePath. If there is a way to do this w/o passing in the StageName, that'd be better. Having it work by default would obviously be best.

srp321 commented 3 years ago

@aguerard, I have faced the same issue. This is an issue, but not sure it it's with API Gateway or for Lambda side implementation. Ideally, since you have used stage, so the stage name should be there in the API gateway endpoint, which it is, but internally API Gateway also maps that to the AWS_PROXY when integration target is with AWS Lambda. Now, in case of spring based AWS Lambda, the endpoint we map doesn't have any stage as such needed in the URL (as in example, the usual endpoint we get is : api/v1/some/resource which is recognised by spring boot based lambda, but with stage name from api gateway, its /dev/api/v1/some/resource which is not recognized by dispatcher servlet as we don't have any request mapping for the same). So, this seems an issue more from API Gateway side. Correct me if i'm wrong. So, for now, the workaround i follow is to not have any stage name, and use the $default one.

ghost commented 3 years ago

@aguerard, I have faced the same issue. This is an issue, but not sure it it's with API Gateway or for Lambda side implementation. Ideally, since you have used stage, so the stage name should be there in the API gateway endpoint, which it is, but internally API Gateway also maps that to the AWS_PROXY when integration target is with AWS Lambda. Now, in case of spring based AWS Lambda, the endpoint we map doesn't have any stage as such needed in the URL (as in example, the usual endpoint we get is : api/v1/some/resource which is recognised by spring boot based lambda, but with stage name from api gateway, its /dev/api/v1/some/resource which is not recognized by dispatcher servlet as we don't have any request mapping for the same). So, this seems an issue more from API Gateway side. Correct me if i'm wrong. So, for now, the workaround i follow is to not have any stage name, and use the $default one.

Thanks for the reply. Once I realized I could use $default that's what I ended up doing, so it's good validation you suggested the same. If I understood the explanation, it's that APIG isn't setting 'stageName' in the event sent to Lambda? If so, then agree it's an APIG issue. If not, then this seems like something the container should use to set context/base path in Spring.

srp321 commented 3 years ago

@aguerard, I have faced the same issue. This is an issue, but not sure it it's with API Gateway or for Lambda side implementation. Ideally, since you have used stage, so the stage name should be there in the API gateway endpoint, which it is, but internally API Gateway also maps that to the AWS_PROXY when integration target is with AWS Lambda. Now, in case of spring based AWS Lambda, the endpoint we map doesn't have any stage as such needed in the URL (as in example, the usual endpoint we get is : api/v1/some/resource which is recognised by spring boot based lambda, but with stage name from api gateway, its /dev/api/v1/some/resource which is not recognized by dispatcher servlet as we don't have any request mapping for the same). So, this seems an issue more from API Gateway side. Correct me if i'm wrong. So, for now, the workaround i follow is to not have any stage name, and use the $default one.

Thanks for the reply. Once I realized I could use $default that's what I ended up doing, so it's good validation you suggested the same. If I understood the explanation, it's that APIG isn't setting 'stageName' in the event sent to Lambda? If so, then agree it's an APIG issue. If not, then this seems like something the container should use to set context/base path in Spring.

Would need to check the default implementation of 'stageName' and how it is deserialized to what key in the case of Lambda from API gateway. Secondly, if it's a container thing, then AWS Lambda doesn't give us an option to change the base path, apart from directly using a container of our own in which case, we can have better control over it.

sapessi commented 3 years ago

I haven't had time to dig into it yet, but I wonder if this is because the payload format version 1.0 includes the stage and the readers/writers make use of it. I would try to use the new V2 payload format and call the defaultHttpApiV2Proxy() method to initialize the handler.

nicofabre commented 3 years ago

Any news about that ? I have a Spark servlet container Java app that configures a endpoint for "/ping". Using the $default stage, it works as it tries to reach https://my-api.us-east-1.amazonaws.com/ping. But as soon as I try another stage, dev for example, the URL is https://my-api.us-east-1.amazonaws.com/dev/ping and my Spark application using handler = SparkLambdaContainerHandler.getHttpApiV2ProxyHandler(); tries to find an endpoint for /dev/ping instead of /ping.

It's annoying. I cannot deploy specific application for each stage. Would it be possible to get rid of this /dev prefix and make Spark searching for /ping endpoint instead of /dev/ping ?

Thank you

nicofabre commented 3 years ago

I would like to propose a change in the code. I checked out the code in my IDE, made the change in a branch named "issue_401". I cannot push this branch to propose my code fix. I get an authorization error.How should I proceed ?

As fallback here is the code fix in this comment :

`package com.amazonaws.serverless.proxy.internal.servlet;

import com.amazonaws.serverless.exceptions.InvalidRequestEventException; import com.amazonaws.serverless.proxy.RequestReader; import com.amazonaws.serverless.proxy.model.ContainerConfig; import com.amazonaws.serverless.proxy.model.HttpApiV2ProxyRequest; import com.amazonaws.services.lambda.runtime.Context;

import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.SecurityContext;

public class AwsHttpApiV2HttpServletRequestReader extends RequestReader<HttpApiV2ProxyRequest, HttpServletRequest> { static final String INVALID_REQUEST_ERROR = "The incoming event is not a valid HTTP API v2 proxy request";

@Override
public HttpServletRequest readRequest(HttpApiV2ProxyRequest request, SecurityContext securityContext, Context lambdaContext, ContainerConfig config) throws InvalidRequestEventException {
    if (request.getRequestContext() == null || request.getRequestContext().getHttp().getMethod() == null || request.getRequestContext().getHttp().getMethod().equals("")) {
        throw new InvalidRequestEventException(INVALID_REQUEST_ERROR);
    }

    this.handleStage(request);

    // clean out the request path based on the container config
    request.setRawPath(stripBasePath(request.getRawPath(), config));

    AwsHttpApiV2ProxyHttpServletRequest servletRequest = new AwsHttpApiV2ProxyHttpServletRequest(request, lambdaContext, securityContext, config);
    servletRequest.setAttribute(HTTP_API_CONTEXT_PROPERTY, request.getRequestContext());
    servletRequest.setAttribute(HTTP_API_STAGE_VARS_PROPERTY, request.getStageVariables());
    servletRequest.setAttribute(HTTP_API_EVENT_PROPERTY, request);
    servletRequest.setAttribute(LAMBDA_CONTEXT_PROPERTY, lambdaContext);
    servletRequest.setAttribute(JAX_SECURITY_CONTEXT_PROPERTY, securityContext);

    return servletRequest;
}

// https://github.com/awslabs/aws-serverless-java-container/issues/401
// AWS API Gateway is adding the stage in the URL at the beginning of the raw path. Remove this stage from the raw path to make the configured
// controllers / enpoints working with the paths without the stage.
private void handleStage(HttpApiV2ProxyRequest request) {
    String stage = request.getRequestContext().getStage();
    String rawPath = request.getRawPath();
    // if the URL begins with "/$stage/", then we consider that this first part of the path is the undesired 'stage' added by API Gateway
    if (stage != null && !stage.isEmpty() && rawPath.startsWith("/" + stage + "/")) {
        request.setRawPath(rawPath.substring(stage.length() + 1));// from "/stage/pets" to "/pets"
    }
}

@Override
protected Class<? extends HttpApiV2ProxyRequest> getRequestClass() {
    return HttpApiV2ProxyRequest.class;
}

}`

deki commented 3 years ago

@nicofabre Would be great if you can submit a Pull Request for that. To do it you need to fork the repo first (see https://docs.github.com/en/github/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork)