feat(router): Ignore trailing slashes in route resolvers inherited from APIGatewayResolver by walmsles · Pull Request #1577 · aws-powertools/powertools-lambda-python

walmsles · 2022-10-09T04:17:39Z

Issue number: #1552

Summary

Ingore trailing slash for routers inheriting from APIGatewayResolver

Changes

AWS API Gateway routes the following 2 paths in the same way "/my/path", "/my/path/", i.e. when routing "/my/path/" it routes exactly as if "/my/path" had been called. The Lambda Powertools router currently treats these as 2 distinct paths so "/my/path/" returns HTTP statusCode of 404.

User experience

No change to user experience. Users will create routes as normal. In the given sample below making API gateway call to path "/my/path/" will correctly ensure the "get_lambda" route function is correctly called even though it is defined as "/my/path"

app = APIGatewayRestResolver()

    @app.get("/my/path")
    def get_lambda() -> Response:
        assert isinstance(app.current_event, APIGatewayProxyEvent)
        assert app.lambda_context == {}
        assert app.current_event.request_context.domain_name == "id.execute-api.us-east-1.amazonaws.com"
        return Response(200, content_types.APPLICATION_JSON, json.dumps({"foo": "value"}))

The code below will work in the same way when called using path "/my/path", "/my/path/"

app = APIGatewayRestResolver()

    @app.get("/my/path/")
    def get_lambda() -> Response:
        assert isinstance(app.current_event, APIGatewayProxyEvent)
        assert app.lambda_context == {}
        assert app.current_event.request_context.domain_name == "id.execute-api.us-east-1.amazonaws.com"
        return Response(200, content_types.APPLICATION_JSON, json.dumps({"foo": "value"}))

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Meet tenets criteria
I have performed a self-review of this change
Changes have been tested
Changes are documented
PR title follows conventional commit semantics

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

View rendered docs/core/event_handler/api_gateway.md

walmsles · 2022-10-09T04:19:33Z

Note: I started working on E2E tests - these are unfinished and will take some more time to complete.

@heitorlessa you mentioned ALB E2E tests already being defined - I was not able to find can you share a link to the test?

heitorlessa · 2022-10-10T05:36:17Z

Note: I started working on E2E tests - these are unfinished and will take some more time to complete.

@heitorlessa you mentioned ALB E2E tests already being defined - I was not able to find can you share a link to the test?

My bad I forgot to mention it's in the V2 branch: https://github.com/awslabs/aws-lambda-powertools-python/blob/v2/tests/e2e/event_handler/test_header_serializer.py#L39

This was for a breaking change feature @rubenfonseca was doing, but we didn't merge the tests (and optimizations) to develop because of V2 proximity -- Would you mind changing the base to v2 as we're close to launch?

There's only one major item pending for V2 (Serverless Application Repository) and we should be good to go.

codecov-commenter · 2022-10-10T05:37:01Z

Codecov Report

Base: 99.74% // Head: 99.74% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (eac4c1a) compared to base (a5755d4).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #1577   +/-   ##
========================================
  Coverage    99.74%   99.74%           
========================================
  Files          124      124           
  Lines         5784     5785    +1     
  Branches       658      658           
========================================
+ Hits          5769     5770    +1     
  Misses           8        8           
  Partials         7        7

Impacted Files	Coverage Δ
aws_lambda_powertools/event_handler/api_gateway.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

walmsles · 2022-10-10T07:52:04Z

My bad I forgot to mention it's in the V2 branch: https://github.com/awslabs/aws-lambda-powertools-python/blob/v2/tests/e2e/event_handler/test_header_serializer.py#L39

This was for a breaking change feature @rubenfonseca was doing, but we didn't merge the tests (and optimizations) to develop because of V2 proximity -- Would you mind changing the base to v2 as we're close to launch?

Ahhh, that makes sense now - nice - it's all in place, so can add my tests pretty quickly - let me move it across.

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

… reference (#1589)

Bumps [mypy-boto3-ssm](https://github.com/youtype/mypy_boto3_builder) from 1.24.81 to 1.24.90. - [Release notes](https://github.com/youtype/mypy_boto3_builder/releases) - [Commits](https://github.com/youtype/mypy_boto3_builder/commits) --- updated-dependencies: - dependency-name: mypy-boto3-ssm dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

walmsles · 2022-10-15T12:29:04Z

Hope my ramblings below make sense, the TL;DR version goes like this:

I have decided to restrict the implementation of this feature to REST API Gateway (V1) since this is the default behaviour of this gateway implementation for a defined route i.e. both "/hello" and "/hello/" resolve to the same lambda integration. I will write E2E tests covering all the API behaviours around this feature so if anything changes it will be picked up.

The E2E tests are amazing! I have been playing with APIs in various guises today (Much appreciation to the person responsible for writing those!) and have come to some conclusions:

The only use-case this feature applies to IS the REST API (V1) Implementation - which works as per my original description
ALB and Lambda URL - all routing is deferred to the actual Lambda, so we should be faithful to RFC 3986 on Path. so as not to close off possible customer use cases.
HTTP API (V2) - actually does not allow the creation of a route with a trailing "/" character and behaves by sending back a "404" response when an invalid path is requested (unlike V1 which is a default 403 - forbidden). If you setup a default route then the HTTP API will behave in similar fashion to ALB and Lambda URL - so again RFC 3986 should apply.

The code in this PR is not up to date as yet since have switched my feature branch to be from V2. Will hopefully push changes tomorrow.

walmsles · 2022-10-16T11:26:51Z

@heitorlessa - This PR is a little mucked up (my fault) - will re-issue a new PR based on V2 and my feature branch, which is required to fix this up.

Have raised new PR - #1609

heitorlessa · 2022-10-17T09:10:29Z

Closing in favour of #1609

walmsles requested a review from a team as a code owner October 9, 2022 04:17

walmsles requested review from heitorlessa and removed request for a team October 9, 2022 04:17

boring-cyborg bot added area/event_handlers documentation Improvements or additions to documentation tests labels Oct 9, 2022

pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 9, 2022

github-actions bot added the feature New feature or functionality label Oct 10, 2022

dependabot bot and others added 16 commits October 11, 2022 12:02

chore(deps-dev): bump flake8-builtins from 1.5.3 to 2.0.0 (#1582)

5d5b4b8

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

docs(homepage): include .NET powertools

154df7c

update changelog with latest changes

73865b3

docs(logger): fix typo. (#1587)

ea28f04

update changelog with latest changes

263ba7b

docs(governance): new form to allow customers self-nominate as public…

5d755a7

… reference (#1589)

docs(idempotency) - Update invalid link target. (#1588)

99d53a7

update changelog with latest changes

0b9e2d1

docs(idempotency): "persisntence" typo (#1596)

74cf2cc

update changelog with latest changes

7dbc6da

docs(governance): allow community to suggest feature content (#1593)

e2d4744

fix(parser): loose validation on SNS fields to support FIFO (#1606)

abb8043

update changelog with latest changes

fb59763

chore(layer): bump to 1.31.1 (v39)

c98e03b

update changelog with latest changes

ec709c3

walmsles changed the base branch from develop to v2 October 16, 2022 11:09

boring-cyborg bot added dependencies Pull requests that update a dependency file github-templates internal Maintenance changes labels Oct 16, 2022

walmsles mentioned this pull request Oct 16, 2022

feat(apigateway): ignore trailing slashes in routes (APIGatewayRestResolver) #1609

Merged

5 tasks

heitorlessa closed this Oct 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(router): Ignore trailing slashes in route resolvers inherited from APIGatewayResolver#1577

feat(router): Ignore trailing slashes in route resolvers inherited from APIGatewayResolver#1577
walmsles wants to merge 16 commits intoaws-powertools:v2from
walmsles:develop

walmsles commented Oct 9, 2022 •

edited by markdownify bot

Loading

Uh oh!

walmsles commented Oct 9, 2022

Uh oh!

heitorlessa commented Oct 10, 2022

Uh oh!

codecov-commenter commented Oct 10, 2022 •

edited

Loading

Uh oh!

walmsles commented Oct 10, 2022

Uh oh!

walmsles commented Oct 15, 2022

Uh oh!

walmsles commented Oct 16, 2022

Uh oh!

heitorlessa commented Oct 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

walmsles commented Oct 9, 2022 • edited by markdownify bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

User experience

Checklist

Acknowledgment

Uh oh!

walmsles commented Oct 9, 2022

Uh oh!

heitorlessa commented Oct 10, 2022

Uh oh!

codecov-commenter commented Oct 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

walmsles commented Oct 10, 2022

Uh oh!

walmsles commented Oct 15, 2022

Uh oh!

walmsles commented Oct 16, 2022

Uh oh!

heitorlessa commented Oct 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

walmsles commented Oct 9, 2022 •

edited by markdownify bot

Loading

codecov-commenter commented Oct 10, 2022 •

edited

Loading