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

--path option is ignored in keys command #150

Closed
babs opened this issue Mar 19, 2018 · 7 comments
Closed

--path option is ignored in keys command #150

babs opened this issue Mar 19, 2018 · 7 comments

Comments

@babs
Copy link

babs commented Mar 19, 2018

keys command seems to ignore path.

Current behavior:

rd keys create -P -t password --path test value
Enter password: 
# Created: keys/value [password]

Expected behavior:

rd keys create -P -t password --path test value
Enter password: 
# Created: keys/test/value [password]
@gschueler
Copy link
Member

The original intention was that you could use either --path keys/blah or append a PATH argument as keys/blah to the end of the command. Currently if both values are set, only the argument is used.

I can see how the behavior is not ideal.

I'd be willing to accept the PR if we get some more +1s on it

@babs
Copy link
Author

babs commented Apr 3, 2018

thing is current implementation forbirds path specification (invalid char in key name) :/

@gschueler
Copy link
Member

@babs oh, can you show an example? that sounds like a bug

@babs
Copy link
Author

babs commented Apr 3, 2018

@gschueler never mind I think I got the way it works.

It's either --path and a fully qualified key or just the key name at keys level.

I thought --path was to define the location of the key (kind of working directory).

--path doesn't expect an argument but changes the behavior of the positional argument PATH (or allows it to be missing?).
I was mislead by the doc [--path -p value] : Storage path, default: keys/

I tried with:

rd keys create -P -t password keys/test/mypassword
Enter password: 
Error: Invalid API Request: The Storage path components must be / separated, not start with a space, and contain only [a-zA-Z0-9+,._-]
[code: api.error.invalid.request; APIv21]
Request failed: 400 Bad Request

and then (assuming path was the "working dir")

rd keys create -P -t password --path test mypassword
Enter password: 
# Created: keys/mypassword [password]

Edit:
I thought the first positional argument PATH was mandatory according to Usage: create options PATH

@babs
Copy link
Author

babs commented Apr 4, 2018

@gschueler not sure how to fix doc to a oid abiguities :/

@gschueler
Copy link
Member

the expected behavior is that:

rd keys create -P -t password --path 'keys/test/mypassword'

works the same as

rd keys create -P -t password  'keys/test/mypassword'

where --path PATH and the PATH argument work the same, and expect the full storage path. although the "keys/" prefix if added will be removed for you.

I will take a closer look at it at some point, but HTH

@gschueler
Copy link
Member

Hi, I think I understand the confusion:

In your example you were doing rd keys create -P -t password --path test value

the command supports a --path <value> input OR specifying the path as the last value on the commandline without any argument indicator.

using --path test value was parsed as --path test and PATH argument value.

in that case the PATH argument was being used instead of the --path value, so you end up with path of keys/value.

The correct usage would have been either --path test/value or simply test/value, using / as the separator char not space ( ).

I think I will fix all of this confusion: I will remove the PATH as the last argument of the commandline, requiring usage of --path/-p flag.

additionally I think it should require that the path include the prefix keys/ which right now is added for you if you do not add it.

gschueler added a commit that referenced this issue Apr 25, 2018
Fix #150 require --path/-p arg to keys subcommands
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants