Proposal: require service_id on last_operation for statelessness

openservicebrokerapi / servicebroker

Open Service Broker API Specification

https://openservicebrokerapi.org/

Apache License 2.0

1.19k stars 434 forks source link

Proposal: require service_id on last_operation for statelessness #346

Closed jtwaleson closed 7 years ago

jtwaleson commented 7 years ago

Currently Service Brokers have to choose at max 2 out of these 3 desirable properties:

Offer multiple Services per Service Broker
Statelessness
Asynchronous operations support

Why?

When Service Brokers offer multiple Services, incoming requests need to be passed to the right Service.
To determine which Service should handle a request, the service_id parameter can be used. Currently service_id is required for all operations except for last_operation, where it is only a suggestion. Note that last_operation is only used for asynchronous operations, hence the 2 out of 3 choice.

A workaround exists. A Service Broker can "broadcast" the last_operation call to all its Services, but this complicates the code and slows down the operation.

I propose we make service_id required in the last_operation call to rid us of this problem.

duglin commented 7 years ago

@jtwaleson I'm not 100% sure but I think you're mixing "service" with "service instance" at times so I might not be understandings you. However, let me make these points:

on last_operation while service_id is optional, the operation string is required if it was returned from the broker. This means that if a broker requires a service_id in order to properly route messages then it can always include it as part of the operation string - thus making it (sort of) required in last_operation messages to that broker.
for consistency I tend to agree with you that it seems like service_id should be required, but we can't turn it into a MUST w/o breaking backwards compatibility - or move to a v3 of the spec. We've had some talks in the past about radically changing how async processing is done if/when we get to a v3 of the spec so I think keeping this issue in mind for that time would be good.

jtwaleson commented 7 years ago

@duglin it is purely about Services, not Service Instances.

Using operation to add the service_id seems like a more robust workaround. I have implemented that instead. See https://github.com/jtwaleson/example-service-broker/commit/298957fe0d2669438e7e9020353de87d2fa5fa28

I understand the concern with backwards compatibility. The spec currently says:

Note: Although the request query parameters service_id and plan_id are not mandatory, the platform SHOULD include them on all last_operation requests it makes to service brokers.

If all existing implementations already adhere to this, perhaps waiting for v3 is not required, but I have no insight into this.

avade commented 7 years ago

Last-operation request already has this field, but it is optional. k8s Service Catalog and CF both support this in the latest releases.

https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#polling-last-operation

Suggesting we close the issue now, but will consider this feedback for v3.

jtwaleson commented 7 years ago

@avade Makes sense, thanks. As the spec suggests the field and both k8s & CF currently support it, I think service brokers that require it can be deployed with care (that is, proper documentation and CI) until v3.