command: Change delim in columnize to handle bars in node names

[dzeban]

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
separator1 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.

[hashicorp-cla]

All committers have signed the CLA.

[hanshasselberg]

@dzeban thank you for your contribution! This is not a correct solution unfortunately, because an agent can be started with your new delimiter in its name:

$ consul agent -dev -node 'bad |##| name'

There are different solutions to this problem: https://en.wikipedia.org/wiki/Delimiter#Solutions

[dzeban]

Frankly, I don't like this solution either - I just tried to implement first solution suggested by @freddygv. Whatever delimiter will be chosen there still will be the case for some bug.

I think we should abandon delimiters altogether and pass a slice of strings. There is a pending PR in ryanuber/columnize that will make this possible but it seems to stuck. We could remove columnize package and do this internally but I think that would be way too invasive for such case.

So I don't really know what is the best way to do here.

[hanshasselberg]

@dzeban I think using the delimiter characters mentioned on the wikipedia link from my previous comment would be a good solution.

[dzeban]

I've changed the PR to us 0x1f - ASCII unit separator as a delimiter.
While we are here I wanted to ask - shouldn't this be changed in other commands like consul operator raft list-peers and consul catalog nodes?

[hanshasselberg]

While we are here I wanted to ask - shouldn't this be changed in other commands like consul operator raft list-peers and consul catalog nodes?

If you could fix them as well, that would be great! 🙏

[dzeban]

I've fixed the columnize everywhere I was able to find.

The CI failed for some unrelated reason - judging by other recent PRs it seems a bit broken for envoy integration.

Pinging @i0rek for review and look into CI jobs.

[hanshasselberg]

Thank you very much @dzeban! Thats amazing! I will review shortly!

[hanshasselberg]

Great work, thanks a lot!

Node           Address         Status  Type    Build     Protocol  DC   Segment
bad |##| name  127.0.0.1:8301  alive   server  1.7.0dev  2         dc1  <all>

👍