Fetching current input parameters values (for a service or a binding)

[mattmcneeney]

Following on from Issue 59 (Service broker can declare supported schema for parameters field), we are expecting that platforms will want to be able to fetch the current values of any service or binding input parameters.

How could this be integrated into the service broker API?

---

Potential idea

Add two new endpoints:

• GET /v2/service_instances/:instance_id to fetch input parameters for a service.
• GET /v2/service_instances/:instance_id/service_bindings/:binding_id to fetch input parameters for a binding.

Concerns:

• Security
• Service brokers having to maintain some kind of state

---

Proposal Doc

(edit by @avade)

Proposal: Fetching Configuration Parameters

[angarg12]

This seems to come back to the bigger question of whether OSB should provide a full REST API. Also I think that the common feeling around here is not to require brokers to keep state.

If we have specific use cases where clients require to retrieve the value of parameters it would make for a much compelling argument in favour of this requirement, instead of guessing a possible future need.

In any case it is probably better to raise this question in the weekly meeting.

[mattmcneeney]

Of course, I've added it to the agenda for today's call. There are some use cases within Cloud Foundry whereby app developers would want to retrieve the configuration of their service instances and bindings. It will be interesting to hear if anyone else has run into this problem.

[gberche-orange]

Use case mentionned during the WG call is that client UIs could display previously submitted parameters fields as to improve “update service instance” UX.

Would be worth investigating whether platforms could expose these endpoints instead of brokers, keeping the OSB API simple and reducing state management constraints on brokers.

In order to avoid allow clients UI fetching and displaying sensitive params:
1- brokers could declare some fields as being sensitive in the declared schemas so that UIs can expect not to fetch & display these fields.
2- alternatively, if the persistence and serving of the field does remain the responsibility of brokers, brokers could choose to not implement these endpoints or partially implementing them (e.g. not returning sensitive fields).

[mattmcneeney]

As well as improving the update service instance UX, this could also improve visibility into existing service instances and bindings. CF is working on a number of service brokers that expose non-sensitive parameters (such as number of nodes in a cluster, disk sizes, etc) that developers would like to be able to see and change via a CLI or UI.

We have already come across use cases whereby brokers have their own dashboards for modifying configuration, so the platform will not always know the correct parameter values, so relying on the platform to provide current parameter values will in some cases be impossible.

If we made these API endpoints optional, that would mean that brokers that are unable to maintain state do not have to, and we can introduce this in version 2 (non-breaking change). Note that we also discussed on the call how some brokers could rely on other services to maintain state.

We could also specify how brokers (if they want to) should respond when asked for parameters that contain sensitive information. For example, a call to:

GET /v2/service_instances/:instance_id

could return

{
  "parameters": {
    "nodes": 10,
    "password": "<redacted>"
}

Alternatively brokers could simply not return sensitive parameters. I'm unsure whether we should specify how brokers should handle secrets within the spec.

@gberche-orange What are your thoughts on this?
@pmorie Have you come across any kubernetes use cases for this?

As CF is now beginning work on the input parameter schemas for the validation through implementation, we're happy to do the same for this piece of work if we can agree on a sensible approach.

[mattmcneeney]

I've put together a document that specifies the Cloud Foundry use case for this, and a proposed solution. Thoughts and feedback very welcome.
Proposal: Fetching Configuration Parameters

[mcelep]

I've had a similar request on CC a while back: cloudfoundry/cloud_controller_ng#680

[duglin]

Ignoring the security aspects of this, I like the idea of supporting GET - we'd just need to make it optional.

[youngm]

Today all of our service brokers expose an INFO end point that can be queried to get information about a service instance. We don't supply sensitive data and we display the info returned in our Web UI. The info usually includes details about parameters passed in or sometimes includes information about the availability of the service. Another example, our "scheduled auto scale" that takes a cron as a service param. One of the items returned by the info endpoint is the next time we expect the cron to run,

It would be nice if the CC could invoke a GET endpoint on the broker where the broker could provide information about a service instance or binding. I would love for support for this to be more formal.

[mattmcneeney]

Hey all. So I've started mocking this together on a branch. My main question is:

• Should this change only add the GET endpoints returning the parameters object, or should the endpoints also return the service instance / binding data returned after a provision / binding creation?

Let me know what you think of master...mattmcneeney:get-endpoints.

Cheers.

[nilebox]

@mattmcneeney considering that even a single GET request makes service broker stateful, I would add GET endpoints for all corresponding PUT requests.

[duglin]

From the f2f notes I see:

Add a GET endpoint for service instances
Add a GET endpoint for service binds 

My take-away from the f2f was that we would add a GET endpoint for all resources.

[mattmcneeney]

I think the branch has both of those GET endpoints. We have Fetching a Service Instance and Fetching a Service Binding.

[mattmcneeney]

Any further thoughts on this all?

[shalako]

Should this change only add the GET endpoints returning the parameters object, or should the endpoints also return the service instance / binding data returned after a provision / binding creation?

I recommend supporting all fields currently supported for responses to the create instance or create binding endpoints, as suggested in the diff linked above.

This proposal looks good to me and I would recommend for moving it to validation-via-implementation phase.

[mattmcneeney]

Sounds good to me @shalako. @pmorie @duglin and other Kubernetes folks; are you happy to start validating the implementation of this?

[duglin]

I think you can remove text like:

All response bodies MUST be a valid JSON Object ({}). This is for future compatibility;

[mattmcneeney]

@duglin I've removed the following text from my PR:

All response bodies MUST be a valid JSON Object (`{}`). This is for future compatibility; it will be easier to add fields in the future if JSON is expected rather than to support the cases when a JSON body might or might not be returned.

As discussed on the call, we probably want this to be removed in the existing spec :)

