-
Notifications
You must be signed in to change notification settings - Fork 756
Add multiline support for RainerScript action detection in rsyslog_remote_loghost rule #14057
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
base: master
Are you sure you want to change the base?
Add multiline support for RainerScript action detection in rsyslog_remote_loghost rule #14057
Conversation
|
Hi @Arden97. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
jan-cerny
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks fine. The automatus test scenarios pass locally for me.
Two things that came to my mind:
-
Please check if there aren't any other similar rules that would need the same modification. I know that there are multiple rules for rsyslog configuration in the project and they might suffer from the same problem.
-
Consider creating another test scenario that would create the same configuration file as the example of the configuration file the reporter of the bug provided in the Jira ticket. I think that would be a good regression test.
|
OVAL checks and tests of the following rules were updated to support both legacy and RainerScript syntax (multi-line included):
|
|
Testing scenario |
shared/macros/10-bash.jinja
Outdated
|
|
||
|
|
||
| {{# | ||
| This macro sets up the rsyslog environment for testing rsyslog_encrypt_offload_actionsendstreamdriverauthmode rule. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to put macros like this to shared/macros/20-test-scenarios.jinja.
shared/macros/10-bash.jinja
Outdated
| {{%- endmacro -%}} | ||
|
|
||
|
|
||
| {{%- macro setup_rsyslog_encrypt_offload_actionsendstreamdriverauthmode() -%}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undocumented macro
shared/macros/10-bash.jinja
Outdated
| It ensures the rsyslog directory structure and rsyslog conf file exist, then removes any existing | ||
| multilined and legacy format entries. | ||
| #}} | ||
| {{%- macro setup_rsyslog_cron_logging () -%}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
coding style
| else | ||
| echo "$LOGHOST_LINE" >> "$CONF_FILE" | ||
| fi | ||
| {{{ setup_rsyslog_remote_loghost("*.* @@192.168.122.1:5000") }}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing end of file
| @@ -0,0 +1,27 @@ | |||
| #!/bin/bash | |||
| # packages = rsyslog | |||
|
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # This TS is a regression test of https://issues.redhat.com/browse/RHEL-104207. |
jan-cerny
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Arden97 So far looks good. Do you plan to make the PR as ready for a review or is there anything else that you plan to do?
I'm currently working on updating ansible remediations for changed |
49e6850 to
c16d5b2
Compare
Description:
action()statements that span multiple linesrsyslog_remote_loghost_rainer_regexnow includes thesflag (dotall mode), which allows the.metacharacter to match newline charactersaction()configurations where parameters are split across multiple lines for readability, which is a common formatting practice in rsyslog configurationsremote_configured_rainer_newline.pass.shthat validates detection of multiline RainerScript action configurationsRationale:
The previous regex pattern with only the
(?m)(multiline) flag could match patterns where^and$anchors work across lines, but the.*portions could not span newlinesFixes # RHEL-104207
Review Hints:
automatusto verify, thatremote_configured_rainer_multilinetest is passingautomatusto test following rules (bothbashandansibleremediations):rsyslog_remote_loghostrsyslog_remote_tlsrsyslog_cron_loggingrsyslog_encrypt_offload_actionsendstreamdriverauthmodersyslog_encrypt_offload_actionsendstreamdrivermodersyslog_encrypt_offload_defaultnetstreamdriver