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

The "members" attribute in the AWS service definition is literally a list of "member" objects #28

Open
sverch opened this issue Mar 7, 2020 · 7 comments
Labels

Comments

@sverch
Copy link

sverch commented Mar 7, 2020

In the AWS service definition you're using as a source for this, there's a "members" field:

"ListUsersResponse": {
  "type": "structure",
  "required": [
    "Users"
  ],
  "members": {
    "Users": {
      "shape": "userListType",
      "documentation": "<p>A list of users.</p>"
    },
    ...
}

You reasonably assumed that "members" is a structural thing to describe what members a "ListUsersResponse" has, and have changed it to "properties" in your generated OpenAPI spec:

ListUsersResponse:
   type: object
   required:
     - Users
   properties:
     Users:
       $ref: '#/components/schemas/userListType'
       description: A list of users.
     ...

However, when hitting the API directly using https://github.com/okigan/awscurl and https://github.com/sverch/aws-signature-proxy, the response actually has "member" sub-objects in it:

$ https_proxy=localhost:8080 curl "https://iam.amazonaws.com/?Action=ListUsers&Version=2010-05-08"
<ListUsersResponse xmlns="https://iam.amazonaws.com/doc/2010-05-08/">
  <ListUsersResult>
    <IsTruncated>false</IsTruncated>
    <Users>
      <member>
        ...
        <UserName>shaun.verch</UserName>
        <Arn>arn:aws:iam::ID:user/shaun.verch</Arn>
        ...
      </member>
    </Users>
  </ListUsersResult>
...
</ListUsersResponse>

When I hit an endpoint that returns multiple results (ListRoles), it actually returns multiple "member" objects rather than multiple "Roles".

@MikeRalphson
Copy link
Contributor

I think the renaming of members to properties is correct here (to get a valid JSON Schema object from the aws-sdk "Shape") and the renaming of member to items is also correct - even though we do it somewhat naively. What seems to be missing is an xmlObject to tell OpenAPI tools that the individual array items should be wrapped in <member></member> tags.

What I don't know is whether this is the case for all xml-based AWS APIs, so paging @pimterry in case we need to refine the logic further.

@MikeRalphson
Copy link
Contributor

The AWS APIs on GitHub are updated now, it will take about 35 minutes for the CI job to finish to publish them to the APIs-Guru API.

@sverch
Copy link
Author

sverch commented Mar 8, 2020

Based on these openapi docs, it looks like the xml.name member that you added in this commit actually changes the name rather than adding a new sub object when you have it on its own.

Although looking further down that page, I think if you also add wrapped : true it will work.

@MikeRalphson
Copy link
Contributor

MikeRalphson commented Mar 8, 2020

The xml.name is set on the items of the array, not on the array itself, where xml.wrapped would be set.

@sverch
Copy link
Author

sverch commented Mar 9, 2020

Ah, so will setting xml.name on array items that are otherwise unnamed cause the items to be wrapped in another sub-object? I don't see anything here about it (this was just a quick search, and I don't know all the options for sure). The xml.wrapped option looks like it was for this case on the array itself, but if naming the sub objects does it too then that's perfect.

@pimterry
Copy link
Contributor

What I don't know is whether this is the case for all xml-based AWS APIs, so paging @pimterry in case we need to refine the logic further.

In short: I have no idea 😃. I've done some digging though.

Your fix here is above for both query and rest-xml protocols. There are a few other interesting examples of each, so picking through a few of them rest-xml protocol AWS docs:

Route53 (rest-xml):

The AWS response example includes:

   <DelegationSet>
      <NameServers>
         <NameServer>ns-2048.awsdns-64.com</NameServer>
         <NameServer>ns-2049.awsdns-65.net</NameServer>
         <NameServer>ns-2050.awsdns-66.org</NameServer>
         <NameServer>ns-2051.awsdns-67.co.uk</NameServer>
      </NameServers>
   </DelegationSet>

I.e. with no <member> tag. That doesn't match our latest OpenAPI spec, which now sets name: 'member'. The AWS spec for this looks like:

    "S2m": {
      "type": "structure",
      "required": [
        "NameServers"
      ],
      "members": {
        "Id": {},
        "CallerReference": {},
        "NameServers": {
          "type": "list",
          "member": {
            "locationName": "NameServer"
          }
        }
      }
    },

Cloudfront (rest-xml):

Similarly the AWS docs for ListTagsForResource shows a response like:

<Tags>
   <Items>
      <Tag>
         <Key>string</Key>
         <Value>string</Value>
      </Tag>
   </Items>
</Tags>

Again no <member>, and again our OpenAPI spec now includes sets name: member. The AWS spec for this case looks like:

    "TagList": {
      "type": "list",
      "member": {
        "shape": "Tag",
        "locationName": "Tag"
      }
    },

ELB (query with an XML namespace):

Looking at the response from DescribeLoadBalancers for example, it seems like every ELB list uses <member> wrappers. Notably the AWS spec doesn't use locationName anywhere. I think the spec that comes out here is correct.

IAM (the original issue) is also using the query protocol.


So, maybe we should actually only do this for the XML query protocol? It's possible that it's relevant to rest-xml but only when an explicit locationName isn't provided, but I can't see any cases where that happens, so maybe not. Any other counter/supporting examples would be very interesting.

@sverch
Copy link
Author

sverch commented Mar 11, 2020

Well I can at least confirm with awscurl that this is what actually gets returned for that cloudfront request (and it has no "members", as described):

$ awscurl --service cloudfront "https://cloudfront.amazonaws.com/2019-03-26/tagging/?Resource=arn%3Aaws%3Acloudfront%3A%ID%3Adistribution%ID"
<?xml version="1.0"?>
<Tags xmlns="http://cloudfront.amazonaws.com/doc/2019-03-26/"><Items><Tag><Key>test</Key><Value>value</Value></Tag></Items></Tags>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants