From 314ab7804d8d92907546d05bc33253433858088e Mon Sep 17 00:00:00 2001 From: Kapil Thangavelu Date: Wed, 24 Jul 2019 18:03:25 -0400 Subject: [PATCH] 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)