[svrc]

Just adding a note here that one other use cases of requiring parameter access is to be able to do an unbind/rebind operation for a given service to refresh its configuration (such as hostnames, etc.). The operator needs to know how the service was initially bound, especially if it's scripted (i.e. this is not likely the same person who did the binding in the first place!).

This is a useful operation to do if the backing service has changed hostnames or IPs, such as in a disaster recovery scenario.

One would think that the "update-service" (i.e. PUT /v2/service_instances/:guid) in theory would cover this case but in practice it doesn't actually refresh the service configuration unless you're changing something (service plans, etc.). Only an unbind/rebind will in practice (at least in CF).

[nilebox]

This is a useful operation to do if the backing service has changed hostnames or IPs, such as in a disaster recovery scenario.

+1 to this.
The similar use case we think of is credentials rotation on the broker side.

[shalako]

@mattmcneeney @duglin
Re: removing All response bodies MUST be a valid JSON Object ({}). This is for future compatibility;

CF will consider a broker response invalid if it does not include a valid JSON object.

[duglin]

@shalako right but since we define a Body I think that sentence is redundant. I think it was only needed before in cases where there was no Body properties defined.

[mattmcneeney]

Can we just add that in one place to the top of the spec and remove from all existing response bodies?

[mattmcneeney]

Branch updated: https://github.com/mattmcneeney/servicebroker/tree/get-endpoints

[shalako]

I see this has been promoted to validation. Great news.

[avade]

@mattmcneeney Do you expect this to be validated in CF and PR'd in time for 2.13 (early September)?

@pmorie will the service-catalog SIG be implementing this feature soon?

[mattmcneeney]

@avade I don't think we'll have feedback on this one by early September unfortunately. We may have support for it in the platform by then, but we probably won't have any brokers supporting the feature to get feedback from.

[mattmcneeney]

@duglin The current branch ( https://github.com/mattmcneeney/servicebroker/blob/get-endpoints/spec.md#fetching-a-service-instance ) only has a 200 response code to the new GET endpoints.

All - do we need to add a 202 Accepted code for when an async operation (currently only for service instance provisioning) is in progress but the resource is not yet ready? Or can the last operation endpoint be relied on for this (when that returns "state": "succeeded", the GET endpoint should be immediately available)?

[duglin]

Couple of comments:

• on instances you don't say that the parameters should match the incoming parameters from the provision request. While its true that the broker might actually change the values for some reason, it might be good to at least correlate the two because as it stands now the text doesn't imply there's any relationship between them at all. I'm hoping that in most cases they will be the exact same otherwise its a bit misleading.
• it talks about matching the schema - does that mean that the schema PR must be merged first?
• fetching a binding should probably be in the Binding section - and fetching an instance should be in the provisioning section - actually, would it make sense to not have the TOC list these separately? Should we instance just have a "Managing Instances" type of section and talk about the CRUD ops for Instances - and then do the same for Bindings? Otherwise to get all of the info for dealing with each you need to jump around a lot.
• I wonder if it should return the other info, like context, service-id, etc...

In the binding:

• s/as a failure database/as a failure/
• should we include the other info - like app_guid, bind_resource, ... ?

The question about 202 doesn't just apply during async, but even sync since a GET could be issued while a provision/bind request is being worked on in a synchronous model.

Overall it seems right, we just need to decide on which fields to return.

Didn't we also talk about allowing a broker to return a subset of the data if it didn't want (or couldn't) return all of it - like the creds?

[Samze]

The platform currently has a way to figure out which optional endpoints the broker supports.

1. Update - plan_updateable: true in the catalog
2. Bind/Unbind - bindable: true in the catalog
3. Last Operation - Provision returns 202 instead of 200

This allows platforms to only send requests to brokers who explicitly declare support for them. The proposal as it stands does not allow any discoverability mechanism these endpoints by the platform.

I think in the face to face we discussed that brokers who do not support these endpoints can just return a 404, however I see a number of flaws with this approach:

1. The platform has to make a request to the broker each time to find out if it supports the endpoint. This could be cached but I'm not sure how you would invalidate this.
2. The platform cannot distinguish between a broker that does not support this endpoint or a broker that temporarily is failing. e.g. routes becoming unregistered and responds with 404s.
3. Old brokers may have implemented that route for some other purpose outside of the spec.

Instead should we follow current convention and add explicit support in the catalog for these endpoints?

[skaegi]

Really late to the game ... my apologies.
Going back to the original use-case, we've been supporting this by having the broker return back a "parameters" property (similar to "credentials") in the response object for provisioning/binding creates and updates.

The idea here is that it is up to the broker to decide which parameters it wants the controller to save. In our implementations fields like passwords, authentication keys, as well as one-time configuration values are not typically returned in this set of parameters. If an admin elects to modify a service instance or binding these saved parameters are used to populate "defaults" in our UI.

--
I'm not saying GET isn't necessarily a good idea, just that there are other approaches to consider that don't add a new verb to an endpoint (and in this case require state storage).

[mattmcneeney]

@skaegi We are seeing many brokers use platform components or the data service it backs onto to store any state it has to keep. I agree though that it is entirely up to the brokers what they want to return in these endpoints, from parameter values to credentials and anything else.

[skaegi]

re: platform components -- that's interesting!
Do you have a mechanism to tell the brokers how they might access a storage service or something similar as part of the registration flow.

[mattmcneeney]

Closing this as it will be resolved by #333