-
Notifications
You must be signed in to change notification settings - Fork 671
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
Store intermediate OVAL check files #9157
Store intermediate OVAL check files #9157
Conversation
Based on our existing folder name pattern, it makes sense to separate OVAL checks that come from templated content from OVAL checks that come directly from the rule content.
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.
I like the idea, the patch LGTM, but I have some questions.
set(OVAL_COMBINE_PATHS "${BUILD_CHECKS_DIR}/shared/oval" "${SSG_SHARED}/checks/oval" "${BUILD_CHECKS_DIR}/oval" "${CMAKE_CURRENT_SOURCE_DIR}/checks/oval") | ||
add_custom_command( | ||
OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/oval-unlinked.xml" | ||
COMMAND env "PYTHONPATH=$ENV{PYTHONPATH}" "${PYTHON_EXECUTABLE}" "${SSG_BUILD_SCRIPTS}/combine_ovals.py" --build-config-yaml "${CMAKE_BINARY_DIR}/build_config.yml" --product-yaml "${CMAKE_CURRENT_SOURCE_DIR}/product.yml" --output "${CMAKE_CURRENT_BINARY_DIR}/oval-unlinked.xml" ${OVAL_COMBINE_PATHS} | ||
COMMAND env "PYTHONPATH=$ENV{PYTHONPATH}" "${PYTHON_EXECUTABLE}" "${SSG_BUILD_SCRIPTS}/combine_ovals.py" --build-config-yaml "${CMAKE_BINARY_DIR}/build_config.yml" --product-yaml "${CMAKE_CURRENT_SOURCE_DIR}/product.yml" --output "${CMAKE_CURRENT_BINARY_DIR}/oval-unlinked.xml" --build-ovals-dir "${CMAKE_CURRENT_BINARY_DIR}/checks/oval" ${OVAL_COMBINE_PATHS} |
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.
Does any part of the build system use the OVALs that are generated into "${CMAKE_CURRENT_BINARY_DIR}/checks/oval"
? If not, what do you think about changing the build system, and separate the jinja processing of OVALs from combining OVALs into a single XML?
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.
This sounds like a good idea. So far the individual checks would only be useful for human inspection.
On the other hand, writing and then reading files can make the build system slower. So I don't know if it's worth it.
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.
let's think about it
This is an irregularity of the build system - everything else (rules, remediations) is processed, written to a file and then loaded from a file in later step. This works this way also the for the templated OVAL checks. Only not-templated OVAL are an exception. Now, it can be confusing that the "compiled" files are actually unused when other "compiled" files are loaded in the build system.
Nevertheless, it would be a task for a separate PR.
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.
I have create this: #9166 to track this desired change.
Code Climate has analyzed commit 9b060ce and detected 3 issues on this pull request. Here's the issue category breakdown:
Note: there is 1 critical issue. 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 42.7% (0.0% change). View more on Code Climate. |
Description:
checks
folder tochecks_from_template
which was essentially what those files were: OVAL checks generated from templates.Rationale: