Normalize format output for vault status [VAULT-508] (#9976)

* normalize format output for vault status

* interim commit

* interim commit

* make formatting idiomatic

* clean up comments

* added formatting test

* updated comments in format test to match godocs

Co-authored-by: HridoyRoy <hridoyroy@Hridoys-MBP.hitronhub.home>
Co-authored-by: HridoyRoy <hridoyroy@Hridoys-MacBook-Pro.local>

api/go.sum

@@ -91,6 +91,7 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
 github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/gorilla/mux v1.7.4/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
 github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
@@ -332,6 +333,7 @@ google.golang.org/protobuf v1.22.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2
 google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU=
 google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU=
 google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGjtUeSXeh4=
+google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c=
 gopkg.in/airbrake/gobrake.v2 v2.0.9/go.mod h1:/h5ZAUhDkGaJfjzjKLSjv6zCL6O0LLBxU4K+aSYdM/U=
 gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

command/format.go

@@ -117,7 +117,8 @@ func (y YamlFormatter) Output(ui cli.Ui, secret *api.Secret, data interface{}) e
 // An output formatter for table output of an object
 type TableFormatter struct{}
 
-// We don't use this
+// We don't use this due to the TableFormatter introducing a bug when the -field flag is supplied:
+// https://github.com/hashicorp/vault/commit/b24cf9a8af2190e96c614205b8cdf06d8c4b6718 .
 func (t TableFormatter) Format(data interface{}) ([]byte, error) {
 	return nil, nil
 }
@@ -132,11 +133,92 @@ func (t TableFormatter) Output(ui cli.Ui, secret *api.Secret, data interface{})
 		return t.OutputList(ui, nil, data)
 	case map[string]interface{}:
 		return t.OutputMap(ui, data.(map[string]interface{}))
+	case SealStatusOutput:
+		return t.OutputSealStatusStruct(ui, nil, data)
 	default:
 		return errors.New("cannot use the table formatter for this type")
 	}
 }
 
