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 Enhancements #3897

Merged
merged 36 commits into from
Feb 12, 2018
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
c14210e
Use Colored UI if stdout is a tty
calvn Feb 1, 2018
58be122
Add format options to operator unseal
calvn Feb 1, 2018
a1fcb87
Add format test on operator unseal
calvn Feb 1, 2018
e149184
Add -no-color output flag, and use BasicUi if no-color flag is provided
calvn Feb 1, 2018
45627d6
Move seal status formatting logic to OutputSealStatus
calvn Feb 1, 2018
ae12135
Apply no-color to warnings from DeprecatedCommands as well
calvn Feb 2, 2018
fa462a4
Add OutputWithFormat to support arbitrary data, add format option to …
calvn Feb 6, 2018
14e70dd
Add ability to output arbitrary list data on TableFormatter
calvn Feb 6, 2018
caae214
Clear up switch logic on format
calvn Feb 6, 2018
e0f96d6
Add format option for list-related commands
calvn Feb 6, 2018
c8cfcb5
Add format option to rest of commands that returns a client API response
calvn Feb 7, 2018
d2ba2fe
Remove initOutputYAML and initOutputJSON, and use OutputWithFormat in…
calvn Feb 7, 2018
8f2a229
Remove outputAsYAML and outputAsJSON, and use OutputWithFormat instead
calvn Feb 7, 2018
204180b
Remove -no-color flag, use env var exclusively to toggle colored output
calvn Feb 7, 2018
2cd9e8e
Fix compile
calvn Feb 7, 2018
c9573ea
Remove -no-color flag in main.go
calvn Feb 7, 2018
7ecec64
Add missing FlagSetOutputFormat
calvn Feb 7, 2018
189f8c9
Fix generate-root/decode test
calvn Feb 8, 2018
60184e9
Merge branch 'master-oss' into cli-enhancements
calvn Feb 8, 2018
a7225da
Migrate init functions to main.go
jefferai Feb 8, 2018
2b3f18d
Add no-color flag back as hidden
jefferai Feb 8, 2018
63d4957
Handle non-supported data types for TableFormatter.OutputList
calvn Feb 9, 2018
53d4534
Merge branch 'cli-enhancements' of github.com:hashicorp/vault into cl…
calvn Feb 9, 2018
4ff62dd
Pull formatting much further up to remove the need to use c.flagForma…
jefferai Feb 9, 2018
3ff53bc
Minor updates
jefferai Feb 9, 2018
a81c42e
Remove unnecessary check
jefferai Feb 9, 2018
79992f3
Fix SSH output and some tests
jefferai Feb 9, 2018
07cf36e
Fix tests
jefferai Feb 9, 2018
7982def
Make race detector not run on generate root since it kills Travis the…
jefferai Feb 9, 2018
9ca29d3
Update docs
calvn Feb 9, 2018
df07a3d
Merge branch 'cli-enhancements' of github.com:hashicorp/vault into cl…
calvn Feb 9, 2018
1583e23
Update docs
calvn Feb 10, 2018
51a07e2
Merge branch 'master-oss' into cli-enhancements
jefferai Feb 11, 2018
9b62480
Address review feedback
jefferai Feb 11, 2018
d0e35e1
Handle --format as well as -format
jefferai Feb 12, 2018
e4ae7ba
Merge branch 'master' into cli-enhancements
calvn Feb 12, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions api/sys_generate_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ func (c *Sys) generateRootUpdateCommon(path, shard, nonce string) (*GenerateRoot
}

type GenerateRootStatusResponse struct {
Nonce string
Started bool
Progress int
Required int
Complete bool
Nonce string `json:"nonce"`
Started bool `json:"started"`
Progress int `json:"progress"`
Required int `json:"required"`
Complete bool `json:"complete"`
EncodedToken string `json:"encoded_token"`
EncodedRootToken string `json:"encoded_root_token"`
PGPFingerprint string `json:"pgp_fingerprint"`
Expand Down
26 changes: 13 additions & 13 deletions api/sys_rekey.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,27 +177,27 @@ type RekeyInitRequest struct {
}

type RekeyStatusResponse struct {
Nonce string
Started bool
T int
N int
Progress int
Required int
Nonce string `json:"nonce"`
Started bool `json:"started"`
T int `json:"t"`
N int `json:"n"`
Progress int `json:"progress"`
Required int `json:"required"`
PGPFingerprints []string `json:"pgp_fingerprints"`
Backup bool
Backup bool `json:"backup"`
}

type RekeyUpdateResponse struct {
Nonce string
Complete bool
Keys []string
Nonce string `json:"nonce"`
Complete bool `json:"complete"`
Keys []string `json:"keys"`
KeysB64 []string `json:"keys_base64"`
PGPFingerprints []string `json:"pgp_fingerprints"`
Backup bool
Backup bool `json:"backup"`
}

type RekeyRetrieveResponse struct {
Nonce string
Keys map[string][]string
Nonce string `json:"nonce"`
Keys map[string][]string `json:"keys"`
KeysB64 map[string][]string `json:"keys_base64"`
}
16 changes: 10 additions & 6 deletions command/audit_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Usage: vault audit list [options]
}

