Whats the correct way v3 broker endpoints handle operation state ?

[philippthun]

Legend

Icon	Meaning
:white_check_mark:	fixed/wanted behaviour
:x:	not fixed but change of behaviour needed
:heavy_minus_sign:	no change of behaviour needed
:question:	questionable behavior, needs further analysis

V3 API

The introduction of the Failed State for Broker operations does come with some unwanted behaviour in some cases.

Service Bindings

POST /v3/service_credential_bindings (i.e. cf bind-service) for already existing binding

Resource State	Observed Behaviour	Wanted Behaviour	PR	Fixed in main
none (synchronous case)	:question:			:heavy_minus_sign:
create succeeded	rejects request (A)	:white_check_mark:		:heavy_minus_sign:
create in progress	rejects request (A)	:white_check_mark:		:heavy_minus_sign:
create failed	rejects request (A)	recreates binding	#2636	:white_check_mark:
delete failed	rejects request (A)	raises error (1)	#2657	:white_check_mark:
delete in progress	rejects request (A)	raises error (1)	#2657	:white_check_mark:

• A: UnprocessableEntity / The app is already bound to the service instance. This is handled as success by cf cli.
• 1: Even if a deletion did not succeed, orphan mitigation should ensure that it eventually will. Thus an error different to The app is already bound to the service instance should be raised. This is because cf cli handles this error as a success.

DELETE /v3/service_credential_bindings/:guid (i.e. cf unbind-service) for already existing binding

Binding State	Observed Behaviour	Wanted Behaviour	PR	Fixed in main
none (synchronous case)	:question:			:heavy_minus_sign:
create succeeded	accepts request	:white_check_mark:		:heavy_minus_sign:
create in progress	accepts request(1)	:white_check_mark:		:heavy_minus_sign:
create failed	accepts request	:white_check_mark:		:heavy_minus_sign:
delete failed	accepts request	:white_check_mark:		:heavy_minus_sign:
delete in progress	rejects request (A)	:white_check_mark:		:heavy_minus_sign:

• A: UnprocessableEntity / There is an operation in progress for the service binding
• 1: From the Open Service Broker API specification: "If a Service Broker accepts the request to delete a Service Binding during the process of it being created, then it MUST have the net effect of halting the current creation process and beginning the deletion [...]."

GET /v3/apps/:guid/env (i.e. cf env) for already existing binding

Binding State	Observed Behaviour	Wanted Behaviour	PR	Fixed in main
none (synchronous case)	:question:			:heavy_minus_sign:
create succeeded	returns binding data	:white_check_mark:		:heavy_minus_sign:
create in progress	does not return binding data	:white_check_mark:		:heavy_minus_sign:
create failed	does not return binding data	:white_check_mark:		:heavy_minus_sign:
delete failed	returns binding data	does not return binding data (1)	#2676 #2682	:white_check_mark:
delete in progress	returns binding data	does not return binding data (1)	#2676 #2682	:white_check_mark:

• 1: Even if deletion did not succeed, orphan mitigation should ensure that it eventually will. Thus binding data should not be returned.

POST /v3/spaces/:guid/actions/apply_manifest (i.e. cf push) for already existing binding

Resource State	Observed Behaviour	Wanted Behaviour	PR	Fixed in main
none (synchronous case)	:question:			:heavy_minus_sign:
create succeeded	skips binding	:white_check_mark:		:heavy_minus_sign:
create in progress	skips binding and includes the invalid (empty) binding in app env	raises error (1)	#2688	:white_check_mark:
create failed	skips binding and includes the invalid (empty) binding in app env	recreates binding	#2636	:white_check_mark:
delete failed	skips binding and includes the invalid (empty) binding in app env	raises error (2)	#2688	:white_check_mark:
delete in progress	raises error (A)	:white_check_mark:		:heavy_minus_sign:

• A: ServiceBindingError / An existing binding is being deleted. Try recreating the binding later.
• 1: Binding requests in this context have to be synchronous. Thus an error should be raised instead. Otherwise an app would be started with the "in progress" (empty) binding in its environment.
• 2: Even if deletion did not succeed, orphan mitigation should ensure that it eventually will. Thus an error should be raised instead so that the app is not started with an empty binding.

