Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(parser): Make ApiGateway version, authorizer fields optional #532

Merged
merged 1 commit into from
Jul 17, 2021
Merged

fix(parser): Make ApiGateway version, authorizer fields optional #532

merged 1 commit into from
Jul 17, 2021

Conversation

walmsles
Copy link
Contributor

@walmsles walmsles commented Jul 17, 2021

**Issue #531 **

Description of changes:

Change to APIGatewayProxyEventModel to make "version" Optional
Change to APIGatewayEventRequestContext to make "authorizer" Optional

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

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

@boring-cyborg
Copy link

boring-cyborg bot commented Jul 17, 2021

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.

@heitorlessa
Copy link
Contributor

Thank you so much @walmsles !

Question: What's the type of API Gateway integration that makes this necessary?

In other words, how did you hit this issue?

@walmsles
Copy link
Contributor Author

walmsles commented Jul 17, 2021

@heitorlessa I created a simple API using LAMBDA_PROXY integration that was very basic - no authentication just default setup for a quick POC test playing around with powertools and parser to see how it works, behaves.

I have also come across this problem setting up sample events using:

sam local generate-event apigateway aws-proxy

I used serverless framework to push the API up to my AWS account.

@heitorlessa heitorlessa self-assigned this Jul 17, 2021
@heitorlessa
Copy link
Contributor

Thanks a lot again @walmsles - Merging this now.

I've just tried deploying an API GW and I don't see those fields in the request too, so either we used an incorrect event or this changed -- hard to know the latter as there isn't an official schema yet.

cc @risenberg-cyberark -- I'll create an issue to create infrastructure for integ test so we can prevent this from happening in the future.

@codecov-commenter
Copy link

codecov-commenter commented Jul 17, 2021

Codecov Report

Merging #532 (17af7af) into develop (5b87bb1) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #532   +/-   ##
========================================
  Coverage    99.23%   99.23%           
========================================
  Files          113      113           
  Lines         4468     4468           
  Branches       243      243           
========================================
  Hits          4434     4434           
  Misses          22       22           
  Partials        12       12           
Impacted Files Coverage Δ
...lambda_powertools/utilities/parser/models/apigw.py 97.40% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b87bb1...17af7af. Read the comment docs.

@heitorlessa heitorlessa added this to the 1.18.0 milestone Jul 17, 2021
@heitorlessa heitorlessa changed the title fix(parser): Make version, requestContext.authorizer Optional for ApiGatewayProxyModel fix(parser): Make ApiGateway version, authorizer fields optional Jul 17, 2021
@heitorlessa heitorlessa merged commit 89337a2 into aws-powertools:develop Jul 17, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented Jul 17, 2021

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

heitorlessa added a commit to heitorlessa/aws-lambda-powertools-python that referenced this pull request Jul 17, 2021
* develop:
  chore(deps): bump boto3 from 1.18.0 to 1.18.1 (aws-powertools#528)
  fix(tracer): mypy generic to preserve decorated method signature (aws-powertools#529)
  fix(parser): Make ApiGateway version, authorizer fields optional (aws-powertools#532)
  fix(mypy): fixes to resolve no implicit optional errors (aws-powertools#521)
  chore(deps): bump boto3 from 1.17.110 to 1.18.0 (aws-powertools#527)
  feat(feat-toggle): New simple feature toggles rule engine (WIP) (aws-powertools#494)
  chore(deps-dev): bump mkdocs-material from 7.1.9 to 7.1.10 (aws-powertools#522)
  chore(deps): bump boto3 from 1.17.102 to 1.17.110 (aws-powertools#523)
  chore(deps-dev): bump isort from 5.9.1 to 5.9.2 (aws-powertools#514)
  feat(mypy): add mypy support to makefile (aws-powertools#508)
  feat(api-gateway): add debug mode (aws-powertools#507)
heitorlessa added a commit to whardier/aws-lambda-powertools-python that referenced this pull request Jul 19, 2021
…ent-subclass

* develop:
  fix(api-gateway): non-greedy route pattern regex (aws-powertools#533)
  chore(deps): bump boto3 from 1.18.0 to 1.18.1 (aws-powertools#528)
  fix(tracer): mypy generic to preserve decorated method signature (aws-powertools#529)
  fix(parser): Make ApiGateway version, authorizer fields optional (aws-powertools#532)
  fix(mypy): fixes to resolve no implicit optional errors (aws-powertools#521)
  chore(deps): bump boto3 from 1.17.110 to 1.18.0 (aws-powertools#527)
@heitorlessa heitorlessa added the bug Something isn't working label Jul 20, 2021
@walmsles walmsles deleted the fix/make-version-authorizer-optional-for-apigw-proxy-model branch July 30, 2021 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants