command: change delim in columnize to funny node names (#6652)

When node name contains vertical bar symbol some commands output is
garbled because `|` is used as a delimiter in `columnize.SimpleFormat`.

This commit changes format string to use `\x1f` - ASCII unit
separator[1] as a delimiter and also adds test to cover this case.

Affected commands:

* `consul catalog nodes`
* `consul members`
* `consul operator raft list-peers`
* `consul intention get`

Fixes #3951.

[1]: https://en.wikipedia.org/wiki/Delimiter#Solutions
This commit is contained in:
Alex Dzyoba 2020-03-09 10:24:56 +00:00 committed by GitHub
parent c46e2ae59b
commit 4137d06f9f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 136 additions and 23 deletions

View File

@ -147,16 +147,16 @@ func printNodes(nodes []*api.Node, detailed bool) (string, error) {
result = simpleNodes(nodes)
}
return columnize.SimpleFormat(result), nil
return columnize.Format(result, &columnize.Config{Delim: string([]byte{0x1f})}), nil
}
func detailedNodes(nodes []*api.Node) []string {
result := make([]string, 0, len(nodes)+1)
header := "Node|ID|Address|DC|TaggedAddresses|Meta"
header := "Node\x1fID\x1fAddress\x1fDC\x1fTaggedAddresses\x1fMeta"
result = append(result, header)
for _, node := range nodes {
result = append(result, fmt.Sprintf("%s|%s|%s|%s|%s|%s",
result = append(result, fmt.Sprintf("%s\x1f%s\x1f%s\x1f%s\x1f%s\x1f%s",
node.Node, node.ID, node.Address, node.Datacenter,
mapToKV(node.TaggedAddresses, ", "), mapToKV(node.Meta, ", ")))
}
@ -166,7 +166,7 @@ func detailedNodes(nodes []*api.Node) []string {
func simpleNodes(nodes []*api.Node) []string {
result := make([]string, 0, len(nodes)+1)
header := "Node|ID|Address|DC"
header := "Node\x1fID\x1fAddress\x1fDC"
result = append(result, header)
for _, node := range nodes {
@ -176,7 +176,7 @@ func simpleNodes(nodes []*api.Node) []string {
if idx > 0 {
id = id[0:idx]
}
result = append(result, fmt.Sprintf("%s|%s|%s|%s",
result = append(result, fmt.Sprintf("%s\x1f%s\x1f%s\x1f%s",
node.Node, id, node.Address, node.Datacenter))
}

View File

@ -163,3 +163,29 @@ func TestCatalogListNodesCommand(t *testing.T) {
}
})
}
func TestCatalogListNodesCommand_verticalBar(t *testing.T) {
t.Parallel()
nodeName := "name|with|bars"
a := agent.NewTestAgent(t, "", `node_name = "`+nodeName+`"`)
defer a.Shutdown()
ui := cli.NewMockUi()
c := New(ui)
c.flags.SetOutput(ui.ErrorWriter)
args := []string{
"-http-addr=" + a.HTTPAddr(),
}
code := c.Run(args)
if code != 0 {
t.Fatalf("bad exit code: %d. %q", code, ui.ErrorWriter.String())
}
// Check for nodeName presense because it should not be parsed by columnize
if !strings.Contains(ui.OutputWriter.String(), nodeName) {
t.Fatalf("expected %q to contain %q", ui.OutputWriter.String(), nodeName)
}
}

View File

@ -66,13 +66,13 @@ func (c *cmd) Run(args []string) int {
// Format the tabular data
data := []string{
fmt.Sprintf("Source:|%s", ixn.SourceString()),
fmt.Sprintf("Destination:|%s", ixn.DestinationString()),
fmt.Sprintf("Action:|%s", ixn.Action),
fmt.Sprintf("ID:|%s", ixn.ID),
fmt.Sprintf("Source:\x1f%s", ixn.SourceString()),
fmt.Sprintf("Destination:\x1f%s", ixn.DestinationString()),
fmt.Sprintf("Action:\x1f%s", ixn.Action),
fmt.Sprintf("ID:\x1f%s", ixn.ID),
}
if v := ixn.Description; v != "" {
data = append(data, fmt.Sprintf("Description:|%s", v))
data = append(data, fmt.Sprintf("Description:\x1f%s", v))
}
if len(ixn.Meta) > 0 {
var keys []string
@ -81,14 +81,14 @@ func (c *cmd) Run(args []string) int {
}
sort.Strings(keys)
for _, k := range keys {
data = append(data, fmt.Sprintf("Meta[%s]:|%s", k, ixn.Meta[k]))
data = append(data, fmt.Sprintf("Meta[%s]:\x1f%s", k, ixn.Meta[k]))
}
}
data = append(data,
fmt.Sprintf("Created At:|%s", ixn.CreatedAt.Local().Format(time.RFC850)),
fmt.Sprintf("Created At:\x1f%s", ixn.CreatedAt.Local().Format(time.RFC850)),
)
c.UI.Output(columnize.SimpleFormat(data))
c.UI.Output(columnize.Format(data, &columnize.Config{Delim: string([]byte{0x1f})}))
return 0
}

View File

@ -122,3 +122,40 @@ func TestCommand_srcDst(t *testing.T) {
require.Equal(0, c.Run(args), ui.ErrorWriter.String())
require.Contains(ui.OutputWriter.String(), id)
}
func TestCommand_verticalBar(t *testing.T) {
t.Parallel()
require := require.New(t)
a := agent.NewTestAgent(t, t.Name(), ``)
defer a.Shutdown()
client := a.Client()
sourceName := "source|name|with|bars"
// Create the intention
var id string
{
var err error
id, _, err = client.Connect().IntentionCreate(&api.Intention{
SourceName: sourceName,
DestinationName: "db",
Action: api.IntentionActionAllow,
}, nil)
require.NoError(err)
}
// Get it
ui := cli.NewMockUi()
c := New(ui)
args := []string{
"-http-addr=" + a.HTTPAddr(),
id,
}
require.Equal(0, c.Run(args), ui.ErrorWriter.String())
// Check for sourceName presense because it should not be parsed by
// columnize
require.Contains(ui.OutputWriter.String(), sourceName)
}

View File

@ -120,7 +120,7 @@ func (c *cmd) Run(args []string) int {
}
// Generate the columnized version
output := columnize.SimpleFormat(result)
output := columnize.Format(result, &columnize.Config{Delim: string([]byte{0x1f})})
c.UI.Output(output)
return 0
@ -146,7 +146,7 @@ func (m ByMemberNameAndSegment) Less(i, j int) bool {
// in a more human-friendly format
func (c *cmd) standardOutput(members []*consulapi.AgentMember) []string {
result := make([]string, 0, len(members))
header := "Node|Address|Status|Type|Build|Protocol|DC|Segment"
header := "Node\x1fAddress\x1fStatus\x1fType\x1fBuild\x1fProtocol\x1fDC\x1fSegment"
result = append(result, header)
for _, member := range members {
addr := net.TCPAddr{IP: net.ParseIP(member.Addr), Port: int(member.Port)}
@ -163,15 +163,15 @@ func (c *cmd) standardOutput(members []*consulapi.AgentMember) []string {
statusString := serf.MemberStatus(member.Status).String()
switch member.Tags["role"] {
case "node":
line := fmt.Sprintf("%s|%s|%s|client|%s|%s|%s|%s",
line := fmt.Sprintf("%s\x1f%s\x1f%s\x1fclient\x1f%s\x1f%s\x1f%s\x1f%s",
member.Name, addr.String(), statusString, build, protocol, dc, segment)
result = append(result, line)
case "consul":
line := fmt.Sprintf("%s|%s|%s|server|%s|%s|%s|%s",
line := fmt.Sprintf("%s\x1f%s\x1f%s\x1fserver\x1f%s\x1f%s\x1f%s\x1f%s",
member.Name, addr.String(), statusString, build, protocol, dc, segment)
result = append(result, line)
default:
line := fmt.Sprintf("%s|%s|%s|unknown||||",
line := fmt.Sprintf("%s\x1f%s\x1f%s\x1funknown\x1f\x1f\x1f\x1f",
member.Name, addr.String(), statusString)
result = append(result, line)
}
@ -183,7 +183,7 @@ func (c *cmd) standardOutput(members []*consulapi.AgentMember) []string {
// their raw format
func (c *cmd) detailedOutput(members []*consulapi.AgentMember) []string {
result := make([]string, 0, len(members))
header := "Node|Address|Status|Tags"
header := "Node\x1fAddress\x1fStatus\x1fTags"
result = append(result, header)
for _, member := range members {
// Get the tags sorted by key
@ -202,7 +202,7 @@ func (c *cmd) detailedOutput(members []*consulapi.AgentMember) []string {
tags := strings.Join(tagPairs, ",")
addr := net.TCPAddr{IP: net.ParseIP(member.Addr), Port: int(member.Port)}
line := fmt.Sprintf("%s|%s|%s|%s",
line := fmt.Sprintf("%s\x1f%s\x1f%s\x1f%s",
member.Name, addr.String(), serf.MemberStatus(member.Status).String(), tags)
result = append(result, line)
}

View File

@ -120,3 +120,29 @@ func TestMembersCommand_statusFilter_failed(t *testing.T) {
t.Fatalf("bad: %d", code)
}
}
func TestMembersCommand_verticalBar(t *testing.T) {
t.Parallel()
nodeName := "name|with|bars"
a := agent.NewTestAgent(t, "", `node_name = "`+nodeName+`"`)
defer a.Shutdown()
ui := cli.NewMockUi()
c := New(ui)
c.flags.SetOutput(ui.ErrorWriter)
args := []string{
"-http-addr=" + a.HTTPAddr(),
}
code := c.Run(args)
if code == 1 {
t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String())
}
// Check for nodeName presense because it should not be parsed by columnize
if !strings.Contains(ui.OutputWriter.String(), nodeName) {
t.Fatalf("bad: %#v", ui.OutputWriter.String())
}
}

View File

@ -68,7 +68,7 @@ func raftListPeers(client *api.Client, stale bool) (string, error) {
}
// Format it as a nice table.
result := []string{"Node|ID|Address|State|Voter|RaftProtocol"}
result := []string{"Node\x1fID\x1fAddress\x1fState\x1fVoter\x1fRaftProtocol"}
for _, s := range reply.Servers {
raftProtocol := s.ProtocolVersion
@ -79,11 +79,11 @@ func raftListPeers(client *api.Client, stale bool) (string, error) {
if s.Leader {
state = "leader"
}
result = append(result, fmt.Sprintf("%s|%s|%s|%s|%v|%s",
result = append(result, fmt.Sprintf("%s\x1f%s\x1f%s\x1f%s\x1f%v\x1f%s",
s.Node, s.ID, s.Address, state, s.Voter, raftProtocol))
}
return columnize.SimpleFormat(result), nil
return columnize.Format(result, &columnize.Config{Delim: string([]byte{0x1f})}), nil
}
func (c *cmd) Synopsis() string {

View File

@ -38,3 +38,27 @@ func TestOperatorRaftListPeersCommand(t *testing.T) {
t.Fatalf("bad: %q, %q", output, expected)
}
}
func TestOperatorRaftListPeersCommand_verticalBar(t *testing.T) {
t.Parallel()
nodeName := "name|with|bars"
a := agent.NewTestAgent(t, "", `node_name = "`+nodeName+`"`)
defer a.Shutdown()
ui := cli.NewMockUi()
c := New(ui)
c.flags.SetOutput(ui.ErrorWriter)
args := []string{"-http-addr=" + a.HTTPAddr()}
code := c.Run(args)
if code != 0 {
t.Fatalf("bad exit code: %d. %q", code, ui.ErrorWriter.String())
}
// Check for nodeName presense because it should not be parsed by columnize
if !strings.Contains(ui.OutputWriter.String(), nodeName) {
t.Fatalf("expected %q to contain %q", ui.OutputWriter.String(), nodeName)
}
}