Allow updated dashboard_url to be returned by update service instance #437

duglin · 2018-02-01T00:31:23Z

We have cases where upon doing an update we have a new dashboard_url
for the service instance. This PR allows for the Platform to indicate
that it supports the Broker sending back a new URL, and it allows for
the Broker to do it.

Signed-off-by: Doug Davis dug@us.ibm.com

cfdreddbot · 2018-02-01T00:31:24Z

Hey duglin!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

mattmcneeney · 2018-02-01T10:55:11Z

Hey @duglin Can you explain the use case for changing the dashboard_url a bit more?

Does this happen when changing plans?

I wonder if either the GET endpoints or extensions proposals could help here...

avade · 2018-02-01T10:57:06Z

I like the idea of the service broker being able to indicate to the platform that it should do a GET. Would work well in the async world.

Likely need to think more on this, also interested in the use case.

mattmcneeney · 2018-02-01T11:03:34Z

This could also be an opportunity to help brokers who may not be able to return a dashboard_url at the start of an async provision (this has always felt weird to me).

duglin · 2018-02-01T15:41:38Z

@mattmcneeney we have situations where updating the plan or parameters causes something similar to a migration and as a result a new dashboard_url is needed. In our cases the URL/creds of the service itself don't change, just the dashboard_url.

fmui · 2018-02-20T11:44:39Z

@mattmcneeney We also have brokers that cannot return a dashboard_url at the start of an async provision. Something like this might help.

n3wscott · 2018-02-21T19:33:09Z

LGTM

I think this is a good idea. It means that the broker could return a dashboard url which could be the pending or history page and then on update could repoint to the final or updated dashboard url.

duglin · 2018-02-27T01:40:56Z

3 more needed

pmorie · 2018-02-27T14:55:56Z

spec.md

@@ -1003,10 +1003,22 @@ For success responses, the following fields are defined:

 | Response Field | Type | Description |
 | --- | --- | --- |
+| dashboard_url | string | The updated URL of a web-based management user interface for the Service Instance; we refer to this as a service dashboard. The URL MUST contain enough information for the dashboard to identify the resource being accessed (`0129d920a083838` in the example below). Note: a Service Broker that wishes to return `dashboard_url` for a Service Instance MUST return it with the initial response to the update request, even if the service is updated asynchronously. If present, MUST be a non-empty string. |


Is this really the updated URL?

I'm not following your question

I assume he means that it's not necessarily updated? It could be the original one?

That's an impl choice I guess - we don't mandate the value sent.

The (possibly) updated URL?

mattmcneeney · 2018-03-06T18:58:17Z

I’m happy to move this into validation through implementation, but whoever picks this up, it would be good to weigh up the pros and cons of using the new service instance GET endpoint to look for an updated dashboard URL after an update has completed vs adding this additional logic.

Using the GET endpoint has the additional benefit of working for async updates where the broker cannot return the dashboard URL at the start of the operation.

duglin · 2018-03-13T16:43:44Z

Agreed to move to validate- thru-impl on today's call

jberkhahn · 2018-05-08T17:08:18Z

This has been implemented in Kube Service Catalog, see the linked PR's up above and review this for merging into the spec.

We have cases where upon doing an update we have a new dashboard_url for the service instance. This PR allows for the Platform to indicate that it supports the Broker sending back a new URL, and it allows for the Broker to do it. Signed-off-by: Doug Davis <dug@us.ibm.com>

Signed-off-by: Doug Davis <dug@us.ibm.com>

duglin · 2018-05-08T17:47:28Z

rebased and ready for review. Removing the "validate thru impl" flag per today's call.

n3wscott · 2018-05-14T17:23:33Z

LGTM

duglin · 2018-05-15T13:38:17Z

Need some reviews of this

duglin · 2018-05-15T17:44:45Z

ping @pmorie @vaikas-google @fmui @zrob

vaikas · 2018-05-15T18:25:17Z

LGTM

zrob · 2018-05-15T21:34:47Z

LGTM

mattmcneeney · 2018-05-21T11:08:45Z

spec.md

 | operation | string | For asynchronous responses, Service Brokers MAY return an identifier representing the operation. The value of this field MUST be provided by the Platform with requests to the [Last Operation](#polling-last-operation) endpoint in a percent-encoded query parameter. If present, MUST be a non-empty string. |

+The ability to include updated metadata in the response message was
+added in version 2.14. Any Platform sending an API Version Header with a
+value of 2.14 or greater MUST use the new values specified in the response


@n3wscott @duglin This seems to conflict with the compatibility matrix? We are saying platforms MUST support this in v2.14 and later.

Yeah it should be added, and that is why we need to commit the matrix PR :D

I think I'm confused! This feature shouldn't be in the matrix (which implies platforms MAY support it) if we are using MUST here, right?

duglin force-pushed the updateResponse branch 2 times, most recently from 05fa96f to 3ead207 Compare February 18, 2018 20:41

pmorie reviewed Feb 27, 2018

View reviewed changes

duglin added the 2 - validating through implementation label Mar 13, 2018

duglin added the do not merge label Mar 13, 2018

duglin changed the title ~~Add support for accepts_updates=true on Update operation~~ Allow updated metadata to be returned on an update() Mar 13, 2018

This was referenced Mar 13, 2018

Update dashboard_url in response to Patch Service Instance requests kubernetes-retired/service-catalog#1830

Closed

Allow dashboard_url in response to update service instance kubernetes-retired/go-open-service-broker-client#116

Merged

jberkhahn mentioned this pull request Apr 2, 2018

Return dashboard url in responses to update service-instance requests kubernetes-retired/service-catalog#1901

Merged

mattmcneeney changed the title ~~Allow updated metadata to be returned on an update()~~ Allow updated dashboard_url to be returned by update service instance Apr 4, 2018

Doug Davis added 2 commits May 8, 2018 10:43

Remove the new flag and just force Platforms to support it in v2.14+

d7e3267

Signed-off-by: Doug Davis <dug@us.ibm.com>

duglin force-pushed the updateResponse branch from 3ead207 to d7e3267 Compare May 8, 2018 17:46

duglin removed 2 - validating through implementation do not merge labels May 8, 2018

mattmcneeney approved these changes May 15, 2018

View reviewed changes

n3wscott merged commit 611c87e into openservicebrokerapi:master May 16, 2018

mattmcneeney reviewed May 21, 2018

View reviewed changes

zrob mentioned this pull request Jun 4, 2018

CF update-service has no option to update dashboard URL cloudfoundry/cloud_controller_ng#995

Closed

duglin deleted the updateResponse branch June 6, 2018 12:53

scottfrederick mentioned this pull request Jul 19, 2018

Allow an updated dashboard_url to be provided when updating a Service Instance spring-cloud/spring-cloud-open-service-broker#111

Closed

liorokman mentioned this pull request Sep 27, 2018

Add missing pieces for supporting OSB 2.14 pivotal-cf/brokerapi#56

Merged

teddyking mentioned this pull request Mar 13, 2020

Add support for OSBAPI 2.14 kubernetes-retired/go-open-service-broker-client#144

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow updated dashboard_url to be returned by update service instance #437

Allow updated dashboard_url to be returned by update service instance #437

duglin commented Feb 1, 2018

cfdreddbot commented Feb 1, 2018

mattmcneeney commented Feb 1, 2018

avade commented Feb 1, 2018

mattmcneeney commented Feb 1, 2018

duglin commented Feb 1, 2018

fmui commented Feb 20, 2018

n3wscott commented Feb 21, 2018 •

edited by avade

Loading

duglin commented Feb 27, 2018

pmorie Feb 27, 2018

duglin Feb 27, 2018

vaikas May 15, 2018

duglin May 15, 2018

vaikas May 15, 2018

mattmcneeney commented Mar 6, 2018

duglin commented Mar 13, 2018

jberkhahn commented May 8, 2018

duglin commented May 8, 2018

n3wscott commented May 14, 2018 •

edited by avade

Loading

duglin commented May 15, 2018

duglin commented May 15, 2018

vaikas commented May 15, 2018 •

edited by avade

Loading

zrob commented May 15, 2018 •

edited by avade

Loading

mattmcneeney May 21, 2018

n3wscott May 22, 2018

mattmcneeney May 22, 2018

Allow updated dashboard_url to be returned by update service instance #437

Allow updated dashboard_url to be returned by update service instance #437

Conversation

duglin commented Feb 1, 2018

cfdreddbot commented Feb 1, 2018

mattmcneeney commented Feb 1, 2018

avade commented Feb 1, 2018

mattmcneeney commented Feb 1, 2018

duglin commented Feb 1, 2018

fmui commented Feb 20, 2018

n3wscott commented Feb 21, 2018 • edited by avade Loading

duglin commented Feb 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattmcneeney commented Mar 6, 2018

duglin commented Mar 13, 2018

jberkhahn commented May 8, 2018

duglin commented May 8, 2018

n3wscott commented May 14, 2018 • edited by avade Loading

duglin commented May 15, 2018

duglin commented May 15, 2018

vaikas commented May 15, 2018 • edited by avade Loading

zrob commented May 15, 2018 • edited by avade Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n3wscott commented Feb 21, 2018 •

edited by avade

Loading

n3wscott commented May 14, 2018 •

edited by avade

Loading

vaikas commented May 15, 2018 •

edited by avade

Loading

zrob commented May 15, 2018 •

edited by avade

Loading