Kludex / mangum

AWS Lambda support for ASGI applications
http://mangum.fastapiexpert.com/
MIT License
1.73k stars 126 forks source link

api_gateway_base_path not supported for ALB? #238

Open ShedPlant opened 2 years ago

ShedPlant commented 2 years ago

Summary

I looked at https://mangum.io/http/ and the GitHub issues and couldn't find this mentioned anywhere.

My Setup

Simplified version of my lambda handler:

def public_foo_function:
    print("Public foo function")

def private_bar_function:
    print("Private bar function")

app = Starlette(
    routes=[
        Route(
            path="/public/foo",
            endpoint=public_foo_function,
        ),
        Route(
            path="/private/bar",
            endpoint=private_bar_function,
        ),
    ]
)

mangum = Mangum(
    app,
    api_gateway_base_path="/prefix",
)

def main(event, context):
    return mangum(event, context)

An external API Gateway v2 HTTP has a route /prefix/public/{proxy+} integrated with the lambda. An internal ALB has a listener rule /prefix/private/* integrated with the lambda.

Requests to <api gateway url>/prefix/public/foo arrive in the lambda and public_foo_function is called, as expected.

From within my VPC, internal requests to <alb url>/prefix/private/bar arrive in the lambda and Starlette returns Not Found 404. This is unexpected.

As a workaround, if I change the private route in Starlette app from /private/bar to /prefix/private/bar, private_bar_function is called.

Considerations

api_gateway_base_path does have 'api gateway' in the name, and ALB is not an API Gateway. So there's an argument it could be intentional?

But, I expected mangum to behave consistently for all of its supported input events.

jordaneremieff commented 2 years ago

@ShedPlant

api_gateway_base_path does have 'api gateway' in the name, and ALB is not an API Gateway. So there's an argument it could be intentional?

I'm not sure if it is intentional or not. API Gateway was originally the only supported event source and ALB support came long after the api_gateway_base_path setting was first included, so it's possible that those contributors of the ALB support assumed to ignore the setting due to the api_gateway_* prefix in the setting name.

Should we replace api_gateway_base_path with a single base_path configuration that behaves the same way for the ALB handler?

ShedPlant commented 2 years ago

@jordaneremieff

Should we replace api_gateway_base_path with a single base_path configuration that behaves the same way for the ALB handler?

Thanks for your reply. That sounds right to me.