fix: Validate Trigger field and test Cognito properties with intrinsic functions #2092

mildaniel · 2021-07-16T01:08:00Z

Issue #, if available:

Description of changes:
SAM transform was allowing invalid values and types for Cognito Trigger to be transformed, resulting in invalid templates that won't deploy.

Checklist:

Write/update tests
make pr passes
Verify transformed template deploys and application functions as expected

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

codecov-commenter · 2021-07-16T23:25:43Z

Codecov Report

Merging #2092 (255c9e1) into develop (e7a1496) will increase coverage by 0.51%.
The diff coverage is 98.36%.

@@             Coverage Diff             @@
##           develop    #2092      +/-   ##
===========================================
+ Coverage    93.58%   94.09%   +0.51%     
===========================================
  Files           90       92       +2     
  Lines         6124     6283     +159     
  Branches      1260     1282      +22     
===========================================
+ Hits          5731     5912     +181     
+ Misses         183      170      -13     
+ Partials       210      201       -9

Impacted Files	Coverage Δ
samtranslator/model/apigateway.py	`97.15% <ø> (ø)`
samtranslator/swagger/swagger.py	`93.66% <88.88%> (+0.29%)`	⬆️
samtranslator/__init__.py	`100.00% <100.00%> (ø)`
samtranslator/feature_toggle/dialup.py	`100.00% <100.00%> (ø)`
samtranslator/feature_toggle/feature_toggle.py	`100.00% <100.00%> (+12.16%)`	⬆️
samtranslator/metrics/metrics.py	`100.00% <100.00%> (ø)`
samtranslator/model/api/api_generator.py	`94.39% <100.00%> (+0.03%)`	⬆️
samtranslator/model/eventsources/pull.py	`84.73% <100.00%> (+6.04%)`	⬆️
samtranslator/model/eventsources/push.py	`91.75% <100.00%> (+0.82%)`	⬆️
samtranslator/model/sam_resources.py	`91.26% <100.00%> (+0.03%)`	⬆️
... and 8 more

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 8f24e41...255c9e1. Read the comment docs.

samtranslator/model/eventsources/push.py

mgrandis

Let's see if we can use the PropertyType mechanism to exclude intrinsics instead.

