Service-catalog: Doc: make sure docs are clear that "parameters" are json and not restricted by configMap naming rules

[duglin]

Once we merge #1079 we're going to leave the user in a very odd state. They'll be able to specify parameters as either:
1 - a list of name/value pairs (not in a secret), or
2 - a raw json object in a secret

I can't think of a good way to explain to an end user how we ended up with these two options but not the obvious other combinations, meaning:
3 - a list of name/value pairs IN a secret, or
4 - a raw json object NOT in a secret

I think we need to add 3 & 4 to present a consistent UX to people.

[pmorie]

The API presented in #1075 is actually:

• Raw json object inline
• Raw json object from a secret key

No key/value pairs supported at the moment, and seems consistent in the sense that you can stick a JSON blob in, either inline or from a secret (which you should use if you have sensitive parameters).

Does that change things for you?

Can we close this issue - since it is about inconsistency - and open issues for specific additional modes we need to come back to once we have design more locked down for the initial beta?

[duglin]

hmm, maybe I'm just misreading it then... when I looked at the design doc and saw:

parameters:
  inlineField: abc

I was viewing inlineField: abc as the name/value pair. I see Parameters is defined as runtime.RawExtension, does that allow for all possible json values, including funky things like /'s in key names?

If so, then I think this might come down to a doc issue because from a newbie's point of view I would have viewed the examples in the design doc as name/value pairs and not raw json (especially when its written as yaml) and walked away (as I did) thinking there was an inconsistency.

[pmorie]

I see Parameters is defined as runtime.RawExtension, does that allow for all possible json values, including funky things like /'s in key names?

Yep, that's exactly what it does. It's basically a place to stick a json blob.

[pmorie]

It does look like key-value pairs due to how the json/yaml is embedded into the serialization of the resource.

[duglin]

ok - I re-titled this to be a doc issue so when we create our baseline docs we can remember to cover it.

[pmorie]

@duglin

@nilebox has written a doc for this in #1075 that is the place where this info should lie, I think

[duglin]

I mean the "real user-facing docs" not the "immediately stale design docs that we shouldn't be checking in" :-)

[pmorie]

@duglin I think @nilebox intends for that PR to be 'real' docs, so specific feedbacks would be good on that PR

[nilebox]

I see Parameters is defined as runtime.RawExtension, does that allow for all possible json value

as @pmorie correctly pointed above, it allows for any valid JSON or even YAML (which is translated into equivalent JSON).

@duglin I think @nilebox intends for that PR to be 'real' docs

@duglin I do intend to check-in the docs right after we merge the code PR, so any comments on improving them are welcome. I should probably at least add a reference to this document somewhere in the main README.

[nilebox]

@duglin I think the title of this issue is still misleading, we don't support configMaps at all yet.

I think we need to add 3 & 4 to present a consistent UX to people.

I don't think that current UX is inconsistent, do you still think so after the discussion above? Why?

I think we should close this issue for now, and create a separate issue for each actual use case blocking users, if there is any. Feel free to raise what is relevant to you.

[duglin]

I don't think I was clear. I don't have a concern with the design doc as it stands today, I think its accurate w.r.t. the PR. What concerns me is that its just a point in time statement and I don't see us updating that doc if the "parameters*" feature changes. That's why I talked about it being "stale".

One of my first experiences with trying to learn Kube was a fair amount of confusion w.r.t. which docs I found in the git repo were actual "up to date" docs vs just old design docs. There were times when the "real docs" had all the info, and there were times when I was directed to some design proposal. But I couldn't assume all design proposals were the latest so I had to guess or play around to find that out. I'm just trying to avoid that confusion.

So, for now I'm ok with checking in this design doc since its better to have this than nothing, but long term (and this is related to what we talked about the other day), once we get that "baseline" set of docs I'd like to remove any "design docs" we have from the repo and make sure our docs themselves are always up to date with the latest info. The design proposals will always live on in issues and PRs (and we could even point to them from the docs if people want to see the gory history), but to me our docs should be the place people go to find out about how to use svc-cat, not the code or design docs.

[nilebox]

@duglin I see your point, and agree with your opinion about the docs. I still see the value of checking in proposals into repo, but that's a separate topic.

The document in https://github.com/kubernetes-incubator/service-catalog/pull/1075 though was not intended to be a design doc but a documentation for the end user. It covers all the ways of passing parameters for Instance/Binding in general, with detailed examples. Also, it doesn't cover Go code, it talks only about user-facing API.

For example, it covers the usage of parameters (inline json) section - the part which hasn't been changed in https://github.com/kubernetes-incubator/service-catalog/pull/1079 at all. So I would expect this doc to be updated with every change of parameters support in Instances and Bindings.

[duglin]

Actually, going back to the original reason I opened this issue... the consistency part. While I now agree that raw json is supported for both secret and non-secret data, I think one of the reasons I initially viewed it as inconsistent is because we're not consistent in how the user gives the data to us - at least not in the examples provided in the design doc. One looks like a list of properties and the other is a serialized JSON object as a string.

I think I understand how we got to that situation (parameters is something we defined, while secrets don't have something like a RawExtension field), it just feels less than ideal.

No ask/suggestion at this time since I'm not sure what we can do about yet, just wanted to jot down a thought that popped into my head.

[duglin]

@nilebox ah ok, I apologize. For some reason I kept thinking it was a "design doc" and not part of our "normal docs". That changes how I want to review it and will try to do so later today.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.