add new arguments to allow vpc/vpce endpoints as intrinsics #1689

wchengru · 2020-08-24T23:26:27Z

Issue #, if available:

Description of changes:

Description of how you validated changes:

Checklist:

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

Examples?

Please reach out in the comments, if you want to add an example. Examples will be
added to sam init through https://github.com/awslabs/aws-sam-cli-app-templates/

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

codecov-commenter · 2020-08-24T23:30:04Z

Codecov Report

Merging #1689 into develop will increase coverage by 0.05%.
The diff coverage is 97.22%.

@@             Coverage Diff             @@
##           develop    #1689      +/-   ##
===========================================
+ Coverage    94.09%   94.15%   +0.05%     
===========================================
  Files           86       86              
  Lines         5423     5437      +14     
  Branches      1084     1087       +3     
===========================================
+ Hits          5103     5119      +16     
+ Misses         148      147       -1     
+ Partials       172      171       -1

Impacted Files	Coverage Δ
samtranslator/swagger/swagger.py	`93.02% <97.22%> (+0.54%)`	⬆️

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 ed2f24e...3dba793. Read the comment docs.

awood45

Test request, as well as a question.

awood45 · 2020-09-03T17:06:15Z

tests/swagger/test_swagger.py

+
+    def test_must_add_vpc_allow_string_and_instrinic(self):
+
+        resourcePolicy = {


A couple changes:

Please make sure the explicit intrinsic entries do NOT have "VPC" or "VPCE" in their strings, to ensure we don't regress on requiring regex matching.

Please include more than one to ensure it's handling the full list.

Can we also test both whitelist and blocklist in a single test, to ensure both are applied?

Updated intrinsic entries and added more test cases.

awood45 · 2020-09-03T17:09:23Z

samtranslator/swagger/swagger.py

+            condition.setdefault("aws:SourceVpc", []).extend(intrinsic_vpc_endpoint_list)
+        if intrinsic_vpce_endpoint_list is not None:
+            condition.setdefault("aws:SourceVpce", []).extend(intrinsic_vpce_endpoint_list)
+        if not condition:


What is this new clause doing?

The plan here is adding a sanity check that this function doesn't write empty endpoint lists into this section. It is similar as the original code at this line.
I just realized that although the three lists in the endpoint_dict shouldn't be empty, but the regex classified vpc/vpce list may still be empty. So I modified this condition to better verify that the endpoint allow/deny lists are not empty.

* docs: fix link to all_policy_templates example file (aws#1682) * fix: Enhance CompatibleRuntimes property of AWS::Serverless::LayerVersion (aws#1683) * Adding Tracing property to State Machine resource (aws#1697) * Step Functions: Adding support for X-Ray tracing * Renaming TracingConfiguration to Tracing Co-authored-by: Vaib Suri <surivaib@amazon.com> * add new arguments to allow vpc/vpce endpoints as intrinsics (aws#1689) * add new arguments to allow vpc/vpce endpoints as intrinsics * Add more unit tests * Add more test cases, and code changes according to comments. * Fix: Updated Slack Invite Link (aws#1712) * Updated Slack Invite Link * Restricted jsonschema to Python 2 * Forced pyrsistent to 0.16 in Python 2 * Reverted Changes to enum34 * Merge master back to develop (aws#1734) * Release v1.26.0 (aws#1680) * feat: add support for VPCEndpointIds in EndpointConfiguration * fix: update formatting with black * docs: update 2016-10-31.md * docs: added api endpointconfiguration example * docs: make example more generic * fix: remove nested EndpointConfiguration types from output * fix: only allow one EndpointConfiguration Type * doc: update example to reflect only allowing one EndpointConfiguration Type * fix : missing UserPool properties (aws#1506) (aws#1581) * fix: resource policy generation for {path+} (aws#1580) * refactor: Remove 2016-10-31 examples * update PR template * adjust pr template * Adding authorization scopes as list validation in ApiGatewayAuthorizer (v1 and v2). (aws#1670) * Adding authorization scopes as list validation in ApiGatewayAuthorizer and ApiGatewayV2Authorizer. * make black. * Adding functional test for invalid auth scope. * adding error condition for invalid test. * removing test template file. * feat: MSK event type support for AWS::Serverless::Function (aws#52) Co-authored-by: Steve Brown <steve@fabric.com> Co-authored-by: jtaylor00 <joetaylor00@gmail.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> Co-authored-by: Alex Wood <awood45@gmail.com> Co-authored-by: Tarun <c2tarun@users.noreply.github.com> * Update __init__.py (aws#1704) * Release/v1.27.0 resolveconflict (aws#1717) * Release v1.26.0 (aws#1680) * feat: add support for VPCEndpointIds in EndpointConfiguration * fix: update formatting with black * docs: update 2016-10-31.md * docs: added api endpointconfiguration example * docs: make example more generic * fix: remove nested EndpointConfiguration types from output * fix: only allow one EndpointConfiguration Type * doc: update example to reflect only allowing one EndpointConfiguration Type * fix : missing UserPool properties (aws#1506) (aws#1581) * fix: resource policy generation for {path+} (aws#1580) * refactor: Remove 2016-10-31 examples * update PR template * adjust pr template * Adding authorization scopes as list validation in ApiGatewayAuthorizer (v1 and v2). (aws#1670) * Adding authorization scopes as list validation in ApiGatewayAuthorizer and ApiGatewayV2Authorizer. * make black. * Adding functional test for invalid auth scope. * adding error condition for invalid test. * removing test template file. * feat: MSK event type support for AWS::Serverless::Function (aws#52) Co-authored-by: Steve Brown <steve@fabric.com> Co-authored-by: jtaylor00 <joetaylor00@gmail.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> Co-authored-by: Alex Wood <awood45@gmail.com> Co-authored-by: Tarun <c2tarun@users.noreply.github.com> * Fix: Updated Slack Invite Link (aws#1712) * Updated Slack Invite Link * Restricted jsonschema to Python 2 * Forced pyrsistent to 0.16 in Python 2 * Reverted Changes to enum34 Co-authored-by: Sriram Madapusi Vasudevan <3770774+sriram-mv@users.noreply.github.com> Co-authored-by: Steve Brown <steve@fabric.com> Co-authored-by: jtaylor00 <joetaylor00@gmail.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> Co-authored-by: Alex Wood <awood45@gmail.com> Co-authored-by: Tarun <c2tarun@users.noreply.github.com> Co-authored-by: Cosh_ <CoshUS@users.noreply.github.com> Co-authored-by: Sriram Madapusi Vasudevan <3770774+sriram-mv@users.noreply.github.com> Co-authored-by: Steve Brown <steve@fabric.com> Co-authored-by: jtaylor00 <joetaylor00@gmail.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> Co-authored-by: Alex Wood <awood45@gmail.com> Co-authored-by: Tarun <c2tarun@users.noreply.github.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Cosh_ <CoshUS@users.noreply.github.com> * Lambdaauth (aws#1733) * Add support for Lambda Authorizers in HttpAPI * Address comments and fix formatting * fix version * Validate input parameters. Update tests * black reformat Co-authored-by: Raymond Wang <14915548+wchengru@users.noreply.github.com> * Feature toggle (aws#1737) * Adding logic to pipe app config providers. Unit test pending * Adding some documentation to config providers. * Adding some unit tests and making black ignore json files. * minor cleanup. * Addressing PR comments. * feature: Support MTLS auth properties in REST and HTTP API domain names (aws#1725) * feature: Support MTLS auth properties in REST and HTTP API domain names * fix unicode != str issue in py2.7 * add SecurityPolicy because default RESTAPI is using TLS1.0 * Add new property DisableExecuteApiEndpoint * black reformat * Address comments * Add tests on invalid templates * address test failures in py2.7 * restart travis tests * fix: adding support for passing target id to EventBridgeRule (aws#1747) * Manage black version using requirement file (aws#1748) * chore: Manage black version in dev.txt - config pre-commit - config development guide - config travis Refer to the commit below in aws-sam-cli aws/aws-sam-cli@d725db5fbfc698a9f0c7582 * Format using black 20.8b1 * Opt-out black fron dev.txt for Python 2 Co-authored-by: Patrick Greenwell <patrickgreenwell@users.noreply.github.com> Co-authored-by: Raymond Wang <14915548+wchengru@users.noreply.github.com> Co-authored-by: vaib-amz <55562387+vaib-amz@users.noreply.github.com> Co-authored-by: Vaib Suri <surivaib@amazon.com> Co-authored-by: Cosh_ <CoshUS@users.noreply.github.com> Co-authored-by: Sriram Madapusi Vasudevan <3770774+sriram-mv@users.noreply.github.com> Co-authored-by: Steve Brown <steve@fabric.com> Co-authored-by: jtaylor00 <joetaylor00@gmail.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> Co-authored-by: Alex Wood <awood45@gmail.com> Co-authored-by: Tarun <c2tarun@users.noreply.github.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Tolledo <ps.tolledo@gmail.com> Co-authored-by: _sam <3804518+aahung@users.noreply.github.com>

wchengru requested a review from awood45 August 24, 2020 23:32

wchengru force-pushed the feature/endpointintrinsiclist branch from f51c03f to a69e74a Compare August 24, 2020 23:44

add new arguments to allow vpc/vpce endpoints as intrinsics

007b2d0

wchengru force-pushed the feature/endpointintrinsiclist branch from a69e74a to 007b2d0 Compare August 31, 2020 16:40

Add more unit tests

a29ff40

awood45 requested changes Sep 3, 2020

View reviewed changes

wchengru force-pushed the feature/endpointintrinsiclist branch from abcab4d to 0c9d5d9 Compare September 3, 2020 18:54

Add more test cases, and code changes according to comments.

3dba793

wchengru force-pushed the feature/endpointintrinsiclist branch from 0c9d5d9 to 3dba793 Compare September 3, 2020 19:06

awood45 approved these changes Sep 3, 2020

View reviewed changes

wchengru merged commit c8ecc83 into aws:develop Sep 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add new arguments to allow vpc/vpce endpoints as intrinsics #1689

add new arguments to allow vpc/vpce endpoints as intrinsics #1689

wchengru commented Aug 24, 2020 •

edited

Loading

codecov-commenter commented Aug 24, 2020 •

edited

Loading

awood45 left a comment

awood45 Sep 3, 2020

wchengru Sep 3, 2020

awood45 Sep 3, 2020

wchengru Sep 3, 2020 •

edited

Loading


		def test_must_add_vpc_allow_string_and_instrinic(self):

		resourcePolicy = {

add new arguments to allow vpc/vpce endpoints as intrinsics #1689

add new arguments to allow vpc/vpce endpoints as intrinsics #1689

Conversation

wchengru commented Aug 24, 2020 • edited Loading

codecov-commenter commented Aug 24, 2020 • edited Loading

Codecov Report

awood45 left a comment

Choose a reason for hiding this comment

awood45 Sep 3, 2020

Choose a reason for hiding this comment

wchengru Sep 3, 2020

Choose a reason for hiding this comment

awood45 Sep 3, 2020

Choose a reason for hiding this comment

wchengru Sep 3, 2020 • edited Loading

Choose a reason for hiding this comment

wchengru commented Aug 24, 2020 •

edited

Loading

codecov-commenter commented Aug 24, 2020 •

edited

Loading

wchengru Sep 3, 2020 •

edited

Loading