+func (t TableFormatter) OutputSealStatusStruct(ui cli.Ui, secret *api.Secret, data interface{}) error {
+	var status SealStatusOutput = data.(SealStatusOutput)
+	var sealPrefix string
+	if status.RecoverySeal {
+		sealPrefix = "Recovery "
+	}
+
+	out := []string{}
+	out = append(out, "Key | Value")
+	out = append(out, fmt.Sprintf("%sSeal Type | %s", sealPrefix, status.Type))
+	out = append(out, fmt.Sprintf("Initialized | %t", status.Initialized))
+	out = append(out, fmt.Sprintf("Sealed | %t", status.Sealed))
+	out = append(out, fmt.Sprintf("Total %sShares | %d", sealPrefix, status.N))
+	out = append(out, fmt.Sprintf("Threshold | %d", status.T))
+
+	if status.Sealed {
+		out = append(out, fmt.Sprintf("Unseal Progress | %d/%d", status.Progress, status.T))
+		out = append(out, fmt.Sprintf("Unseal Nonce | %s", status.Nonce))
+	}
+
+	if status.Migration {
+		out = append(out, fmt.Sprintf("Seal Migration in Progress | %t", status.Migration))
+	}
+
+	out = append(out, fmt.Sprintf("Version | %s", status.Version))
+	out = append(out, fmt.Sprintf("Storage Type | %s", status.StorageType))
+
+	if status.ClusterName != "" && status.ClusterID != "" {
+		out = append(out, fmt.Sprintf("Cluster Name | %s", status.ClusterName))
+		out = append(out, fmt.Sprintf("Cluster ID | %s", status.ClusterID))
+	}
+
+	// Output if HA is enabled
+	out = append(out, fmt.Sprintf("HA Enabled | %t", status.HAEnabled))
+
+	if status.HAEnabled {
+		mode := "sealed"
+		if !status.Sealed {
+			out = append(out, fmt.Sprintf("HA Cluster | %s", status.LeaderClusterAddress))
+			mode = "standby"
+			showLeaderAddr := false
+			if status.IsSelf {
+				mode = "active"
+			} else {
+				if status.LeaderAddress == "" {
+					status.LeaderAddress = "<none>"
+				}
+				showLeaderAddr = true
+			}
+			out = append(out, fmt.Sprintf("HA Mode | %s", mode))
+
+			// This is down here just to keep ordering consistent
+			if showLeaderAddr {
+				out = append(out, fmt.Sprintf("Active Node Address | %s", status.LeaderAddress))
+			}
+
+			if status.PerfStandby {
+				out = append(out, fmt.Sprintf("Performance Standby Node | %t", status.PerfStandby))
+				out = append(out, fmt.Sprintf("Performance Standby Last Remote WAL | %d", status.PerfStandbyLastRemoteWAL))
+			}
+		}
+	}
+
+	if status.RaftCommittedIndex > 0 {
+		out = append(out, fmt.Sprintf("Raft Committed Index | %d", status.RaftCommittedIndex))
+	}
+	if status.RaftAppliedIndex > 0 {
+		out = append(out, fmt.Sprintf("Raft Applied Index | %d", status.RaftAppliedIndex))
+	}
+	if status.LastWAL != 0 {
+		out = append(out, fmt.Sprintf("Last WAL | %d", status.LastWAL))
+	}
+
+	ui.Output(tableOutput(out, &columnize.Config{
+		Delim: "|",
+	}))
+	return nil
+}
+
 func (t TableFormatter) OutputList(ui cli.Ui, secret *api.Secret, data interface{}) error {
 	t.printWarnings(ui, secret)
 
@@ -306,41 +388,7 @@ func (t TableFormatter) OutputMap(ui cli.Ui, data map[string]interface{}) error
 
 // 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(ui) {
+	sealStatusOutput := SealStatusOutput{SealStatusResponse: *status}
-	case "table":
-	default:
-		return OutputData(ui, status)
-	}
-
-	var sealPrefix string
-	if status.RecoverySeal {
-		sealPrefix = "Recovery "
-	}
-
-	out := []string{}
-	out = append(out, "Key | Value")
-	out = append(out, fmt.Sprintf("%sSeal Type | %s", sealPrefix, status.Type))
-	out = append(out, fmt.Sprintf("Initialized | %t", status.Initialized))
-	out = append(out, fmt.Sprintf("Sealed | %t", status.Sealed))
-	out = append(out, fmt.Sprintf("Total %sShares | %d", sealPrefix, status.N))
-	out = append(out, fmt.Sprintf("Threshold | %d", status.T))
-
-	if status.Sealed {
-		out = append(out, fmt.Sprintf("Unseal Progress | %d/%d", status.Progress, status.T))
-		out = append(out, fmt.Sprintf("Unseal Nonce | %s", status.Nonce))
-	}
-
-	if status.Migration {
-		out = append(out, fmt.Sprintf("Seal Migration in Progress | %t", status.Migration))
-	}
-
-	out = append(out, fmt.Sprintf("Version | %s", status.Version))
-	out = append(out, fmt.Sprintf("Storage Type | %s", status.StorageType))
-
-	if status.ClusterName != "" && status.ClusterID != "" {
-		out = append(out, fmt.Sprintf("Cluster Name | %s", status.ClusterName))
-		out = append(out, fmt.Sprintf("Cluster ID | %s", status.ClusterID))
-	}
 
 	// Mask the 'Vault is sealed' error, since this means HA is enabled, but that
 	// we cannot query for the leader since we are sealed.
@@ -354,48 +402,17 @@ func OutputSealStatus(ui cli.Ui, client *api.Client, status *api.SealStatusRespo
 		return 1
 	}
 
-	// Output if HA is enabled
+	// copy leaderStatus fields into sealStatusOutput for display later
-	out = append(out, fmt.Sprintf("HA Enabled | %t", leaderStatus.HAEnabled))
+	sealStatusOutput.HAEnabled = leaderStatus.HAEnabled
-
+	sealStatusOutput.IsSelf = leaderStatus.IsSelf
-	if leaderStatus.HAEnabled {
+	sealStatusOutput.LeaderAddress = leaderStatus.LeaderAddress
-		mode := "sealed"
+	sealStatusOutput.LeaderClusterAddress = leaderStatus.LeaderClusterAddress
-		if !status.Sealed {
+	sealStatusOutput.PerfStandby = leaderStatus.PerfStandby
-			out = append(out, fmt.Sprintf("HA Cluster | %s", leaderStatus.LeaderClusterAddress))
+	sealStatusOutput.PerfStandbyLastRemoteWAL = leaderStatus.PerfStandbyLastRemoteWAL
-			mode = "standby"
+	sealStatusOutput.LastWAL = leaderStatus.LastWAL
-			showLeaderAddr := false
+	sealStatusOutput.RaftCommittedIndex = leaderStatus.RaftCommittedIndex
-			if leaderStatus.IsSelf {
+	sealStatusOutput.RaftAppliedIndex = leaderStatus.RaftAppliedIndex
-				mode = "active"
+	OutputData(ui, sealStatusOutput)
-			} else {
-				if leaderStatus.LeaderAddress == "" {
-					leaderStatus.LeaderAddress = "<none>"
-				}
-				showLeaderAddr = true
-			}
-			out = append(out, fmt.Sprintf("HA Mode | %s", mode))
-
-			// This is down here just to keep ordering consistent
-			if showLeaderAddr {
-				out = append(out, fmt.Sprintf("Active Node Address | %s", leaderStatus.LeaderAddress))
-			}
-
-			if leaderStatus.PerfStandby {
-				out = append(out, fmt.Sprintf("Performance Standby Node | %t", leaderStatus.PerfStandby))
-				out = append(out, fmt.Sprintf("Performance Standby Last Remote WAL | %d", leaderStatus.PerfStandbyLastRemoteWAL))
-			}
-		}
-	}
-
-	if leaderStatus.RaftCommittedIndex > 0 {
-		out = append(out, fmt.Sprintf("Raft Committed Index | %d", leaderStatus.RaftCommittedIndex))
-	}
-	if leaderStatus.RaftAppliedIndex > 0 {
-		out = append(out, fmt.Sprintf("Raft Applied Index | %d", leaderStatus.RaftAppliedIndex))
-	}
-	if leaderStatus.LastWAL != 0 {
-		out = append(out, fmt.Sprintf("Last WAL | %d", leaderStatus.LastWAL))
-	}
-
-	ui.Output(tableOutput(out, nil))
 	return 0
 }
 
@@ -408,3 +425,19 @@ func looksLikeDuration(k string) bool {
 		k == "duration" || strings.HasSuffix(k, "_duration") ||
 		k == "lease_max" || k == "ttl_max"
 }
+
+// This struct is responsible for capturing all the fields to be output by a
+// vault status command, including fields that do not come from the status API.
+// Currently we are adding the fields from api.LeaderResponse
+type SealStatusOutput struct {
+	api.SealStatusResponse
+	HAEnabled                bool   `json:"ha_enabled"`
+	IsSelf                   bool   `json:"is_self,omitempty""`
+	LeaderAddress            string `json:"leader_address,omitempty"`
+	LeaderClusterAddress     string `json:"leader_cluster_address,omitempty"`
+	PerfStandby              bool   `json:"performance_standby,omitempty"`
+	PerfStandbyLastRemoteWAL uint64 `json:"performance_standby_last_remote_wal,omitempty"`
+	LastWAL                  uint64 `json:"last_wal,omitempty"`
+	RaftCommittedIndex       uint64 `json:"raft_committed_index,omitempty"`
+	RaftAppliedIndex         uint64 `json:"raft_applied_index,omitempty"`
+}

command/format_test.go

@@ -2,6 +2,7 @@ package command
 
 import (
 	"bytes"
+	"fmt"
 	"os"
 	"strings"
 	"testing"
@@ -69,6 +70,8 @@ func TestYamlFormatter(t *testing.T) {
 func TestTableFormatter(t *testing.T) {
 	os.Setenv(EnvVaultFormat, "table")
 	ui := mockUi{t: t}
+
+	// Testing secret formatting
 	s := api.Secret{Data: map[string]interface{}{"k": "something"}}
 	if err := outputWithFormat(ui, &s, &s); err != 0 {
 		t.Fatal(err)
@@ -78,6 +81,145 @@ func TestTableFormatter(t *testing.T) {
 	}
 }
 
+// TestStatusFormat tests to verify that the embedded struct
+// SealStatusOutput ignores omitEmpty fields and prints out
+// fields in the embedded struct explicitly. It also checks the spacing,
+// indentation, and delimiters of table formatting explicitly.
+func TestStatusFormat(t *testing.T) {
+	ui := mockUi{t: t}
+	os.Setenv(EnvVaultFormat, "table")
+
+	statusHA := getMockStatusData(false)
+	statusOmitEmpty := getMockStatusData(true)
+
+	// Testing that HA fields are formatted properly for table.
+	// All fields (including new HA fields) are expected
+	if err := outputWithFormat(ui, nil, statusHA); err != 0 {
+		t.Fatal(err)
+	}
+
+	expectedOutputString :=
+		`Key                           Value
+---                           -----
+Recovery Seal Type            type
+Initialized                   true
+Sealed                        true
+Total Recovery Shares         2
+Threshold                     1
+Unseal Progress               3/1
+Unseal Nonce                  nonce
+Seal Migration in Progress    true
+Version                       version
+Storage Type                  storage type
+Cluster Name                  cluster name
+Cluster ID                    cluster id
+HA Enabled                    true
+Raft Committed Index          3
+Raft Applied Index            4
+Last WAL                      2`
+
+	if expectedOutputString != output {
+		fmt.Printf("%s\n%+v\n %s\n%+v\n", "output found was: ", output, "versus", expectedOutputString)
+		t.Fatal("format output for status does not match expected format. Check print statements above.")
+	}
+
+	// Testing that omitEmpty fields are omitted from status
+	// no HA fields are expected, except HA Enabled
+	if err := outputWithFormat(ui, nil, statusOmitEmpty); err != 0 {
+		t.Fatal(err)
+	}
+
+	expectedOutputString =
+		`Key                           Value
+---                           -----
+Recovery Seal Type            type
+Initialized                   true
+Sealed                        true
+Total Recovery Shares         2
+Threshold                     1
+Unseal Progress               3/1
+Unseal Nonce                  nonce
+Seal Migration in Progress    true
+Version                       version
+Storage Type                  n/a
+HA Enabled                    false`
+
+	if expectedOutputString != output {
+		fmt.Printf("%s\n%+v\n %s\n%+v\n", "output found was: ", output, "versus", expectedOutputString)
+		t.Fatal("format output for status does not match expected format. Check print statements above.")
+	}
+}
+
+// getMockStatusData outputs a SealStatusOutput struct from format.go to be used
+// for testing. The emptyfields parameter specifies whether the struct will be
+// initialized with all the omitempty fields as empty or not.
+func getMockStatusData(emptyFields bool) SealStatusOutput {
+	var status SealStatusOutput
+	var sealStatusResponseMock api.SealStatusResponse
+	if !emptyFields {
+		sealStatusResponseMock = api.SealStatusResponse{
+			Type:         "type",
+			Initialized:  true,
+			Sealed:       true,
+			T:            1,
+			N:            2,
+			Progress:     3,
+			Nonce:        "nonce",
+			Version:      "version",
+			Migration:    true,
+			ClusterName:  "cluster name",
+			ClusterID:    "cluster id",
+			RecoverySeal: true,
+			StorageType:  "storage type",
+		}
+
+		// must initialize this struct without explicit field names due to embedding
+		status = SealStatusOutput{
+			sealStatusResponseMock,
+			true,                     // HAEnabled
+			true,                     // IsSelf
+			"leader address",         // LeaderAddress
+			"leader cluster address", // LeaderClusterAddress
+			true,                     // PerfStandby
+			1,                        // PerfStandbyLastRemoteWAL
+			2,                        // LastWAL
+			3,                        // RaftCommittedIndex
+			4,                        // RaftAppliedIndex
+		}
+	} else {
+		sealStatusResponseMock = api.SealStatusResponse{
+			Type:         "type",
+			Initialized:  true,
+			Sealed:       true,
+			T:            1,
+			N:            2,
+			Progress:     3,
+			Nonce:        "nonce",
+			Version:      "version",
+			Migration:    true,
+			ClusterName:  "",
+			ClusterID:    "",
+			RecoverySeal: true,
+			StorageType:  "",
+		}
+
+		// must initialize this struct without explicit field names due to embedding
+		status = SealStatusOutput{
+			sealStatusResponseMock,
+			false, // HAEnabled
+			false, // IsSelf
+			"",    // LeaderAddress
+			"",    // LeaderClusterAddress
+			false, // PerfStandby
+			0,     // PerfStandbyLastRemoteWAL
+			0,     // LastWAL
+			0,     // RaftCommittedIndex
+			0,     // RaftAppliedIndex
+		}
+	}
+	return status
+}
+
 func Test_Format_Parsing(t *testing.T) {
 	defer func() {
 		os.Setenv(EnvVaultCLINoColor, "")