-
Notifications
You must be signed in to change notification settings - Fork 7
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 oscap debug #264
Add oscap debug #264
Conversation
44c7904
to
324a6c5
Compare
Added 3 tests in total:
|
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.
cis_workstation_l1
is fast-to-scan profile and ok-ish for oscap-debug/vm-scan.py
.
But for oscap-debug/sysctl-only.py
it would be better to use other profile with more sysctl rules, for example ANSSI High (45 sysctl rules vs 71).
Moreover, I've seen freezes also on Nevermind, file_*
rules which should be fast-to-scan as well (checking file permission/ownership). That's another 76 rules with chance to hit the freeze. So changing sysctl-only.py
to something like textfilecontent-rules.py
(as textfilecontent54
is the suspected problematic oscap probe) should make that more efficient.file_*
are noticeably slower than sysctl
.
lib/util/test_metadata.py
Outdated
test_metadata = yaml.safe_load(f) | ||
self.update(test_metadata) | ||
|
||
# dict's .copy() returns 'dict', not 'TestMetadata' |
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.
The comment confused me. My understanding was you want to get dict
from .copy()
, not TestMetadata
. But it's the opposite.
Change it to something like
# return `TestMetadata` for `.copy()`, not `dict`
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.
Honestly, that piece of code is >1 year old, so I was recently re-reading it myself and it wasn't confusing to me, so I guess I see both versions as reasonable. I'll use yours just to be safe. 😄
We could probably even remove it, it was for the relatively rare case somebody would want to create a copy of the instance, which was the case in the original codebase this snippet came from (early runcontest).
Got error on rhel9
|
Okay, I was able to reproduce it on 9.6, it worked on 9.5, will fix.
|
Actually, it was not 9.6, it was just RHEL-8, which I presume was your version as well and "rhel9" was a typo. I tried using the older syntax of
and that got rid of the error on RHEL-8 (while still working fine on 9), but it didn't make the test work on 8, now gdb just prints
and exits with error. So I will probably limit the gdb-style tests to RHEL-9+, which should be good enough. We'll see if any freezing fixes helped on RHEL-8 simply by running normal tests. |
9bdbddd
to
dc4f413
Compare
Rebased + added a commit to avoid running these in productization. Ie.
|
Signed-off-by: Jiri Jaburek <[email protected]>
In other instances of util.subprocess_run() and Guest.ssh() we simply let the user use subprocess.PIPE directly when wanted. This allows for flexibility like 'stderr=subprocess.STDOUT' as well. Further, newer Python versions have 'capture_output=True' as a shorthand for 'stdout=PIPE' + 'stderr=PIPE', so the user can always use that. So let's get rid of the special ssh()-specific 'capture'. Signed-off-by: Jiri Jaburek <[email protected]>
This is for (repeated) snapshotting within one test, not for sharing snapshots across tests. The random tag (instead of an empty '') will also safeguard against accidentally re-using snapshots by multiple tests using Guest() without a tag, but then trying to g.snapshotted() without installation. Signed-off-by: Jiri Jaburek <[email protected]>
Signed-off-by: Jiri Jaburek <[email protected]>
Signed-off-by: Jiri Jaburek <[email protected]>
Default to using all space for '/'. Signed-off-by: Jiri Jaburek <[email protected]>
Signed-off-by: Jiri Jaburek <[email protected]>
The sysctl-only scan is much simpler and faster, but doesn't seem to reliably reproduce the freeze. The nested VM test runs much faster than a full host OS scan, and is able to easily reproduce it. Signed-off-by: Jiri Jaburek <[email protected]>
Signed-off-by: Jiri Jaburek <[email protected]>
(Not sure we should merge this in any form, this is just for possibly temporary testing and having it as a PR is convenient as our CI can run it.)