openservicebrokerapi / servicebroker

Open Service Broker API Specification
https://openservicebrokerapi.org/
Apache License 2.0
1.19k stars 434 forks source link

Example Java MySQL broker is not compliant with spec #290

Closed wryun closed 6 years ago

wryun commented 7 years ago

https://github.com/openservicebrokerapi/servicebroker/blob/master/gettingStarted.md helpfully references some example service brokers. However https://github.com/cloudfoundry-community/cf-mysql-java-broker is a little bit too minimal/wrong:

https://github.com/cloudfoundry-community/cf-mysql-java-broker/issues/4 https://github.com/cloudfoundry-community/cf-mysql-java-broker/issues/3

Not to mention completely ignoring service_id/plan_id, which I guess doesn't make it non-compliant but makes it a little odd as an example.

Should it be removed from the README, or at least a warning added? Feels like it's a possible source of confusion.

wryun commented 7 years ago

Also, re https://github.com/cloudfoundry-community/cf-mysql-java-broker/issues/4, since the spec says that one can do multiple PUTs to the same binding as long as they have the same parameters, I believe this forces brokers that generate passwords to be stateful (i.e. they have to keep the password so they can return it for subsequent binding requests), OR that generated passwords have to be passed to the broker by the marketplace (which seems confusing). Is this correct?

duglin commented 7 years ago

this raises an interesting question of whether we want to be gatekeepers and have some kind of compliant check for each one we add to the list (possibly even a conformance test suite at some point), or do we just let anyone on the list w/o a check?

Right now we have a (sort of) warning that we don't claim any of the ones listed are good/bad/compliant/etc... so we at least are up-front about that. :-)

My recommendation is that we leave this "as-is" for now but consider if we want to do some kind of conformance test suite in the future.

@wryun can you open up a separate issue for your 2nd comment because I think that's a different topic and one we should consider independently?

wryun commented 7 years ago

I think it's ok (and necessary) for this to be a grey area in the absence of conformance tests. On the other hand, I would consider removing (or flagging as low quality) examples like this if someone realises they're clearly wrong, since there is a fuzzy line somewhere - to take it to a hyperbolic extreme, I'm assuming you wouldn't accept a PR for an API example that only implemented a PUT of a service_instance (and no catalog or binding).

I'll raise the comment separately. Being new to OSB, I wasn't sure whether it was worth raising.

fmui commented 7 years ago

I'm currently developing a OSB test framework for platforms and brokers. It's basically a proxy between the two. It checks platform requests and broker responses for compliance and logs spec violations. I hope I can open source that at some point.

duglin commented 7 years ago

Sounds cool