Skip to content

Conversation

Jacco
Copy link
Contributor

@Jacco Jacco commented Aug 6, 2019

Issue #229, if available:

Description of changes:

Added support for Cognito event sources
It adds only one Permission per Lambda (logical_id de-dupe)
It changes to LambdaConfig property of the Cognito UserPool it refers to

Description of how you validated changes:

Updated the example "examples/2016-10-31/api_cognito_auth"
Transformed and created stack in my account
Checked is PreSignUp trigger correctly refers to the Lambda

Checklist:

  • Write/update tests
  • make pr passes
  • Update documentation
  • Verify transformed template deploys and application functions as expected
  • Add/update example to examples/2016-10-31

other todos:

  • Check is also "User Migration" can be set this way

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Jacco Kulman added 5 commits August 6, 2019 12:53
- test script fixed
- test added few cases to cognito_userpool_with_event
- some make pr fixes
- only one permission needs to be created
- allow string or array for Trigger
@codecov-io
Copy link

codecov-io commented Aug 6, 2019

Codecov Report

Merging #1066 into develop will decrease coverage by 0.21%.
The diff coverage is 81.48%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1066      +/-   ##
===========================================
- Coverage    94.81%   94.59%   -0.22%     
===========================================
  Files           69       70       +1     
  Lines         3276     3328      +52     
  Branches       639      651      +12     
===========================================
+ Hits          3106     3148      +42     
- Misses          89       93       +4     
- Partials        81       87       +6
Impacted Files Coverage Δ
samtranslator/translator/verify_logical_id.py 100% <ø> (ø) ⬆️
samtranslator/model/eventsources/push.py 87.53% <80.85%> (-1.08%) ⬇️
samtranslator/model/cognito.py 85.71% <85.71%> (ø)
...lator/plugins/application/serverless_app_plugin.py 83.44% <0%> (ø) ⬆️

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 5090228...97f12db. Read the comment docs.

@Jacco Jacco changed the title Cognito event sources #229 feat: Cognito event sources #229 Aug 6, 2019
Copy link
Contributor

@keetonian keetonian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all, this looks good to me other than the one nit in the docs. I'm going to deploy the updated example to make sure it works as expected and read up on this feature to make sure this is all correct before we merge it in. Thank you for this contribution!

@jlhood jlhood requested a review from keetonian August 13, 2019 17:24
Copy link
Contributor

@keetonian keetonian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran the example and verified that it works as expected. Thank you for this feature!

@praneetap praneetap self-assigned this Aug 16, 2019
@praneetap
Copy link
Contributor

Thank you for submitting this feature! :)

@praneetap praneetap merged commit c71f404 into aws:develop Aug 19, 2019
@praneetap praneetap mentioned this pull request Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants