augeas icon indicating copy to clipboard operation
augeas copied to clipboard

rsyslog: modern name for the discard action ("stop") is not recognized

Open lersek opened this issue 2 years ago • 5 comments

rsyslog deprecated the legacy name for the discard action, namely the tilde sign ~, in commit https://github.com/rsyslog/rsyslog/commit/2ae9e456db899, in favor of the RainerScript directive stop.

cloud-init made use of the new directive in commit https://github.com/canonical/cloud-init/commit/b613de733fa7.

At commit 4f3bbeb3f782, augeas relies on Syslog.action for recognizing the discard action, but the latter does not understand stop yet, it only understands the legacy directive.

Please consider recognizing stop. Thanks.

Reference: https://bugzilla.redhat.com/show_bug.cgi?id=2155147

lersek avatar Dec 20 '22 11:12 lersek

@lersek could you craft a patch yourself?

igalic avatar Dec 20 '22 13:12 igalic

@igalic The code change does not look complicated (just add an alternative to ~); the reason I'm currently reluctant to do it myself is my past experience with testability of lens changes. That has been nothing short of horrific. My goal would be to run a particular lens against a particular test file that's located in any random location in my filesystem, such as my $HOME/tmp. I've never been able to do that; and anything I've found on the net always implies hacking the test config into preexistent test cases or whatever. Terrible testing experience.

Another issue I've had with testing previously is that some of the augeas error messages are completely incomprehensible. Sometimes there are complaints about the lens modification (even before reaching a test file), and they make no sense. One looks into the C source files then... and those make even less sense; they are impenetrable. I understand parsing theory is complex (I've done my fair share in it), but the code lacks comments, variable names are useless, etc.

In short: I can try the patch myself, but I will need serious hand-holding. For starters, can you please tell me how I can test the change in the easiest possible way, without having to create a brand new test scenario (and/or hacking an existent one)? Thanks.

lersek avatar Dec 21 '22 09:12 lersek

For example, with this patch committed on top of current master:

diff --git a/tests/root/etc/rsyslog.d/21-cloudinit.conf b/tests/root/etc/rsyslog.d/21-cloudinit.conf
new file mode 100644
index 000000000000..150d800f15be
--- /dev/null
+++ b/tests/root/etc/rsyslog.d/21-cloudinit.conf
@@ -0,0 +1,6 @@
+# Log cloudinit generated log messages to file
+:syslogtag, isequal, "[CLOUDINIT]" /var/log/cloud-init.log
+
+# comment out the following line to allow CLOUDINIT messages through.
+# Doing so means you'll also get CLOUDINIT messages in /var/log/syslog
+& stop

I'd expect ./src/try to fail, but it doesn't -- it continues succeeding. That indicates the new test file is being ignored. How do I fix that? Note that lenses/tests/test_rsyslog.aug already contains

$IncludeConfig /etc/rsyslog.d/*.conf

but it's not taking effect (presumably augeas just parses this directive, but does nothing with it; i.e., it doesn't actually implement inclusion).

How do I make ./src/try parse the new 21-cloudinit.conf test file? Thank you.

lersek avatar Dec 21 '22 10:12 lersek

Regarding the rsyslog config snippet in https://github.com/hercules-team/augeas/issues/794#issuecomment-1361111325, there are two issues in fact:

(1) The multi-action parsing from commit 5181105bae84 / #653 cannot deal with comments and newlines embedded in the action list. The config_sep rule does not apply in that context.

In other words, the following file (as tests/root/etc/rsyslog.d/21-cloudinit.conf) parses fine:

# Log cloudinit generated log messages to file
:syslogtag, isequal, "[CLOUDINIT]" /var/log/cloud-init.log
& ~

producing

$ ./src/try cli
augtool> print /files/etc/rsyslog.d/21-cloudinit.conf
/files/etc/rsyslog.d/21-cloudinit.conf
/files/etc/rsyslog.d/21-cloudinit.conf/#comment = "Log cloudinit generated log messages to file"
/files/etc/rsyslog.d/21-cloudinit.conf/filter
/files/etc/rsyslog.d/21-cloudinit.conf/filter/property = "syslogtag"
/files/etc/rsyslog.d/21-cloudinit.conf/filter/operation = "isequal"
/files/etc/rsyslog.d/21-cloudinit.conf/filter/value = "[CLOUDINIT]"
/files/etc/rsyslog.d/21-cloudinit.conf/filter/action[1]
/files/etc/rsyslog.d/21-cloudinit.conf/filter/action[1]/file = "/var/log/cloud-init.log"
/files/etc/rsyslog.d/21-cloudinit.conf/filter/action[2]
/files/etc/rsyslog.d/21-cloudinit.conf/filter/action[2]/discard

but the following (i.e., with the comments/newlines added back in) leads to a parse error:

# Log cloudinit generated log messages to file
:syslogtag, isequal, "[CLOUDINIT]" /var/log/cloud-init.log

# comment out the following line to allow CLOUDINIT messages through.
# Doing so means you'll also get CLOUDINIT messages in /var/log/syslog
& ~

(2) Even if I remove the empty lines and comments from within the multi-action list (tests/root/etc/rsyslog.d/21-cloudinit.conf):

# Log cloudinit generated log messages to file
:syslogtag, isequal, "[CLOUDINIT]" /var/log/cloud-init.log
& stop

and extend the discard action in lenses/syslog.aug such that it recognize stop as an alternative:

diff --git a/lenses/syslog.aug b/lenses/syslog.aug
index bf5f7b422a07..78bda57f2065 100644
--- a/lenses/syslog.aug
+++ b/lenses/syslog.aug
@@ -202,7 +202,7 @@ module Syslog =
        (* View: discard
         discards matching messages
         *)
-       let discard = [ label "discard" . Util.del_str "~" ]
+       let discard = [ label "discard" . ( Util.del_str "~" | Util.del_str "stop" ) ]
 
        (* View: action
         an action is either a file, a host, users, a program, or discard

the desired discard action is mis-parsed as a user action:

$ ./src/try cli
augtool> print /files/etc/rsyslog.d/21-cloudinit.conf
/files/etc/rsyslog.d/21-cloudinit.conf
/files/etc/rsyslog.d/21-cloudinit.conf/#comment = "Log cloudinit generated log messages to file"
/files/etc/rsyslog.d/21-cloudinit.conf/filter
/files/etc/rsyslog.d/21-cloudinit.conf/filter/property = "syslogtag"
/files/etc/rsyslog.d/21-cloudinit.conf/filter/operation = "isequal"
/files/etc/rsyslog.d/21-cloudinit.conf/filter/value = "[CLOUDINIT]"
/files/etc/rsyslog.d/21-cloudinit.conf/filter/action[1]
/files/etc/rsyslog.d/21-cloudinit.conf/filter/action[1]/file = "/var/log/cloud-init.log"
/files/etc/rsyslog.d/21-cloudinit.conf/filter/action[2]
/files/etc/rsyslog.d/21-cloudinit.conf/filter/action[2]/user = "stop"

The rules need to be overhauled more deeply for addressing both issues than what I can do.

lersek avatar Dec 22 '22 09:12 lersek

BTW just introducing tests/root/etc/rsyslog.d/21-cloudinit.conf is enough for triggering the problem -- the new file is covered immediately (for example in make check) due to commit 03d6082d0f749 / #475.

lersek avatar Dec 22 '22 09:12 lersek