fix: update the method by which IAM roles are applied

edaniszewski commented 4 years ago

This implementation is a pretty sizable departure of the previous implementation attempt (https://github.com/serverless/serverless-google-cloudfunctions/pull/219) which did not work as anticipated (https://github.com/serverless/serverless-google-cloudfunctions/issues/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:someone@somewhere.io

...

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:someone@somewhere.io

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:someone@somewhere.io
    - role: customRole
      members:
      - allUsers

functions:
  first:
    handler: http
    events:
      - http: path
    allowUnauthenticated: true
    iam:
        bindings:
        - role: roles/cloudfunctions.admin
          members:
          - user:someone-else@somewhere.io
        - role: roles/cloudfunctions.invoker
          members:
          - user:someone-else@somewhere.io

the full set of IAM policies would get merged to

- role: roles/cloudfunctions.invoker
  members:
  - allUsers
  - user:someone@somewhere.io
  - user:someone-else@somewhere.io
- role: customRole
  members:
  - allUsers
- role:  roles/cloudfunctions.admin
  members:
  - user:someone-eelse@somewhere.io

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:

205

edaniszewski commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 3 years ago

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

mrlucasrib commented 3 years ago

@medikoo Please approve this fix

cgossain commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

HobbyProjects commented 3 years ago

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

Ashar2shahid commented 3 years ago

Plans on getting this merged?

cgossain commented 3 years ago

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 commented 3 years ago

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

actuchicks commented 3 years ago

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.