fix: update the method by which IAM roles are applied #223

edaniszewski · 2020-06-30T21:43:32Z

This implementation is a pretty sizable departure of the previous implementation attempt (#219) which did not work as anticipated (#222) because it seems like the documented configuration/behavior for setting via deploymentmanager does not work as expected.

This approach does offer a bit better granularity of how functions IAM policies can be defined though, which is nice.

IAM roles are applied after a function is deployed/updated.

Serverless: Packaging service...
Serverless: Excluding development dependencies...
Serverless: Compiling function "first"...
Serverless: Uploading artifacts...
Serverless: Artifacts successfully uploaded...
Serverless: Updating deployment...
Serverless: Checking deployment update progress...
..........................
Serverless: Done...
Serverless: Setting IAM policies...
Service Information
...

Note: In order to apply IAM policies to functions, the serviceaccount you are using for serverless deploys will need the cloudfunctions.functions.setIamPolicy permission. This permission is not granted by the default cloudfunctions roles so will require you to create a custom policy.

If you do not have the appropriate permissions, you should get an error message stating you lack the permission:

  Error --------------------------------------------------
 
  Error: Permission 'cloudfunctions.functions.setIamPolicy' denied on resource 'projects/project/locations/us-central1/functions/test-svc-dev-first' (or resource may not exist).
      at Gaxios._request (/Users/edaniszewski/dev/edaniszewski/serverless-google-cloudfunctions/node_modules/gaxios/src/gaxios.ts:108:15)
      at processTicksAndRejections (internal/process/task_queues.js:89:5)
      at JWT.requestAsync (/Users/edaniszewski/dev/edaniszewski/serverless-google-cloudfunctions/node_modules/google-auth-library/build/src/auth/oauth2client.js:341:18)

IAM policies can be set a number of different ways.

Globally

service:  test-svc

provider:
  name: google
  runtime: python37
  region: us-central1
  project: project-name
  credentials: ~/.gcloud/keyfile.json
  
  # allowUnauthenticated mirrors the gcloud command line --allow-unauthenticated which
  # adds the 'roles/cloudfunctions.invoker' role for the 'allUsers' member. Setting it globally
  # applies it to all configured functions.
  allowUnauthenticated: true

  # Finer-grained IAM settings. This allows you to specify any number of bindings for functions,
  # whether they are for built-in roles or custom roles.  Setting it globally here applies the bindings
  # to all configured functions.
  iam:
    bindings:
    - role: roles/cloudfunctions.invoker
      members:
      - user:[email protected]

...

Per-function

service:  test-svc

provider:
  name: google
  runtime: python37
  region: us-central1
  project: project-name
  credentials: ~/.gcloud/keyfile.json

functions:
  first:
    handler: http
    events:
      - http: path

    # Sets 'roles/cloudfunctions.invoker' for 'allUsers' for  this function
    allowUnauthenticated: true

    # Custom IAM declarations for this function.
    iam:
        bindings:
        - role: roles/cloudfunctions.invoker
          members:
          - user:[email protected]

Reference on how to define members: https://cloud.google.com/iam/docs/reference/rest/v1/Policy#Binding

IAM policies are merged between the global/function-local scope as well. This definition:

service:  test-svc

provider:
  name: google
  runtime: python37
  region: us-central1
  project: project-name
  credentials: ~/.gcloud/keyfile.json
  iam:
    bindings:
    - role: roles/cloudfunctions.invoker
      members:
      - user:[email protected]
    - role: customRole
      members:
      - allUsers

functions:
  first:
    handler: http
    events:
      - http: path
    allowUnauthenticated: true
    iam:
        bindings:
        - role: roles/cloudfunctions.admin
          members:
          - user:[email protected]
        - role: roles/cloudfunctions.invoker
          members:
          - user:[email protected]

the full set of IAM policies would get merged to

- role: roles/cloudfunctions.invoker
  members:
  - allUsers
  - user:[email protected]
  - user:[email protected]
- role: customRole
  members:
  - allUsers
- role:  roles/cloudfunctions.admin
  members:
  - user:[email protected]

There are some caveats with this approach which this PR does not address:

If you deploy a function with an IAM binding and later remove that IAM binding from your serverless config, the IAM binding is not removed from the function. I didn't see any API documentation (https://cloud.google.com/functions/docs/reference/rest/v1/projects.locations.functions) for how to remove IAM bindings, so to do this, you would need to sls remove and re-deploy.
If setting IAM policies fail, the deployment is not cleaned up. I think this is okay behavior. The sls deploy command will fail, but since IAM policies are applied to functions after deployment manager is setup and creates the functions, a failure to set policy will not automatically remove functions.
There is currently no explicit check that the configured IAM policies were applied. This could be added in the future if needed, but it seems like its already implicitly there by virtue of the setIamPolicy call failing or not.

Fixes #222

Also related:

How to deploy public function on GCF? #205

medikoo

Thank you @edaniszewski Please see my comments, they're mostly style related

medikoo · 2020-07-01T07:11:12Z

deploy/lib/setIamPolicy.js

+'use strict';
+
+const _ = require('lodash');
+const BbPromise = require('bluebird');


Let's stick just to native promises (note we can fully use async/await). I believe we should get rid of bluebird, as we plant do so in a Framework).

If you find it difficult with native promises, let me know, I'll try to help

I've tried to move to using native promises, but since Im not really experienced in the area and don't have other examples to go by, I haven't been able to get it to work quite right. If you have an example somewhere of what changing from bluebird to native looks like, it'd be much appreciated!

medikoo · 2020-07-01T07:13:24Z

deploy/lib/setIamPolicy.js

+ }
+ this.serverless.cli.log('Setting IAM policies...');
+
+ _.forEach(policies, (value, key) => {


Let's use Object.entries().forEach instead (no point to use lodash, when we can do same with native functions)

medikoo · 2020-07-01T07:13:52Z

deploy/lib/setIamPolicy.js

+ },
+ };
+
+ this.provider.request(


This looks as orphaned promise

medikoo · 2020-07-01T07:14:26Z

deploy/lib/setIamPolicy.js

+ `Unable to set IAM bindings (${value}) for "${key}": function not found for`,
+ ` project "${this.serverless.service.provider.project}" in region "${this.options.region}".`,
+ ].join('');
+ throw new Error(errorMessage);


Let' use ServerlessError, it'll provide more user friendly message (regular errors should be thrown only in case of programmer errors, but not operational errors)

medikoo · 2020-07-01T07:14:59Z

package/lib/compileFunctions.js

+ // Collect the configured IAM bindings at the function and provider level and merge the
+ // members for each defined role. This transforms the array of IAM bindings into a mapping
+ // in order to easily merge.
+ const iamBindings = _.reduce(


Let's use antive reduce

medikoo · 2020-07-01T07:15:11Z

package/lib/compileFunctions.js

+ // members for each defined role. This transforms the array of IAM bindings into a mapping
+ // in order to easily merge.
+ const iamBindings = _.reduce(
+ _.concat(


Let's use native concat

edaniszewski · 2020-07-02T12:21:37Z

I've updated the PR with some of your recommendations. I'm not sure I've handled the orphaned promises correctly, so I may need further guidance there.

I also didn't move towards using native promises instead of bluebird since it feels like transitioning from bluebird to native is a bit out-of-scope for this PR and feels to me like that project-wide change should be in its own PR

medikoo · 2020-07-02T12:31:57Z

it feels like transitioning from bluebird to native is a bit out-of-scope for this PR

Yes definitely, but to avoid increase of technical debt we should not introduce any new code on Bluebird (it's not a problem to use native promises together with Bluebird). Any new code should be constructed with async/await and native promises.

edaniszewski · 2020-07-08T13:20:40Z

@medikoo - sorry for the delay here and being a bother, but do you know of any examples (e.g. in the serverless repo or elsewhere) of migrating from bluebird to native promises? I'm a bit new to JS promises, and even though I feel kinda close to getting it right, I'm hoping there is an example for me to use as a reference and validate against.

KavinJey · 2020-09-04T06:15:39Z

Wondering what needs to be done for this merge? New to OSS but would love IAM capabilities with GCP Functions

mrlucasrib · 2020-11-14T13:06:26Z

@medikoo Please approve this fix

cgossain · 2020-11-19T23:48:47Z

Just throwing in my two cents, would love for the PR to be merged as I'm looking for a way to "allowUnauthenticated" on deploy as opposed to having to manually add the permission per function in google cloud console.

medikoo · 2020-11-20T13:49:08Z

@KavinJey @mrlucasrib @cgossain Do you think you can finalize this PR? Aside of suggestions pointed in review process it needs now updating with master

ansjin · 2021-03-08T17:23:31Z

Any plans for merging this PR soon? I am looking for the "allowUnauthenticated" option.

HobbyProjects · 2021-03-26T09:39:07Z

Any status update on this? This is certainly a very useful PR and a very much needed one.

Ashar2shahid · 2021-04-03T18:20:02Z

Plans on getting this merged?

cgossain · 2021-04-03T21:15:42Z

I too would be interested in this functionality. I had to write a script just to get around this but it’s an extra thing to manage.

medikoo · 2021-04-06T07:55:55Z

This PR is not finalized. Can you can help with finalizing it?

actuchicks · 2021-04-28T04:50:48Z

It would be great if this fix could be merged soon. We're using deployment manager to manage all our cloud functions and currently have to manually assign permissions to public HTTP functions after deploy to work around this bug.

J-Rojas

@edaniszewski I have provided examples as to how you can use native promises instead of bluebird. Please apply these changes and resolve any merge conflicts. It would great to get this merged.

J-Rojas · 2021-05-12T10:34:32Z

deploy/lib/setIamPolicy.js

+ }
+ });
+
+ return BbPromise.all(promises);


The return can be rewritten as
return Promise.all(promises)

J-Rojas · 2021-05-12T10:39:54Z

deploy/lib/setIamPolicy.js

+
+module.exports = {
+ setIamPolicy() {
+ return BbPromise.bind(this).then(this.getFunctions).then(this.setPolicies);


Can be rewritten as:
return Promise.resolve().then(() => this.getFunctions()).then(() => this.setPolicies())

J-Rojas · 2021-05-12T10:41:10Z

deploy/lib/setIamPolicy.js

+ // If there are no IAM policies configured with any function, there is nothing to
+ // do here.
+ if (!policies || !Object.keys(policies).length) {
+ return BbPromise.resolve();


Can be rewritten as
return Promise.resolve()

J-Rojas · 2021-05-12T10:49:16Z

In the meanwhile... this is a decent workaround, although you'll have to install another plugin.

#205 (comment)

* removes the previous implementation using deploymentmanager, as it does not appear to work as documented * adds support for setting IAM via cloudfunctions API * adds test cases

edaniszewski · 2021-05-12T12:47:51Z

@J-Rojas thanks for the tips on how to update. Simple stuff, but since I'm not really a JS developer I was just unfamiliar.

Hopefully with these updates, this should now be ready for review + merge.

J-Rojas · 2021-05-12T16:53:16Z

@edaniszewski not a problem, I'm glad I can help and thanks for taking the time to fix this.

J-Rojas · 2021-05-12T21:26:21Z

@medikoo bump... can we get this merged since your requested changes have been addressed?

Tom5om · 2021-09-23T23:40:23Z

I really need this too!

aaron5670 · 2022-09-14T20:28:15Z

Status of this MR?

cgossain · 2022-10-02T16:15:02Z

+1 on this PR; would be very useful!

and3rson · 2022-10-09T23:21:18Z

Any updates on this? This is really blocking us from moving to GCP from AWS.

edaniszewski mentioned this pull request Jun 30, 2020

How to deploy public function on GCF? #205

Open

medikoo suggested changes Jul 1, 2020

View reviewed changes

medikoo assigned edaniszewski Jul 1, 2020

medikoo added the bug label Jul 1, 2020

medikoo mentioned this pull request Jul 17, 2020

fix: Fix deployment status when the function name is specified #225

Merged

medikoo added the help wanted label Jul 17, 2020

J-Rojas suggested changes May 12, 2021

View reviewed changes

edaniszewski added 4 commits May 12, 2021 08:16

fix: update the method by which IAM roles are applied

53c34c2

* removes the previous implementation using deploymentmanager, as it does not appear to work as documented * adds support for setting IAM via cloudfunctions API * adds test cases

review updates

23dd8df

test updates

acba8ca

replace BbPromise with native Promise

f354536

edaniszewski force-pushed the iam-redo branch from b980fee to f354536 Compare May 12, 2021 12:44

ericrav mentioned this pull request Oct 16, 2022

feat: configure custom IAM roles with set permissions #298

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update the method by which IAM roles are applied #223

fix: update the method by which IAM roles are applied #223

edaniszewski commented Jun 30, 2020 •

edited

medikoo left a comment

medikoo Jul 1, 2020

edaniszewski Jul 17, 2020

medikoo Jul 1, 2020

medikoo Jul 1, 2020

medikoo Jul 1, 2020

medikoo Jul 1, 2020

medikoo Jul 1, 2020

edaniszewski commented Jul 2, 2020

medikoo commented Jul 2, 2020

edaniszewski commented Jul 8, 2020

KavinJey commented Sep 4, 2020

mrlucasrib commented Nov 14, 2020

cgossain commented Nov 19, 2020

medikoo commented Nov 20, 2020 •

edited

ansjin commented Mar 8, 2021

HobbyProjects commented Mar 26, 2021

Ashar2shahid commented Apr 3, 2021

cgossain commented Apr 3, 2021

medikoo commented Apr 6, 2021

actuchicks commented Apr 28, 2021

J-Rojas left a comment

J-Rojas May 12, 2021

J-Rojas May 12, 2021 •

edited

J-Rojas May 12, 2021 •

edited

J-Rojas commented May 12, 2021

edaniszewski commented May 12, 2021

J-Rojas commented May 12, 2021

J-Rojas commented May 12, 2021 •

edited

Tom5om commented Sep 23, 2021

aaron5670 commented Sep 14, 2022

cgossain commented Oct 2, 2022

and3rson commented Oct 9, 2022

fix: update the method by which IAM roles are applied #223

Are you sure you want to change the base?

fix: update the method by which IAM roles are applied #223

Conversation

edaniszewski commented Jun 30, 2020 • edited

medikoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edaniszewski commented Jul 2, 2020

medikoo commented Jul 2, 2020

edaniszewski commented Jul 8, 2020

KavinJey commented Sep 4, 2020

mrlucasrib commented Nov 14, 2020

cgossain commented Nov 19, 2020

medikoo commented Nov 20, 2020 • edited

ansjin commented Mar 8, 2021

HobbyProjects commented Mar 26, 2021

Ashar2shahid commented Apr 3, 2021

cgossain commented Apr 3, 2021

medikoo commented Apr 6, 2021

actuchicks commented Apr 28, 2021

J-Rojas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

J-Rojas May 12, 2021 • edited

Choose a reason for hiding this comment

J-Rojas May 12, 2021 • edited

Choose a reason for hiding this comment

J-Rojas commented May 12, 2021

edaniszewski commented May 12, 2021

J-Rojas commented May 12, 2021

J-Rojas commented May 12, 2021 • edited

Tom5om commented Sep 23, 2021

aaron5670 commented Sep 14, 2022

cgossain commented Oct 2, 2022

and3rson commented Oct 9, 2022

edaniszewski commented Jun 30, 2020 •

edited

medikoo commented Nov 20, 2020 •

edited

J-Rojas May 12, 2021 •

edited

J-Rojas May 12, 2021 •

edited

J-Rojas commented May 12, 2021 •

edited