Fix invalid output of syslog IPv6 servers #1933
Conversation
wen587
requested review from
tsvanduyn and
qiluo-msft
and removed request for
tsvanduyn
|
This pull request introduces 1 alert when merging 7e4eb10 into e63f47e - view on LGTM.com new alerts:
|
| elif re_ipv6.match(line): | ||
| server = re_ipv6.match(line).group(1) | ||
| else: | ||
| continue |
There was a problem hiding this comment.
I don't think so. I am checking all lines in rsyslogd.conf.
Even if only check lines start with *.* @, it is possible we have tcp format server starts with *.* @@. It is not enabled in the rsyslogd.conf.
Seems current show run syslog command only care about UDP server
| To match below cases: | ||
| *.* @IPv4:port | ||
| *.* @[IPv4]:port | ||
| *.* @[IPv6]:port |
There was a problem hiding this comment.
The port is optional and the default is 514. ref: https://linux.die.net/man/5/rsyslog.conf #Closed
There was a problem hiding this comment.
I can make the port optional.
show/main.py
Outdated
| syslog_servers = [] | ||
| syslog_dict = {} | ||
| re_ipv4_1 = re.compile(r'^\*\.\* @(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}):\d+') |
There was a problem hiding this comment.
Thanks for being strict when writing a regex!
However since it is a show command, and the data is from a file in filesystem, should we be more relax and do not check for a valid ipv4/v6 address?
we can delegate the check to rsyslogd -N1. If anything wrong, we report an empty syslog running config. #Closed
There was a problem hiding this comment.
Hi Qi, according to rsyslog IPv6 support, I think we can just consider the cases of *.* @[IPv4]:port and *.* @[IPv6]:port which are all included in square bracket.
This way, I think the initial commit in this PR which checks ending ']' is enough.
| @@ -1356,16 +1356,32 @@ def show_run_snmp(db, ctx): | |||
| @runningconfiguration.command() | |||
| @click.option('--verbose', is_flag=True, help="Enable verbose output") | |||
| def syslog(verbose): | |||
There was a problem hiding this comment.
Could you add some unit tests?
#### What I did Modify the filter function of `show runningconfiguration syslog` #### How I did it Filter by ending "]" rather than split by ":" #### How to verify it add a syslog v6 server `sudo config syslog add f587::1:1` run command `show runningconfiguration syslog` #### Previous command output (if the output of a command-line utility has changed) ``` admin@vlab-01:~$ show runningconfiguration syslog Syslog Servers ---------------- [10.0.0.5] [10.0.0.6] [f587 ``` #### New command output (if the output of a command-line utility has changed) ``` admin@vlab-01:~$ show runningconfiguration syslog Syslog Servers ---------------- [10.0.0.5] [10.0.0.6] [f587::1:1] ```
What I did
Modify the filter function of
show runningconfiguration syslogHow I did it
Filter by ending "]" rather than split by ":"
How to verify it
add a syslog v6 server
sudo config syslog add f587::1:1run command
show runningconfiguration syslogPrevious command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)