Skip to content

Commit

Permalink
address review feedback, allow api gw resource policy from file
Browse files Browse the repository at this point in the history
  • Loading branch information
kapilt committed Jul 25, 2019
1 parent 2f0038e commit 314ab78
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 29 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
=====
Expand Down
8 changes: 4 additions & 4 deletions chalice/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
14 changes: 11 additions & 3 deletions chalice/deploy/deployer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion chalice/deploy/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion chalice/deploy/swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions chalice/deploy/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))


Expand Down
14 changes: 8 additions & 6 deletions docs/source/topics/configfile.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 23 additions & 3 deletions tests/unit/deploy/test_deployer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/deploy/test_swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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": "*",
Expand All @@ -589,7 +589,7 @@ def test_can_custom_resource_policy_with_cfn(sample_app):
}
}
}]
}
})
)

doc = swagger_gen.generate_swagger(sample_app, rest_api)
Expand All @@ -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": "*",
Expand All @@ -628,7 +628,7 @@ def test_can_auto_resource_policy_with_cfn(sample_app):
}
}
}]
}
})
)

doc = swagger_gen.generate_swagger(sample_app, rest_api)
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/deploy/test_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 314ab78

Please sign in to comment.