Skip to content

Conversation

rhodie27
Copy link

See issue #114.

First draft of the generic actions (aka extensions) proposal on a per-service-instance basis.

A couple notes:

  1. In addition to updating Provisioning, I added a separate section for Extensions at the bottom b/c I suspect we will add actions/extensions at the service (and broker) level next. A lot of this can likely be handled in the same way so it made sense to me to separate it out now.
  2. The OpenAPI, Auth & adheres_to sections obviously need more content/wording, but I wanted to get some quick feedback on the direction.
  3. I haven't really looked at any changes required outside of spec.md yet.
  4. I'm an awful speller today, so good luck.

@cfdreddbot
Copy link

Hey rhodie27!

Thanks for submitting this pull request!

All pull request submitters and commit authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

If you are confident that you're covered under a Corporate CLA, please make sure you've publicized your membership in the appropriate Github Org, per these instructions.

Once you've publicized your membership, one of the owners of this repository can close and reopen this pull request, and dreddbot will take another look.

@rhodie27
Copy link
Author

Reopen please, my org was private for some reason. I fixed the CI issues.

@duglin duglin closed this Jan 26, 2018
@duglin duglin reopened this Jan 26, 2018
@cfdreddbot
Copy link

Hey rhodie27!

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.

spec.md Outdated

##### Extension APIs Object

The `extension_apis` object MAY be used to describe any additional endpoint needed related to a Service Instance. An examples of this might include backup and restore endpoints for a database. If present, MUST return a `discovery_url`. See [Extensions](#extensions) for more information.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you wrap non-table lines at 80 columns to be consistent with the rest of the doc?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if present, each item in the extension_apis array MUST include a discovery_url property.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although, I think this MUST is redundant with the REQUIRED aspect of the discovery_url you mention in the table below. So it might be best to remove here so we're not duplicating requirements.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I got these.

spec.md Outdated

| Response field | Type | Description |
| --- | --- | --- |
| discovery_url | string | A URI pointing to a valid OpenAPI 3.0+ document describing the API extension(s) to the Service Instance including server location, endpoints, parameters and any other detail required for invocation. The location of the API extension endpoint(s) can be local to the Service Broker or on a remote server. The Platform MUST execute these API extensions on behalf of the client. MUST be a valid URI. See [OpenAPI](#openapi-document) for more information.|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/required for invocation/needed for invocation/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably use the '*' pattern to indicate that its a required field.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than "The Platform MUST execute...", which sounds like the platform is required to invoke these even if it has no need to, I would suggest that we remove that sentence from here. Instead, since this is really more of a generic statement about the entire concept and not about this one field, let's add a sentence to the intro section at line 849 about this issue. And perhaps there say something like:

These APIs are extensions to the Open Service Broker API. As such they are intended to be invoked by the Platform on behalf of its clients.

I tried to think of a way to make it normative but it felt a bit odd. I don't think we ever say that the normal OSBAPIs MUST be invoked by the Platform, although its implied. So I think we can probably live with the same non-normative-ness in these too. WDYT?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense. I changed it.

spec.md Outdated
| Response field | Type | Description |
| --- | --- | --- |
| discovery_url | string | A URI pointing to a valid OpenAPI 3.0+ document describing the API extension(s) to the Service Instance including server location, endpoints, parameters and any other detail required for invocation. The location of the API extension endpoint(s) can be local to the Service Broker or on a remote server. The Platform MUST execute these API extensions on behalf of the client. MUST be a valid URI. See [OpenAPI](#openapi-document) for more information.|
| credentials | object | A Service Broker MAY return authentication details for running any of the extension API calls, especially for those running on remote servers. If not present, platforms can assume that standard broker authentication will work for the new endpoint(s). See [Extension Authentication](#extension-api-authentication) for more information.|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not present, the same authentication mechanism used for the normal Open Service Broker APIs MUST work for the new endpoint(s).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

spec.md Outdated
| --- | --- | --- |
| discovery_url | string | A URI pointing to a valid OpenAPI 3.0+ document describing the API extension(s) to the Service Instance including server location, endpoints, parameters and any other detail required for invocation. The location of the API extension endpoint(s) can be local to the Service Broker or on a remote server. The Platform MUST execute these API extensions on behalf of the client. MUST be a valid URI. See [OpenAPI](#openapi-document) for more information.|
| credentials | object | A Service Broker MAY return authentication details for running any of the extension API calls, especially for those running on remote servers. If not present, platforms can assume that standard broker authentication will work for the new endpoint(s). See [Extension Authentication](#extension-api-authentication) for more information.|
| adheres_to | string | A URI pointing to a specification detailing the implementation guidelines for the OpenAPI document hosted at the `discovery_url`. If present, must be a valid URI.|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if instead of "pointing" (which sort of implies there has to be a server listening at the endpoint), we should say "referring". Almost the same but perhaps wishy-washy enough to then allow us to add: While this property is a URI, there is no requirement for there to be an actual server listening at that endpoint. This value is meant to provide a unique identifier representing the set of extensions APIs supported.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Wishy-washy it is!

spec.md Outdated
"discovery_url": "http://example-openapi-doc.example.com/extensions",
"credentials":[{
"tokenURL": "https://example.com/api/oauth/token"
}]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comma

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. Got it.

spec.md Outdated

### Extension API on Remote Servers

Additional API endpoints MAY be executed on a remote server, however the OpenAPI document MUST include the servers `url` so that the Platform will know how to invoke the endpoint(s). If the server `context` parameter of the OpenAPI document is set to local host the Platform can assume the extension API endpoints are to be invoked using the Service Broker host and port.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/can assume.../MUST use....

I assume that its just the host (localhost) that is swapped out in favor of the original host/port, right? meaning any path stuff in the OpenAPI doc should still be used right? We should probably say that if so. You may want to also say that any port number specified will be replace too if "localhost" is used - if that's your intent.

Copy link
Author

@rhodie27 rhodie27 Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm talking specifically about the server object, https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#serverObject, url. It can or cannot include a port. I'm think we want to say that if the doc just says http://localhost for url that signifies that it should run on the SB host/port. However, there is an issue if the OAPI doc provides more than one url. Perhaps we can say the doc should provide a url with a description "Service Broker host" to clearly identify the right one, whether it's localhost or other...

I need to think about this more, so I didn't modify it. Can get to it tomorrow.

spec.md Outdated

### Extension API Authentication

The Service Broker MAY use the the same broker authentication method for invocation of extension endpoints by not providing credentials as part of a `extensions_api` object. If no credentials are present in an `extensions_api` object, the Platform can assume that the default broker authentication is required.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a dup from what's above - I would suggest that we only say it once - and I think the one from above is good enough. You may be able to just move all of this section up above - not sure its worthy enough for just one para. Actually, would it read better to just move this entire "Extension" section into what we have above since it kind of feels like a repeat of the same material and anything new could just be sprinkled up there. Just a thought.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wasn't sure. Like I wrote in PR, while writing this I separated it out b/c a lot of it seemed like it would be redundant if we added extensions on other resources down the line. But, obviously, we could always abstract it out at that point in time.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it your way, for now. If others chime in and want it separated out I can always revert.

@duglin
Copy link
Contributor

duglin commented Jan 26, 2018

@rhodie27 overall I think it looks good! Just some editorial suggestions.

@duglin
Copy link
Contributor

duglin commented Jan 29, 2018

@rhodie27 any chance of addressing the comments before tomorrow's call?

spec.md Outdated
Extension API endpoints MAY be executed on a remote server, however the OpenAPI
document MUST include the servers `url` so that the Platform will know how to
invoke the endpoint(s). If the server `context` parameter of the OpenAPI
document is set to localhost the Platform can assume the extension API endpoints
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may want to backtick localhost

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we s/can assume/MUST assume/ ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I think that makes sense. I can't think of a reason they wouldn't want to assume that.

spec.md Outdated

| Response field | Type | Description |
| --- | --- | --- |
| discovery_url | string | A URI pointing to a valid OpenAPI 3.0+ document describing the API extension(s) to the Service Instance including server location, endpoints, parameters and any other detail the platform needs for invocation. The location of the API extension endpoint(s) can be local to the Service Broker or on a remote server. MUST be a valid URI. The returned OpenAPI document MUST be in json format. See the [OpenAPI Specification](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md) for more information. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to add a * and the * Fields with an asterisk are REQUIRED. sentence after the description of the fields.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I thought I did. Must've reverted. There now.

Copy link
Contributor

@n3wscott n3wscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am more in favor of the change without the additions of operation_display_names and alt_instance_id.

Like on the call we agreed that we need to start implementing this feature and see how much resistance there is to implementing a useful feature. I hope we can do with with the bare minimum and then add nice-to-have features once we find that they are indeed nice-to-haves.

spec.md Outdated
| discovery_url* | string | A URI pointing to a valid OpenAPI 3.0+ document describing the API extension(s) to the Open Service Broker API including, endpoints, parameters, authentication mechanism and any other detail the platform needs for invocation. The location of the API extension endpoint(s) can be local to the Service Broker or on a remote server. If located on a remote server acccessing the OpenAPI document MUST NOT require authentication. MUST be a valid URI. If `discovery_url` is a path, the Platform can assume it is to be called relative to the basepath of the Service Broker. The returned OpenAPI document MUST be in json format. See the [OpenAPI Specification](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md) for more information. |
| server_url | string | A URI pointing to a remote server where API extensions will run. This URI will be used as the basepath for the endpoint(s) described by the `discovery_url` OpenAPI document. If no `server_url` is present, the Platform MUST assume the extension API endpoint(s) are to be invoked using the Service Broker host and port. If present, MUST be a valid URI. If `server_url` is a path, the Platform can assume it is to be called relative to the basepath of the Service Broker. |
| credentials | object | A Service Broker MAY return authentication details for running any of the extension API calls, especially for those running on remote servers. If not present, the same authentication mechanism used for the normal Open Service Broker APIs MUST work for the new endpoint(s). If the Service Broker wants to use alternate methods of authentication, (e.g. on remote servers) it MUST provide details to that mechanism in the OpenAPI document via one or more [Security Scheme Objects](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#securitySchemeObject). Next, each extension endpoint in the OpenAPI document MUST have one or more [Security Requirement Objects](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#securityRequirementObject) defined that matches a [Security Scheme Object](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#securitySchemeObject). Lastly, the parameters needed by each [Security Scheme Object](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#securitySchemeObject), (e.g. username and password for basic authentication), MUST be provided as part of the `extension_api` object within the `credentials` object. If credentials are present in an `extension_api` object, the Platform will need to verify the authentication method from the OpenAPI document. |
| alt_instance_id | string | Refers to a parameter in the `discovery_url` OpenAPI document that maps directly to the Service Broker API `instance_id` parameter. If the extension API endpoint(s) use a different value to represent a Service Broker instance, then `alt_instance_id` MUST be present. If not present, the Platform can assume `instance_id` means Service Broker Instance in the OpenAPI document. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to push back on this field entirely and I apologize for not being in the original discussions for it.

I think alt_instance_id is an attempt at serving pre-processed open api information and the correct way to solve this issue is with vendor extensions in the open api doc. You could add a key, say "x-osb-instance-id": "instance_name" on the definition of the resource and it will allow you to do the same thing without polluting the response from provisioning.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apology accepted. I agree, this could be done via vendor extentions, however, the overall theme I have been trying to push is to not make the service provider change their OpenAPI document to fit the service broker's needs. Perhaps the service broker author doesn't have that ability/power or the service provider doesn't want a million different vendor customizations in their pretty swagger. I guess we could always say that the Service Broker author needs to maintain their own OpenAPI document... Sounds messy.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vote is to remove this for now. Mike to add language about the openapi doc must do this somehow.

spec.md Outdated
| server_url | string | A URI pointing to a remote server where API extensions will run. This URI will be used as the basepath for the endpoint(s) described by the `discovery_url` OpenAPI document. If no `server_url` is present, the Platform MUST assume the extension API endpoint(s) are to be invoked using the Service Broker host and port. If present, MUST be a valid URI. If `server_url` is a path, the Platform can assume it is to be called relative to the basepath of the Service Broker. |
| credentials | object | A Service Broker MAY return authentication details for running any of the extension API calls, especially for those running on remote servers. If not present, the same authentication mechanism used for the normal Open Service Broker APIs MUST work for the new endpoint(s). If the Service Broker wants to use alternate methods of authentication, (e.g. on remote servers) it MUST provide details to that mechanism in the OpenAPI document via one or more [Security Scheme Objects](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#securitySchemeObject). Next, each extension endpoint in the OpenAPI document MUST have one or more [Security Requirement Objects](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#securityRequirementObject) defined that matches a [Security Scheme Object](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#securitySchemeObject). Lastly, the parameters needed by each [Security Scheme Object](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#securitySchemeObject), (e.g. username and password for basic authentication), MUST be provided as part of the `extension_api` object within the `credentials` object. If credentials are present in an `extension_api` object, the Platform will need to verify the authentication method from the OpenAPI document. |
| alt_instance_id | string | Refers to a parameter in the `discovery_url` OpenAPI document that maps directly to the Service Broker API `instance_id` parameter. If the extension API endpoint(s) use a different value to represent a Service Broker instance, then `alt_instance_id` MUST be present. If not present, the Platform can assume `instance_id` means Service Broker Instance in the OpenAPI document. |
| operation_display_names | object | A Service Broker MAY match operations in the `discovery_url` OpenAPI document with user consumable display names. If present, each key in this object MUST match an [operationID](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#operationObject) from the `discovery_url` OpenAPI document, with each value representing a human readable string. Platforms can use this object to show end users operations that are available on their Service Instance prior to calling the `discovery_url` OpenAPI document. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also push back on this field. operation_display_names could be accomplished in other ways, like a lookup service that takes the adhears_to (plus more data?) url and returns the same list, maybe hosted by the broker.

I think we need to draw a line on the data that is returned from a provision is data that is specific to the instance that was created, and when instances of the same type start to return the exact same data is a red flag for me.

Are we going to need the platform to remember the exact operation_display_names per instance? Because if we let it be in the response, the broker could make this list dynamically as provisioning happens.

Then we also need to talk about how does this list get updated as the instance is upgraded under the brokers control and new extensions are enabled. The only way to be sure is to fetch and process the open api doc.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also push back on this field. operation_display_names could be accomplished in other ways, like a lookup service that takes the adhears_to (plus more data?) url and returns the same list, maybe hosted by the broker.

The request from a few different people, (mostly RH), is they want a way to get a list of keywords associated with the service instance actions, prior to having to load and parse the openapi document. Adhears_to will not be available 100% of the time, and typically will not be used when you are interested in dynamically generating any returned openapi document.

I think we need to draw a line on the data that is returned from a provision is data that is specific to the instance that was created, and when instances of the same type start to return the exact same data is a red flag for me.

I do agree we need to draw a line, I believe this is it for validating phase. We can always remove/change if it doesn't work IRL.

Are we going to need the platform to remember the exact operation_display_names per instance? Because if we let it be in the response, the broker could make this list dynamically as provisioning happens.

The same could be said for the OpenAPI document, in general. Maybe it's different per service_instance...

Then we also need to talk about how does this list get updated as the instance is upgraded under the brokers control and new extensions are enabled. The only way to be sure is to fetch and process the open api doc.

The same response will be required on a service_instance update. I just haven't added that to the PR yet.

@duglin
Copy link
Contributor

duglin commented Feb 28, 2018 via email


The `extension_api` object MAY be used to describe any additional endpoint
to the Open Service Broker API. An example of this could be lifecycle
management of a Service Instance, (e.g. "Day Two Operations"), like Backup,
Copy link
Contributor

@gberche-orange gberche-orange Mar 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest the specs to clarify when an extension_api should be defined and used by broker authors w.r.t. to asking 1st class support in the OSB API.

Are platforms expected to provide UI/CLI tooling for any declared extensions (i.e. dynamically generating UI from the openAPI document) ? Would generated UIs and user workflows be restricted to single API endpoint calls, or would instead spawn across multiple API calls (e.g. list backups, restore backup, delete backup) ?

Are platforms instead expected to additionally support specific curated extensions and bring optimized user experience for them? If so, where/how would such curated extensions be defined and published ?

Can the specs recommend use-cases for extensions such as:
1- operator specific extensions : a platform operators is adding custom features to a broker and brings custom UI on along it.
2- service specific extensions (say stop/resume operation) that only applies to a category of services
3- vendor specific extensions (say K8S/CF vendor X way of offering backup which differ from vendor Y)
4- platform specific extensions (e.g. CF AR service metrics ingestion which differs from K8S metrics ingestion) that can't be supported by platform-specific profiles

Would the mentioned day 2 operations seem be fitting some of the cases above?

  • Backup, Restore: vendor specific extensions
  • Stop, Start, Restart and Pause: service specific extensions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest the specs to clarify when an extension_api should be defined and used by broker authors w.r.t. to asking 1st class support in the OSB API.

The current thinking is it is entirely up to the broker author, and the platform can choose to provide, at minimum, the OpenAPI UI (or even just the OpenAPI url). Then platforms can extend the UI based on 'adheres_to'.

Are platforms expected to provide UI/CLI tooling for any declared extensions (i.e. dynamically generating UI from the openAPI document) ? Would generated UIs and user workflows be restricted to single API endpoint calls, or would instead spawn across multiple API calls (e.g. list backups, restore backup, delete backup) ?

We had talked about how to batch calls but it became clear that this could be designed forever and we decided to support just the single endpoint as a first step, but nothing stops a clever broker from providing batch/aggregation on their own.

Are platforms instead expected to additionally support specific curated extensions and bring optimized user experience for them? If so, where/how would such curated extensions be defined and published ?

adheres_to is the mechanism we think will do this. If it is proven that this is not enough we can always revisit the spec to add what is lacking.

Can the specs recommend use-cases for extensions such as:
1- operator specific extensions : a platform operators is adding custom features to a broker and brings custom UI on along it.
2- service specific extensions (say stop/resume operation) that only applies to a category of services
3- vendor specific extensions (say K8S/CF vendor X way of offering backup which differ from vendor Y)
4- platform specific extensions (e.g. CF AR service metrics ingestion which differs from K8S metrics ingestion) that can't be supported by platform-specific profiles

I suspect this is out of scope for the current PR, at least until we see some real world emergent behavior for generic actions.

@duglin
Copy link
Contributor

duglin commented Aug 17, 2018

I believe we were supposed to find out how much interest we each had on this one... from our side we are still interested in the idea in general but have no immediate need for it. I guess that means its a "nice to have" but not a "show stopper" or "must have" for us right now.

@Samze
Copy link
Contributor

Samze commented Apr 29, 2019

After a period of inactivity we are now revisiting generic instance extensions. There is a new proposal doc here.

@mattmcneeney
Copy link
Contributor

Closing this as we are planning on moving forward with #670

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.