Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
lersek opened this issue Dec 20, 2022 · 5 comments
Open

Comments

@lersek
Copy link
Contributor

lersek commented Dec 20, 2022

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

cloud-init made use of the new directive in commit canonical/cloud-init@b613de733fa7.

At commit 4f3bbeb, 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

@igalic
Copy link

igalic commented Dec 20, 2022

@lersek could you craft a patch yourself?

@lersek
Copy link
Contributor Author

lersek commented Dec 21, 2022

@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
Copy link
Contributor Author

lersek commented Dec 21, 2022

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
Copy link
Contributor Author

lersek commented Dec 22, 2022

Regarding the rsyslog config snippet in #794 (comment), there are two issues in fact:

(1) The multi-action parsing from commit 5181105 / #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
Copy link
Contributor Author

lersek commented Dec 22, 2022

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 03d6082 / #475.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants