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
167 stars 118 forks source link

PCF update instance parameters failure #345

Closed MadaManu closed 2 years ago

MadaManu commented 2 years ago

Hi,

The below problem seems to arise when trying to update service instance configuration parameters. Service broker error: 400 BAD_REQUEST "Failed to read HTTP message"; nested exception is org.springframework.core.codec.DecodingException: JSON decoding error: Unrecognized field "service_id" (class org.springframework.cloud.servicebroker.model.instance.UpdateServiceInstanceRequest$PreviousValues), not marked as ignorable; nested exception is com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException: Unrecognized field "service_id" (class org.springframework.cloud.servicebroker.model.instance.UpdateServiceInstanceRequest$PreviousValues), not marked as ignorable (2 known properties: "plan_id", "maintenance_info"]) at [Source: (io.netty.buffer.ByteBufInputStream); line: 6, column: 20] (through reference chain: org.springframework.cloud.servicebroker.model.instance.UpdateServiceInstanceRequest["previous_values"]->org.springframework.cloud.servicebroker.model.instance.UpdateServiceInstanceRequest$PreviousValues["service_id"])

It seems that the pcf controller is sending the update call to the instance and it's payload contains service_id within previous_values. But the springframework servicebroker model does not define this, hence failing to decode the json.

Versions used: CC API Version: 2.164.0 and 3.99.0 According to this page - These should be compatible with OSB spec 2.15.

The 2.15 OSB spec, defines the previous_values as below - reference

Previous Values Object

Request Field Type Description
service_id string Deprecated; determined to be unnecessary as the value is immutable. If present, it MUST be the ID of the Service Offering for the Service Instance.
plan_id string If present, it MUST be the ID of the Service Plan prior to the update.
organization_id string Deprecated as it was redundant information. The organization ID for the Service Instance MUST be provided by Platforms in the top-level field context. If present, it MUST be the ID of the organization specified for the Service Instance.
space_id string Deprecated as it was redundant information. The space ID for the Service Instance MUST be provided by Platforms in the top-level field context. If present, it MUST be the ID of the space specified for the Service Instance.
maintenance_info Maintenance Info The maintenance information that was used when the Service Instance was provisioned or when it was last updated.

According to this page Version compatibility the OSB API 2.15 is in 3.3.x of this library (Spring Cloud Open Service Broker). However, we are using spring-cloud-open-service-broker 3.3.2 and the above mentioned issue happens.

Digging a bit more, it looks like the UpdateServiceInstanceRequest>PreviousValues class has fields for only planId and maintenanceInfo, even if the spec of OSB API 2.15 has the service_id field defined and ONLY noted as deprecated - which is different from REMOVED.

I tried going back in versions of the library to see if I can find another maybe older version that did NOT remove this field, and couldn't find any.

Are there other people having this issue when using their OSB to update service instances in PCF? Should I consider opening a PR with a quick fix for 3.3.x version to include this field as per OSB API 2.15 specs?

MadaManu commented 2 years ago

The same thing happens with the other fields that are Deprecated. Overall these fields service_id, organization_id, space_id are not defined in PreviousValues class and are not ignored. This causes the decode to fail if any of the above fields are present in the request from controller to the OSB.

KevinMimacom commented 2 years ago

Also facing this issue.

royclarkson commented 2 years ago

Thanks for reporting and including the details of the issue you are experiencing.

royclarkson commented 2 years ago

My initial review confirms your info that we are indeed missing these fields in the PreviousValues class. I'm not sure where the disconnect happened, but I'll work on adding them. Thanks again.