-
Notifications
You must be signed in to change notification settings - Fork 435
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
Should the X-Api-Info-Location header be a part of the spec? #676
Comments
Hey @vorbidan Thanks for opening this issue. There are two headers sent on those requests today which may help you understand where the request originated from:
Did you look at either of those fields and see if it is possible to understand which platform the call originated from? |
@mattmcneeney - thank you for following up with the response. |
That makes sense. What about having a |
What would the platform_guid be? a URI, or UUID? |
Good question, maybe a GUID doesn't make sense. Maybe a URL is better, but do we have to worry about passing CC API URLs to service providers for security reasons? @waterlink @williammartin any thoughts on this? |
What does Pivotal use in |
Right now, CF sends
|
Great!
In either case @mattmcneeney 's security concern needs to be addressed |
About the security concern. I’m not sure what you’re hinting at? What does the CC URL give to the potential attacker? And even if it does have some concern, I can see how an attacker can get this URL by a variety of other means cheaply and quickly (including social engineering, or just signing up for the account if the platform is hosted; it’s also not hard to guess given enough information about the target). |
Well if we are already sending this information today, then I think my concern is void. If this is a header we send today then it would make sense for it to be an optional part of the spec. Any concerns about this from the svc-cat side? |
It looks like it makes a lot of sense to just add the header as an optional part of the spec, to bring it on par with implementation. |
@jberkhahn would you mind this header being part of the core spec? Would you use |
That would be difficult as service catalog doesn't inherently know where the kube cluster is running, and also the part issuing the requests (the service catalog controller) is an entirely different piece from the part you normally talk to (the main kube apiserver). On top of that, the actual api of the apiserver is obfuscated behind a go client lib, so I don't even know what endpoint on the apiserver I would put in there even if I did know the apiserver's address. So yeah, I don't really know what we'd put in there. I'll try to bring this up at a SIG meeting and get back to you |
Great, thanks @jberkhahn So depending on the outcome from that discussion, we have two options:
Let me know which of these seems most suitable for you @jberkhahn and we can look to open a PR. |
If a broker hands out different credentials to the platform instances, it can use these to identify the calling platform. There is no need for an additional header. |
@fmui That's true, but I think there are many cases where that doesn't happen today |
Dug a bit more into this. The k8s equivalent is a kubectl command called |
Cool - will update the PR to move this to profile only since it sounds like only CF will use this. |
What is the problem?
When the service broker is used across multiple platform instances, it would be nice to understand which instance is making a call.
It seems that Spring Cloud Open Service Broker implementation has such a header defined, however, I could not find traces of it in the spec..
Who does this affect?
Does this affect service broker authors, platform authors or developers?
Do you have any proposed solutions?
Include an X-Api-Info-Location header as a part of the specification and allow for platforms to set this header during all requests (catalog, provision, bind, unbind, update, deprovision)
The format of the header MUST be:
Example:
Where the value, when decoded, is:
Additional context
Here is how Pivotal describes the usage:
The text was updated successfully, but these errors were encountered: