search

cloudfoundry / cf-java-client

Java Client Library for Cloud Foundry
Apache License 2.0
328 stars 318 forks source link

DefaultServices.updateInstance() rejects plan updates despites plan is updateable. #1044

Open gberche-orange opened 4 years ago

gberche-orange commented 4 years ago

The OSB API states regarding a service offering metadata that its updateable flag can be overriden in plans.

https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#service-offering-object

Response Field Type Description

[...] plan_updateable | boolean | Whether the Service Offering supports upgrade/downgrade for Service Plans by default. Service Plans can override this field (see Service Plan). Please note that the misspelling of the attribute plan_updatable as plan_updateable was done by mistake. We have opted to keep that misspelling instead of fixing it and thus breaking backward compatibility. Defaults to false.

https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#service-plan-object

Response Field Type Description

[...] plan_updateable | boolean | Whether the Plan supports upgrade/downgrade/sidegrade to another version. This field is OPTIONAL. If specificed, this takes precedence over the Service Offering's plan_updateable field. If not specified, the default is derived from the Service Offering. Please note that the attribute is intentionally misspelled as plan_updateable for legacy reasons.

Unfortunately the CC API reuses the two fields, but isn't precise enough to state that plan.updateable override serviceoffering.plan_updateable

https://apidocs.cloudfoundry.org/12.42.0/services/retrieve_a_particular_service.html

Name Description Default Valid Values Example Values
plan_updateable A boolean describing that an instance of this service can be updated to a different plan false    

https://apidocs.cloudfoundry.org/12.42.0/service_plans/retrieve_a_particular_service_plan.html

"plan_updateable": true,

As a result, cf-java-client refuses service instance updates unless the service definition is updateable, regardless than the service plan is updateable.

https://github.com/cloudfoundry/cf-java-client/blob/8ec06b4cdd61dda0f0ba5e4d546651b880735faa/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/services/DefaultServices.java#L515-L517

Given that CloudFoundry itself likely performs the checks, is there any reason to also perform the check client-side ?

If there is a good reason, then the logic should be fixed to only reject when both serviceplan.plan_updateable=false and servicedefinition.plan_updateable=false

dmikusa commented 2 years ago

Agreed. I'm not sure it makes sense to have that logic client-side. This might have been due to legacy behavior, but I don't really know why it was set up that way. We'll take this as a bug.