From b5b34750f5abec2a50c394ceb501be55651c469f Mon Sep 17 00:00:00 2001 From: Kapil Thangavelu Date: Mon, 1 Jul 2019 18:29:52 -0400 Subject: [PATCH 1/6] api gateway endpoint configuration --- .pylintrc | 2 +- CHANGELOG.rst | 3 ++ chalice/awsclient.py | 7 ++-- chalice/cli/factory.py | 2 + chalice/config.py | 5 +++ chalice/constants.py | 2 +- chalice/deploy/deployer.py | 1 + chalice/deploy/models.py | 1 + chalice/deploy/planner.py | 30 +++++++++++---- chalice/deploy/validate.py | 13 +++++++ chalice/package.py | 1 + docs/source/topics/configfile.rst | 7 +++- tests/functional/test_awsclient.py | 3 +- tests/unit/deploy/test_deployer.py | 1 + tests/unit/deploy/test_models.py | 2 + tests/unit/deploy/test_planner.py | 59 ++++++++++++++++++++++++++++-- tests/unit/deploy/test_validate.py | 16 ++++++++ 17 files changed, 138 insertions(+), 17 deletions(-) diff --git a/.pylintrc b/.pylintrc index 64c848c3d..88933ae96 100644 --- a/.pylintrc +++ b/.pylintrc @@ -59,7 +59,7 @@ confidence= # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use"--disable=all --enable=classes # --disable=W" -disable=R0201,W0613,I0021,I0020,C0111,W1618,W1619,R0902,R0903,W0231,W0611,R0913,W0703,C0330,R0204,I0011,R0904,R0205,R1705,R1710,W0403 +disable=R0201,W0613,I0021,I0020,C0111,W1618,W1619,R0902,R0903,W0231,W0611,R0913,W0703,C0330,R0204,I0011,R0904,R0205,R1705,R1710,W0403,C0302 [REPORTS] diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 245740ea1..36c9d27c1 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -8,6 +8,9 @@ Next Release (TBD) * Add experimental support for websockets (`#1017 `__) +* API Gateway Endpoint Type Configuration (Regional and Edge) + (`#1160 https://github.com/aws/chalice/pull/1160`__) + 1.9.1 ===== diff --git a/chalice/awsclient.py b/chalice/awsclient.py index c6940538c..abc4c9313 100644 --- a/chalice/awsclient.py +++ b/chalice/awsclient.py @@ -460,11 +460,12 @@ def rest_api_exists(self, rest_api_id): except client.exceptions.NotFoundException: return False - def import_rest_api(self, swagger_document): - # type: (Dict[str, Any]) -> str + def import_rest_api(self, swagger_document, endpoint_type): + # type: (Dict[str, Any], str) -> str client = self._client('apigateway') response = client.import_rest_api( - body=json.dumps(swagger_document, indent=2) + body=json.dumps(swagger_document, indent=2), + parameters={'endpointConfigurationTypes': endpoint_type} ) rest_api_id = response['id'] return rest_api_id diff --git a/chalice/cli/factory.py b/chalice/cli/factory.py index 2b6d736ba..b20bffef0 100644 --- a/chalice/cli/factory.py +++ b/chalice/cli/factory.py @@ -19,6 +19,7 @@ from chalice.package import AppPackager # noqa from chalice.constants import DEFAULT_STAGE_NAME from chalice.constants import DEFAULT_APIGATEWAY_STAGE_NAME +from chalice.constants import DEFAULT_ENDPOINT_TYPE from chalice.logs import LogRetriever from chalice import local from chalice.utils import UI # noqa @@ -142,6 +143,7 @@ def create_config_obj(self, chalice_stage_name=DEFAULT_STAGE_NAME, user_provided_params = {} # type: Dict[str, Any] default_params = {'project_dir': self.project_dir, 'api_gateway_stage': DEFAULT_APIGATEWAY_STAGE_NAME, + 'api_gateway_endpoint_type': DEFAULT_ENDPOINT_TYPE, 'autogen_policy': True} try: config_from_disk = self.load_project_config() diff --git a/chalice/config.py b/chalice/config.py index 89b6dd0d3..c86a2fc85 100644 --- a/chalice/config.py +++ b/chalice/config.py @@ -223,6 +223,11 @@ def api_gateway_stage(self): return self._chain_lookup('api_gateway_stage', varies_per_chalice_stage=True) + @property + def api_gateway_endpoint_type(self): + # type: () -> str + return self._chain_lookup('api_gateway_endpoint_type') + @property def minimum_compression_size(self): # type: () -> int diff --git a/chalice/constants.py b/chalice/constants.py index 89c03dfe2..0328e1a65 100644 --- a/chalice/constants.py +++ b/chalice/constants.py @@ -45,7 +45,7 @@ def index(): DEFAULT_STAGE_NAME = 'dev' DEFAULT_APIGATEWAY_STAGE_NAME = 'api' - +DEFAULT_ENDPOINT_TYPE = 'EDGE' DEFAULT_LAMBDA_TIMEOUT = 60 DEFAULT_LAMBDA_MEMORY_SIZE = 128 diff --git a/chalice/deploy/deployer.py b/chalice/deploy/deployer.py index f37469f52..3b86a057a 100644 --- a/chalice/deploy/deployer.py +++ b/chalice/deploy/deployer.py @@ -462,6 +462,7 @@ def _create_rest_api_model(self, return models.RestAPI( resource_name='rest_api', swagger_doc=models.Placeholder.BUILD_STAGE, + endpoint_type=config.api_gateway_endpoint_type, minimum_compression=minimum_compression, api_gateway_stage=config.api_gateway_stage, lambda_function=lambda_function, diff --git a/chalice/deploy/models.py b/chalice/deploy/models.py index 3ae01b7a8..aef047acc 100644 --- a/chalice/deploy/models.py +++ b/chalice/deploy/models.py @@ -189,6 +189,7 @@ class RestAPI(ManagedModel): swagger_doc = attrib() # type: DV[Dict[str, Any]] minimum_compression = attrib() # type: str api_gateway_stage = attrib() # type: str + endpoint_type = attrib() # type: str lambda_function = attrib() # type: LambdaFunction authorizers = attrib(default=Factory(list)) # type: List[LambdaFunction] diff --git a/chalice/deploy/planner.py b/chalice/deploy/planner.py index a07f1f541..4e0ab13e0 100644 --- a/chalice/deploy/planner.py +++ b/chalice/deploy/planner.py @@ -3,6 +3,7 @@ from typing import List, Dict, Any, Optional, Union, Tuple, Set, cast # noqa from typing import Sequence # noqa +from chalice.constants import DEFAULT_ENDPOINT_TYPE from chalice.config import Config, DeployedResources # noqa from chalice.utils import OSUtils # noqa from chalice.deploy import models @@ -843,17 +844,17 @@ def _plan_restapi(self, resource): # There's also a set of instructions that are needed # at the end of deploying a rest API that apply to both # the update and create case. + shared_plan_patch_ops = [{ + 'op': 'replace', + 'path': '/minimumCompressionSize', + 'value': resource.minimum_compression}] shared_plan_epilogue = [ models.APICall( method_name='update_rest_api', params={ 'rest_api_id': Variable('rest_api_id'), - 'patch_operations': [{ - 'op': 'replace', - 'path': '/minimumCompressionSize', - 'value': resource.minimum_compression, - }], - }, + 'patch_operations': shared_plan_patch_ops + } ), models.APICall( method_name='add_permission_for_apigateway', @@ -876,6 +877,12 @@ def _plan_restapi(self, resource): name='rest_api_url', variable_name='rest_api_url', ), + models.RecordResourceValue( + resource_type='rest_api', + resource_name=resource.resource_name, + name='endpoint_type', + value=resource.endpoint_type + ), ] # type: List[InstructionMsg] for auth in resource.authorizers: shared_plan_epilogue.append( @@ -891,7 +898,8 @@ def _plan_restapi(self, resource): plan = shared_plan_preamble + [ (models.APICall( method_name='import_rest_api', - params={'swagger_document': resource.swagger_doc}, + params={'swagger_document': resource.swagger_doc, + 'endpoint_type': resource.endpoint_type}, output_var='rest_api_id', ), "Creating Rest API\n"), models.RecordResourceVariable( @@ -908,6 +916,14 @@ def _plan_restapi(self, resource): ] + shared_plan_epilogue else: deployed = self._remote_state.resource_deployed_values(resource) + if deployed.get('endpoint_type', + DEFAULT_ENDPOINT_TYPE) != resource.endpoint_type: + shared_plan_patch_ops.append({ + 'op': 'replace', + 'path': '/endpointConfiguration/types/{}'.format( + deployed.get('endpoint_type', DEFAULT_ENDPOINT_TYPE)), + 'value': resource.endpoint_type + }) plan = shared_plan_preamble + [ models.StoreValue( name='rest_api_id', diff --git a/chalice/deploy/validate.py b/chalice/deploy/validate.py index f394ccfea..7219202af 100644 --- a/chalice/deploy/validate.py +++ b/chalice/deploy/validate.py @@ -45,6 +45,19 @@ def validate_configuration(config): validate_python_version(config) validate_unique_function_names(config) validate_feature_flags(config.chalice_app) + validate_endpoint_type(config) + + +def validate_endpoint_type(config): + # type: (app.Chalice) -> None + + if not config.api_gateway_endpoint_type: + return + valid_types = ('EDGE', 'REGIONAL') + if config.api_gateway_endpoint_type not in valid_types: + raise ValueError( + "api gateway endpoint type must be one of %s" % ( + ", ".join(valid_types))) def validate_feature_flags(chalice_app): diff --git a/chalice/package.py b/chalice/package.py index 33a871912..875fcd339 100644 --- a/chalice/package.py +++ b/chalice/package.py @@ -166,6 +166,7 @@ def _generate_restapi(self, resource, template): resources['RestAPI'] = { 'Type': 'AWS::Serverless::Api', 'Properties': { + 'EndpointConfiguration': resource.endpoint_type, 'StageName': resource.api_gateway_stage, 'DefinitionBody': resource.swagger_doc, } diff --git a/docs/source/topics/configfile.rst b/docs/source/topics/configfile.rst index bb5022f86..f72719dbd 100644 --- a/docs/source/topics/configfile.rst +++ b/docs/source/topics/configfile.rst @@ -34,13 +34,18 @@ information about chalice stages. key will automatically be created for you. See the examples section below for some stage specific configurations. +* ``api_gateway_endpoint_type`` - The endpoint configuration of the + deployed API Gateway which determines how the API will be accessed, + can be EDGE, REGIONAL, PRIVATE. Note this value can only be set as a + top level key and defaults to EDGE. For more information see + https://amzn.to/2LofApt + The following config values can either be specified per stage config or as a top level key which is not tied to a specific key. Whenever a stage specific configuration value is needed, the ``stages`` mapping is checked first. If no value is found then the top level keys will be checked. - * ``api_gateway_stage`` - The name of the API gateway stage. This will also be the URL prefix for your API (``https://endpoint/prefix/your-api``). diff --git a/tests/functional/test_awsclient.py b/tests/functional/test_awsclient.py index 24989d2ce..b2b776a93 100644 --- a/tests/functional/test_awsclient.py +++ b/tests/functional/test_awsclient.py @@ -1670,12 +1670,13 @@ def test_import_rest_api(stubbed_session): apig = stubbed_session.stub('apigateway') swagger_doc = {'swagger': 'doc'} apig.import_rest_api( + parameters={'endpointConfigurationTypes': 'EDGE'}, body=json.dumps(swagger_doc, indent=2)).returns( {'id': 'rest_api_id'}) stubbed_session.activate_stubs() awsclient = TypedAWSClient(stubbed_session) - rest_api_id = awsclient.import_rest_api(swagger_doc) + rest_api_id = awsclient.import_rest_api(swagger_doc, 'EDGE') stubbed_session.verify_stubs() assert rest_api_id == 'rest_api_id' diff --git a/tests/unit/deploy/test_deployer.py b/tests/unit/deploy/test_deployer.py index d12575948..3f11cafb2 100644 --- a/tests/unit/deploy/test_deployer.py +++ b/tests/unit/deploy/test_deployer.py @@ -1228,6 +1228,7 @@ def test_can_generate_swagger_builder(self): resource_name='foo', swagger_doc=models.Placeholder.BUILD_STAGE, minimum_compression='', + endpoint_type='EDGE', api_gateway_stage='api', lambda_function=None, ) diff --git a/tests/unit/deploy/test_models.py b/tests/unit/deploy/test_models.py index f5c9fc8c9..21a54e490 100644 --- a/tests/unit/deploy/test_models.py +++ b/tests/unit/deploy/test_models.py @@ -41,6 +41,7 @@ def test_can_default_to_no_auths_in_rest_api(lambda_function): swagger_doc={'swagger': '2.0'}, minimum_compression='', api_gateway_stage='api', + endpoint_type='EDGE', lambda_function=lambda_function, ) assert rest_api.dependencies() == [lambda_function] @@ -54,6 +55,7 @@ def test_can_add_authorizers_to_dependencies(lambda_function): swagger_doc={'swagger': '2.0'}, minimum_compression='', api_gateway_stage='api', + endpoint_type='EDGE', lambda_function=lambda_function, authorizers=[auth1, auth2], ) diff --git a/tests/unit/deploy/test_planner.py b/tests/unit/deploy/test_planner.py index 05991ebda..9eaf0f687 100644 --- a/tests/unit/deploy/test_planner.py +++ b/tests/unit/deploy/test_planner.py @@ -882,16 +882,19 @@ def test_can_plan_rest_api(self): rest_api = models.RestAPI( resource_name='rest_api', swagger_doc={'swagger': '2.0'}, + endpoint_type='EDGE', minimum_compression='100', api_gateway_stage='api', lambda_function=function, ) plan = self.determine_plan(rest_api) self.assert_loads_needed_variables(plan) + assert plan[4:] == [ models.APICall( method_name='import_rest_api', - params={'swagger_document': {'swagger': '2.0'}}, + params={'swagger_document': {'swagger': '2.0'}, + 'endpoint_type': 'EDGE'}, output_var='rest_api_id', ), models.RecordResourceVariable( @@ -937,11 +940,50 @@ def test_can_plan_rest_api(self): name='rest_api_url', variable_name='rest_api_url' ), + models.RecordResourceValue( + resource_type='rest_api', + resource_name='rest_api', + name='endpoint_type', + value=rest_api.endpoint_type + ), ] assert list(self.last_plan.messages.values()) == [ 'Creating Rest API\n' ] + def test_update_no_op_endpoint(self): + function = create_function_resource('function_name') + rest_api = models.RestAPI( + resource_name='rest_api', + swagger_doc={'swagger': '2.0'}, + minimum_compression='', + api_gateway_stage='api', + endpoint_type='EDGE', + lambda_function=function, + ) + self.remote_state.declare_resource_exists(rest_api) + self.remote_state.deployed_values['rest_api'] = { + 'rest_api_id': 'my_rest_api_id', + } + plan = self.determine_plan(rest_api) + + update = [ + p for p in plan + if isinstance(p, models.APICall) + and p.method_name == 'update_rest_api'] + + assert update == [ + models.APICall( + method_name='update_rest_api', + params={ + 'rest_api_id': Variable('rest_api_id'), + 'patch_operations': [{ + 'op': 'replace', + 'path': '/minimumCompressionSize', + 'value': ''}, + ], + })] + def test_can_update_rest_api(self): function = create_function_resource('function_name') rest_api = models.RestAPI( @@ -949,6 +991,7 @@ def test_can_update_rest_api(self): swagger_doc={'swagger': '2.0'}, minimum_compression='', api_gateway_stage='api', + endpoint_type='REGIONAL', lambda_function=function, ) self.remote_state.declare_resource_exists(rest_api) @@ -991,8 +1034,11 @@ def test_can_update_rest_api(self): 'patch_operations': [{ 'op': 'replace', 'path': '/minimumCompressionSize', - 'value': '', - }], + 'value': ''}, + {'op': 'replace', + 'path': '/endpointConfiguration/types/EDGE', + 'value': 'REGIONAL'} + ], }, ), models.APICall( @@ -1016,6 +1062,12 @@ def test_can_update_rest_api(self): name='rest_api_url', variable_name='rest_api_url' ), + models.RecordResourceValue( + resource_type='rest_api', + resource_name='rest_api', + name='endpoint_type', + value=rest_api.endpoint_type + ), ] @@ -1409,6 +1461,7 @@ def create_rest_api_model(self): resource_name='rest_api', swagger_doc={'swagger': '2.0'}, minimum_compression='', + endpoint_type='EDGE', api_gateway_stage='api', lambda_function=None, ) diff --git a/tests/unit/deploy/test_validate.py b/tests/unit/deploy/test_validate.py index 0e13c8c0a..0d7768b38 100644 --- a/tests/unit/deploy/test_validate.py +++ b/tests/unit/deploy/test_validate.py @@ -12,6 +12,7 @@ from chalice.deploy.validate import validate_route_content_types from chalice.deploy.validate import validate_unique_function_names from chalice.deploy.validate import validate_feature_flags +from chalice.deploy.validate import validate_endpoint_type from chalice.deploy.validate import ExperimentalFeatureError @@ -233,6 +234,21 @@ def index(): sample_app.api.binary_types) is None +def test_can_validate_endpoint_type(sample_app): + config = Config.create( + chalice_app=sample_app, api_gateway_endpoint_type='EDGE2') + with pytest.raises(ValueError): + validate_endpoint_type(config) + + config = Config.create( + chalice_app=sample_app, api_gateway_endpoint_type='REGIONAL') + validate_endpoint_type(config) + + config = Config.create( + chalice_app=sample_app, api_gateway_endpoint_type='PRIVATE') + validate_endpoint_type(config) + + def test_can_validate_feature_flags(sample_app): # The _features_used is marked internal because we don't want # chalice users to access it, but this attribute is intended to be From 0657b87e69b8e4bb289a316f2fff1c0699b673b8 Mon Sep 17 00:00:00 2001 From: Kapil Thangavelu Date: Fri, 19 Jul 2019 14:29:45 -0400 Subject: [PATCH 2/6] apigw endpoint type configuration, switch from recorded value to fetch current endpoint --- .pylintrc | 2 +- chalice/awsclient.py | 11 ++--- chalice/config.py | 3 +- chalice/deploy/planner.py | 37 ++++++++-------- chalice/deploy/validate.py | 3 +- docs/source/topics/configfile.rst | 12 +++--- tests/functional/test_awsclient.py | 6 +-- tests/unit/deploy/test_planner.py | 68 +++++++----------------------- tests/unit/deploy/test_validate.py | 3 +- 9 files changed, 57 insertions(+), 88 deletions(-) diff --git a/.pylintrc b/.pylintrc index 88933ae96..64c848c3d 100644 --- a/.pylintrc +++ b/.pylintrc @@ -59,7 +59,7 @@ confidence= # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use"--disable=all --enable=classes # --disable=W" -disable=R0201,W0613,I0021,I0020,C0111,W1618,W1619,R0902,R0903,W0231,W0611,R0913,W0703,C0330,R0204,I0011,R0904,R0205,R1705,R1710,W0403,C0302 +disable=R0201,W0613,I0021,I0020,C0111,W1618,W1619,R0902,R0903,W0231,W0611,R0913,W0703,C0330,R0204,I0011,R0904,R0205,R1705,R1710,W0403 [REPORTS] diff --git a/chalice/awsclient.py b/chalice/awsclient.py index abc4c9313..d5ff7373d 100644 --- a/chalice/awsclient.py +++ b/chalice/awsclient.py @@ -450,15 +450,16 @@ def get_rest_api_id(self, name): return api['id'] return None - def rest_api_exists(self, rest_api_id): - # type: (str) -> bool + def get_rest_api(self, rest_api_id): + # type: (str) -> Dict[str, Any] """Check if an an API Gateway REST API exists.""" client = self._client('apigateway') try: - client.get_rest_api(restApiId=rest_api_id) - return True + result = client.get_rest_api(restApiId=rest_api_id) + result.pop('ResponseMetadata', None) + return result except client.exceptions.NotFoundException: - return False + return {} def import_rest_api(self, swagger_document, endpoint_type): # type: (Dict[str, Any], str) -> str diff --git a/chalice/config.py b/chalice/config.py index c86a2fc85..27b38ed8a 100644 --- a/chalice/config.py +++ b/chalice/config.py @@ -226,7 +226,8 @@ def api_gateway_stage(self): @property def api_gateway_endpoint_type(self): # type: () -> str - return self._chain_lookup('api_gateway_endpoint_type') + return self._chain_lookup('api_gateway_endpoint_type', + varies_per_chalice_stage=True) @property def minimum_compression_size(self): diff --git a/chalice/deploy/planner.py b/chalice/deploy/planner.py index 4e0ab13e0..2c15cbeb6 100644 --- a/chalice/deploy/planner.py +++ b/chalice/deploy/planner.py @@ -1,9 +1,9 @@ +# pylint: disable=too-many-lines from collections import OrderedDict from typing import List, Dict, Any, Optional, Union, Tuple, Set, cast # noqa from typing import Sequence # noqa -from chalice.constants import DEFAULT_ENDPOINT_TYPE from chalice.config import Config, DeployedResources # noqa from chalice.utils import OSUtils # noqa from chalice.deploy import models @@ -108,7 +108,7 @@ def _resource_exists_restapi(self, resource): except ValueError: return False rest_api_id = deployed_values['rest_api_id'] - return self._client.rest_api_exists(rest_api_id) + return bool(self._client.get_rest_api(rest_api_id)) def _resource_exists_websocketapi(self, resource): # type: (models.WebsocketAPI) -> bool @@ -847,7 +847,9 @@ def _plan_restapi(self, resource): shared_plan_patch_ops = [{ 'op': 'replace', 'path': '/minimumCompressionSize', - 'value': resource.minimum_compression}] + 'value': resource.minimum_compression} + ] # type: List[Dict] + shared_plan_epilogue = [ models.APICall( method_name='update_rest_api', @@ -877,12 +879,6 @@ def _plan_restapi(self, resource): name='rest_api_url', variable_name='rest_api_url', ), - models.RecordResourceValue( - resource_type='rest_api', - resource_name=resource.resource_name, - name='endpoint_type', - value=resource.endpoint_type - ), ] # type: List[InstructionMsg] for auth in resource.authorizers: shared_plan_epilogue.append( @@ -916,14 +912,21 @@ def _plan_restapi(self, resource): ] + shared_plan_epilogue else: deployed = self._remote_state.resource_deployed_values(resource) - if deployed.get('endpoint_type', - DEFAULT_ENDPOINT_TYPE) != resource.endpoint_type: - shared_plan_patch_ops.append({ - 'op': 'replace', - 'path': '/endpointConfiguration/types/{}'.format( - deployed.get('endpoint_type', DEFAULT_ENDPOINT_TYPE)), - 'value': resource.endpoint_type - }) + shared_plan_epilogue.insert( + 0, + models.APICall( + method_name='get_rest_api', + params={'rest_api_id': Variable('rest_api_id')}, + output_var='rest_api') + ) + shared_plan_patch_ops.append({ + 'op': 'replace', + 'path': StringFormat( + '/endpointConfiguration/types/%s' % ( + '{rest_api[endpointConfiguration][types][0]}'), + ['rest_api']), + 'value': resource.endpoint_type} + ) plan = shared_plan_preamble + [ models.StoreValue( name='rest_api_id', diff --git a/chalice/deploy/validate.py b/chalice/deploy/validate.py index 7219202af..9effbd1b1 100644 --- a/chalice/deploy/validate.py +++ b/chalice/deploy/validate.py @@ -49,8 +49,7 @@ def validate_configuration(config): def validate_endpoint_type(config): - # type: (app.Chalice) -> None - + # type: (Config) -> None if not config.api_gateway_endpoint_type: return valid_types = ('EDGE', 'REGIONAL') diff --git a/docs/source/topics/configfile.rst b/docs/source/topics/configfile.rst index f72719dbd..b5951b5a9 100644 --- a/docs/source/topics/configfile.rst +++ b/docs/source/topics/configfile.rst @@ -34,12 +34,6 @@ information about chalice stages. key will automatically be created for you. See the examples section below for some stage specific configurations. -* ``api_gateway_endpoint_type`` - The endpoint configuration of the - deployed API Gateway which determines how the API will be accessed, - can be EDGE, REGIONAL, PRIVATE. Note this value can only be set as a - top level key and defaults to EDGE. For more information see - https://amzn.to/2LofApt - The following config values can either be specified per stage config or as a top level key which is not tied to a specific key. Whenever a stage specific configuration value is needed, the ``stages`` mapping @@ -50,6 +44,12 @@ be checked. will also be the URL prefix for your API (``https://endpoint/prefix/your-api``). +* ``api_gateway_endpoint_type`` - The endpoint configuration of the + deployed API Gateway which determines how the API will be accessed, + can be EDGE, REGIONAL. Note this value can only be set as a + top level key and defaults to EDGE. For more information see + https://amzn.to/2LofApt + * ``minimum_compression_size`` - An integer value that indicates the minimum compression size to apply to the API gateway. If this key is specified in both a stage specific config option diff --git a/tests/functional/test_awsclient.py b/tests/functional/test_awsclient.py index b2b776a93..b1205a342 100644 --- a/tests/functional/test_awsclient.py +++ b/tests/functional/test_awsclient.py @@ -64,11 +64,11 @@ def test_put_role_policy(stubbed_session): def test_rest_api_exists(stubbed_session): stubbed_session.stub('apigateway').get_rest_api( - restApiId='api').returns({}) + restApiId='api').returns({'id': 'api'}) stubbed_session.activate_stubs() awsclient = TypedAWSClient(stubbed_session) - assert awsclient.rest_api_exists('api') + assert awsclient.get_rest_api('api') stubbed_session.verify_stubs() @@ -81,7 +81,7 @@ def test_rest_api_not_exists(stubbed_session): stubbed_session.activate_stubs() awsclient = TypedAWSClient(stubbed_session) - assert not awsclient.rest_api_exists('api') + assert not awsclient.get_rest_api('api') stubbed_session.verify_stubs() diff --git a/tests/unit/deploy/test_planner.py b/tests/unit/deploy/test_planner.py index 9eaf0f687..d566cc943 100644 --- a/tests/unit/deploy/test_planner.py +++ b/tests/unit/deploy/test_planner.py @@ -940,50 +940,11 @@ def test_can_plan_rest_api(self): name='rest_api_url', variable_name='rest_api_url' ), - models.RecordResourceValue( - resource_type='rest_api', - resource_name='rest_api', - name='endpoint_type', - value=rest_api.endpoint_type - ), ] assert list(self.last_plan.messages.values()) == [ 'Creating Rest API\n' ] - def test_update_no_op_endpoint(self): - function = create_function_resource('function_name') - rest_api = models.RestAPI( - resource_name='rest_api', - swagger_doc={'swagger': '2.0'}, - minimum_compression='', - api_gateway_stage='api', - endpoint_type='EDGE', - lambda_function=function, - ) - self.remote_state.declare_resource_exists(rest_api) - self.remote_state.deployed_values['rest_api'] = { - 'rest_api_id': 'my_rest_api_id', - } - plan = self.determine_plan(rest_api) - - update = [ - p for p in plan - if isinstance(p, models.APICall) - and p.method_name == 'update_rest_api'] - - assert update == [ - models.APICall( - method_name='update_rest_api', - params={ - 'rest_api_id': Variable('rest_api_id'), - 'patch_operations': [{ - 'op': 'replace', - 'path': '/minimumCompressionSize', - 'value': ''}, - ], - })] - def test_can_update_rest_api(self): function = create_function_resource('function_name') rest_api = models.RestAPI( @@ -1000,6 +961,7 @@ def test_can_update_rest_api(self): } plan = self.determine_plan(rest_api) self.assert_loads_needed_variables(plan) + assert plan[4:] == [ models.StoreValue(name='rest_api_id', value='my_rest_api_id'), models.RecordResourceVariable( @@ -1027,6 +989,11 @@ def test_can_update_rest_api(self): 'account_id': Variable('account_id'), 'rest_api_id': Variable('rest_api_id')}, ), + models.APICall( + method_name='get_rest_api', + params={'rest_api_id': Variable('rest_api_id')}, + output_var='rest_api' + ), models.APICall( method_name='update_rest_api', params={ @@ -1036,8 +1003,11 @@ def test_can_update_rest_api(self): 'path': '/minimumCompressionSize', 'value': ''}, {'op': 'replace', - 'path': '/endpointConfiguration/types/EDGE', - 'value': 'REGIONAL'} + 'value': 'REGIONAL', + 'path': StringFormat( + '/endpointConfiguration/types/%s' % ( + '{rest_api[endpointConfiguration][types][0]}'), + ['rest_api'])} ], }, ), @@ -1062,12 +1032,6 @@ def test_can_update_rest_api(self): name='rest_api_url', variable_name='rest_api_url' ), - models.RecordResourceValue( - resource_type='rest_api', - resource_name='rest_api', - name='endpoint_type', - value=rest_api.endpoint_type - ), ] @@ -1526,7 +1490,7 @@ def test_rest_api_exists_no_deploy(self, no_deployed_values): remote_state = RemoteState( self.client, no_deployed_values) assert not remote_state.resource_exists(rest_api) - assert not self.client.rest_api_exists.called + assert not self.client.get_rest_api.called def test_rest_api_exists_with_existing_deploy(self): rest_api = self.create_rest_api_model() @@ -1537,11 +1501,11 @@ def test_rest_api_exists_with_existing_deploy(self): 'rest_api_id': 'my_rest_api_id', }] } - self.client.rest_api_exists.return_value = True + self.client.get_rest_api.return_value = {'apiId': 'my_rest_api_id'} remote_state = RemoteState( self.client, DeployedResources(deployed_resources)) assert remote_state.resource_exists(rest_api) - self.client.rest_api_exists.assert_called_with('my_rest_api_id') + self.client.get_rest_api.assert_called_with('my_rest_api_id') def test_rest_api_not_exists_with_preexisting_deploy(self): rest_api = self.create_rest_api_model() @@ -1552,11 +1516,11 @@ def test_rest_api_not_exists_with_preexisting_deploy(self): 'rest_api_id': 'my_rest_api_id', }] } - self.client.rest_api_exists.return_value = False + self.client.get_rest_api.return_value = {} remote_state = RemoteState( self.client, DeployedResources(deployed_resources)) assert not remote_state.resource_exists(rest_api) - self.client.rest_api_exists.assert_called_with('my_rest_api_id') + self.client.get_rest_api.assert_called_with('my_rest_api_id') def test_websocket_api_exists_no_deploy(self, no_deployed_values): rest_api = self.create_websocket_api_model() diff --git a/tests/unit/deploy/test_validate.py b/tests/unit/deploy/test_validate.py index 0d7768b38..dd89b6dff 100644 --- a/tests/unit/deploy/test_validate.py +++ b/tests/unit/deploy/test_validate.py @@ -246,7 +246,8 @@ def test_can_validate_endpoint_type(sample_app): config = Config.create( chalice_app=sample_app, api_gateway_endpoint_type='PRIVATE') - validate_endpoint_type(config) + with pytest.raises(ValueError): + validate_endpoint_type(config) def test_can_validate_feature_flags(sample_app): From 9c586f5a3cfb93c562751df56f3322a95e4c516a Mon Sep 17 00:00:00 2001 From: Kapil Thangavelu Date: Fri, 19 Jul 2019 23:52:16 -0400 Subject: [PATCH 3/6] support private api endpoints and api gw resource policy configuration --- CHANGELOG.rst | 2 +- chalice/config.py | 12 ++++ chalice/deploy/deployer.py | 32 +++++++++- chalice/deploy/models.py | 1 + chalice/deploy/planner.py | 42 +++++++------ chalice/deploy/validate.py | 15 ++++- docs/source/topics/configfile.rst | 10 +++- tests/unit/deploy/test_deployer.py | 36 +++++++++++- tests/unit/deploy/test_planner.py | 94 ++++++++++++++++++++++++------ tests/unit/deploy/test_validate.py | 25 ++++++-- 10 files changed, 223 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 36c9d27c1..5491c54a4 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -8,7 +8,7 @@ Next Release (TBD) * Add experimental support for websockets (`#1017 `__) -* API Gateway Endpoint Type Configuration (Regional and Edge) +* API Gateway Endpoint Type Configuration (`#1160 https://github.com/aws/chalice/pull/1160`__) diff --git a/chalice/config.py b/chalice/config.py index 27b38ed8a..e11df747b 100644 --- a/chalice/config.py +++ b/chalice/config.py @@ -229,6 +229,18 @@ def api_gateway_endpoint_type(self): return self._chain_lookup('api_gateway_endpoint_type', varies_per_chalice_stage=True) + @property + def api_gateway_endpoint_vpce(self): + # type: () -> str + return self._chain_lookup('api_gateway_endpoint_vpce', + varies_per_chalice_stage=True) + + @property + def api_gateway_policy(self): + # type: () -> Optional[Dict] + return self._chain_lookup('api_gateway_policy', + varies_per_chalice_stage=True) + @property def minimum_compression_size(self): # type: () -> int diff --git a/chalice/deploy/deployer.py b/chalice/deploy/deployer.py index 3b86a057a..656478c76 100644 --- a/chalice/deploy/deployer.py +++ b/chalice/deploy/deployer.py @@ -81,7 +81,7 @@ """ - +# pylint: disable=too-many-lines import json import os import textwrap @@ -459,6 +459,10 @@ def _create_rest_api_model(self, handler_name=auth.handler_string, stage_name=stage_name, ) authorizers.append(auth_lambda) + policy = config.api_gateway_policy + if (config.api_gateway_endpoint_type == 'PRIVATE' and not policy): + policy = self._get_default_private_api_policy(config) + return models.RestAPI( resource_name='rest_api', swagger_doc=models.Placeholder.BUILD_STAGE, @@ -467,8 +471,34 @@ def _create_rest_api_model(self, api_gateway_stage=config.api_gateway_stage, lambda_function=lambda_function, authorizers=authorizers, + policy=json.dumps(policy) ) + def _get_default_private_api_policy(self, config): + # type: (Config) -> Dict[str, Any] + statements = [{ + "Effect": "Allow", + "Principal": "*", + "Action": "execute-api:Invoke", + "Resource": [ + ("arn:aws:execute-api:{region_name}:" + "{account_id}:{rest_api_id}/*") + ]}, + {"Effect": "Deny", + "Principal": "*", + "Action": "execute-api:Invoke", + "Resource": [ + ("arn:aws:execute-api:{region_name}:" + "{account_id}:{rest_api_id}/*") + ], + "Condition": { + "StringNotEquals": { + "aws:SourceVpce": config.api_gateway_endpoint_vpce + } + }}] + + return {"Version": "2012-10-17", "Statement": statements} + def _create_websocket_api_model( self, config, # type: Config diff --git a/chalice/deploy/models.py b/chalice/deploy/models.py index aef047acc..416346225 100644 --- a/chalice/deploy/models.py +++ b/chalice/deploy/models.py @@ -191,6 +191,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] authorizers = attrib(default=Factory(list)) # type: List[LambdaFunction] def dependencies(self): diff --git a/chalice/deploy/planner.py b/chalice/deploy/planner.py index 2c15cbeb6..9ca11026f 100644 --- a/chalice/deploy/planner.py +++ b/chalice/deploy/planner.py @@ -865,6 +865,11 @@ def _plan_restapi(self, resource): 'account_id': Variable('account_id'), 'rest_api_id': Variable('rest_api_id')}, ), + models.APICall( + method_name='deploy_rest_api', + params={'rest_api_id': Variable('rest_api_id'), + 'api_gateway_stage': resource.api_gateway_stage}, + ), models.StoreValue( name='rest_api_url', value=StringFormat( @@ -904,12 +909,7 @@ def _plan_restapi(self, resource): name='rest_api_id', variable_name='rest_api_id', ), - models.APICall( - method_name='deploy_rest_api', - params={'rest_api_id': Variable('rest_api_id'), - 'api_gateway_stage': resource.api_gateway_stage}, - ), - ] + shared_plan_epilogue + ] else: deployed = self._remote_state.resource_deployed_values(resource) shared_plan_epilogue.insert( @@ -944,19 +944,23 @@ def _plan_restapi(self, resource): 'swagger_document': resource.swagger_doc, }, ), "Updating rest API\n"), - models.APICall( - method_name='deploy_rest_api', - params={'rest_api_id': Variable('rest_api_id'), - 'api_gateway_stage': resource.api_gateway_stage}, - ), - models.APICall( - method_name='add_permission_for_apigateway', - params={'function_name': function_name, - 'region_name': Variable('region_name'), - 'account_id': Variable('account_id'), - 'rest_api_id': Variable('rest_api_id')}, - ), - ] + shared_plan_epilogue + ] + + 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 def _get_role_arn(self, resource): diff --git a/chalice/deploy/validate.py b/chalice/deploy/validate.py index 9effbd1b1..68d4853d7 100644 --- a/chalice/deploy/validate.py +++ b/chalice/deploy/validate.py @@ -46,13 +46,26 @@ def validate_configuration(config): validate_unique_function_names(config) validate_feature_flags(config.chalice_app) validate_endpoint_type(config) + validate_resource_policy(config) + + +def validate_resource_policy(config): + # type: (Config) -> None + if config.api_gateway_endpoint_type != 'PRIVATE': + return + if config.api_gateway_policy: + return + if not config.api_gateway_endpoint_vpce: + raise ValueError( + ("Private Endpoints require api_gateway_policy or " + "api_gateway_endpoint_vpce specified")) def validate_endpoint_type(config): # type: (Config) -> None if not config.api_gateway_endpoint_type: return - valid_types = ('EDGE', 'REGIONAL') + valid_types = ('EDGE', 'REGIONAL', 'PRIVATE') if config.api_gateway_endpoint_type not in valid_types: raise ValueError( "api gateway endpoint type must be one of %s" % ( diff --git a/docs/source/topics/configfile.rst b/docs/source/topics/configfile.rst index b5951b5a9..9953b6b64 100644 --- a/docs/source/topics/configfile.rst +++ b/docs/source/topics/configfile.rst @@ -46,10 +46,18 @@ be checked. * ``api_gateway_endpoint_type`` - The endpoint configuration of the deployed API Gateway which determines how the API will be accessed, - can be EDGE, REGIONAL. Note this value can only be set as a + can be EDGE, REGIONAL, PRIVATE. Note this value can only be set as a top level key and defaults to EDGE. For more information see https://amzn.to/2LofApt +* ``api_gateway_endpoint_vpce`` - When configuring a Private API a VPC + Endpoint must be specified to configure a default resource policy on + the API if an explicit policy is not specified. + +* ``api_gateway_policy`` - The IAM resource policy for the REST API. + Note this should be a dictionary, and several variables are + interpolated in strings {account_id}, {region_name}, {rest_api_id}. + * ``minimum_compression_size`` - An integer value that indicates the minimum compression size to apply to the API gateway. If this key is specified in both a stage specific config option diff --git a/tests/unit/deploy/test_deployer.py b/tests/unit/deploy/test_deployer.py index 3f11cafb2..d603724ee 100644 --- a/tests/unit/deploy/test_deployer.py +++ b/tests/unit/deploy/test_deployer.py @@ -1,3 +1,4 @@ +import json import os import socket @@ -472,12 +473,16 @@ def create_config(self, app, app_name='lambda-only', iam_role_arn=None, policy_file=None, api_gateway_stage='api', autogen_policy=False, security_group_ids=None, - subnet_ids=None, reserved_concurrency=None, layers=None): + subnet_ids=None, reserved_concurrency=None, layers=None, + api_gateway_endpoint_type=None, + api_gateway_endpoint_vpce=None): kwargs = { 'chalice_app': app, 'app_name': app_name, 'project_dir': '.', 'api_gateway_stage': api_gateway_stage, + 'api_gateway_endpoint_type': api_gateway_endpoint_type, + 'api_gateway_endpoint_vpce': api_gateway_endpoint_vpce } if iam_role_arn is not None: # We want to use an existing role. @@ -699,6 +704,35 @@ def test_scheduled_event_models(self, scheduled_event_app): assert isinstance(event.lambda_function, models.LambdaFunction) assert event.lambda_function.resource_name == 'foo' + def test_can_build_private_rest_api(self, rest_api_app): + config = self.create_config(rest_api_app, + app_name='rest-api-app', + api_gateway_endpoint_type='PRIVATE', + api_gateway_endpoint_vpce='vpce-abc123') + builder = ApplicationGraphBuilder() + application = builder.build(config, stage_name='dev') + rest_api = application.resources[0] + assert isinstance(rest_api, models.RestAPI) + assert json.loads(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}/*')]}, + {'Action': 'execute-api:Invoke', + 'Condition': { + 'StringNotEquals': { + 'aws:SourceVpce': 'vpce-abc123'}}, + 'Effect': 'Deny', + 'Principal': '*', + 'Resource': [( + 'arn:aws:execute-api:{region_name}:' + '{account_id}:{rest_api_id}/*')]}], + } + def test_can_build_rest_api(self, rest_api_app): config = self.create_config(rest_api_app, app_name='rest-api-app', diff --git a/tests/unit/deploy/test_planner.py b/tests/unit/deploy/test_planner.py index d566cc943..3d68cc995 100644 --- a/tests/unit/deploy/test_planner.py +++ b/tests/unit/deploy/test_planner.py @@ -877,6 +877,35 @@ 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( @@ -903,9 +932,6 @@ def test_can_plan_rest_api(self): name='rest_api_id', variable_name='rest_api_id', ), - models.APICall(method_name='deploy_rest_api', - params={'rest_api_id': Variable('rest_api_id'), - 'api_gateway_stage': 'api'}), models.APICall( method_name='update_rest_api', params={ @@ -915,7 +941,7 @@ def test_can_plan_rest_api(self): 'path': '/minimumCompressionSize', 'value': '100', }], - }, + } ), models.APICall( method_name='add_permission_for_apigateway', @@ -926,6 +952,9 @@ def test_can_plan_rest_api(self): 'rest_api_id': Variable('rest_api_id'), } ), + models.APICall(method_name='deploy_rest_api', + params={'rest_api_id': Variable('rest_api_id'), + 'api_gateway_stage': 'api'}), models.StoreValue( name='rest_api_url', value=StringFormat( @@ -945,6 +974,43 @@ def test_can_plan_rest_api(self): 'Creating Rest API\n' ] + def test_can_update_rest_api_with_policy(self): + function = create_function_resource('function_name') + rest_api = models.RestAPI( + resource_name='rest_api', + swagger_doc={'swagger': '2.0'}, + minimum_compression='', + api_gateway_stage='api', + endpoint_type='EDGE', + policy="{'Statement': []}", + lambda_function=function, + ) + self.remote_state.declare_resource_exists(rest_api) + self.remote_state.deployed_values['rest_api'] = { + 'rest_api_id': 'my_rest_api_id', + } + plan = self.determine_plan(rest_api) + + assert plan[8].params == { + 'patch_operations': [ + {'op': 'replace', + 'path': '/minimumCompressionSize', + 'value': ''}, + {'op': 'replace', + 'path': StringFormat( + ("/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']) + }], + 'rest_api_id': Variable("rest_api_id") + } + def test_can_update_rest_api(self): function = create_function_resource('function_name') rest_api = models.RestAPI( @@ -977,18 +1043,6 @@ def test_can_update_rest_api(self): 'swagger_document': {'swagger': '2.0'}, }, ), - models.APICall( - method_name='deploy_rest_api', - params={'rest_api_id': Variable('rest_api_id'), - 'api_gateway_stage': 'api'}, - ), - models.APICall( - method_name='add_permission_for_apigateway', - params={'function_name': 'appname-dev-function_name', - 'region_name': Variable('region_name'), - 'account_id': Variable('account_id'), - 'rest_api_id': Variable('rest_api_id')}, - ), models.APICall( method_name='get_rest_api', params={'rest_api_id': Variable('rest_api_id')}, @@ -1007,7 +1061,8 @@ def test_can_update_rest_api(self): 'path': StringFormat( '/endpointConfiguration/types/%s' % ( '{rest_api[endpointConfiguration][types][0]}'), - ['rest_api'])} + ['rest_api'])}, + {'op': 'remove', 'path': '/policy'} ], }, ), @@ -1018,6 +1073,11 @@ def test_can_update_rest_api(self): 'account_id': Variable("account_id"), 'function_name': 'appname-dev-function_name'}, output_var=None), + models.APICall( + method_name='deploy_rest_api', + params={'rest_api_id': Variable('rest_api_id'), + 'api_gateway_stage': 'api'}, + ), models.StoreValue( name='rest_api_url', value=StringFormat( diff --git a/tests/unit/deploy/test_validate.py b/tests/unit/deploy/test_validate.py index dd89b6dff..e39b9cc59 100644 --- a/tests/unit/deploy/test_validate.py +++ b/tests/unit/deploy/test_validate.py @@ -13,6 +13,7 @@ from chalice.deploy.validate import validate_unique_function_names from chalice.deploy.validate import validate_feature_flags from chalice.deploy.validate import validate_endpoint_type +from chalice.deploy.validate import validate_resource_policy from chalice.deploy.validate import ExperimentalFeatureError @@ -234,6 +235,25 @@ def index(): sample_app.api.binary_types) is None +def test_can_validate_resource_policy(sample_app): + config = Config.create( + chalice_app=sample_app, api_gateway_endpoint_type='PRIVATE') + with pytest.raises(ValueError): + validate_resource_policy(config) + + config = Config.create( + chalice_app=sample_app, + api_gateway_endpoint_vpce='vpce-abc123', + api_gateway_endpoint_type='PRIVATE') + validate_resource_policy(config) + + config = Config.create( + chalice_app=sample_app, + api_gateway_policy={'Statement': []}, + api_gateway_endpoint_type='PRIVATE') + validate_resource_policy(config) + + def test_can_validate_endpoint_type(sample_app): config = Config.create( chalice_app=sample_app, api_gateway_endpoint_type='EDGE2') @@ -244,11 +264,6 @@ def test_can_validate_endpoint_type(sample_app): chalice_app=sample_app, api_gateway_endpoint_type='REGIONAL') validate_endpoint_type(config) - config = Config.create( - chalice_app=sample_app, api_gateway_endpoint_type='PRIVATE') - with pytest.raises(ValueError): - validate_endpoint_type(config) - def test_can_validate_feature_flags(sample_app): # The _features_used is marked internal because we don't want From 117adaea7ff8a81735de34d688dee73722486519 Mon Sep 17 00:00:00 2001 From: Kapil Thangavelu Date: Sat, 20 Jul 2019 15:37:27 -0400 Subject: [PATCH 4/6] cfn package support for private api endpoints --- chalice/deploy/deployer.py | 28 ++++------ chalice/deploy/swagger.py | 52 ++++++++++++++++- tests/unit/deploy/test_deployer.py | 19 +++---- tests/unit/deploy/test_swagger.py | 89 +++++++++++++++++++++++++++++- 4 files changed, 154 insertions(+), 34 deletions(-) diff --git a/chalice/deploy/deployer.py b/chalice/deploy/deployer.py index 656478c76..29b3736dc 100644 --- a/chalice/deploy/deployer.py +++ b/chalice/deploy/deployer.py @@ -480,23 +480,15 @@ 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}/*") - ]}, - {"Effect": "Deny", - "Principal": "*", - "Action": "execute-api:Invoke", - "Resource": [ - ("arn:aws:execute-api:{region_name}:" - "{account_id}:{rest_api_id}/*") - ], - "Condition": { - "StringNotEquals": { - "aws:SourceVpce": config.api_gateway_endpoint_vpce - } - }}] - + "Resource": ( + "arn:aws:execute-api:{region_name}:" + "{account_id}:{rest_api_id}/*"), + "Condition": { + "StringEquals": { + "aws:SourceVpce": config.api_gateway_endpoint_vpce + } + } + }] return {"Version": "2012-10-17", "Statement": statements} def _create_websocket_api_model( @@ -858,7 +850,7 @@ def __init__(self, swagger_generator): def handle_restapi(self, config, resource): # type: (Config, models.RestAPI) -> None swagger_doc = self._swagger_generator.generate_swagger( - config.chalice_app) + config.chalice_app, resource) resource.swagger_doc = swagger_doc diff --git a/chalice/deploy/swagger.py b/chalice/deploy/swagger.py index ea27bf742..e7ca18b58 100644 --- a/chalice/deploy/swagger.py +++ b/chalice/deploy/swagger.py @@ -1,11 +1,13 @@ import copy +import json import inspect -from typing import Any, List, Dict, Optional # noqa +from typing import Any, List, Dict, Optional, Union # noqa from chalice.app import Chalice, RouteEntry, Authorizer, CORSConfig # noqa from chalice.app import ChaliceAuthorizer from chalice.deploy.planner import StringFormat +from chalice.deploy.models import RestAPI # noqa from chalice.utils import to_cfn_resource_name @@ -32,8 +34,8 @@ def __init__(self, region, deployed_resources): self._region = region self._deployed_resources = deployed_resources - def generate_swagger(self, app): - # type: (Chalice) -> Dict[str, Any] + def generate_swagger(self, app, rest_api=None): + # type: (Chalice, Optional[RestAPI]) -> Dict[str, Any] api = copy.deepcopy(self._BASE_TEMPLATE) api['info']['title'] = app.app_name self._add_binary_types(api, app) @@ -238,6 +240,50 @@ 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/tests/unit/deploy/test_deployer.py b/tests/unit/deploy/test_deployer.py index d603724ee..b86542370 100644 --- a/tests/unit/deploy/test_deployer.py +++ b/tests/unit/deploy/test_deployer.py @@ -719,18 +719,13 @@ def test_can_build_private_rest_api(self, rest_api_app): {'Action': 'execute-api:Invoke', 'Effect': 'Allow', 'Principal': '*', - 'Resource': [ - ('arn:aws:execute-api:{region_name}:' - '{account_id}:{rest_api_id}/*')]}, - {'Action': 'execute-api:Invoke', - 'Condition': { - 'StringNotEquals': { - 'aws:SourceVpce': 'vpce-abc123'}}, - 'Effect': 'Deny', - 'Principal': '*', - 'Resource': [( + 'Resource': ( 'arn:aws:execute-api:{region_name}:' - '{account_id}:{rest_api_id}/*')]}], + '{account_id}:{rest_api_id}/*'), + 'Condition': { + 'StringEquals': { + 'aws:SourceVpce': 'vpce-abc123'}}}, + ] } def test_can_build_rest_api(self, rest_api_app): @@ -1271,7 +1266,7 @@ def test_can_generate_swagger_builder(self): p = SwaggerBuilder(generator) p.handle(config, rest_api) assert rest_api.swagger_doc == {'swagger': '2.0'} - generator.generate_swagger.assert_called_with(app) + generator.generate_swagger.assert_called_with(app, rest_api) class TestDeploymentPackager(object): diff --git a/tests/unit/deploy/test_swagger.py b/tests/unit/deploy/test_swagger.py index 6111611e2..97d482246 100644 --- a/tests/unit/deploy/test_swagger.py +++ b/tests/unit/deploy/test_swagger.py @@ -1,8 +1,10 @@ +import json + from chalice.deploy.swagger import SwaggerGenerator, CFNSwaggerGenerator from chalice import CORSConfig from chalice.app import CustomAuthorizer, CognitoUserPoolAuthorizer from chalice.app import IAMAuthorizer, Chalice - +from chalice.deploy.models import RestAPI import mock from pytest import fixture @@ -565,6 +567,91 @@ def foo(): } +def test_can_custom_resource_policy_with_cfn(sample_app): + swagger_gen = CFNSwaggerGenerator() + rest_api = RestAPI( + resource_name='dev', + swagger_doc={}, + lambda_function=None, + minimum_compression="", + api_gateway_stage="xyz", + endpoint_type="PRIVATE", + policy=json.dumps({ + 'Statement': [{ + "Effect": "Allow", + "Principal": "*", + "Action": "execute-api:Invoke", + "Resource": [( + "arn:aws:execute-api:{region_name}:" + "{account_id}:{rest_api_id}/*"), + "arn:aws:exceute-api:*:*:*/*" + ], + "Condition": { + "StringEquals": { + "aws:SourceVpce": "vpce-abc123" + } + } + }] + }) + ) + + doc = swagger_gen.generate_swagger(sample_app, rest_api) + assert doc['x-amazon-apigateway-policy'] == { + 'Statement': [{ + 'Action': 'execute-api:Invoke', + 'Condition': {'StringEquals': { + 'aws:SourceVpce': 'vpce-abc123'}}, + 'Effect': 'Allow', + 'Principal': '*', + 'Resource': [ + {'Fn:Sub': ( + 'arn:aws:execute-api:${AWS::Region}' + ':${AWS:AccoundId}:*/*')}, + "arn:aws:exceute-api:*:*:*/*"] + }] + } + + +def test_can_auto_resource_policy_with_cfn(sample_app): + swagger_gen = CFNSwaggerGenerator() + rest_api = RestAPI( + resource_name='dev', + swagger_doc={}, + lambda_function=None, + minimum_compression="", + api_gateway_stage="xyz", + endpoint_type="PRIVATE", + policy=json.dumps({ + 'Statement': [{ + "Effect": "Allow", + "Principal": "*", + "Action": "execute-api:Invoke", + "Resource": ( + "arn:aws:execute-api:{region_name}:" + "{account_id}:{rest_api_id}/*"), + "Condition": { + "StringEquals": { + "aws:SourceVpce": "vpce-abc123" + } + } + }] + }) + ) + + doc = swagger_gen.generate_swagger(sample_app, rest_api) + assert doc['x-amazon-apigateway-policy'] == { + 'Statement': [{ + 'Action': 'execute-api:Invoke', + 'Condition': {'StringEquals': { + 'aws:SourceVpce': 'vpce-abc123'}}, + 'Effect': 'Allow', + 'Principal': '*', + 'Resource': {'Fn:Sub': ( + 'arn:aws:execute-api:${AWS::Region}:${AWS:AccoundId}:*/*')} + }] + } + + def test_will_custom_auth_with_cfn(sample_app): swagger_gen = CFNSwaggerGenerator() From 2f0038e29be13206d7f54b8ec3fb8d2f903643d2 Mon Sep 17 00:00:00 2001 From: Kapil Thangavelu Date: Wed, 24 Jul 2019 12:41:25 -0400 Subject: [PATCH 5/6] address review feedback, use static policy and simplify --- chalice/config.py | 8 ++--- chalice/deploy/deployer.py | 6 ++-- chalice/deploy/models.py | 3 +- chalice/deploy/planner.py | 14 -------- chalice/deploy/swagger.py | 51 ++++-------------------------- chalice/deploy/validate.py | 9 ++++++ tests/unit/deploy/test_deployer.py | 7 ++-- tests/unit/deploy/test_planner.py | 39 ++--------------------- tests/unit/deploy/test_swagger.py | 26 ++++++--------- tests/unit/deploy/test_validate.py | 15 +++++++++ 10 files changed, 51 insertions(+), 127 deletions(-) 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( From 314ab7804d8d92907546d05bc33253433858088e Mon Sep 17 00:00:00 2001 From: Kapil Thangavelu Date: Wed, 24 Jul 2019 18:03:25 -0400 Subject: [PATCH 6/6] address review feedback, allow api gw resource policy from file --- CHANGELOG.rst | 2 ++ chalice/config.py | 8 ++++---- chalice/deploy/deployer.py | 14 +++++++++++--- chalice/deploy/models.py | 2 +- chalice/deploy/swagger.py | 2 +- chalice/deploy/validate.py | 8 ++++---- docs/source/topics/configfile.rst | 14 ++++++++------ tests/unit/deploy/test_deployer.py | 26 +++++++++++++++++++++++--- tests/unit/deploy/test_swagger.py | 10 +++++----- tests/unit/deploy/test_validate.py | 4 ++-- 10 files changed, 61 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5491c54a4..b15aa915b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -11,6 +11,8 @@ Next Release (TBD) * API Gateway Endpoint Type Configuration (`#1160 https://github.com/aws/chalice/pull/1160`__) +* API Gateway Resource Policy Configuration + (`#1160 https://github.com/aws/chalice/pull/1160`__) 1.9.1 ===== diff --git a/chalice/config.py b/chalice/config.py index 77e0bd214..ffee34474 100644 --- a/chalice/config.py +++ b/chalice/config.py @@ -236,10 +236,10 @@ def api_gateway_endpoint_vpce(self): varies_per_chalice_stage=True) @property - def api_gateway_policy(self): - # type: () -> Dict - return self._chain_lookup('api_gateway_policy', - varies_per_chalice_stage=True) or {} + def api_gateway_policy_file(self): + # type: () -> str + return self._chain_lookup('api_gateway_policy_file', + varies_per_chalice_stage=True) @property def minimum_compression_size(self): diff --git a/chalice/deploy/deployer.py b/chalice/deploy/deployer.py index 197921053..3b0a43689 100644 --- a/chalice/deploy/deployer.py +++ b/chalice/deploy/deployer.py @@ -459,9 +459,17 @@ def _create_rest_api_model(self, handler_name=auth.handler_string, stage_name=stage_name, ) authorizers.append(auth_lambda) - policy = config.api_gateway_policy - if (config.api_gateway_endpoint_type == 'PRIVATE' and not policy): - policy = self._get_default_private_api_policy(config) + + policy = None + policy_path = config.api_gateway_policy_file + if (config.api_gateway_endpoint_type == 'PRIVATE' and not policy_path): + policy = models.IAMPolicy( + document=self._get_default_private_api_policy(config)) + elif policy_path: + policy = models.FileBasedIAMPolicy( + document=models.Placeholder.BUILD_STAGE, + filename=os.path.join( + config.project_dir, '.chalice', policy_path)) return models.RestAPI( resource_name='rest_api', diff --git a/chalice/deploy/models.py b/chalice/deploy/models.py index 726fe00ca..d84b3bab9 100644 --- a/chalice/deploy/models.py +++ b/chalice/deploy/models.py @@ -192,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=Factory(dict)) # type: Dict[str, Any] + policy = attrib(default=None) # type: Optional[IAMPolicy] authorizers = attrib(default=Factory(list)) # type: List[LambdaFunction] def dependencies(self): diff --git a/chalice/deploy/swagger.py b/chalice/deploy/swagger.py index 8ba4bb3b9..c961ecc44 100644 --- a/chalice/deploy/swagger.py +++ b/chalice/deploy/swagger.py @@ -45,7 +45,7 @@ def generate_swagger(self, app, rest_api=None): 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 + api['x-amazon-apigateway-policy'] = rest_api.policy.document def _add_binary_types(self, api, app): # type: (Dict[str, Any], Chalice) -> None diff --git a/chalice/deploy/validate.py b/chalice/deploy/validate.py index ab3d110b0..9861aa693 100644 --- a/chalice/deploy/validate.py +++ b/chalice/deploy/validate.py @@ -58,15 +58,15 @@ def validate_resource_policy(config): "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: + if config.api_gateway_policy_file and config.api_gateway_endpoint_vpce: raise ValueError( - "Can only specify one of api_gateway_policy and " + "Can only specify one of api_gateway_policy_file and " "api_gateway_endpoint_vpce") - if config.api_gateway_policy: + if config.api_gateway_policy_file: return if not config.api_gateway_endpoint_vpce: raise ValueError( - ("Private Endpoints require api_gateway_policy or " + ("Private Endpoints require api_gateway_policy_file or " "api_gateway_endpoint_vpce specified")) diff --git a/docs/source/topics/configfile.rst b/docs/source/topics/configfile.rst index 9953b6b64..0a6a39978 100644 --- a/docs/source/topics/configfile.rst +++ b/docs/source/topics/configfile.rst @@ -51,12 +51,14 @@ be checked. https://amzn.to/2LofApt * ``api_gateway_endpoint_vpce`` - When configuring a Private API a VPC - Endpoint must be specified to configure a default resource policy on - the API if an explicit policy is not specified. - -* ``api_gateway_policy`` - The IAM resource policy for the REST API. - Note this should be a dictionary, and several variables are - interpolated in strings {account_id}, {region_name}, {rest_api_id}. + Endpoint id must be specified to configure a default resource policy on + the API if an explicit policy is not specified. This value can be a + list or a string of endpoint ids. + +* ``api_gateway_policy_file`` - A file pointing to an IAM resource + policy for the REST API. If not specified chalice will autogenerate + this policy when endpoint_type is PRIVATE. This filename is relative + to the ``.chalice`` directory. * ``minimum_compression_size`` - An integer value that indicates the minimum compression size to apply to the API gateway. If diff --git a/tests/unit/deploy/test_deployer.py b/tests/unit/deploy/test_deployer.py index 0aca359dd..3f6d44dbc 100644 --- a/tests/unit/deploy/test_deployer.py +++ b/tests/unit/deploy/test_deployer.py @@ -474,12 +474,15 @@ def create_config(self, app, app_name='lambda-only', autogen_policy=False, security_group_ids=None, subnet_ids=None, reserved_concurrency=None, layers=None, api_gateway_endpoint_type=None, - api_gateway_endpoint_vpce=None): + api_gateway_endpoint_vpce=None, + api_gateway_policy_file=None, + project_dir='.'): kwargs = { 'chalice_app': app, 'app_name': app_name, - 'project_dir': '.', + 'project_dir': project_dir, 'api_gateway_stage': api_gateway_stage, + 'api_gateway_policy_file': api_gateway_policy_file, 'api_gateway_endpoint_type': api_gateway_endpoint_type, 'api_gateway_endpoint_vpce': api_gateway_endpoint_vpce } @@ -712,7 +715,7 @@ 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 rest_api.policy == { + assert rest_api.policy.document == { 'Version': '2012-10-17', 'Statement': [ {'Action': 'execute-api:Invoke', @@ -725,6 +728,23 @@ def test_can_build_private_rest_api(self, rest_api_app): ] } + def test_can_build_private_rest_api_custom_policy( + self, tmpdir, rest_api_app): + config = self.create_config(rest_api_app, + app_name='rest-api-app', + api_gateway_policy_file='foo.json', + api_gateway_endpoint_type='PRIVATE', + project_dir=str(tmpdir)) + tmpdir.mkdir('.chalice').join('foo.json').write( + serialize_to_json({'Version': '2012-10-17', 'Statement': []})) + + builder = ApplicationGraphBuilder() + application = builder.build(config, stage_name='dev') + rest_api = application.resources[0] + rest_api.policy.document == { + 'Version': '2012-10-17', 'Statement': [] + } + def test_can_build_rest_api(self, rest_api_app): config = self.create_config(rest_api_app, app_name='rest-api-app', diff --git a/tests/unit/deploy/test_swagger.py b/tests/unit/deploy/test_swagger.py index c4f774a04..99e4e86d4 100644 --- a/tests/unit/deploy/test_swagger.py +++ b/tests/unit/deploy/test_swagger.py @@ -2,7 +2,7 @@ from chalice import CORSConfig from chalice.app import CustomAuthorizer, CognitoUserPoolAuthorizer from chalice.app import IAMAuthorizer, Chalice -from chalice.deploy.models import RestAPI +from chalice.deploy.models import RestAPI, IAMPolicy import mock from pytest import fixture @@ -574,7 +574,7 @@ def test_can_custom_resource_policy_with_cfn(sample_app): minimum_compression="", api_gateway_stage="xyz", endpoint_type="PRIVATE", - policy={ + policy=IAMPolicy({ 'Statement': [{ "Effect": "Allow", "Principal": "*", @@ -589,7 +589,7 @@ def test_can_custom_resource_policy_with_cfn(sample_app): } } }] - } + }) ) doc = swagger_gen.generate_swagger(sample_app, rest_api) @@ -616,7 +616,7 @@ def test_can_auto_resource_policy_with_cfn(sample_app): minimum_compression="", api_gateway_stage="xyz", endpoint_type="PRIVATE", - policy={ + policy=IAMPolicy({ 'Statement': [{ "Effect": "Allow", "Principal": "*", @@ -628,7 +628,7 @@ def test_can_auto_resource_policy_with_cfn(sample_app): } } }] - } + }) ) doc = swagger_gen.generate_swagger(sample_app, rest_api) diff --git a/tests/unit/deploy/test_validate.py b/tests/unit/deploy/test_validate.py index 302742818..8e9ab082f 100644 --- a/tests/unit/deploy/test_validate.py +++ b/tests/unit/deploy/test_validate.py @@ -256,14 +256,14 @@ def test_can_validate_resource_policy(sample_app): config = Config.create( chalice_app=sample_app, - api_gateway_policy={'Statement': []}, + api_gateway_policy_file='xyz.json', 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_policy_file='bar.json', api_gateway_endpoint_type='PRIVATE') with pytest.raises(ValueError): validate_resource_policy(config)