func (c *AuditListCommand) Flags() *FlagSets {
set := c.flagSet(FlagSetHTTP)
set := c.flagSet(FlagSetHTTP | FlagSetOutputFormat)

f := set.NewFlagSet("Command Options")

Expand Down Expand Up @@ -99,13 +99,17 @@ func (c *AuditListCommand) Run(args []string) int {
return 0
}

if c.flagDetailed {
c.UI.Output(tableOutput(c.detailedAudits(audits), nil))
switch Format() {
case "table":
if c.flagDetailed {
c.UI.Output(tableOutput(c.detailedAudits(audits), nil))
return 0
}
c.UI.Output(tableOutput(c.simpleAudits(audits), nil))
return 0
default:
return OutputData(c.UI, audits)
}

c.UI.Output(tableOutput(c.simpleAudits(audits), nil))
return 0
}

func (c *AuditListCommand) simpleAudits(audits map[string]*api.Audit) []string {
Expand Down
19 changes: 12 additions & 7 deletions command/auth_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Usage: vault auth list [options]
}

func (c *AuthListCommand) Flags() *FlagSets {
set := c.flagSet(FlagSetHTTP)
set := c.flagSet(FlagSetHTTP | FlagSetOutputFormat)

f := set.NewFlagSet("Command Options")

Expand All @@ -55,7 +55,8 @@ func (c *AuthListCommand) Flags() *FlagSets {
Target: &c.flagDetailed,
Default: false,
Usage: "Print detailed information such as configuration and replication " +
"status about each auth method.",
"status about each auth method. This option is only applicable to " +
"table-formatted output.",
})

return set
Expand Down Expand Up @@ -95,13 +96,17 @@ func (c *AuthListCommand) Run(args []string) int {
return 2
}

if c.flagDetailed {
c.UI.Output(tableOutput(c.detailedMounts(auths), nil))
switch Format() {
case "table":
if c.flagDetailed {
c.UI.Output(tableOutput(c.detailedMounts(auths), nil))
return 0
}
c.UI.Output(tableOutput(c.simpleMounts(auths), nil))
return 0
default:
return OutputData(c.UI, auths)
}

c.UI.Output(tableOutput(c.simpleMounts(auths), nil))
return 0
}

func (c *AuthListCommand) simpleMounts(auths map[string]*api.AuthMount) []string {
Expand Down
26 changes: 23 additions & 3 deletions command/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ type BaseCommand struct {
flagTLSSkipVerify bool
flagWrapTTL time.Duration

flagFormat string
flagField string
flagFormat string
flagField string
flagNoColor bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this and the var below if we are just using the envvar

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was to allow it in a flag but not advertise it. Now that you mention autocorrect below, I wonder if it's better to just remove it rather than have people get it autocorrected and then complain that it's not documented. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the code will be a lot simpler if we drop the flag entirely. People are more likely to "never want colored output" that "not want colored output for this specific command", and even in the latter case, they can specify the envvar first.


tokenHelper token.TokenHelper

Expand Down Expand Up @@ -152,6 +153,11 @@ func (c *BaseCommand) flagSet(bit FlagSetBit) *FlagSets {
c.flagsOnce.Do(func() {
set := NewFlagSets(c.UI)

// These flag sets will apply to all leaf subcommands.
// TODO: Optional, but FlagSetHTTP can be safely removed from the individual
// Flags() subcommands.
bit = bit | FlagSetHTTP

if bit&FlagSetHTTP != 0 {
f := set.NewFlagSet("HTTP Options")

Expand Down Expand Up @@ -237,6 +243,15 @@ func (c *BaseCommand) flagSet(bit FlagSetBit) *FlagSets {
"The TTL is specified as a numeric string with suffix like \"30s\" " +
"or \"5m\".",
})

f.BoolVar(&BoolVar{
Name: "no-color",
Target: &c.flagNoColor,
Default: false,
Hidden: true,
EnvVar: EnvVaultCLINoColor,
Usage: "Print the output without ANSI color escape sequences.",
})
}

if bit&(FlagSetOutputField|FlagSetOutputFormat) != 0 {
Expand All @@ -260,7 +275,7 @@ func (c *BaseCommand) flagSet(bit FlagSetBit) *FlagSets {
Name: "format",
Target: &c.flagFormat,
Default: "table",
EnvVar: "VAULT_FORMAT",
EnvVar: EnvVaultFormat,
Completion: complete.PredictSet("table", "json", "yaml"),
Usage: "Print the output in the given format. Valid formats " +
"are \"table\", \"json\", or \"yaml\".",
Expand Down Expand Up @@ -317,6 +332,11 @@ func (f *FlagSets) Parse(args []string) error {
return f.mainSet.Parse(args)
}

// Parsed reports whether the command-line flags have been parsed.
func (f *FlagSets) Parsed() bool {
return f.mainSet.Parsed()
}

// Args returns the remaining args after parsing.
func (f *FlagSets) Args() []string {
return f.mainSet.Args()
Expand Down
30 changes: 11 additions & 19 deletions command/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ import (
physZooKeeper "github.com/hashicorp/vault/physical/zookeeper"
)

const (
// EnvVaultCLINoColor is an env var that toggles colored UI output.
EnvVaultCLINoColor = `VAULT_CLI_NO_COLOR`
// EnvVaultFormat is the output format
EnvVaultFormat = `VAULT_FORMAT`
)

var (
auditBackends = map[string]audit.Factory{
"file": auditFile.Factory,
Expand Down Expand Up @@ -149,7 +156,9 @@ func (c *DeprecatedCommand) Help() string {

// Run wraps the embedded Run command and prints a warning about deprecation.
func (c *DeprecatedCommand) Run(args []string) int {
c.warn()
if Format() == "table" {
c.warn()
}
return c.Command.Run(args)
}

Expand All @@ -166,24 +175,7 @@ func (c *DeprecatedCommand) warn() {
var Commands map[string]cli.CommandFactory
var DeprecatedCommands map[string]cli.CommandFactory

func init() {
ui := &cli.ColoredUi{
ErrorColor: cli.UiColorRed,
WarnColor: cli.UiColorYellow,
Ui: &cli.BasicUi{
Writer: os.Stdout,
ErrorWriter: os.Stderr,
},
}

serverCmdUi := &cli.ColoredUi{
ErrorColor: cli.UiColorRed,
WarnColor: cli.UiColorYellow,
Ui: &cli.BasicUi{
Writer: os.Stdout,
},
}

func initCommands(ui, serverCmdUi cli.Ui) {
loginHandlers := map[string]LoginHandler{
"aws": &credAws.CLIHandler{},
"centrify": &credCentrify.CLIHandler{},
Expand Down
79 changes: 58 additions & 21 deletions command/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"errors"
"fmt"
"os"
"sort"
"strings"

Expand All @@ -19,29 +20,37 @@ const (
hopeDelim = "♨"
)

func OutputSecret(ui cli.Ui, format string, secret *api.Secret) int {
return outputWithFormat(ui, format, secret, secret)
type FormatOptions struct {
Format string
}

func OutputList(ui cli.Ui, format string, secret *api.Secret) int {
return outputWithFormat(ui, format, secret, secret.Data["keys"])
func OutputSecret(ui cli.Ui, secret *api.Secret) int {
return outputWithFormat(ui, secret, secret)
}

func outputWithFormat(ui cli.Ui, format string, secret *api.Secret, data interface{}) int {
// If we had a colored UI, pull out the nested ui so we don't add escape
// sequences for outputting json, etc.
colorUI, ok := ui.(*cli.ColoredUi)
if ok {
ui = colorUI.Ui
func OutputList(ui cli.Ui, data interface{}) int {
switch data.(type) {
case *api.Secret:
secret := data.(*api.Secret)
return outputWithFormat(ui, secret, secret.Data["keys"])
default:
return outputWithFormat(ui, nil, data)
}
}

func OutputData(ui cli.Ui, data interface{}) int {
return outputWithFormat(ui, nil, data)
}

formatter, ok := Formatters[strings.ToLower(format)]
func outputWithFormat(ui cli.Ui, secret *api.Secret, data interface{}) int {
formatter, ok := Formatters[Format()]
if !ok {
ui.Error(fmt.Sprintf("Invalid output format: %s", format))
ui.Error(fmt.Sprintf("Invalid output format: %s", Format()))
return 1
}

if err := formatter.Output(ui, secret, data); err != nil {
ui.Error(fmt.Sprintf("Could not output secret: %s", err.Error()))
ui.Error(fmt.Sprintf("Could not parse output: %s", err.Error()))
return 1
}
return 0
Expand All @@ -58,6 +67,14 @@ var Formatters = map[string]Formatter{
"yml": YamlFormatter{},
}

func Format() string {
format := os.Getenv(EnvVaultFormat)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we retrieve this as part of an init function instead of doing the envvar lookup each time?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just got away from init functions in favor of being able to intercept the args earlier in the main run function. This isn't going to be meaningfully slow for interactive uses, is there a reason not to just call this a couple of times?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've experienced Getenv being slow (in relative terms), particularly on Windows systems. I wonder if we could use a Once instead and cache the result?

if format == "" {
format = "table"
}
return format
}

// An output formatter for json output of an object
type JsonFormatter struct{}

Expand Down Expand Up @@ -87,19 +104,32 @@ type TableFormatter struct {
}

func (t TableFormatter) Output(ui cli.Ui, secret *api.Secret, data interface{}) error {
// TODO: this should really use reflection like the other formatters do
if s, ok := data.(*api.Secret); ok {
return t.OutputSecret(ui, s)
switch data.(type) {
case *api.Secret:
return t.OutputSecret(ui, secret)
case []interface{}:
return t.OutputList(ui, secret, data)
case []string:
return t.OutputList(ui, nil, data)
default:
return errors.New("Cannot use the table formatter for this type")
}
if s, ok := data.([]interface{}); ok {
return t.OutputList(ui, secret, s)
}
return errors.New("Cannot use the table formatter for this type")
}

func (t TableFormatter) OutputList(ui cli.Ui, secret *api.Secret, list []interface{}) error {
func (t TableFormatter) OutputList(ui cli.Ui, secret *api.Secret, data interface{}) error {
t.printWarnings(ui, secret)

switch data.(type) {
case []interface{}:
case []string:
ui.Output(tableOutput(data.([]string), nil))
return nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a default case returning an error here quoting that its an unknown type? This avoids panic when the data is actually cast to []interface{} below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done below.

default:
return errors.New("Error: table formatter cannot output list for this data type")
}

list := data.([]interface{})

if len(list) > 0 {
keys := make([]string, len(list))
for i, v := range list {
Expand Down Expand Up @@ -208,7 +238,14 @@ func (t TableFormatter) OutputSecret(ui cli.Ui, secret *api.Secret) error {
return nil
}

// OutputSealStatus will print *api.SealStatusResponse in the CLI according to the format provided
func OutputSealStatus(ui cli.Ui, client *api.Client, status *api.SealStatusResponse) int {
switch Format() {
case "table":
default:
return OutputData(ui, status)
}

var sealPrefix string
if status.RecoverySeal {
sealPrefix = "Recovery "
Expand Down
Loading