Support update on Bindings

[vaikas]

There should be a way to update a binding object. Once a binding is created, one can only delete it. But there are some use cases where being able to support update on Binding would be nice to have. For example, a user of a service could apply quota adjustment that would be on per binding. The model proposed for Service Instance actions could be one way to achieve this.
Other use cases might include credential rotation as well as route modifications.

[avade]

@shalako was there a reason for not having update-binding? Sounds useful to update the arbitrary params.

In CF it may require the application to be restaged to pick up the altered binding. I am not sure if restaging would be part of the default workflow.

[youngm]

CF would require a restart if the change in params changed the bound credentials. But, I can think of some use cases for updating a binding that wouldn't change credentials in CF.

For example, if I have a service that uses an up stream load balancer to provide rate limiting to a route, I could change that rate limit for a single route by updating the params on the route service binding with out unbinding the route service and re-binding with new arbitrary parameters.

[shalako]

@avade Wasn't implemented before because no one asked for it. No reason we couldn't do it soon, except there are so many other things being asked for also ;)

[bmelville]

I will plan to write a proposal doc for this shortly.

[mattmcneeney]

@vaikas-google I've taken a quick first stab at this. Let me know if you think this is going in the right direction. I realised we didn't really discuss what a broker should return when a binding is updated, so I've said it can return the same information as with a create binding for now.

https://github.com/mattmcneeney/servicebroker/blob/support-update-on-bindings/spec.md

[tsjsdbd]

can we just call the Binding API again for update binding ?
because in the create Binding api PUT /v2/service_instances/:instance_id/service_bindings/:binding_id it already include the binding ID which generated by the first binding. the Broker can hold it as an update operation.

what we should consider is when should we trigger the update action, for example:

1. at instance update, may cause all credential need be updated.
2. just update one of the credentials only for one of the instance user (like only for a special app).
3. the instance's credential will updated every 3 months it self (not need be trigger from outside the service instance).

[mattmcneeney]

Hi @tsjsdbd - in the API today performing an update on a binding is not supported. This is something that has been raised before though and is something we have been considering via a PATCH request (like for updating a service instance).

Are your use cases for this regarding updating credentials?

[tsjsdbd]

Hi, @mattmcneeney , the Binding API is more like: hey, please tell me the newest credential of the instance, if the instance is updated in use case 1, we just need ask Broker again: hey, please tell me the newest credential of the instance.

then, let's consider use case 2, when update only one credential. the Broker also can decide the new credential from the information of the Binding request, for example:
first Binding api call with

instance: mysql
app: example_app1
parameters:
  user_name: tsjsdbd
  password: Huawei@123

the broker can create the first credential for the app example_app1 on mysql instance
when call the Binding api again with new information:

instance: mysql
app: example_app1
parameters:
  user_name: tsjsdbd
  password: Huawei@321

the Broker know the account tsjsdbd is exist, and not need create but just update the password is enough.

only in use case 3, we have no idea. because the instance change the credential itself. we can not decide when to call the api hey, please tell me the newest credential of the instance. seems like we need a flag on Service object, this flag tell us when and how frequently get the newest information from instance.(but this maybe not a good idea)

I think we are all agree that it is very careful to add new api to Broker, right?

[duglin]

I like the general direction of this. A couple of comments:

• I'd like the PUT to be idempotent, or at least move in that direction. So, I'd prefer that a PUT to an existing Binding doesn't generate new creds, it'll return the existing ones.
• I'd remove "service_id" from the input since it can't be changed - or if we think its needed due to those pesky stateless brokers, then mandate that it MUST be the same. How a stateless broker detects this is unknown to me.
• It would be nice if there were a way to generate new creds w/o having to pass in the old data - for two reasons, 1) because it might be a challenge for the platform to get then, and 2) this would force the broker to "diff" against the old stuff to see if there's a change - which could be hard. So, if we could do something like use the absence of a plan_id to indicate that we just want to rotate the creds but not change anything else, I think that would be good.

[tsjsdbd]

let focus on when should we fresh the credential, new, our Update Response is:

{
  "dashboard_url": "http://example-url",
  "base_info": {
    "instance_type": "Template",
    "actual_id":     "stack-guid-here"
    },
  "refresh_credential": true, # <=== Add a new flag to indicate need refresh credential
  "userdata": "user data info"
}

Or once instance is updated, we need call all of Bindings refresh of this instance.

[mattmcneeney]

I'm not sure I totally follow the above. Is the general consensus that we want to use the same PUT request for creating and updating a binding? I'd be hesitant to do that due to the inconsistency with service instances (PUT for creating, PATCH for updating`).

[shalako]

I agree with @mattmcneeney. PUT for creating, PATCH for updating. If you want a new cred, use PUT with a new ID. If you want to fetch existing creds, the upcoming GET enables this.

[tsjsdbd]

OK，Let's nail down using PATCH method for update cred.
Then let's discuss when to do the update action. As our team's discussion it have some point:

1. When Instance is Updated, all the creds of this instance need be updated too. (but this maybe not necessory)
2. (improment of 1) we set some flag on Parameters of Instance, when update instance, we check which parameter is changed, then dicide whether update cred. this mean just change the size of instance will not update the cred.
3. Add if need update cred flag in Broker's Update Instance Response API. through that we dicide whether update the cred of the instance.

[mattmcneeney]

@tsjsdbd For obtaining new credentials, why not just create a new binding? In many cases, a binding really just means a set of credentials.

If that's not possible, your broker could accept a parameter to update binding (say "rotate_credentials": true), and then you could use the GET endpoints (not released yet but coming in #159) to fetch the credentials. Thoughts?

[duglin]

I'd prefer to update an existing Binding rather than create a new one because there may be a cost associated with each Binding.

However, having said that, the process of rotating credentials could be tricky. Between the time the new creds are created and the app is provided those new creds, it (the app) might try to talk to the service with the old creds and we don't necessarily want that to fail. But, how does the broker know its safe to delete the old creds? The easiest might be to create a new Binding, then the platform can delete the old one when it knows the app has the new creds (and has been restarted, if needed). But give my first statement, I'm wondering if it would make any sense at all to create a second Binding but include a reference to the first so that the Broker knows we're in the process of rotating creds and doesn't treat it like a totally new Binding (and charge us). The broker could even have a timeout turned on for the first Binding if the Platform forgets to delete it.

Another option is to have two sets of creds within a single Binding, then the platform would have to do another PATCH to delete the first set of creds when ready. Can't decide if that's cleaner or more complex though - but it would avoid the need to distinguish between two types of Bindings.

[bmelville]

One of the reasons we originally proposed this is that bindings really aren't just credentials. That may be what they are on the surface, but a binding may have lots of other expensive stuff behind it, e.g., proxies, firewalls, etc., and it would be beneficial not to have to duplicate that effort every time credentials need to rotate.

It seems to me that bindings are actually two separate things, the credentials, and the binding resource itself. As a binding operator I may want to:

1. Update binding parameters to adjust some function of the binding itself, and,
2. Rotate credentials.

Is there maybe an intersect here with the Actions proposal to do credential rotation?

[mattmcneeney]

I rebased the branch with this work on (here), but since this is likely to land after async bindings ( #137 ), it will need updating at that time to support async updates.

There still seem to be outstanding queries regarding whether binding updates should be used for credential rotations.

[mattmcneeney]

Branch updated: https://github.com/mattmcneeney/servicebroker/tree/support-update-on-bindings
(Depends on #137 which depends on #159)

[duglin]

8/29 call: Wait for aysync PR/issue to be resolved before we review/discuss this one

[llamadew]

Looks like this is already covered as previously mentioned by bmelville #123 (comment)
An example use case is an app autoscaler service. There are no service specific creds. The binding does represent a few things:

1. a relationship between the service and the service .
2. a "resource" operating in the service (e.g. the scaling engine) for the bound app.
3. a configuration defining how that service functions for the bound app.

The ability to update #3 without updating/recreating/redefining #1 or #2 is a high impact on UX. The main benefit is that users can use the common service broker interface to pass service specific (arbitrary) config and not have to know the API of the service itself. This is especially impactful in the cases where the API for the service itself is not a standard or well known interface and is a burden for users to learn.

[n3wscott]

Reviving this thread.

[mattmcneeney]

@n3wscott the previous attempt to revive this thread didn't go so well!

I'm interested to hear if people still need this? Can we revisit what we think a binding update gives us that an unbind/rebind doesn't?

[mattmcneeney]

Closing due to inactivity. Please reopen if this is still urgent for anyone. Note that the issue of credential rotation is being discussed in #176