-
Notifications
You must be signed in to change notification settings - Fork 158
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
Fixed knife vault show -F json and knife vault list -F json don't always output valid JSON #348
Conversation
Signed-off-by: sanga17 <sausekar@msystechnologies.com>
Signed-off-by: sanga17 <sausekar@msystechnologies.com>
@@ -35,7 +35,7 @@ def run | |||
bags.each_key do |bagname| | |||
vaultbags.push(bagname) if bag_is_vault?(bagname) | |||
end | |||
output vaultbags.join("\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this going to result in hard to read output on the CLI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tas50, nothing impacted on the CLI's output, but if we use join it is causing issues for JSON format, as posted in the issue.
split_vault_keys(bag)[1].each do |item| | ||
output item | ||
end | ||
output split_vault_keys(bag)[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's more than 1 key this will only show a single key. Is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tas50,yes it is intentional as split_vault_keys method return array of keys and items
chef-vault/lib/chef/knife/vault_base.rb
Line 81 in 9e9d28e
[keys, items] |
And regarding the change made in PR, below are some o/ps before change and after change:
Before Change:
knife vault show val_passwords
root
root1
knife vault show val_passwords -F json
"root"
"root1"
knife vault show val_passwords -F yaml
--- root
...
--- root1
...
After Change:
knife vault show val_passwords
root
root1
knife vault show val_passwords -F json
[
"root",
"root1"
]
knife vault show val_passwords -F yaml
---
- root
- root1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a breaking change
Description
Fixed knife vault show -F json and knife vault list -F json don't always output valid JSON
Related Issue
Fixed: #313
Types of changes
Checklist: