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 a warning to AutoMatus #10394

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

jan-cerny
Copy link
Collaborator

This adds a warning that prevents a confusion in situation when a test scenario has a profile in its header but the rule isn't a part of that profile but is present in the built data stream.

For more context, see:
#10369

Rationale:

This can save a lot of time when investigating errors in Automatus output.

Review Hints:

  • use steps to reproduce from Automatus struggles with profile-specific test scenarios #10369
  • use your favourite rule that uses a lot of profile headers in their test scenarios, for example selinux_state: python3 tests/automatus.py rule --libvirt qemu:///system ssgts_rhel8 --scenario selinux_enforcing.pass.sh selinux_state
  • find some rule ad try adding and removing and changing various profiles headers to some test scenario, eg. no_line.fail.sh in sshd_rekey_limit and watch when the message apears, try both profiles that the rule is part of and profiles that the rule isn't part of: python3 tests/automatus.py rule --libvirt qemu:///system ssgts_rhel8 --scenario no_line.fail.sh sshd_rekey_limit

@jan-cerny jan-cerny added this to the 0.1.68 milestone Mar 30, 2023
@github-actions
Copy link

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@jan-cerny
Copy link
Collaborator Author

I will not modify the amount of _check_rule_scenario parameters because I didn't change nor introduce its definition.

@jan-cerny
Copy link
Collaborator Author

I have rebased this PR on the top of the latest upstream master branch.

@jan-cerny jan-cerny added the Test Suite Update in Test Suite. label Mar 30, 2023
@ggbecker
Copy link
Member

ggbecker commented Apr 3, 2023

This is a good addition although in my opinion we should in future PRs slowly get away from the profiles test scenario metadata in favor of using variables metadata as we can define a XCCDF variable to make the test scenario independent of profile variables as this is something that can change over time for example.

@jan-cerny
Copy link
Collaborator Author

This is a good addition although in my opinion we should in future PRs slowly get away from the profiles test scenario metadata in favor of using variables metadata as we can define a XCCDF variable to make the test scenario independent of profile variables as this is something that can change over time for example.

I fully agree.

Do you want some changes related to this in this PR?

@jan-cerny
Copy link
Collaborator Author

There is a fail in the Testing farm CentOS stream 8 job:

:: [ 16:31:39 ] :: [   FAIL   ] :: Rules not passing after remediation:

xccdf_org.ssgproject.content_rule_accounts_password_pam_pwhistory_remember_password_auth - fail

xccdf_org.ssgproject.content_rule_accounts_password_pam_pwhistory_remember_system_auth - fail 

But, it's in the /Sanity/machine-hardening/stig test which doesn't use AutoMatus at all. I guess it isn't related to the PR?

This adds a warning that prevents a confusion in situation when
a test scenario has a profile in its header but the rule isn't
a part of that profile but is present in the built data stream.

For more context, see:
ComplianceAsCode#10369
@jan-cerny
Copy link
Collaborator Author

I have rebased this PR on the top of the latest upstream master branch.

@ggbecker
Copy link
Member

ggbecker commented Apr 4, 2023

This is a good addition although in my opinion we should in future PRs slowly get away from the profiles test scenario metadata in favor of using variables metadata as we can define a XCCDF variable to make the test scenario independent of profile variables as this is something that can change over time for example.

I fully agree.

Do you want some changes related to this in this PR?

Not necessarily, I would prefer a separate PR for that.

@codeclimate
Copy link

codeclimate bot commented Apr 4, 2023

Code Climate has analyzed commit bbbc85f and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 52.4% (0.0% change).

View more on Code Climate.

@jan-cerny
Copy link
Collaborator Author

Great!

@marcusburghardt
Copy link
Member

@ggbecker , are you ok assigning this PR for you and reviewing it?

@ggbecker ggbecker self-assigned this Apr 11, 2023
@ggbecker ggbecker merged commit d5adab2 into ComplianceAsCode:master Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Test Suite Update in Test Suite.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants