Skip to content
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

Cli does not parse integer for cups #193

Closed
xtreme-sameer-vohra opened this issue Jun 26, 2014 · 5 comments
Closed

Cli does not parse integer for cups #193

xtreme-sameer-vohra opened this issue Jun 26, 2014 · 5 comments

Comments

@xtreme-sameer-vohra
Copy link

Cli garbles the credentials section of a service when cups is used with a integer inside the params hash.

cf -v is cf version 6.1.2-6a013ca

Port as integer
cf cups broken-service -p '{"hostname":"db.example.com", "port":3306}'

Port as string
cf cups working-service -p '{"hostname":"db.example.com", "port":"3306"}'

Output from logs/env.log for app bound to the two services above

VCAP_SERVICES={"user-provided":[{"name":"broken-service","label":"user-provided","tags":[],"credentials":{"\port\:3306}":"","hostname":"db.example.com","{\hostname\:\db.example.com\":""},"syslog_drain_url":""},{"name":"working-service","label":"user-provided","tags":[],"credentials":{"hostname":"db.example.com","port":"3306"},"syslog_drain_url":""}]}
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/73971288.

@tedsuo
Copy link

tedsuo commented Jun 26, 2014

@xtreme-sameer-vohra @tjarratt looks like the CLI is presuming your JSON conforms to map[string]string when really it should be looking for map[string]interface{}, since integers appear to be valid values for cups parameters.

This raises another issue. We can fix the JSON parameters pretty easily, but what about interactive parameters? Either we only allow string values, or we have to infer the type. Inference opens the door to inconsistency, if a value is supposed to be a string but can sometimes look like a number.

@xtreme-sameer-vohra
Copy link
Author

Great point. Interactive mode certainly adds more complexity.
As mentioned by you, the API accepts integers. Does it also accept a nested object or any valid JSON string? That would further complicate the interactive mode.

I guess an option would be to require a user to specify the key + type when using interactive mode.
Something along the lines of the rails cli when creating models.

Based on the examples shown in the CLI help for cups, I automatically assumed it would happily accept valid JSON.

@tjarratt
Copy link
Contributor

tjarratt commented Jul 1, 2014

Based on the current API documentation, it seemingly does not specify what values you can store, so it seems reasonable to assume it allows nested objects and any valid JSON.

I'm not a huge fan of the interactive mode (we tried really hard to remove this as much as possible for v6.0 because it makes scripting hard). My feeling is that we can make @tedsuo's proposed change to make the map a map[string]interface{} since that would be backwards compatible. Interactive parameters can continue to be handled as strings, since that would have been the existing behavior. Honestly, I would be surprised if more than five people have ever interactively created a user provided service.

lvarvel pushed a commit that referenced this issue Jul 10, 2014
@lcddave
Copy link
Contributor

lcddave commented Jul 22, 2014

Fixed

@lcddave lcddave closed this as completed Jul 22, 2014
haydonryan pushed a commit to haydonryan/cli that referenced this issue Nov 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants