aws-powertools / powertools-lambda-python

A developer toolkit to implement Serverless best practices and increase developer velocity.
https://docs.powertools.aws.dev/lambda/python/latest/
MIT No Attribution
2.83k stars 391 forks source link

Feature request: HTTP resolver middleware should have access to path parameters #4609

Open kimsappi opened 3 months ago

kimsappi commented 3 months ago

Use case

HTTP resolver middlewares should have access to path parameters to help with use cases like resource-specific authnz, logging enrichment, etc. Access is already available in an undocumented manner that may be considered "private" according to PEP 8 (see "User Experience" below).

Solution/User Experience

Current:

def middleware(app: APIGatewayRestResolver, next_middleware: NextMiddleware) -> Response:
    path_params = app.context.get("_route_args")
    logger.append_keys(path_params=path_params)
    return next_middleware(app)

New:

def middleware(app: APIGatewayRestResolver, next_middleware: NextMiddleware) -> Response:
    path_params = app.context.path_parameters
    logger.append_keys(path_params=path_params)
    return next_middleware(app)

Alternative solutions

Perhaps it would be ideal to have the parsed path parameters available via app.current_event instead (as with API Gateway resolvers), but using app.context is certainly easier and more in line with the current state. Providing data in the powertools app.current_event that is not present in a typical Lambda handler's event could also lead to confusion.

Acknowledgment

boring-cyborg[bot] commented 3 months ago

Thanks for opening your first issue here! We'll come back to you as soon as we can. In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

kimsappi commented 3 months ago

Here's one solution for implementation (I can make a PR if this is deemed a valuable suggestion, tips for how to document it welcome):

diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py
index 7c4d6769..4cbd2bd8 100644
--- a/aws_lambda_powertools/event_handler/api_gateway.py
+++ b/aws_lambda_powertools/event_handler/api_gateway.py
@@ -423,7 +423,7 @@ class Route:
             print("=================")

         # Add Route Arguments to app context
-        app.append_context(_route_args=route_arguments)
+        app.context["_route_args"] = route_arguments

         # Call the Middleware Wrapped _call_stack function handler with the app
         return self._middleware_stack(app)
@@ -898,10 +898,17 @@ class ResponseBuilder(Generic[ResponseEventT]):
         }

+class RouterContext(dict):
+    @property
+    def path_parameters(self) -> Dict[str, Any]:
+        """Get path parameters for the current request."""
+        return self.get("_route_args", {})
+
+
 class BaseRouter(ABC):
     current_event: BaseProxyEvent
     lambda_context: LambdaContext
-    context: dict
+    context: RouterContext
     _router_middlewares: List[Callable] = []
     processed_stack_frames: List[str] = []

@@ -1317,6 +1324,11 @@ class BaseRouter(ABC):

     def append_context(self, **additional_context):
         """Append key=value data as routing context"""
+        if "_route_args" in additional_context:
+            warnings.warn(
+                "The '_route_args' context key is used internally by powertools and should not be overwrit
ten",
+                stacklevel=2,
+            )
         self.context.update(**additional_context)

     def clear_context(self):
@@ -1421,7 +1433,7 @@ def _registered_api_adapter(
         The API Response Object

     """
-    route_args: Dict = app.context.get("_route_args", {})
+    route_args: Dict = app.context.path_parameters
     logger.debug(f"Calling API Route Handler: {route_args}")
     return app._to_response(next_middleware(**route_args))

@@ -1494,7 +1506,7 @@ class ApiGatewayResolver(BaseRouter):
         self._debug = self._has_debug(debug)
         self._enable_validation = enable_validation
         self._strip_prefixes = strip_prefixes
-        self.context: Dict = {}  # early init as customers might add context before event resolution
+        self.context = RouterContext()  # early init as customers might add context before event resolutio
n
         self.processed_stack_frames = []
         self._response_builder_class = ResponseBuilder[BaseProxyEvent]

@@ -2453,7 +2465,7 @@ class Router(BaseRouter):
         self._routes: Dict[tuple, Callable] = {}
         self._routes_with_middleware: Dict[tuple, List[Callable]] = {}
         self.api_resolver: Optional[BaseRouter] = None
-        self.context = {}  # early init as customers might add context before event resolution
+        self.context = RouterContext()  # early init as customers might add context before event resolutio
n
         self._exception_handlers: Dict[Type, Callable] = {}

     def route(
diff --git a/tests/functional/event_handler/required_dependencies/test_router.py b/tests/functional/event_h
andler/required_dependencies/test_router.py
index d96f5035..b6111017 100644
--- a/tests/functional/event_handler/required_dependencies/test_router.py
+++ b/tests/functional/event_handler/required_dependencies/test_router.py
@@ -1,16 +1,23 @@
+import pytest
+
 from aws_lambda_powertools.event_handler import (
     ALBResolver,
     APIGatewayHttpResolver,
+    ApiGatewayResolver,
     APIGatewayRestResolver,
     LambdaFunctionUrlResolver,
     Response,
 )
+from aws_lambda_powertools.event_handler.middlewares import NextMiddleware
+from aws_lambda_powertools.event_handler.openapi.params import Path
 from aws_lambda_powertools.event_handler.router import (
     ALBRouter,
     APIGatewayHttpRouter,
     APIGatewayRouter,
     LambdaFunctionUrlRouter,
+    Router,
 )
+from aws_lambda_powertools.shared.types import Annotated
 from aws_lambda_powertools.utilities.data_classes import (
     ALBEvent,
     APIGatewayProxyEvent,
@@ -74,3 +81,71 @@ def test_lambda_function_url_router_event_type():
     app.include_router(router)
     result = app(load_event("lambdaFunctionUrlEvent.json"), {})
     assert result["body"] == "routed"
+
+
+@pytest.mark.parametrize(
+    "router,resolver,event_file",
+    [
+        (ALBRouter, ALBResolver, "albEvent.json"),
+        (APIGatewayRouter, APIGatewayRestResolver, "apiGatewayProxyEvent.json"),
+        (APIGatewayHttpRouter, APIGatewayHttpResolver, "apiGatewayProxyV2Event_GET.json"),
+        (LambdaFunctionUrlRouter, LambdaFunctionUrlResolver, "lambdaFunctionUrlEvent.json"),
+    ],
+)
+def test_path_parameters_in_context(
+    router: Router,
+    resolver: ApiGatewayResolver,
+    event_file: str,
+) -> None:
+    app = resolver(enable_validation=True)
+    router = router()
+    path_params = {
+        "str_param": "str_value",
+        "int_param": 3,
+    }
+
+    def bar(app: APIGatewayRestResolver, next_middleware: NextMiddleware) -> Response[str]:
+        assert app.context.path_parameters == path_params
+        return next_middleware(app)
+
+    @router.route(rule="/<str_param>/<int_param>", method=["GET"], middlewares=[bar])
+    def foo(str_param: Annotated[str, Path()], int_param: Annotated[int, Path()]) -> Response[str]:
+        return Response(status_code=200, body="routed")
+
+    app.include_router(router)
+    event = load_event(event_file)
+    event["path"] = event["rawPath"] = f"/{path_params['str_param']}/{path_params['int_param']}"
+    result = app(event, {})
+    assert result["body"] == "routed"
+
+
+@pytest.mark.parametrize(
+    "router,resolver,event_file",
+    [
+        (ALBRouter, ALBResolver, "albEvent.json"),
+        (APIGatewayRouter, APIGatewayRestResolver, "apiGatewayProxyEvent.json"),
+        (APIGatewayHttpRouter, APIGatewayHttpResolver, "apiGatewayProxyV2Event_GET.json"),
+        (LambdaFunctionUrlRouter, LambdaFunctionUrlResolver, "lambdaFunctionUrlEvent.json"),
+    ],
+)
+def test_path_parameters_static_path(
+    router: Router,
+    resolver: ApiGatewayResolver,
+    event_file: str,
+) -> None:
+    app = resolver(enable_validation=True)
+    router = router()
+
+    def bar(app: APIGatewayRestResolver, next_middleware: NextMiddleware) -> Response[str]:
+        assert app.context.path_parameters == {}
+        return next_middleware(app)
+
+    @router.route(rule="/static", method=["GET"], middlewares=[bar])
+    def foo() -> Response[str]:
+        return Response(status_code=200, body="routed")
+
+    app.include_router(router)
+    event = load_event(event_file)
+    event["path"] = event["rawPath"] = "/static"
+    result = app(event, {})
+    assert result["body"] == "routed"
heitorlessa commented 3 months ago

Hey Kim, did you face any issues accessing via the app.current_event within the middleware?

The app instance is the same for both middlewares and routes by design.

PS: I’m reading on my phone but will check tomorrow on my laptop more attentively in case I missed anything.

On Sun, 23 Jun 2024 at 12:35, Kim Säppi @.***> wrote:

Here's one solution for implementation (I can make a PR if this is deemed a valuable suggestion, tips for how to document it welcome):

diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index 7c4d6769..4cbd2bd8 100644--- a/aws_lambda_powertools/event_handler/api_gateway.py+++ b/aws_lambda_powertools/event_handler/api_gateway.py@@ -423,7 +423,7 @@ class Route: print("=================")

     # Add Route Arguments to app context-        app.append_context(_route_args=route_arguments)+        app.context["_route_args"] = route_arguments

     # Call the Middleware Wrapped _call_stack function handler with the app
     return self._middleware_stack(app)@@ -898,10 +898,17 @@ class ResponseBuilder(Generic[ResponseEventT]):
     }

+class RouterContext(dict):+ @property+ def path_parameters(self) -> Dict[str, Any]:+ """Get path parameters for the current request."""+ return self.get("_route_args", {})++ class BaseRouter(ABC): current_event: BaseProxyEvent lambda_context: LambdaContext- context: dict+ context: RouterContext _router_middlewares: List[Callable] = [] processed_stack_frames: List[str] = [] @@ -1317,6 +1324,11 @@ class BaseRouter(ABC):

 def append_context(self, **additional_context):
     """Append key=value data as routing context"""+        if "_route_args" in additional_context:+            warnings.warn(+                "The '_route_args' context key is used internally by powertools and should not be overwrit

ten",+ stacklevel=2,+ ) self.context.update(**additional_context)

 def clear_context(self):@@ -1421,7 +1433,7 @@ def _registered_api_adapter(
     The API Response Object

 """-    route_args: Dict = app.context.get("_route_args", {})+    route_args: Dict = app.context.path_parameters
 logger.debug(f"Calling API Route Handler: {route_args}")
 return app._to_response(next_middleware(**route_args))

@@ -1494,7 +1506,7 @@ class ApiGatewayResolver(BaseRouter): self._debug = self._has_debug(debug) self._enable_validation = enable_validation self._strip_prefixes = strip_prefixes- self.context: Dict = {} # early init as customers might add context before event resolution+ self.context = RouterContext() # early init as customers might add context before event resolutio n self.processed_stack_frames = [] self._response_builder_class = ResponseBuilder[BaseProxyEvent] @@ -2453,7 +2465,7 @@ class Router(BaseRouter): self._routes: Dict[tuple, Callable] = {} self._routes_with_middleware: Dict[tuple, List[Callable]] = {} self.api_resolver: Optional[BaseRouter] = None- self.context = {} # early init as customers might add context before event resolution+ self.context = RouterContext() # early init as customers might add context before event resolutio n self._exception_handlers: Dict[Type, Callable] = {}

 def route(diff --git a/tests/functional/event_handler/required_dependencies/test_router.py b/tests/functional/event_h

andler/required_dependencies/test_router.py index d96f5035..b6111017 100644--- a/tests/functional/event_handler/required_dependencies/test_router.py+++ b/tests/functional/event_handler/required_dependencies/test_router.py@@ -1,16 +1,23 @@+import pytest+ from aws_lambda_powertools.event_handler import ( ALBResolver, APIGatewayHttpResolver,+ ApiGatewayResolver, APIGatewayRestResolver, LambdaFunctionUrlResolver, Response, )+from aws_lambda_powertools.event_handler.middlewares import NextMiddleware+from aws_lambda_powertools.event_handler.openapi.params import Path from aws_lambda_powertools.event_handler.router import ( ALBRouter, APIGatewayHttpRouter, APIGatewayRouter, LambdaFunctionUrlRouter,+ Router, )+from aws_lambda_powertools.shared.types import Annotated from aws_lambda_powertools.utilities.data_classes import ( ALBEvent, APIGatewayProxyEvent,@@ -74,3 +81,71 @@ def test_lambda_function_url_router_event_type(): app.include_router(router) result = app(load_event("lambdaFunctionUrlEvent.json"), {}) assert result["body"] == @.(+ "router,resolver,event_file",+ [+ (ALBRouter, ALBResolver, "albEvent.json"),+ (APIGatewayRouter, APIGatewayRestResolver, "apiGatewayProxyEvent.json"),+ (APIGatewayHttpRouter, APIGatewayHttpResolver, "apiGatewayProxyV2Event_GET.json"),+ (LambdaFunctionUrlRouter, LambdaFunctionUrlResolver, "lambdaFunctionUrlEvent.json"),+ ],+)+def test_path_parameters_in_context(+ router: Router,+ resolver: ApiGatewayResolver,+ event_file: str,+) -> None:+ app = resolver(enable_validation=True)+ router = router()+ path_params = {+ "str_param": "str_value",+ "int_param": 3,+ }++ def bar(app: APIGatewayRestResolver, next_middleware: NextMiddleware) -> Response[str]:+ assert app.context.path_parameters == path_params+ return next_middleware(app)++ @router.route(rule="//", method=["GET"], middlewares=[bar])+ def foo(str_param: Annotated[str, Path()], int_param: Annotated[int, Path()]) -> Response[str]:+ return Response(status_code=200, body="routed")++ app.include_router(router)+ event = load_event(event_file)+ event["path"] = event["rawPath"] = f"/{path_params['str_param']}/{path_params['int_param']}"+ result = app(event, {})+ assert result["body"] == @.(+ "router,resolver,event_file",+ [+ (ALBRouter, ALBResolver, "albEvent.json"),+ (APIGatewayRouter, APIGatewayRestResolver, "apiGatewayProxyEvent.json"),+ (APIGatewayHttpRouter, APIGatewayHttpResolver, "apiGatewayProxyV2Event_GET.json"),+ (LambdaFunctionUrlRouter, LambdaFunctionUrlResolver, "lambdaFunctionUrlEvent.json"),+ ],+)+def test_path_parameters_static_path(+ router: Router,+ resolver: ApiGatewayResolver,+ event_file: str,+) -> None:+ app = resolver(enable_validation=True)+ router = router()++ def bar(app: APIGatewayRestResolver, next_middleware: NextMiddleware) -> Response[str]:+ assert app.context.path_parameters == {}+ return next_middleware(app)++ @router.route(rule="/static", method=["GET"], middlewares=[bar])+ def foo() -> Response[str]:+ return Response(status_code=200, body="routed")++ app.include_router(router)+ event = load_event(event_file)+ event["path"] = event["rawPath"] = "/static"+ result = app(event, {})+ assert result["body"] == "routed"

— Reply to this email directly, view it on GitHub https://github.com/aws-powertools/powertools-lambda-python/issues/4609#issuecomment-2184937310, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZPQBCPW6VPWQM7EXGAOILZI2QGJAVCNFSM6AAAAABJYH5IB6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBUHEZTOMZRGA . You are receiving this because you are subscribed to this thread.Message ID: @.*** .com>

kimsappi commented 3 months ago

Hey Kim, did you face any issues accessing via the app.current_event within the middleware? The app instance is the same for both middlewares and routes by design.

No, there is no issue with that. The problem is that I would like to have access to the parsed path parameters in middlewares. This is available in app.current_event for API Gateway, where it's a native feature of API Gateway and part of the Lambda event ("pathParameters"): https://docs.aws.amazon.com/lambda/latest/dg/services-apigateway.html#apigateway-example-event

However, other resolvers like ALB don't have this luxury since routing is handled exclusively with ALBResolver and path parameters aren't part of the Lambda event: https://docs.aws.amazon.com/lambda/latest/dg/services-alb.html

In a route, path parameters can be accessed like this:

@router.route(rule="/<str_param>/<int_param>", method=["GET"], middlewares=[bar])
def foo(str_param: Annotated[str, Path()], int_param: Annotated[int, Path()]) -> Response[str]:
    return Response(status_code=200, body=f"{str_param}/{int_param}")

But the middleware bar would, as far as I'm aware, need to parse them from the raw path or use the undocumented app.context["_route_args"] (unless using an API Gateway resolver).

heitorlessa commented 3 months ago

Gotcha! It sounds like a stable albeit longer solution would be to create a standardised Request object (I remember there’s an issue for that).

Similar to Starlette, this would also help rely less on the event structure and do it the legwork for customers.

We’re making a release this Thursday but if you’d like to help speed this up in July perhaps, we’d love help drafting a Request object to expose path, query string (and multi-value), headers (and multi-value), cookies, etc.

On Sun, 23 Jun 2024 at 18:58, Kim Säppi @.***> wrote:

Hey Kim, did you face any issues accessing via the app.current_event within the middleware? The app instance is the same for both middlewares and routes by design.

No, there is no issue with that. The problem is that I would like to have access to the parsed path parameters in middlewares. This is available in app.current_event for API Gateway, where it's a native feature of API Gateway and part of the Lambda event ("pathParameters"): https://docs.aws.amazon.com/lambda/latest/dg/services-apigateway.html#apigateway-example-event

However, other resolvers like ALB don't have this luxury since routing is handled exclusively with ALBResolver and path parameters aren't part of the Lambda event: https://docs.aws.amazon.com/lambda/latest/dg/services-alb.html

In a route, path parameters can be accessed like this:

@router.route(rule="//", method=["GET"], middlewares=[bar])def foo(str_param: Annotated[str, Path()], int_param: Annotated[int, Path()]) -> Response[str]: return Response(status_code=200, body=f"{str_param}/{int_param}")

But the middleware bar would, as far as I'm aware, need to parse them from the raw path or use the undocumented app.context["_route_args"] (unless using an API Gateway resolver).

— Reply to this email directly, view it on GitHub https://github.com/aws-powertools/powertools-lambda-python/issues/4609#issuecomment-2185153117, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZPQBHM2RZP6JSEJJGC3H3ZI35ERAVCNFSM6AAAAABJYH5IB6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBVGE2TGMJRG4 . You are receiving this because you commented.Message ID: @.*** com>

kimsappi commented 3 months ago

I think that sounds very good. I wasn't able to find the issue right now, but I'll keep an eye out. Perhaps it doesn't make sense to implement a temporary solution if a better solution is being planned. Maybe it's best to close this ticket?

heitorlessa commented 3 months ago

Turns out the issue I mentioned was the precursor to have our new Data Validation feature (#1955) - we inject request Payloads, Headers, Query String, Path parameters, etc. inferring from type annotation.

We should keep this issue altogether. Unless @leandrodamascena disagrees, I think we should add this feature into the data validation middleware only to not confuse customers with the DX. If we see a Request model, we inject it.

If we stick by WSGI "standard", we can keep it simple like Starlette while handling multi-value query string and multi-value headers.

leandrodamascena commented 2 months ago

We can discuss this when you get back from the PTO @heitorlessa!