Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

first pass at iam policy example for deploy user #894

Merged
merged 4 commits into from
Sep 29, 2018

Conversation

Bartvds
Copy link
Contributor

@Bartvds Bartvds commented May 26, 2017

This is basically a collection from #244

I split it up into statements so people can remove chunks easily. Sadly the policy lingo is a bit limited and the only way to comment a Statement is a Sid value with only CamelCase value.

Anyway, I'm by no means an expert on zappa or IAM and this is cargo culted so please review.

Maybe this should be provided with some example (patterns) to make it more specific? Put in markdown instead of JSON?

@coveralls
Copy link

coveralls commented May 26, 2017

Coverage Status

Coverage remained the same at 72.615% when pulling 175f2b3 on Bartvds:policy into 9ff3e18 on Miserlou:master.

@jakul
Copy link
Contributor

jakul commented May 31, 2017

@Bartvds CreateMultipartUpload is needed for me!

Calling deploy for stage uatold..
Warning! You may have a namespace collision with app.wsgi.the_app! You may want to rename that file.
Warning! Your project and virtualenv have the same name! You may want to re-create your venv with a new name, or explicitly define a 'project_name', as this may cause errors.
Downloading and installing dependencies..
 - cffi==1.9.1: Using locally cached manylinux wheel
 - cryptography==1.7.2: Warning! Using precompiled lambda package version 1.4 instead!
 - greenlet==0.4.12: Using locally cached manylinux wheel
 - pymongo==3.4.0: Using locally cached manylinux wheel
Packaging project as zip..
Uploading s-user-old-uatold-1496245451.zip (23.5MiB)..
  0%|                                                                    | 0.00/24.6M [00:00<?, ?B/s]Failed to upload s-user-old-uatold-1496245451.zip to zappa-uploads-uat-old/s-user-old-uatold-1496245451.zip: An error occurred (AccessDenied) when calling the CreateMultipartUpload operation: Access Denied

Error: Unable to upload to S3. Quitting.

Perhaps you didn't need it because your sample zip file is small and mine is large?

@jakul
Copy link
Contributor

jakul commented Jun 15, 2017

+1 for the rest of the permissions though. I've copied/pasted these into my project and they work fine for all my usecases.

@ebertti
Copy link

ebertti commented Jun 27, 2017

👍

@levesquejf
Copy link

lambda:GetFunctionConfiguration is required to update an existing deployment.

Persons comment said this permission was needed
@coveralls
Copy link

Coverage Status

Coverage increased (+0.9%) to 73.469% when pulling 6f6f52e on Bartvds:policy into 9ff3e18 on Miserlou:master.

1 similar comment
@coveralls
Copy link

coveralls commented Jul 24, 2017

Coverage Status

Coverage increased (+0.9%) to 73.469% when pulling 6f6f52e on Bartvds:policy into 9ff3e18 on Miserlou:master.

@philvarner
Copy link

philvarner commented Nov 11, 2017

I'm interested in doing something about permissions within Zappa. I've been thinking about these things:

  1. Improve the documentation such that it's much more apparent that the permissions in the policy given to the default role are far broader than they strictly need to be, and that most production deployments (not just those in a "corporate environment") will probably need to use manually-managed roles configured via 'role_name'.

  2. Get the above example policy committed so at least there are examples to work from that aren't just in github threads (like this one).

  3. Precisely document which Zappa features require which permissions in the current policy and the example minimal policy, e.g., if you don't turn X-Ray on, you don't need the X-Ray permissions (that's a pretty obvious one, but many of the other ones aren't -- particularly the ENI ones). Part of this is separating the permissions required for Zappa (e.g., just no-op in the lambda) and which are granted so that most code that uses common AWS services (e.g., dynamodb) will just work without needing to explicitly grant 'extra_permissions'.

  4. Convert the existing ASSUME_POLICY and ATTACH_POLICY to troposphere code. One of the primary benefits is simply being able to document exactly why a permission is being added to the policy, without needing to break the JSON string literal up into a bunch of concatenated strings and comments.

  5. As for making this more configurable within Zappa, I was thinking that there could be a set of policies instead of a single one (i.e., ATTACH_POLICY) that could be specified in the Zappa config. One option might be a parameter "canned_role" with options "all" (for the existing one, which is the default for backwards compatibility) or "minimal" (just to deploy a no-op handler), and then also add whatever additional permissions necessary for the features you've enabled, e.g., xray. My feeling is that users should be required to add whatever permissions their code needs via 'extra_permissions' rather than adding the common ones (e.g., dynamodb) by default, since in most prod envs people aren't going to want to grant a single Lambda access to all their dynamo/sqs/s3/etc resources. Another option could be to specify a function name to pass the troposphere Template object (from the previous item) into, and then you could write whatever code you needed to to modify the permissions without having to dump your 'extra_permissions' JSON into the zappa config file.

@philvarner
Copy link

Created a PR for the documentation issue (#1 above) #1233

@philvarner
Copy link

the apigateway permissions can be reduced to "apigateway:*", as the only one that's missing is HEAD, and adding that doesn't seem to be a concern.

@coltyharrison
Copy link

When using zappa cerify, I ran into this error:

botocore.errorfactory.BadRequestException: An error occurred (BadRequestException) when calling the CreateDomainName operation: Unable to associate certificate [CERTIFICATE ARN] with CloudFront. This error may prevent the domain name [DOMAIN NAME] from being used in API Gateway for up to 40 minutes. Please ensure the certificate domain name matches the requested domain name, and that this user has permission to call cloudfront:UpdateDistribution on '*' resources.

Can we add cloudfront:UpdateDistribution on * resources to this?

@dpretty
Copy link
Contributor

dpretty commented Aug 16, 2018

Thanks to everyone who's been working on this, it helps a lot!

One issue though is the Action "ec2:DescribeVpcsRequest" appears to be invalid, it's not listed here: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_Operations.html

I assume if this policy has been working for people, then this action can just be omitted.

Pinging #244 (comment #244 (comment)) as that appears to be the original source of "ec2:DescribeVpcsRequest"

@dpretty
Copy link
Contributor

dpretty commented Aug 16, 2018

@jakul It appears CreateMultipartUpload isn't a valid S3 action: https://docs.aws.amazon.com/IAM/latest/UserGuide/list_amazons3.html#amazons3-actions-as-permissions

It appears s3:PutObject is all that's required for multipart uploads: https://docs.aws.amazon.com/AmazonS3/latest/dev/mpuAndPermissions.html

You've probably already resolved this, as your comment is over a year old, but I figure it's worth mentioning for anyone else looking into this issue.

@yiannistri
Copy link

yiannistri commented Aug 19, 2018

Thanks for all the contributions so far. I've had to set up a VPC recently so I had to add the following permissions to my policy.

{
    "Effect": "Allow",
    "Action": [
        "ec2:CreateNetworkInterface",
        "ec2:DeleteNetworkInterface",
        "ec2:DescribeNetworkInterfaces"
    ],
    "Resource":"*"
}

They are also documented here

@jneves jneves merged commit 0da9eda into Miserlou:master Sep 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.