Add support for per-stage IAM policy files #272
Conversation
This is handled by .chalice/deployed.json. To accomplish this, I had to update the signature of the API Gateway deployer to pass in the deployed lambda_arn, which is needed for the swagger document. I was considering using the "existing resources" object, but it's not clear if allowing a partially hydrated deployed resources object is a good idea or not. For now I'm just explicitly passing in the data I need from previous deployement steps (just the lambda_arn).
Allows us to add more functionality in the future without having to mess with the signature.
This is may potentially get more involved once we account for multiple files. This object all the logic out of coordinating with the policy generation code into its own class. This allows the ApplicationPolicyHandler to focus solely on whether or not we should be loading policy files (and from where).
Previously there was only a single .chalice/policy.json file that you could specify. Now you can provide a stage specific policy file to control this value. The default value for this file is .chalice/policy-<stage>.json, but to help with migration the .chalice/policy.json file is still supported for the dev stage if the .chalice/policy-dev.json file does not exist.
jamesls
requested review from
dstufft,
stealthycoin,
JordonPhillips and
kyleknap
Codecov Report
@@ Coverage Diff @@
## package #272 +/- ##
===========================================
+ Coverage 85.75% 85.87% +0.11%
===========================================
Files 17 18 +1
Lines 1664 1678 +14
Branches 201 202 +1
===========================================
+ Hits 1427 1441 +14
Misses 184 184
Partials 53 53
Continue to review full report at Codecov.
|
| LambdaDeploymentPackager(), | ||
| ApplicationPolicyHandler(osutils), | ||
| # TODO: remove duplication here. |
There was a problem hiding this comment.
Did you miss a todo here? Seems like you can share the instance of the policy generator.
There was a problem hiding this comment.
Ended up being removed due to refactoring in the env var PR (https://github.com/awslabs/chalice/pull/273/files#diff-7637287bcfcc038a2e99fb26ac726348R32)
| assert generated == json.loads(previous_policy) | ||
|
|
||
|
|
||
| def test_can_provide_stage_specific_policy_for_other_stage(app_policy, |
There was a problem hiding this comment.
Is there a test for when autogen_policy is True, but the stage is not dev? Looks like you have one for when the stage is dev, but not when it is the non default stage.
There was a problem hiding this comment.
Just pushed a commit that adds a test for this case.
This reworks some of the existing IAM policy logic to properly be "stage aware":
.chalice/policy-<stage>.jsonfile.iam_policy_filedev, stage we'll also check.chalice/policy.json(the pre-existing filename) if there's no.chalice/policy-dev.jsonfile.Note that this builds on #271, so you'll see that diff as well until #271 is merged.
Also, I'll be sending a follow up PR that adds docs/upgrade notes for all the package related changes.