address review feedback, use static policy and simplify

chalice/config.py

@@ -2,7 +2,7 @@
 import sys
 import json
 
-from typing import Dict, Any, Optional, List  # noqa
+from typing import Dict, Any, Optional, List, Union  # noqa
 from chalice import __version__ as current_chalice_version
 from chalice.app import Chalice  # noqa
 from chalice.constants import DEFAULT_STAGE_NAME
@@ -231,15 +231,15 @@ def api_gateway_endpoint_type(self):
 
     @property
     def api_gateway_endpoint_vpce(self):
-        # type: () -> str
+        # type: () -> Union[str, List[str]]
         return self._chain_lookup('api_gateway_endpoint_vpce',
                                   varies_per_chalice_stage=True)
 
     @property
     def api_gateway_policy(self):
-        # type: () -> Optional[Dict]
+        # type: () -> Dict
         return self._chain_lookup('api_gateway_policy',
-                                  varies_per_chalice_stage=True)
+                                  varies_per_chalice_stage=True) or {}
 
     @property
     def minimum_compression_size(self):

chalice/deploy/deployer.py

@@ -471,7 +471,7 @@ def _create_rest_api_model(self,
             api_gateway_stage=config.api_gateway_stage,
             lambda_function=lambda_function,
             authorizers=authorizers,
-            policy=json.dumps(policy)
+            policy=policy
         )
 
     def _get_default_private_api_policy(self, config):
@@ -480,9 +480,7 @@ def _get_default_private_api_policy(self, config):
             "Effect": "Allow",
             "Principal": "*",
             "Action": "execute-api:Invoke",
-            "Resource": (
-                "arn:aws:execute-api:{region_name}:"
-                "{account_id}:{rest_api_id}/*"),
+            "Resource": "arn:aws:execute-api:*:*:*",
             "Condition": {
                 "StringEquals": {
                     "aws:SourceVpce": config.api_gateway_endpoint_vpce

chalice/deploy/models.py

@@ -1,3 +1,4 @@
+# pylint: disable=line-too-long
 import enum
 from typing import List, Dict, Optional, Any, TypeVar, Union, Set  # noqa
 from typing import cast
@@ -191,7 +192,7 @@ class RestAPI(ManagedModel):
     api_gateway_stage = attrib()                 # type: str
     endpoint_type = attrib()                     # type: str
     lambda_function = attrib()                   # type: LambdaFunction
-    policy = attrib(default=None)                # type: Optional[str]
+    policy = attrib(default=Factory(dict))       # type: Dict[str, Any]
     authorizers = attrib(default=Factory(list))  # type: List[LambdaFunction]
 
     def dependencies(self):

chalice/deploy/planner.py

@@ -946,20 +946,6 @@ def _plan_restapi(self, resource):
                 ), "Updating rest API\n"),
             ]
 
-            if not resource.policy:
-                shared_plan_patch_ops.append({
-                    'op': 'remove',
-                    'path': '/policy'})
-
-        if resource.policy:
-            shared_plan_patch_ops.append({
-                'op': 'replace',
-                'path': '/policy',
-                'value': StringFormat(
-                    resource.policy,
-                    ['region_name', 'account_id', 'rest_api_id'])
-            })
-
         plan.extend(shared_plan_epilogue)
         return plan
 

chalice/deploy/swagger.py

@@ -1,5 +1,4 @@
 import copy
-import json
 import inspect
 
 from typing import Any, List, Dict, Optional, Union  # noqa
@@ -40,8 +39,14 @@ def generate_swagger(self, app, rest_api=None):
         api['info']['title'] = app.app_name
         self._add_binary_types(api, app)
         self._add_route_paths(api, app)
+        self._add_resource_policy(api, rest_api)
         return api
 
+    def _add_resource_policy(self, api, rest_api):
+        # type: (Dict[str, Any], Optional[RestAPI]) -> None
+        if rest_api and rest_api.policy:
+            api['x-amazon-apigateway-policy'] = rest_api.policy
+
     def _add_binary_types(self, api, app):
         # type: (Dict[str, Any], Chalice) -> None
         api['x-amazon-apigateway-binary-media-types'] = app.api.binary_types
@@ -240,50 +245,6 @@ def __init__(self):
         # type: () -> None
         pass
 
-    def generate_swagger(self, app, rest_api=None):
-        # type: (Chalice, Optional[RestAPI]) -> Dict[str, Any]
-        api = super(CFNSwaggerGenerator, self).generate_swagger(app, rest_api)
-        if rest_api and rest_api.policy and rest_api.policy != 'null':
-            self._add_resource_policy(rest_api, api)
-        return api
-
-    def _add_resource_policy(self, rest_api, api):
-        # type: (RestAPI, Dict[str, Any]) -> None
-        policy = json.loads(str(rest_api.policy))
-        self._insert_fn_sub_list(policy['Statement'])
-        api['x-amazon-apigateway-policy'] = policy
-
-    def _insert_fn_sub_list(self, p):
-        # type: (List) -> None
-        key_map = dict(
-            region_name='${AWS::Region}',
-            rest_api_id='*',
-            account_id='${AWS:AccoundId}')
-        for idx, v in enumerate(list(p)):
-            if isinstance(v, dict):
-                return self._insert_fn_sub_dict(v)
-            for k in key_map:
-                if "{%s}" % k in v:
-                    p[idx] = {'Fn:Sub': v.format(**key_map)}
-                    break
-
-    def _insert_fn_sub_dict(self, p):
-        # type: (Dict) -> None
-        key_map = dict(
-            region_name='${AWS::Region}',
-            rest_api_id='*',
-            account_id='${AWS:AccoundId}')
-
-        for k, v in list(p.items()):
-            if isinstance(v, list):
-                return self._insert_fn_sub_list(v)
-            if isinstance(v, dict):
-                return self._insert_fn_sub_dict(v)
-            for r in key_map:
-                if "{%s}" % r in v:
-                    p[k] = {'Fn:Sub': v.format(**key_map)}
-                    break
-
     def _uri(self, lambda_arn=None):
         # type: (Optional[str]) -> Any
         return {

chalice/deploy/validate.py

@@ -51,8 +51,17 @@ def validate_configuration(config):
 
 def validate_resource_policy(config):
     # type: (Config) -> None
+    if (config.api_gateway_endpoint_type != 'PRIVATE' and
+            config.api_gateway_endpoint_vpce):
+        raise ValueError(
+            "config.api_gateway_endpoint_vpce should only be "
+            "specified for PRIVATE api_gateway_endpoint_type")
     if config.api_gateway_endpoint_type != 'PRIVATE':
         return
+    if config.api_gateway_policy and config.api_gateway_endpoint_vpce:
+        raise ValueError(
+            "Can only specify one of api_gateway_policy and "
+            "api_gateway_endpoint_vpce")
     if config.api_gateway_policy:
         return
     if not config.api_gateway_endpoint_vpce:

tests/unit/deploy/test_deployer.py

@@ -1,4 +1,3 @@
-import json
 import os
 
 import socket
@@ -713,15 +712,13 @@ def test_can_build_private_rest_api(self, rest_api_app):
         application = builder.build(config, stage_name='dev')
         rest_api = application.resources[0]
         assert isinstance(rest_api, models.RestAPI)
-        assert json.loads(rest_api.policy) == {
+        assert rest_api.policy == {
             'Version': '2012-10-17',
             'Statement': [
                 {'Action': 'execute-api:Invoke',
                  'Effect': 'Allow',
                  'Principal': '*',
-                 'Resource': (
-                     'arn:aws:execute-api:{region_name}:'
-                     '{account_id}:{rest_api_id}/*'),
+                 'Resource': 'arn:aws:execute-api:*:*:*',
                  'Condition': {
                      'StringEquals': {
                          'aws:SourceVpce': 'vpce-abc123'}}},

tests/unit/deploy/test_planner.py

@@ -877,35 +877,6 @@ def assert_loads_needed_variables(self, plan):
                 to_var='api_handler_lambda_arn'),
         ]
 
-    def test_can_plan_private_rest_api(self):
-        function = create_function_resource('function_name')
-        rest_api = models.RestAPI(
-            resource_name='rest_api',
-            swagger_doc={'swagger': '2.0'},
-            endpoint_type='PRIVATE',
-            policy="{'Statement': []}",
-            minimum_compression='100',
-            api_gateway_stage='api',
-            lambda_function=function,
-        )
-        plan = self.determine_plan(rest_api)
-        self.assert_loads_needed_variables(plan)
-
-        assert plan[6].params == {
-            'rest_api_id': Variable("rest_api_id"),
-            'patch_operations': [
-                {'op': 'replace',
-                 'path': '/minimumCompressionSize',
-                 'value': '100'},
-                {'op': 'replace',
-                 'path': '/policy',
-                 'value': StringFormat(
-                     "{'Statement': []}",
-                     ['region_name', 'account_id', 'rest_api_id'])
-                 }
-            ]
-        }
-
     def test_can_plan_rest_api(self):
         function = create_function_resource('function_name')
         rest_api = models.RestAPI(
@@ -1001,13 +972,8 @@ def test_can_update_rest_api_with_policy(self):
                      ("/endpointConfiguration/types/"
                       "{rest_api[endpointConfiguration][types][0]}"),
                      ['rest_api']),
-                 'value': 'EDGE'},
-                {'op': 'replace',
-                 'path': '/policy',
-                 'value': StringFormat(
-                     "{'Statement': []}",
-                     ['region_name', 'account_id', 'rest_api_id'])
-                 }],
+                 'value': 'EDGE'}
+            ],
             'rest_api_id': Variable("rest_api_id")
         }
 
@@ -1062,7 +1028,6 @@ def test_can_update_rest_api(self):
                              '/endpointConfiguration/types/%s' % (
                                 '{rest_api[endpointConfiguration][types][0]}'),
                              ['rest_api'])},
-                        {'op': 'remove', 'path': '/policy'}
                     ],
                 },
             ),

tests/unit/deploy/test_swagger.py

@@ -1,5 +1,3 @@
-import json
-
 from chalice.deploy.swagger import SwaggerGenerator, CFNSwaggerGenerator
 from chalice import CORSConfig
 from chalice.app import CustomAuthorizer, CognitoUserPoolAuthorizer
@@ -576,14 +574,13 @@ def test_can_custom_resource_policy_with_cfn(sample_app):
         minimum_compression="",
         api_gateway_stage="xyz",
         endpoint_type="PRIVATE",
-        policy=json.dumps({
+        policy={
             'Statement': [{
                 "Effect": "Allow",
                 "Principal": "*",
                 "Action": "execute-api:Invoke",
-                "Resource": [(
-                    "arn:aws:execute-api:{region_name}:"
-                    "{account_id}:{rest_api_id}/*"),
+                "Resource": [
+                    "arn:aws:execute-api:*:*:*",
                     "arn:aws:exceute-api:*:*:*/*"
                 ],
                 "Condition": {
@@ -592,7 +589,7 @@ def test_can_custom_resource_policy_with_cfn(sample_app):
                     }
                 }
             }]
-        })
+        }
     )
 
     doc = swagger_gen.generate_swagger(sample_app, rest_api)
@@ -604,9 +601,7 @@ def test_can_custom_resource_policy_with_cfn(sample_app):
             'Effect': 'Allow',
             'Principal': '*',
             'Resource': [
-                {'Fn:Sub': (
-                    'arn:aws:execute-api:${AWS::Region}'
-                    ':${AWS:AccoundId}:*/*')},
+                'arn:aws:execute-api:*:*:*',
                 "arn:aws:exceute-api:*:*:*/*"]
             }]
     }
@@ -621,21 +616,19 @@ def test_can_auto_resource_policy_with_cfn(sample_app):
         minimum_compression="",
         api_gateway_stage="xyz",
         endpoint_type="PRIVATE",
-        policy=json.dumps({
+        policy={
             'Statement': [{
                 "Effect": "Allow",
                 "Principal": "*",
                 "Action": "execute-api:Invoke",
-                "Resource": (
-                    "arn:aws:execute-api:{region_name}:"
-                    "{account_id}:{rest_api_id}/*"),
+                "Resource": "arn:aws:execute-api:*:*:*/*",
                 "Condition": {
                     "StringEquals": {
                         "aws:SourceVpce": "vpce-abc123"
                     }
                 }
             }]
-        })
+        }
     )
 
     doc = swagger_gen.generate_swagger(sample_app, rest_api)
@@ -646,8 +639,7 @@ def test_can_auto_resource_policy_with_cfn(sample_app):
                 'aws:SourceVpce': 'vpce-abc123'}},
             'Effect': 'Allow',
             'Principal': '*',
-            'Resource': {'Fn:Sub': (
-                'arn:aws:execute-api:${AWS::Region}:${AWS:AccoundId}:*/*')}
+            'Resource': 'arn:aws:execute-api:*:*:*/*',
             }]
     }
 

tests/unit/deploy/test_validate.py

@@ -247,12 +247,27 @@ def test_can_validate_resource_policy(sample_app):
         api_gateway_endpoint_type='PRIVATE')
     validate_resource_policy(config)
 
+    config = Config.create(
+        chalice_app=sample_app,
+        api_gateway_endpoint_vpce='vpce-abc123',
+        api_gateway_endpoint_type='REGIONAL')
+    with pytest.raises(ValueError):
+        validate_resource_policy(config)
+
     config = Config.create(
         chalice_app=sample_app,
         api_gateway_policy={'Statement': []},
         api_gateway_endpoint_type='PRIVATE')
     validate_resource_policy(config)
 
+    config = Config.create(
+        chalice_app=sample_app,
+        api_gateway_endpoint_vpce=['vpce-abc123', 'vpce-bdef'],
+        api_gateway_policy={'Statement': []},
+        api_gateway_endpoint_type='PRIVATE')
+    with pytest.raises(ValueError):
+        validate_resource_policy(config)
+
 
 def test_can_validate_endpoint_type(sample_app):
     config = Config.create(