diff --git a/chalice/config.py b/chalice/config.py index e11df747b..77e0bd214 100644 --- a/chalice/config.py +++ b/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): diff --git a/chalice/deploy/deployer.py b/chalice/deploy/deployer.py index 29b3736dc..197921053 100644 --- a/chalice/deploy/deployer.py +++ b/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 diff --git a/chalice/deploy/models.py b/chalice/deploy/models.py index 416346225..726fe00ca 100644 --- a/chalice/deploy/models.py +++ b/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): diff --git a/chalice/deploy/planner.py b/chalice/deploy/planner.py index 9ca11026f..18d731198 100644 --- a/chalice/deploy/planner.py +++ b/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 diff --git a/chalice/deploy/swagger.py b/chalice/deploy/swagger.py index e7ca18b58..8ba4bb3b9 100644 --- a/chalice/deploy/swagger.py +++ b/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 { diff --git a/chalice/deploy/validate.py b/chalice/deploy/validate.py index 68d4853d7..ab3d110b0 100644 --- a/chalice/deploy/validate.py +++ b/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: diff --git a/tests/unit/deploy/test_deployer.py b/tests/unit/deploy/test_deployer.py index b86542370..0aca359dd 100644 --- a/tests/unit/deploy/test_deployer.py +++ b/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'}}}, diff --git a/tests/unit/deploy/test_planner.py b/tests/unit/deploy/test_planner.py index 3d68cc995..02d7603ef 100644 --- a/tests/unit/deploy/test_planner.py +++ b/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'} ], }, ), diff --git a/tests/unit/deploy/test_swagger.py b/tests/unit/deploy/test_swagger.py index 97d482246..c4f774a04 100644 --- a/tests/unit/deploy/test_swagger.py +++ b/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:*:*:*/*', }] } diff --git a/tests/unit/deploy/test_validate.py b/tests/unit/deploy/test_validate.py index e39b9cc59..302742818 100644 --- a/tests/unit/deploy/test_validate.py +++ b/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(