…c functions (aws#2092) * Add headers whenever cors is set * Fix Cognito trigger validation * Make templates deployable * Removed trigger value validation * Fix Python2 string matching test issue * Use PropertyType mechanism to exclude intrinsics * Remove unused import

* chore: don't install integration tests (#1964) * Remove unnecessary use of comprehension (#1805) * fix: Grammatical error in README.md (#1965) * fix: Added SAR Support Check (#1972) * Added SAR Support Check * Added docstring and Removed Instance Initialization for Class Method * update pyyaml version to get the security update (#1974) * fix the request parameter parsing, the value can contain dots (#1975) * fix the request parameter parsing, the value can contain dots * fix the unit test for python 2.7 * use built in split, instead of concatenating the string * Documentation: fix incorrect header (#1979) Fixed the incorrectly formatted header for HTTP API section * fix: mutable default values in method definitions (#1997) * fix: remove explicit logging level set in single module (#1998) * fix: Fail when Intrinsics are in SourceVPC list instead of IntrinsicsSourceVPC (#1999) * feat: Adding support for metric publishing. (#2062) * Adding support for metric publishing, manually tested and tests pending. * Adding make action for running test with html coverage. * Adding unit tests for metrics. * Writing integration tests for metrics publishing. * Black reformatting * Fixing unit test. * Addressing PR comments. * Clearing __init__py from test module. * Updating documentation for publish method. Co-authored-by: Tarun Mall <tarun@amazon.noreply.github.com> * Update AppConfig boto3 client config to shorten timeout (#2070) * Update AppConfig boto3 client config to shorten timeout * Update timeout config of AppConfig boto3 client * feat: Add ValidateBody/ValidateParameters attribute to API models (#2026) * Added validators to the swagger definitions when a model is required * Remove approach of trying an extra field for validators * WIP - tests deps not working * Finished and fixed all tests to make pr command was complaining. Fixed case where a validator could have a / only and changed it to translate as 'root' * py2.7 complaint * Fix py2.7 order of validators on output folder * WIP - adding validator fields * Set another strategy to set the validators without affect existent clients with model required * Set another strategy to set the validators without affect existent clients with model required * Remove mistaken file pushed * Remove mistaken file pushed * Partial coments fixes * Implemented case where we can set a only limited combinations on the specs * Fix case test when multiple path are using same validator * Added openapi3 specific tests * Added openapi3 specific tests * Fix requested changes from review: Renamed from ValidateRequest -> ValidateParameters also renamed everything from validator_request. Moved validators names according with the comum use. * new approach to not cause KeyErrors * Setting validator to the only given method not all methods on the path * removed extra space on comment - lint * Normalized method name path that comes from the template Co-authored-by: Rondineli Gomes de Araujo <rgomesdearaujo@rgomesdearaujo-mbp.ad.shuttercorp.net> Co-authored-by: Rondineli <rondineli.gomes.araujo@gmail.com> Co-authored-by: Rondineli <rgomesdearaujo@yahoo.com.br> * fix: fail with invalid resource, when RetentionPolicy has unresolved intrinsic value (#2074) * fix: fail with invalid resource, when RetentionPolicy has unresolved intrinsic value * make black * remove extra formatting * add test case for !ref * test: Migration of combination integration tests (#1970) * fix: Compare integration tests results using hash of file content instead of image compare (#2086) * change yaml.load to yaml.safe_load for the security best practice * use yaml_parse for consistant style * remove pillow library for image comparing, use hash instead * make it compatible with py2 * Remove logging of SAR service call response (#2064) * Test CloudWatchEvent Properties With Intrinsic Functions (#2090) * Add headers whenever cors is set * Add cw event success cases * Replace GetAtt with condition * Cleanup test yaml * Cleanup test yaml * Remove files * Add intrinsic to pattern field * Fix wrong files committed * test: Added intrinsic tests for Function SNS Event (#2101) * fix: Validate Trigger field and test Cognito properties with intrinsic functions (#2092) * Add headers whenever cors is set * Fix Cognito trigger validation * Make templates deployable * Removed trigger value validation * Fix Python2 string matching test issue * Use PropertyType mechanism to exclude intrinsics * Remove unused import * feat(Intrinsic Tests): Added Tests for Kinesis Event Source with Intrinsics (#2103) * Added Tests for Kinesis EventSource with Intrinsics * Fixed Formatting with Black * fix: Don't attempt to refetch a swagger method object by its name as we already have it (#2031) * Fix a bad logic where a hash key is modified then refetched which results in KeyError * add unit tests and update other instance of same usage on line 534 * fix: Fix the same code in open_api.py Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Mathieu Grandis <grmathie@amazon.com> * test: Adding intrinsic tests for Function S3 Event (#2100) * fix(bug): Check if Identity.ReauthorizeEvery equals zero (#2105) * Revert "Revert "Issue 1508 remove check requiring identity ... (#1577)" (#2038)" This reverts commit ed3c283. * Update implementation to support intrinsics/ add more tests to validate changes * Fixing hashes for py2 * Run make black * Handle pr feedback * Add another unit tests to cover the original issue Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> * chore: Update PR Checklist to include writing intrinsic tests (#2106) * Update PR Checklist to include writing intrinsic tests * Handle pr feedback Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> * Delete .travis.yml (#2102) * fix: Correct grammar in the waiting for changeset message (#2027) * chore: bump version to 1.39.0 (#2128) Co-authored-by: Chih-Hsuan Yen <yan12125@gmail.com> Co-authored-by: Harsh Mishra <erbeusgriffincasper@gmail.com> Co-authored-by: Pranav <54665036+Pranav016@users.noreply.github.com> Co-authored-by: Cosh_ <CoshUS@users.noreply.github.com> Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Co-authored-by: Ben <freiberg.ben@gmail.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com> Co-authored-by: Tarun <c2tarun@users.noreply.github.com> Co-authored-by: Tarun Mall <tarun@amazon.noreply.github.com> Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com> Co-authored-by: Rondineli <Rondineli@users.noreply.github.com> Co-authored-by: Rondineli Gomes de Araujo <rgomesdearaujo@rgomesdearaujo-mbp.ad.shuttercorp.net> Co-authored-by: Rondineli <rondineli.gomes.araujo@gmail.com> Co-authored-by: Rondineli <rgomesdearaujo@yahoo.com.br> Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com> Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Co-authored-by: Ahmed Elbayaa <72949274+elbayaaa@users.noreply.github.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> Co-authored-by: Ichinose Shogo <shogo82148@gmail.com> Co-authored-by: Ryan Parman <ryan@ryanparman.com>

* Release/v1.39.0 (#2141) * chore: don't install integration tests (#1964) * Remove unnecessary use of comprehension (#1805) * fix: Grammatical error in README.md (#1965) * fix: Added SAR Support Check (#1972) * Added SAR Support Check * Added docstring and Removed Instance Initialization for Class Method * update pyyaml version to get the security update (#1974) * fix the request parameter parsing, the value can contain dots (#1975) * fix the request parameter parsing, the value can contain dots * fix the unit test for python 2.7 * use built in split, instead of concatenating the string * Documentation: fix incorrect header (#1979) Fixed the incorrectly formatted header for HTTP API section * fix: mutable default values in method definitions (#1997) * fix: remove explicit logging level set in single module (#1998) * fix: Fail when Intrinsics are in SourceVPC list instead of IntrinsicsSourceVPC (#1999) * feat: Adding support for metric publishing. (#2062) * Adding support for metric publishing, manually tested and tests pending. * Adding make action for running test with html coverage. * Adding unit tests for metrics. * Writing integration tests for metrics publishing. * Black reformatting * Fixing unit test. * Addressing PR comments. * Clearing __init__py from test module. * Updating documentation for publish method. Co-authored-by: Tarun Mall <tarun@amazon.noreply.github.com> * Update AppConfig boto3 client config to shorten timeout (#2070) * Update AppConfig boto3 client config to shorten timeout * Update timeout config of AppConfig boto3 client * feat: Add ValidateBody/ValidateParameters attribute to API models (#2026) * Added validators to the swagger definitions when a model is required * Remove approach of trying an extra field for validators * WIP - tests deps not working * Finished and fixed all tests to make pr command was complaining. Fixed case where a validator could have a / only and changed it to translate as 'root' * py2.7 complaint * Fix py2.7 order of validators on output folder * WIP - adding validator fields * Set another strategy to set the validators without affect existent clients with model required * Set another strategy to set the validators without affect existent clients with model required * Remove mistaken file pushed * Remove mistaken file pushed * Partial coments fixes * Implemented case where we can set a only limited combinations on the specs * Fix case test when multiple path are using same validator * Added openapi3 specific tests * Added openapi3 specific tests * Fix requested changes from review: Renamed from ValidateRequest -> ValidateParameters also renamed everything from validator_request. Moved validators names according with the comum use. * new approach to not cause KeyErrors * Setting validator to the only given method not all methods on the path * removed extra space on comment - lint * Normalized method name path that comes from the template Co-authored-by: Rondineli Gomes de Araujo <rgomesdearaujo@rgomesdearaujo-mbp.ad.shuttercorp.net> Co-authored-by: Rondineli <rondineli.gomes.araujo@gmail.com> Co-authored-by: Rondineli <rgomesdearaujo@yahoo.com.br> * fix: fail with invalid resource, when RetentionPolicy has unresolved intrinsic value (#2074) * fix: fail with invalid resource, when RetentionPolicy has unresolved intrinsic value * make black * remove extra formatting * add test case for !ref * test: Migration of combination integration tests (#1970) * fix: Compare integration tests results using hash of file content instead of image compare (#2086) * change yaml.load to yaml.safe_load for the security best practice * use yaml_parse for consistant style * remove pillow library for image comparing, use hash instead * make it compatible with py2 * Remove logging of SAR service call response (#2064) * Test CloudWatchEvent Properties With Intrinsic Functions (#2090) * Add headers whenever cors is set * Add cw event success cases * Replace GetAtt with condition * Cleanup test yaml * Cleanup test yaml * Remove files * Add intrinsic to pattern field * Fix wrong files committed * test: Added intrinsic tests for Function SNS Event (#2101) * fix: Validate Trigger field and test Cognito properties with intrinsic functions (#2092) * Add headers whenever cors is set * Fix Cognito trigger validation * Make templates deployable * Removed trigger value validation * Fix Python2 string matching test issue * Use PropertyType mechanism to exclude intrinsics * Remove unused import * feat(Intrinsic Tests): Added Tests for Kinesis Event Source with Intrinsics (#2103) * Added Tests for Kinesis EventSource with Intrinsics * Fixed Formatting with Black * fix: Don't attempt to refetch a swagger method object by its name as we already have it (#2031) * Fix a bad logic where a hash key is modified then refetched which results in KeyError * add unit tests and update other instance of same usage on line 534 * fix: Fix the same code in open_api.py Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Mathieu Grandis <grmathie@amazon.com> * test: Adding intrinsic tests for Function S3 Event (#2100) * fix(bug): Check if Identity.ReauthorizeEvery equals zero (#2105) * Revert "Revert "Issue 1508 remove check requiring identity ... (#1577)" (#2038)" This reverts commit ed3c283. * Update implementation to support intrinsics/ add more tests to validate changes * Fixing hashes for py2 * Run make black * Handle pr feedback * Add another unit tests to cover the original issue Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> * chore: Update PR Checklist to include writing intrinsic tests (#2106) * Update PR Checklist to include writing intrinsic tests * Handle pr feedback Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> * Delete .travis.yml (#2102) * fix: Correct grammar in the waiting for changeset message (#2027) * chore: bump version to 1.39.0 (#2128) Co-authored-by: Chih-Hsuan Yen <yan12125@gmail.com> Co-authored-by: Harsh Mishra <erbeusgriffincasper@gmail.com> Co-authored-by: Pranav <54665036+Pranav016@users.noreply.github.com> Co-authored-by: Cosh_ <CoshUS@users.noreply.github.com> Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Co-authored-by: Ben <freiberg.ben@gmail.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com> Co-authored-by: Tarun <c2tarun@users.noreply.github.com> Co-authored-by: Tarun Mall <tarun@amazon.noreply.github.com> Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com> Co-authored-by: Rondineli <Rondineli@users.noreply.github.com> Co-authored-by: Rondineli Gomes de Araujo <rgomesdearaujo@rgomesdearaujo-mbp.ad.shuttercorp.net> Co-authored-by: Rondineli <rondineli.gomes.araujo@gmail.com> Co-authored-by: Rondineli <rgomesdearaujo@yahoo.com.br> Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com> Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Co-authored-by: Ahmed Elbayaa <72949274+elbayaaa@users.noreply.github.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> Co-authored-by: Ichinose Shogo <shogo82148@gmail.com> Co-authored-by: Ryan Parman <ryan@ryanparman.com> * feat: ARM support (#2163) * feat: ARM architecture support for Function and Layer * Add docs/tests for Globals Architectures (#85) Co-authored-by: Renato Valenzuela <37676028+valerena@users.noreply.github.com> Co-authored-by: Chih-Hsuan Yen <yan12125@gmail.com> Co-authored-by: Harsh Mishra <erbeusgriffincasper@gmail.com> Co-authored-by: Pranav <54665036+Pranav016@users.noreply.github.com> Co-authored-by: Cosh_ <CoshUS@users.noreply.github.com> Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Co-authored-by: Ben <freiberg.ben@gmail.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com> Co-authored-by: Tarun <c2tarun@users.noreply.github.com> Co-authored-by: Tarun Mall <tarun@amazon.noreply.github.com> Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com> Co-authored-by: Rondineli <Rondineli@users.noreply.github.com> Co-authored-by: Rondineli Gomes de Araujo <rgomesdearaujo@rgomesdearaujo-mbp.ad.shuttercorp.net> Co-authored-by: Rondineli <rondineli.gomes.araujo@gmail.com> Co-authored-by: Rondineli <rgomesdearaujo@yahoo.com.br> Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com> Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Co-authored-by: Ahmed Elbayaa <72949274+elbayaaa@users.noreply.github.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> Co-authored-by: Ichinose Shogo <shogo82148@gmail.com> Co-authored-by: Ryan Parman <ryan@ryanparman.com> Co-authored-by: Renato Valenzuela <37676028+valerena@users.noreply.github.com>

mildaniel and others added 6 commits June 14, 2021 11:44

Add headers whenever cors is set

87a8b0f

Merge branch 'aws:develop' into develop

d0f27b0

Fix Cognito trigger validation

654a5af

Make templates deployable

94d22d9

Removed trigger value validation

c546425

Fix Python2 string matching test issue

52bf73b

mildaniel added the pr/internal label Jul 16, 2021

mgrandis changed the title ~~Validate Trigger Field and Test Cognito Properties With Intrinsic Functions~~ fix: Validate Trigger field and test Cognito properties with intrinsic functions Jul 20, 2021

moelasmar approved these changes Jul 20, 2021

View reviewed changes

mgrandis reviewed Jul 21, 2021

View reviewed changes

samtranslator/model/eventsources/push.py Outdated Show resolved Hide resolved

mgrandis suggested changes Jul 21, 2021

View reviewed changes

mildaniel added 2 commits July 21, 2021 17:38

Use PropertyType mechanism to exclude intrinsics

f56dc25

Remove unused import

255c9e1

mildaniel requested a review from mgrandis July 22, 2021 00:48

mgrandis approved these changes Jul 23, 2021

View reviewed changes

mildaniel merged commit 0e45f20 into aws:develop Jul 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Validate Trigger field and test Cognito properties with intrinsic functions #2092

fix: Validate Trigger field and test Cognito properties with intrinsic functions #2092

mildaniel commented Jul 16, 2021

codecov-commenter commented Jul 16, 2021 •

edited

mgrandis left a comment

fix: Validate Trigger field and test Cognito properties with intrinsic functions #2092

fix: Validate Trigger field and test Cognito properties with intrinsic functions #2092

Conversation

mildaniel commented Jul 16, 2021

codecov-commenter commented Jul 16, 2021 • edited

Codecov Report

mgrandis left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jul 16, 2021 •

edited