bcgov/common-object-management-service

OpenAPI spec problems

Closed this issue · 3 comments

pbolduc commented

Describe the bug

I have identified the following issues in the openapi spec. Most are the example values being incorrect for fields. There are also a number of unused component/responses defined. I suspect either you plan to add usages of these in the future, or you removed usages of them. I would be better to only include items actually used in one or more of the operations.

  1. The example value for the Last-Modified field should match the format of the date-time format as defined by RFC 3339, section 5.6, for example, 2017-07-21T17:32:28Z. See: openapi data-types

  2. The example value for s3VersionId exceeds the maximum value for int32. int32 is a signed-32 bit number and has a maximum value of 2,147,483,647, however, the example value is 1,647,462,569,641 (formatted). See openapi data-types

  3. The example value for bucketId is wrong. The last set of hex characters is 13 long, so the example uuid is not valid.

  4. The example value for the value property should be an object, not a string.

  5. There are number of unused components

jujaga commented
  1. We will be keeping the Last-Modified example the same as this is the value that Dell ECS Object Storage is yielding in the header.
  2. These should have been changed to strings - good catch!
  3. Good catch - fixed
  4. Changing the example to just have a {} instead, but we'll need to revisit at a later date to re-align what's actually being emitted in a 422.
  5. We're going to leave these unused components in, especially the response ones as we generally have those as our scaffolding for our other API specs. S3-VersionList isn't used at the moment, but we may be using it again if we reintroduce a direct to S3 query endpoint in future versions.
pbolduc commented
  1. We will be keeping the Last-Modified example the same as this is the value that Dell ECS Object Storage is yielding in the header.

That is fine, but the returned value will not be consumable by applications calling the API because it will not be able to parse the value. I would suggest you change the type to string if you want to maintain that format. The example value is not complaint with the OpenAPI specification.

In the future, you may want to include openapi spec validation. I used vacuum but ignore a lot of the warnings it gave based on naming conventions.

jujaga commented

Fair enough. We will need to revisit this at a later date. I wasn't aware of the vacuum tool, but we can definitely investigate adding that to our processes going forwards! 👍