Update clamav database before running rpminspect #4
Conversation
| # Update the clamav database | ||
| freshclam | ||
| if [ $? -ne 0 ]; then | ||
| echo "ERROR: Updating the ClamAV database failed." |
There was a problem hiding this comment.
rather a warning? Errors are usually fatal, but we just continue here.
There was a problem hiding this comment.
The script runs with set -e, so it will bail out immediately if freshclam commands returns a non-zero status code -- this line won't be executed.
There was a problem hiding this comment.
Also, we should only update the database if the script was executed without the third optional "$test_name" parameter, or if "$test_name" = "virus" (i.e. not when we run any other inspection).
There was a problem hiding this comment.
Solid feedback; we can remove the $? check entire if using set -e to validate our outputs.
If we agree this is the proper route to go, I can add the logic around the $test_name - I just wanted to make sure the logic/place makes sense. If so, I can update it. Is the rebase still being worked on?
There was a problem hiding this comment.
I think the question is what should happen if the update fails. Should we just print warning and continue? Or should the test fail?
I'd probably vote for printing a warning.
Is the rebase still being worked on?
I dropped the ball there :/ I was actually thinking about ditching bash completely and rewriting the runner script in Python as it is getting more and more complicated: #3
|
This is OSCI-1769. |
|
Thanks for the pull request. Please see the comments above. |
|
I was rewriting the script today and I squeezed this change in in the following commit: 5b9c459 Thanks ;) |
If running the virus scan in rpminspect, it will throw an error like this:
LibClamAV Warning: **************************************************
LibClamAV Warning: *** The virus database is older than 7 days! ***
LibClamAV Warning: *** Please update it as soon as possible. ***
LibClamAV Warning: **************************************************
Running freshclam, as we do in GHA here, solves this error:
https://github.com/rpminspect/rpminspect/blob/master/osdeps/fedora/post.sh#L29
I'm guessing we'll need to make a similar fix in downstream in the Dockerfile, though I know you are working on an update to this script so you tell me the best way to proceed. :) @msrb