Closed duglin closed 7 years ago
IMO as long as whatever restrictions we all work with all existing platforms and brokers, then we could probably add this in a minor version. Technically it's a breaking change, but in reality it isn't, and I don't see the benefit of holding off on making the spec clearer for both broker authors and platform authors to understand.
All brokers in CF that I have seen use GUIDS. Also the spec examples all say service-guid-here
or plan-guid-here
.
To avoid a "breaking-change", how about we update this:
"Using a GUID is RECOMMENDED to ensure compatibility with platforms."
Side note: This would be a good 3.0
thing to iron out of the API. Shall we start tracking these?
We've already seen brokers send non-GUIDs (e.g. hashicorp/cf-vault-service-broker sends an ID with a period) - so that exposes the problem with trying to treat a "RECOMMENDED" as an implied "MUST" - its not really :-)
In interest of full disclosure, as much as it would be nice to limit the chars, the spec-geek in me can't see us doing that with 1) breaking backwards compat, or 2) getting buy-in from ALL existing brokers - which I have no idea how to do since there could be private CF deploys/brokers that we'll never know about.
Yes we should keep a v3.0 issue list. Maybe just create a "v3.0" label for those issue.
Interesting.
So what is the plan for working around this in k8s?
@avade see https://github.com/kubernetes-incubator/service-catalog/issues/802 near the bottom for a proposal that covers this issue and others.
its not been agreed to but it is an idea...
From today's OSB API call - CF doesn't use these IDs for anything other than in responses back to the broker. They create their own (other) GUIDs for their own internal usage/reference.
From today's call:
123 456
@zrob thanks for gathering the info for us.
if the field is used as a path segment in API calls, the characters /
and %
and the exact strings .
and ..
are extremely problematic and should be prohibited.
I would assume most impls would/should just URL-encode them to avoid any problems.
And by that I meant always URL-encode them w/o actually looking for problematic chars. We should probably add some text to the spec to remind people about this.
URL-encoding doesn't actually fix problems with those specific characters in path segments... http handler libraries, load balancers, and proxies are notoriously bad at passing those through transparently.
Since we really only need to worry about this on messages from the platform to the broker, I think this falls into the category of "its your own fault if things break" :-) Meaning, if there are problem components on the receiving end (the broker) then the broker shouldn't use chars that don't work in its own infrastructure.
Just to reiterate a suggestion I made in issue #137, a common identifier requirement I have seen used in various places is the URI unrestricted character set, along with a length limit: https://tools.ietf.org/html/rfc3986#section-2.3
I'll also point out that there are cases where GUID is overly restrictive. Many broker providers will have ID systems in place that conform to sane character sets but do not conform to GUID standard. Forcing them to have to potentially generate and store these GUIDs may be problematic.
An example case I have seen is a broker that is stateless and relies on other infrastructure to store state which provides its own ID.
From the f2f last week:
ServiceID and PlanID are defined as a non-empty strings w/o any restrictions on the exact set of characters that can be used - however, the spec does RECOMMEND a GUID be used.
This leaves the question of whether the intent was to really leave it totally wide open for any characters (including things like forward/back-slashes, punctuation and spaces).
If in Kubernetes we were to use these IDs as the primary look-up key then we'd run into issues because the rules in Kube for these fields are: should be up to maximum length of 253 characters and consist of lower case alphanumeric characters, -, and .
Question for OSB API group to consider: can/should we be more specific about the valid character set of these IDs w/o requiring a version bump in the spec?