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

Feature Proposal: Customize the default IAM role on function level #4313

Open
balassy opened this Issue Sep 26, 2017 · 13 comments

Comments

Projects
None yet
6 participants
@balassy

balassy commented Sep 26, 2017

Description

Currently when I define IAM role statements on function level, they will override the default generated IAM role customized at the provider level. However there can be functions that would need additional permissions to operate properly. Following the principle of least privilege it would be great, if the IAM role generated for a function could not only replace the default role, but there would be an option to extend or customize the provider level role on function level.

Examples:

  • I have multiple functions, but only 1 of them needs to access S3.
  • I have multiple functions that needs to access S3, but only 1 of them needs to delete from S3.

If this is already possible, please extend to existing documentation to describe how.

Additional Data

  • Serverless Framework Version you're using: 1.23.0

Thanks for creating and maintaining this project!

@horike37

This comment has been minimized.

Member

horike37 commented Oct 9, 2017

Thank you for sending a great feature proposal @balassy 💯
We are likely to need more feedback from someone else so that we know the detail of specification or how to set up looks like in serverless.yml 👍

@balassy

This comment has been minimized.

balassy commented Oct 9, 2017

Thanks for the feedback, Takahiro.

Currently you can define an iamRoleStatements property on the provider level with effect, action and resource, and a role element with a name on function level. I propose a new extendIamRoleStatements (any other name can work well of course) element on function level which allows configuring effects, actions and resources, and the union of the provider level and the function level statements is applied to the function during deployment.

How can I help?

@theburningmonk

This comment has been minimized.

theburningmonk commented Dec 6, 2017

+100 to this, many people (including folks at AWS) I speak to wants the ability to define per-function policies easily, you can already do it with the serverless framework but it's rather clumsy and adds a lot of friction (requires setting up the role as custom CF resource, and have to manually include the permissions the Serverless framework adds for you out by default, eg. CWLogs), and as such I don’t know of anyone who actually do this in production.

service: new-service

provider:
  name: aws
  iamRoleStatements:
    -  Effect: "Allow"
       Action:
         - "s3:ListBucket"
       Resource:
         Fn::Join:
           - ""
           - - "arn:aws:s3:::"
             - Ref: ServerlessDeploymentBucket

functions:
  func0:
    ...
    iamRoleStatements:
      -  Effect: "Allow"
         Action:
           - "s3:PutObject"
         Resource:
           Fn::Join:
             - ""
             - - "arn:aws:s3:::"
               - Ref: ServerlessDeploymentBucket
               - "/*"
    func1:
      ...
    func2:
      ...

In the above example, func1 and func2 would inherit the IAM role created at the service level, with the permission for s3:ListBuckets. But since func0 specifies its own IAM statement, these statements should be concatenated with the service level IAM statements to create a new role, and assigned to the func0 function, giving it permissions for both s3:ListBuckets and s3:PutObject.

@horike37

This comment has been minimized.

Member

horike37 commented Dec 13, 2017

Thank you for the proposal @theburningmonk and @balassy 👍
I almost agree with #4313 (comment) example.

Additionally, we might want to consider the case which overriding the IAM role created at the service level instead of inheritance.
imo, as default, inheriting the service level, and you can completely override by adding optional property under iamRoleStatements at the function level.
It would be something like this.

functions:
  func0: 
    iamRoleStatements:
      type: override #you can specify `override` or `inherit `
      definitation:
        -  Effect: "Allow"
           Action:
             - "s3:PutObject"
           Resource:
             Fn::Join:
               - ""
               - - "arn:aws:s3:::"
                 - Ref: ServerlessDeploymentBucket
                 - "/*"

You can omit type property. In this case, the IAM role at the function level inherits it at the service level.

functions:
  func0: 
    iamRoleStatements:
      -  Effect: "Allow"
         Action:
           - "s3:PutObject"
         Resource:
           Fn::Join:
             - ""
             - - "arn:aws:s3:::"
               - Ref: ServerlessDeploymentBucket
               - "/*"

What do you think?
cc @serverless/framework-maintainers

@theburningmonk

This comment has been minimized.

theburningmonk commented Dec 13, 2017

@horike37 that's a good shout to allow for overriding the inherited permissions, looks good to me

@tommedema

This comment has been minimized.

tommedema commented Dec 13, 2017

@horike37 this looks good with the exception that it's not entirely consistent with the normal function-level parameters behavior. These normally override (rather than inherit) from the provider-level parameters. It might be worthwhile to explicitly deviate from this here, but it's good to be aware of this before doing so.

Perhaps having a boolean inherit (true/false) would make more sense, and to have it default to false rather than true (because this is more secure and more consistent). Note that if you override, I would still except the serverless default roles to be effective.

@glicht

This comment has been minimized.

glicht commented Jan 21, 2018

Is there any progress with this feature request? I would really love seeing this as part of the framework. It will make life easier and providing control over inherit/override sounds like the way to go.

I would like to help out implementing this if no one is currently working on this.

@glicht

This comment has been minimized.

glicht commented Jan 31, 2018

After looking into this a bit and learning how plugins work, I've implement this as a separate plugin. Please take a look at: https://github.com/functionalone/serverless-iam-roles-per-function for a full description of the plugin. Feedback and comments are welcome.

@theburningmonk

This comment has been minimized.

theburningmonk commented Feb 1, 2018

@glicht this looks great, I'll give it a spin tomorrow and let you know if I have any feedbacks, really great work!

@balassy

This comment has been minimized.

balassy commented Feb 2, 2018

Thank you @glicht, this looks awesome!

@jthomerson

This comment has been minimized.

Contributor

jthomerson commented Jun 26, 2018

@horike37, et al: the plugin by @glicht is great, and does exactly what's needed - makes it easy to define a custom role per function, and to extend the default role. I'd really like to see this in core so that we don't have to use a plugin for it. Also, right now @glicht has to duplicate a lot of logic that's already in core for creating the default role (e.g. CloudWatch permissions, SQS/SNS/VPC/etc depending on the events for the function). This seems like a natural fit for Serverless core.

@glicht any thought of making a PR?

@horike37

This comment has been minimized.

Member

horike37 commented Jun 26, 2018

Fully agreed with @jthomas 👍
Would be great if we can have this feature in sls core.

@tommedema

This comment has been minimized.

tommedema commented Aug 23, 2018

This is a much needed feature. In addition, the current documentation is rather vague on the subject.

It says that if you define function level iamRoleStatements, the provider level ones will be gone. And you'd have to recreate roles for cloudwatch logs etc.

Yet in my case this was totally not happening: I set s3:* allow iamRoleStatements at the function level, and the function was still writing logs to cloudwatch just fine (apparently there was no permission issue there), but it failed to write to s3. When I moved the permissions to the provider level it could write to s3 too. Very strange.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment