Google-api-python-client: cloudresourcemanager.project.getIamPolicy recently started throwing 'Missing required parameter "body"'

Created on 11 Jul 2019  路  9Comments  路  Source: googleapis/google-api-python-client

Replicated in version 1.7.7, 1.7.8, and 1.7.9 on MacOS, and various flavors of linux with python 3.7

Prior to July 3rd, the following code worked, and returned the IAM policy for a given project:

from googleapiclient.discovery import build
service = build('cloudresourcemanager','v1')
req = service.projects().getIamPolicy(resource='my-project')
resp = req.execute()
print(resp)

Since then, our existing applications using that functionality started throwing an error that the required parameter body was missing. At first I thought google made a breaking change to the discovery document, but upon checking, the discovery doc does not require the body parameter. Then I started poking around and I see this library adds it as required here: https://github.com/googleapis/google-api-python-client/blob/master/googleapiclient/discovery.py#L494

All of the code related to this, that I noticed, is over 7 years old. So its unlikely that a change to the library broke anything. One thing that COULD explain the issue is a few lines below the line i linked above... IF the discovery document didn't previously send a request field in the method description, this library would make the body optional. I have no way to determine if that actually happened, as I don't have any older cached discovery documents for the cloudresourcemanager v1 API.

I realize what I'm reporting here isn't a bug with this library, but something appears to have changed the behavior in this case (and potentially others) so I wanted to:

  • report it (so others can find it)
  • see if you have a way to determine what happened

Thanks!

p1 bug

Most helpful comment

Other folks were impacted by this change and reached out via the internal issue tracker.

After discussion with folks on the team, the recommendation was to pass an empty body and let the server do the validation. I will create a PR shortly.

All 9 comments

I forgot to include the now-working code

from googleapiclient.discovery import build
service = build('cloudresourcemanager','v1')
req = service.projects().getIamPolicy(resource='my-project', body={})
resp = req.execute()
print(resp)

Those are simplified versions of what we were using that stopped working, but here is a link to the PR to fix the issue in the actual code that was running that I noticed the issue in: https://github.com/forseti-security/resource-policy-evaluation-library/pull/18

I modified the method description for getIamPolicy manually and removed the request field, and confirmed that "fixed" the required parameter missing error.

The problem is, it also broke it if I apply the fix to my project in the PR mentioned above: https://github.com/forseti-security/resource-policy-evaluation-library/pull/18

So I still think a change in the discovery document caused this, but if there's a chance it gets reverted, and I apply the fix to my application, it will break again. If anyone has some contacts at google to figure out if there was a change, that would be helpful in determining how I need to proceed

@jceresini Thank you for the report! Do you have any information on the time this started happening?

The best I can do is provide the timestamp of the last log I have that indicated that our application successfully evaluated the IAM policy on a project. That evaluation is event-driven based on a pub/sub feed of changes, so its only as accurate as the frequency of changes. Anyway, the last log is from July 3rd at 18:00:02 UTC. In some cases there are several hours between changes that our application should act on, but there are typically at least a few dozen per day.

@jceresini That's very helpful, thank you for tracking that timestamp down.

A change was made to the GetIamPolicyRequest object around that time. Properties used to be empty, and now has 'GetPolicyOptions'.

Current Version (20190708)

    "GetIamPolicyRequest": {
      "description": "Request message for `GetIamPolicy` method.",
      "id": "GetIamPolicyRequest",
      "properties": {
        "options": {
          "$ref": "GetPolicyOptions",
          "description": "OPTIONAL: A `GetPolicyOptions` object for specifying options to\n`GetIamPolicy`. This field is only used by Cloud IAM."
        }
      },

Previous:

    "GetIamPolicyRequest": {
      "description": "Request message for `GetIamPolicy` method.",
      "id": "GetIamPolicyRequest",
      "properties": {},
      "type": "object"
    },

body is optional for requests with no parameters, but a parameter was added in a recent update. The change to the discovery doc is non-breaking (an optional parameter being added). Our client broke because it doesn't distinguish between optional and required parameters, AFAICT.

GetPolicyOptions will not be removed (that would definitely be a breaking change to the API). You can also absolutely pass in body even if it's not required.

https://github.com/googleapis/google-api-python-client/blob/b5d8755fe54efda07fb17d52da91903e7a413fbb/googleapiclient/discovery.py#L494-L500 https://github.com/googleapis/google-api-python-client/blob/b5d8755fe54efda07fb17d52da91903e7a413fbb/googleapiclient/discovery.py#L1174-L1191.

Thanks for the info, that all makes perfect sense. I think that fact that the client broke is a concern. I would assume others are affected by this. Should it be updated to require the body ONLY if there are _required_ properties in the request field in the method description?

Would you accept a PR for such a change?

@jceresini A PR would most definitely be appreciated!

My only concern is that "OPTIONAL" is marked inside the string for description. I'm not sure if this is the standard way (across APIs), or if this was done one-off for cloud resource manager. I'll look into that and get back to you.

Ah, fair point. I didn't realize it was only present in the description of the property. That's kind of a bummer

Other folks were impacted by this change and reached out via the internal issue tracker.

After discussion with folks on the team, the recommendation was to pass an empty body and let the server do the validation. I will create a PR shortly.

Was this page helpful?
0 / 5 - 0 ratings