Add Open API document for OSB API

[n3wscott]

Relates to https://github.com/openservicebrokerapi/servicebroker/issues/160

Ready.

[cfdreddbot]

Hey n3wscott!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

[n3wscott]

@rhodie27 Yes, there is an open bug with swagger editor not pulling in parameters from components correctly. https://github.com/swagger-api/swagger-ui/issues/3976 https://github.com/swagger-api/swagger-ui/issues/3558

But I think what is in this PR is correct based on the Open API spec. The swagger UI works correctly.

I will double check the spec... I think I am correct and the tools have not caught up to the new stuff in 3.0.0. https://swagger.io/docs/specification/components/

[mattmcneeney]

Rendered version: http://petstore.swagger.io/?url=https://raw.githubusercontent.com/n3wscott/servicebroker/9bc40f42ecaa3ab89fe632404a3eb56f15fb03b8/openapi.yaml

[fmui]

Here are some random nit-picking comments.

• In the spec we several places with wording like this: "MUST be a non-empty string". "minLength: 1" should cover this.
• In the schema definition Service -> properties -> requires, items should be an array of enums (syslog_drain, route_forwarding, volume_mount).
• The schema of the status code '409' of 'PUT /v2/service_instances/{instance_id}' could be an Error, not an Object.
• The status code '400' of 'PUT /v2/service_instances/{instance_id}/service_bindings/{binding_id}' is lacking a content definition.
• The definitions of "route" and "*_url" properties and could be enhanced by "format: url".
• The conventions for service and plan metadata are not in the spec, but we link to them from the spec. Should we do something similar here?

[n3wscott]

@fmui

In the spec we several places with wording like this: "MUST be a non-empty string". "minLength: 1" should cover this.

I would vote for no validation in the first version of the swagger spec. It might be useful to not have the validation in to allow folks to poke at bad brokers. If we still want validation, could it be a followup exercise?

The schema of the status code '409' of 'PUT /v2/service_instances/{instance_id}' could be an Error, not an Object.

The spec says MAY for an error and expected to be {}:

| 409 Conflict | MUST be returned if a Service Instance with the same id already exists but with different attributes. The expected response body is `{}`, though the description field MAY be used to return a user-facing error message, as described in [Service Broker Errors](#service-broker-errors).|

The conventions for service and plan metadata are not in the spec, but we link to them from the spec. Should we do something similar here?

I opted to only use the spec.

I have updated based on feedback, adding the missing return types and enums, but I am hesitating to add more validation to the doc just because it is hard to know when to stop. I think it could be a valuable addition to the Open API document, but it is still possible to do as a followup and we could do it one resources

[fmui]

So, what's the meaning of this excerpt:

| 409 Conflict | MUST be returned if a Service Instance with the same id already exists but with different attributes. The expected response body is {}, though the description field MAY be used to return a user-facing error message, as described in Service Broker Errors.|

1. You can add a description, but you better shouldn't. The platform doesn't expect it.
2. This a broker error, but you don't have to provide a description.

[n3wscott]

@fmui it means that 409 must at least return {} and optionally return a service broker error.

[n3wscott]

@avade thanks for the review! I will fix those missing return params up when I next get a chance.

[avade]

LGTM

[avade]

I propose that this is now merged and further refinements are new PRs

LGTM

[angarg12]

Would it make sense to put a link to the rendered swagger on the repo README so that it is easily accessible by people?

[angarg12]

LGTM

[n3wscott]

Would it make sense to put a link to the rendered swagger on the repo README so that it is easily accessible by people?

Sounds like a good follow-up. Maybe even a symlink to a git tag when next release happens.

[duglin]

I'm new to OAPI but is there a way in the "Example Value" that it generates to have it do something like add a "?" when a field is optional? Showing that example is really helpful, but I'm missing the required/optional aspect of each field.

[duglin]

@maximilien ^^

[n3wscott]

is there a way in the "Example Value" that it generates to have it do something like add a "?" when a field is optional

@duglin This is a quirk of the default swagger ui, click the tab next to "example value" called "Model" and you will see the object in table format with the required fields with descriptions if the model has them.

[duglin]

Ah, I didn't realize the "Model" thing was a link - nice

[vaikas]

LGTM

[duglin]

@n3wscott did you want to fix the two spots @mattmcneeney commented on?

[n3wscott]

did you want to fix the two spots @mattmcneeney commented on?

@duglin the error pr is not in yet I think

[duglin]

Sorry, not sure what you mean by "error pr"

[duglin]

Matt are your issues resolved? github shows you're still requesting changes

[mattmcneeney]

Yes - but if we can get #409 merged then it'd be awesome to update the OpenAPI doc with that change before merging it in (as it makes many of the errors much simpler)

[duglin]

@n3wscott I think there are a few edits that people want to see before we merge this - any chance of getting those in?

[mattmcneeney]

LGTM