search

spring-cloud / spring-cloud-open-service-broker

Spring Cloud project for creating service brokers that conform to the Open Server Broker API specification
https://spring.io/projects/spring-cloud-open-service-broker
Apache License 2.0
168 stars 118 forks source link

Support 200 and 409 response codes for existing binding requests #321

Closed andrejb-dev closed 1 year ago

andrejb-dev commented 3 years ago

To generate the response in ServiceInstanceBindingController.createServiceInstanceBinding this little method is called:

https://github.com/spring-cloud/spring-cloud-open-service-broker/blob/a32b84e1836f37b74a40bf1a8262c9c924f63cb8/spring-cloud-open-service-broker-core/src/main/java/org/springframework/cloud/servicebroker/controller/ServiceInstanceBindingController.java#L144-L155

When the binding existed the method returns 200 OK. But based on OSB api spec (both v2.15 and v2.16), the response should fail with 409 Conflict

409 Conflict MUST be returned if a Service Binding with the same id, for the same Service Instance, already exists or is being created but with different parameters.

Maybe I misunderstood that or am I missing something? Thanks.

royclarkson commented 3 years ago

Thanks for reporting! I'll review the spec and double check our tests.

royclarkson commented 3 years ago

hmm. this is a bit ambiguous. the spec also says a 200 OK SHOULD be returned if the Service Binding already exists and the requested parameters are identical to the existing Service Binding.

andrejb-dev commented 3 years ago

I see. I think the difference is in the parameters (if same 200 OK, if different 409 conflict)

royclarkson commented 3 years ago

response.isBindingExisted() isn't adequate to support those two different scenarios. It might make sense to deprecate this property and provide an alternative method for configuring or returning the correct response code. If we do that, then it would be the responsibility of the developer to determine whether the parameters are identical or not.

royclarkson commented 1 year ago

We already have a ServiceInstanceBindingExistsException that you can throw to return a 409, so this change isn't correct. Reopening this issue to reevaluate how to handle this.