Error handling

When the synchronous creation of a service binding failed, ~~AppApplyManifest triggered a deletion~~ no deletion is triggered (in addition to the orphan mitigation already performed by the ServiceClientProvider). The result of the failed creation is kept in the database for debugging purposes. In consecutive runs the failed binding was considered as an existing binding and provisioning a new one was skipped. The app was started with an invalid empty binding (the failed one) but success was reported to cf push user. (Fixed with #2636)

Service Keys

POST /v3/service_credential_bindings (i.e. cf create-service-key) for already existing key

Resource State	Observed Behaviour	Wanted Behaviour	PR	Fixed in main
none (synchronous case)	:question:			:heavy_minus_sign:
create succeeded	rejects request (A)	:white_check_mark:		:heavy_minus_sign:
create in progress	rejects request (A)	:white_check_mark:		:heavy_minus_sign:
create failed	rejects request (A)	recreates key	#2636	:white_check_mark:
delete failed	rejects request (A)	raises error (1)	#2657	:white_check_mark:
delete in progress	rejects request (A)	raises error (1)	#2657	:white_check_mark:

• A: UnprocessableEntity / The binding name is invalid. Key binding names must be unique. The service instance already has a key binding with name 'mykey'.
• 1: Even if deletion did not succeed, orphan mitigation should ensure that it eventually will. Thus an error should be raised as we are not sure the resource is deleted and can be created again.

DELETE /v3/service_credential_bindings/:guid (i.e. cf delete-service-key) for already existing key

Resource State	Observed Behaviour	Wanted Behaviour	PR	Fixed in main
none (synchronous case)	:question:			:heavy_minus_sign:
create succeeded	accepts request	:white_check_mark:		:heavy_minus_sign:
create in progress	accepts request(1)	:white_check_mark:		:heavy_minus_sign:
create failed	accepts request	:white_check_mark:		:heavy_minus_sign:
delete failed	accepts request	:white_check_mark:		:heavy_minus_sign:
delete in progress	rejects request (A)	:white_check_mark:		:heavy_minus_sign:

• A: UnprocessableCreate / There is an operation in progress for the service binding
• 1: From the Open Service Broker API specification: "If a Service Broker accepts the request to delete a Service Binding during the process of it being created, then it MUST have the net effect of halting the current creation process and beginning the deletion [...]."

GET /v3/service_credential_bindings/:guid/details (i.e. cf service-key) for already existing key

Resource State	Observed Behaviour	Wanted Behaviour	PR	Fixed in main
none (synchronous case)	:question:			:heavy_minus_sign:
create succeeded	shows key data	:white_check_mark:		:heavy_minus_sign:
create in progress	shows key data	raises error (A)	#2636	:white_check_mark:
create failed	shows key data	raises error (A)	#2636	:white_check_mark:
delete failed	shows key data	raises error (1)	#2679 #2682	:white_check_mark:
delete in progress	shows key data	raises error (1)	#2679 #2682	:white_check_mark:

• A: ResourceNotFound / Creation of service key in progress / Creation of service key failed
• 1: Even if deletion did not succeed, orphan mitigation should ensure that it eventually will. Thus an error should be raised instead so that the user is not supplied with a potentially not working service credential key.

Service Route Bindings

... to be done ...

Service Instances

See #2713.

V2 API

Also the V2 API may be affected since both V2 and V3 share the same model and db table. So the assumption that could be looked at is that V2 coding might not handle the "failed" states correctly. In the worst case a mixed usage of V3 and V2 could introduce unwanted/undefined behaviour. Might be worth a look what happens in the failed state in V2 then.

Service Bindings

... to be done ...

Service Keys

... to be done ...

Service Route Bindings

... to be done ...

Service Instances

... to be done ...

[philippthun]

Reopening as there are still (less critical) issues to be fixed and further entities that need to be checked.

[philippthun]

Reopening...

[philippthun]

Everything done :tada: