-
Notifications
You must be signed in to change notification settings - Fork 710
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
Refactor ssg.build_ovals module #10048
Refactor ssg.build_ovals module #10048
Conversation
ssg/build_ovals.py
Outdated
|
||
xml_content = process_file_with_macros(_path, local_env_yaml) | ||
|
||
if not _check_is_applicable_for_product(xml_content, product): |
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.
Hello @jan-cerny , I see similar piece of code in the function _process_oval_file. Would it be possible to deduplicate some parts of those functions?
Refactor the code by splitting the code into 2 logical blocks and putting them each in a separate function: - get_checks_from_benchmark - get_checks_from_directories
This commit introduces a new command line option `--include-benchmark`. If this option is specified by the user, OVAL checks from rule directories in the benchmark directory tree (eg. /linux_os/guide tree) will be included in the generated OVAL. Adding these files is the current behavior of the script so it doesn't add a new feature, instead it allows users to turn this feature off by not providing this command line option. This commit also rewords and clarifies the help text for the "ovaldirs" argument but it doesn't change the behavior of the "ovaldirs" argument.
3c244a6
to
ce77c7c
Compare
The code has many issues that are triggering CodeClimate problems. We tried to address them by simple refactoring, but then we realized that more refactoring would make the code more readable and understandable. There are 2 branches of the code logic: 1. processing OVAL checks from benchmark directories (eg. /linux_os/guide) 2. processing OVAL checks from shared directories (eg. /shared/checks) These 2 logic branches have similar code and it's possible to unify them. However, I found the unification easier when I change the code to a class.
ce77c7c
to
682892a
Compare
@vojtapolasek Thanks for the excellent suggestion! I have realized that there were similar blocks of code that differ only in small details. The difference was that some code processed OVALs from benchmark (ie. from rule directories) and other code processed OVALs generated from templates or from |
Code Climate has analyzed commit 682892a and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 0.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 49.3% (-0.4% change). View more on Code Climate. |
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.
Thank you for all the improvements and tests as well.
Description:
This is a series of refactoring and improvements in build_ovals.py. These commits have been extracted from PR #10035 making review easier, because 10 commits have to be created to pass CodeClimate.
Then, additional refactoring has been performed, read the discussion below.
Rationale: