-
Notifications
You must be signed in to change notification settings - Fork 434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add X-Api-Info-Location header
to the profile document
#686
base: master
Are you sure you want to change the base?
Add X-Api-Info-Location header
to the profile document
#686
Conversation
✅ Hey mattmcneeney! The commit authors and yourself have already signed the CLA. |
Thank you @mattmcneeney
I personally like the proposed format with an explicit platform type mentioned in the value, besides, it is consistent with the Originating Identity header. |
Ah, good point. I missed the bit about CF not sending the |
The other thing I don't like about this is the fact that all other headers start with |
If this header is only supported by CF, the specification should only go into the profile document. |
It is currently supported by CF, I think the goal is to make it optional for other platforms (k8s) as well, but we have not heard from @jberkhahn if it is even possible... Don't you think that because os these many questions it is a bit premature to be adding it to the spec PR? |
I responded in the issue that there really isn't a k8s equivalent to this, for several reasons. I don't think adding this to the spec is a good idea. |
That makes sense. Are you happy if I detail this header in the profile doc only? |
7f50305
to
5be67c6
Compare
5be67c6
to
b9785b5
Compare
Updated the PR, and also removed the header from the openapi and swagger docs since this isn't part of the core spec. cc @jberkhahn |
X-Api-Info-Location header
X-Api-Info-Location header
to the profile document
Hey @vorbidan We discussed this on today's call and spoke about whether or not we could, instead of using this existing header that CF sends, add a field to the context object along the lines of What do you think? |
@mattmcneeney |
I've moved this header to the profile doc. Reviews please! |
Review please @fmui @tinygrasshopper |
@zrob pointed out on the call today that with the CF CAPI v2->v3 transition going on, the location and contents of this header may change. We might want to wait until we have this functionality in v3. Heroku also may have a URL and other info that they may want to make available to service providers. This would help their service brokers support multiple platforms where they need to call back into them. @waterlink mentioned that service brokers will also need to know how to interact with the platform API. The We could introduce a second header informing the service broker of the platform that the URL relates too, or we could include it in the encoded header value, e.g.:
Let's continue the discussion on here. |
@mattmcneeney Good point on CF CAPI v3. There does not seem to be an equivalent of |
No objections on today's call on postponing this until the CAPI v3 transition is over. |
The Here are the current pains associated with using the CC API v2/info endpoint in the Osb cloudfoundry profile:
{
"name": "",
"build": "",
"support": "https://support.run.pivotal.io",
"version": 0,
"description": "Cloud Foundry sponsored by Pivotal",
"authorization_endpoint": "https://login.run.pivotal.io",
"token_endpoint": "https://uaa.run.pivotal.io",
"min_cli_version": "6.22.0",
"min_recommended_cli_version": "latest",
"app_ssh_endpoint": "ssh.run.pivotal.io:2222",
"app_ssh_host_key_fingerprint": "e7:13:4e:32:ee:39:62:df:54:41:d7:f7:8b:b2:a7:6b",
"app_ssh_oauth_client": "ssh-proxy",
"doppler_logging_endpoint": "wss://doppler.run.pivotal.io:443",
"api_version": "2.141.0",
"osbapi_version": "2.15",
"routing_endpoint": "https://api.run.pivotal.io/routing",
"user": "526cc2f7-..."
} I first considered having the X-Api-Info-Location header instead contain relevant content for brokers such as: {
"authorization_endpoint": "https://login.platform.example.com",
"token_endpoint": "https://uaa.platform.example.com",
"service_instance_permission_endpoint": "https://api.platform.example.com/v2/service_instances/:guid/permissions"
} However, after a second thought, these fields might be better suited in the response of the Benefits of including OIDC and Service-instance-permission endpoints in the context object on the provisionning endpoint:
|
Removing the v2.16 milestone from this, and leaving this blocked given the comment above:
|
What is the problem this PR solves?
Closes #676
Checklist: