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

Change loading of OVAL result reports #140

Merged
merged 6 commits into from
Apr 12, 2023

Conversation

Honny1
Copy link
Member

@Honny1 Honny1 commented Mar 8, 2023

This PR changes the approach of loading the OVAL results report and brings special logic to determine where is present the OVAL result report with the OVAL definitions that are used in the CPE dictionary and CPE AL.

The OVAL result report element is selected according to the references in the rule results. The selected element is the one that is not referenced in the rule results. If multiple report elements are not referenced in the rule result, OVAL definitions for CPE are selected from all report elements. In the case of duplicate definitions, the first definition or the definition that has a result other than not evaluated is chosen.

@Honny1 Honny1 force-pushed the change-loading-of-oval-reports branch 2 times, most recently from 9eb7fa5 to 7b17d1f Compare March 13, 2023 10:58
@Honny1 Honny1 marked this pull request as ready for review March 13, 2023 15:35
@Honny1 Honny1 force-pushed the change-loading-of-oval-reports branch from 7b17d1f to e80a3b0 Compare March 15, 2023 12:25
@Honny1 Honny1 marked this pull request as draft March 16, 2023 13:44
@Honny1 Honny1 force-pushed the change-loading-of-oval-reports branch from e80a3b0 to ea79b62 Compare March 16, 2023 15:04
@Honny1 Honny1 marked this pull request as ready for review March 16, 2023 15:51
@Honny1 Honny1 mentioned this pull request Mar 21, 2023
@Honny1 Honny1 force-pushed the change-loading-of-oval-reports branch 2 times, most recently from c105d74 to cfd6b11 Compare March 29, 2023 13:15
@Honny1 Honny1 force-pushed the change-loading-of-oval-reports branch from cfd6b11 to b2c112b Compare April 5, 2023 15:44
return None
check_name = check_ref.get("name")
if rule.oval_definition_id is None:
rule.oval_definition_id = check_name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method is a 'getter', but it sometimes mutates the object. On top of that, it is a static private method. That's very confusing.

@@ -75,6 +75,14 @@ def _get_benchmark_element(self):
benchmark_el = self.root
return benchmark_el

@staticmethod
def _get_oval_definition_reference(rules):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be either references or reference_list.

OVAL_and_CPE_tree_builder = OVALAndCPETreeBuilder( # pylint: disable=C0103
self.root, group_parser,
self._get_applicable_cpe_ids_for_machine(report.profile_info.cpe_platforms_for_profile)
self._get_applicable_cpe_ids_for_machine(report.profile_info.cpe_platforms_for_profile),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ProfileInfo class already has get_cpe_platforms_that_satisfy_evaluation_target getter, which kinda does the same under a different name. This is inconsistent.

I'd say that _get_applicable_cpe_ids_for_machine should be part of ProfileInfo (named like get_list_of_cpe_platforms_that_satisfy_evaluation_target) and get_cpe_platforms_that_satisfy_evaluation_target should use it to join a list into a string.

return self.reports_with_oval_definitions[self.cpe_source]
logging.warning((
"The given input does not contain a clear mapping of the OVAL definition used"
" for CPE checks. The results of the OVAL definition in the CPE checks could "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the way you split strings consistent: space either goes to the end or to the start of the line.

if oval_def is not None and rule.oval_definition_id in report:
logging.warning(
("The given input contains the duplicate results of"
" the OVAL definition (\"%s\")."),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the way you split strings consistent: space either goes to the end or to the start of the line.

@Honny1 Honny1 force-pushed the change-loading-of-oval-reports branch from b2c112b to 55330f5 Compare April 12, 2023 12:49
@Honny1
Copy link
Member Author

Honny1 commented Apr 12, 2023

Changes:

  • Consistently split strings
  • Reworked _get_oval_check_href method
  • Renamed _get_oval_definition_reference method to _get_oval_definition_references
  • The _get_applicable_cpe_ids_for_machine method has been moved to ProfileInfo
  • Renamed _get_applicable_cpe_ids_for_machine method to get_list_of_cpe_platforms_that_satisfy_evaluation_target

…ysis system and specify which OVAL result report is for CPE
…nd rename the method to get_list_of_cpe_platforms_that_satisfy_evaluation_target
@Honny1 Honny1 force-pushed the change-loading-of-oval-reports branch from 55330f5 to 98df938 Compare April 12, 2023 12:58
@Honny1 Honny1 requested a review from evgenyz April 12, 2023 13:45
@evgenyz evgenyz merged commit 98df938 into OpenSCAP:main Apr 12, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants