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

Add syslog_files rules test scenarios #4743

Merged
merged 3 commits into from
Aug 22, 2019

Conversation

iokomin
Copy link
Contributor

@iokomin iokomin commented Aug 19, 2019

Description:

In order to get #4379 verified introduced test scenarios for rsyslog_files_* rules.
Test scenarios check include log rules, include() object (rsyslog v8.33.0+: RHEL8, OL8, Fedora) and $IncludeConfig directive.

Added tests for rules:

  • rsyslog_files_groupownership
  • rsyslog_files_ownership
  • rsyslog_files_permissions

Testing:

  • Checked ol, rhel, and fedora builds to pass
  • Using OL7.6 VM libvirt backend checked scenarios to pass
  • Verified corresponding DEBUG "*-initial.verbose.log" logs for OVAL definitions to filter and check expected test configs and log files.

Copy link
Contributor

@mildas mildas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The preparation part (create test files, test config and directory) is still same for each test scenario, the only difference is number of files.
Could you create function for it (e.g. with number of files as parameter)? This function could be placed in group_ensure_rsyslog_log_file_configuration folder and used by all test scenarios you made. And test scenarios would only call it and then contain the important part - setting ownership/permissions and creating rsyslog config.

@iokomin iokomin force-pushed the add_rule_syslog_files_tests branch from 4793431 to 08be3bc Compare August 20, 2019 15:56
@iokomin
Copy link
Contributor Author

iokomin commented Aug 20, 2019

@mildas thanks for your review, suggested changes make sense to me.
Please find updated version of PR, it should address all requested changes.

Copy link
Contributor

@mildas mildas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for update. I have just few more minor issues, mostly with so much empty lines, so please remove some of them (e.g. between chgrp, chmod etc.)

Signed-off-by: Ilya Okomin <ilya.okomin@oracle.com>
Signed-off-by: Ilya Okomin <ilya.okomin@oracle.com>
Signed-off-by: Ilya Okomin <ilya.okomin@oracle.com>
@iokomin iokomin force-pushed the add_rule_syslog_files_tests branch from 08be3bc to c4655f4 Compare August 21, 2019 14:20
@mildas mildas self-assigned this Aug 22, 2019
@mildas
Copy link
Contributor

mildas commented Aug 22, 2019

Tried these test on top of #4379 fix (on rhel8 and fedora30) and they pass/fail as expected. Thank you!

@mildas mildas merged commit 365a295 into ComplianceAsCode:master Aug 22, 2019
@iokomin iokomin deleted the add_rule_syslog_files_tests branch August 22, 2019 08:28
@yuumasato yuumasato added this to the 0.1.46 milestone Aug 22, 2019
This pull request was closed.
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

Successfully merging this pull request may close these issues.

3 participants