Prepare OVAL object model for integration #11206

Honny1 · 2023-10-16T16:35:44Z

This PR adds and enhances the OVAL object model capabilities for integration into combine_ovals.py.

New capabilities:

Finding references to OVAL components
Remove OVAL definitions with all referenced OVAL components (preserves components used in other OVAL components).
Check if the OVAL definition is applicable to the product.
Check that the Affected attribute in the metadata is not empty.
Add error logging or other warnings

Enhanced capabilities:

Load from shorthands has been redesigned.
Fixed fields in the element generator in the generated OVAL document.

Review Hints:

Test script

test.py

Execution of test script:

python3 test.py

This test script generates one document for each OVAL definition with all referenced OVAL components and then performs validation using oscap oval validate FILE_PATH, but ignores validation for the ocp and eks products.

Note: the test script processes all OVAL documents in the build directory.

Unit tests

Run unit tests:

tox

openshift-ci · 2023-10-16T16:35:55Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

github-actions · 2023-10-16T16:37:22Z

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment

Oracle Linux 8 Environment

jan-cerny · 2023-10-17T08:00:32Z

@Honny1 What is the intention behind OVALContainer and OVALShorthand classes?

Honny1 · 2023-10-17T08:23:09Z

@jan-cerny OVALContainer is the parent class of the OVALDocument and OVALShorthand classes. OVALShorthand represents a shorthand with OVAL definitions.

jan-cerny · 2023-10-17T08:57:52Z

@Honny1 Cool, that sounds natural. But, I'm curious about their purpose. Won't the shorthand format be parsed and converted to objects?

Honny1 · 2023-10-18T07:12:25Z

/test all

Honny1 · 2023-10-18T14:46:59Z

@jan-cerny Yes, OVALShorthand is an object representation for shorthand.

jan-cerny · 2023-10-19T09:35:45Z

@Honny1 And why do we need this representation? Feel free to reply in a more detailed way.

Honny1 · 2023-10-23T09:13:19Z

@jan-cerny When loading shortcuts, you can load a shortcut containing several OVAL definitions. One of them may be applicable for a given platform and the other may not. For this reason, I decided to load the entire shortcut into an OVALShorthand object and then remove the unusable OVAL definition. The OVALShordhand inherits capabilities from OVALContainer. For example, look up references in an OVAL definition and then just preserve all referenced components of the OVAL definition. These capabilities will also be used to slim down OVAL documents for lightweight DS.

jan-cerny · 2023-10-24T08:57:43Z

ssg/oval_object_model/oval_container.py

+    return "external_variable" in component.tag
+
+
+def _handle_existing_id(component, component_dict):


This function looks very similar to a part of append function in ssg/build_ovals.py. You should first extract the common code out to a function and then call that function both here and there to prevent unwanted code duplication and responsibility duplication.

In the next PR, I plan to rework ssg/build_ovals.py and combine_ovals.py and remove some duplicated and unused code. So this duplication will be resolved.

jan-cerny · 2023-10-24T08:58:05Z

ssg/oval_object_model/oval_container.py

+
+
+def _handle_existing_id(component, component_dict):
+    # ID is identical, but OVAL entities are semantically difference =>


Suggested change

# ID is identical, but OVAL entities are semantically difference =>

# ID is identical, but OVAL entities are semantically different =>

jan-cerny · 2023-10-24T09:04:28Z

ssg/oval_object_model/oval_shorthand.py

+        if value is None:
+            return True
+        return False


This could be simplified to return value is None.

jan-cerny · 2023-10-24T09:08:56Z

ssg/oval_object_model/oval_container.py

+                    "ERROR: A definition that does not match the rule ID '{}'"
+                    " is not present in the shorthand."


I'm confused by this. Should the "does not" be there?

jan-cerny · 2023-10-24T09:11:37Z

tests/unit/ssg-module/test_oval_object_model/test_oval_document.py

@@ -0,0 +1,51 @@
+import pytest
+
+from test_load_and_store import _load_oval_document, OVAL_DOCUMENT_PATH


A small nit pick: I think that the tests are somehow exceptional and it actually could be more readable if the path is redefined here in this file instead of importing the constant from other unit test file.

Honny1 · 2023-10-25T06:26:52Z

/packit retest-failed

…o the product

…tadata field is not empty

codeclimate · 2023-10-25T14:19:43Z

Code Climate has analyzed commit 5eb2824 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 88.2% (50% is the threshold).

This pull request will bring the total coverage in the repository to 58.4%.

View more on Code Climate.

jan-cerny

I hav found that the feedback from previous drive-by's has been addressed, but I'm sad that there are no PR comments that would make this situation transparent for everyone.

I can see that almost all the changes are in the oval_object_model module and the unit tests for the oval_object_model module. At this moment, the oval_object_model module isn't used anywhere in the build system. The only changes outside of this that indeed affect our build system are small refactoring changes in ssg/build_ovals.py and ssg/utils.py that is the extraction of code to the get_fixed_product_version function. From this point of view, the changes are safe for the project. However, I'm missing this information in the PR description. Also, I would prefer if this specific refactoring change would be submitted as a separate PR.

I have played with the attached test.py script. I have noticed that it assumes the existence of the ./build/single_ovals/ directory which isn't created by that. But if I create this directory manually, it creates the expected OVAL files.

Overall, I evaluate the contents of this PR as a good preparation for further steps. Thanks for having the unit tests!

openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Oct 16, 2023

Honny1 force-pushed the integration-of-OVAL-Object-mode branch from 4b54a04 to 853d46b Compare October 16, 2023 16:41

Honny1 added enhancement General enhancements to the project. OVAL OVAL update. Related to the systems assessments. New Feature Issues or pull requests related to new Features. labels Oct 16, 2023

Honny1 force-pushed the integration-of-OVAL-Object-mode branch 4 times, most recently from f3913d4 to f6fbece Compare October 16, 2023 17:14

Honny1 force-pushed the integration-of-OVAL-Object-mode branch 9 times, most recently from 5ccd6e8 to d71e097 Compare October 17, 2023 20:03

jan-cerny self-assigned this Oct 18, 2023

Honny1 force-pushed the integration-of-OVAL-Object-mode branch 3 times, most recently from c31c2e6 to a6cdb3f Compare October 18, 2023 14:03

Honny1 force-pushed the integration-of-OVAL-Object-mode branch from a6cdb3f to 8efe315 Compare October 23, 2023 16:30

jan-cerny reviewed Oct 24, 2023

View reviewed changes

Honny1 force-pushed the integration-of-OVAL-Object-mode branch 2 times, most recently from 265ea7a to 7ccbeea Compare October 24, 2023 12:41

Honny1 force-pushed the integration-of-OVAL-Object-mode branch from 7ccbeea to 51c55f5 Compare October 25, 2023 09:08

Honny1 added 12 commits October 25, 2023 16:04

Add a method to the OVAL Definition that checks if it is applicable t…

bfa253f

…o the product

Add a method to the OVAL definition that checks if the Affected in Me…

564d1a8

…tadata field is not empty

Add methods for obtaining references

3253f05

Update info about generator in genereted OVAL document

2f4c86f

Add logging and fix imports

61d7cea

Update tests

ed09f13

Create class for stroing references of OVAL definitions

31fde9b

Create OVAL container class

1714dfb

Create OVAL shorthand class

3f3d7d4

Inherit features from OVAL container to OVAL document

3f14a04

Fix missing tag of affected element child

9d48820

Add Unit tests

5eb2824

Honny1 force-pushed the integration-of-OVAL-Object-mode branch from 51c55f5 to 5eb2824 Compare October 25, 2023 14:04

Honny1 marked this pull request as ready for review October 25, 2023 14:23

openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Oct 25, 2023

Honny1 requested a review from jan-cerny October 25, 2023 14:23

jan-cerny added this to the 0.1.71 milestone Oct 25, 2023

jan-cerny approved these changes Oct 26, 2023

View reviewed changes

jan-cerny merged commit 246f43d into ComplianceAsCode:master Oct 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare OVAL object model for integration #11206

Prepare OVAL object model for integration #11206

Honny1 commented Oct 16, 2023 •

edited

Loading

openshift-ci bot commented Oct 16, 2023

github-actions bot commented Oct 16, 2023

jan-cerny commented Oct 17, 2023

Honny1 commented Oct 17, 2023

jan-cerny commented Oct 17, 2023

Honny1 commented Oct 18, 2023

Honny1 commented Oct 18, 2023

jan-cerny commented Oct 19, 2023

Honny1 commented Oct 23, 2023

jan-cerny Oct 24, 2023

Honny1 Oct 24, 2023

jan-cerny Oct 24, 2023

jan-cerny Oct 24, 2023

jan-cerny Oct 24, 2023

jan-cerny Oct 24, 2023

Honny1 commented Oct 25, 2023

codeclimate bot commented Oct 25, 2023

jan-cerny left a comment •

edited

Loading

		return "external_variable" in component.tag


		def _handle_existing_id(component, component_dict):



		def _handle_existing_id(component, component_dict):
		# ID is identical, but OVAL entities are semantically difference =>

		"ERROR: A definition that does not match the rule ID '{}'"
		" is not present in the shorthand."

		@@ -0,0 +1,51 @@
		import pytest

		from test_load_and_store import _load_oval_document, OVAL_DOCUMENT_PATH

Prepare OVAL object model for integration #11206

Prepare OVAL object model for integration #11206

Conversation

Honny1 commented Oct 16, 2023 • edited Loading

Review Hints:

Test script

Unit tests

openshift-ci bot commented Oct 16, 2023

github-actions bot commented Oct 16, 2023

jan-cerny commented Oct 17, 2023

Honny1 commented Oct 17, 2023

jan-cerny commented Oct 17, 2023

Honny1 commented Oct 18, 2023

Honny1 commented Oct 18, 2023

jan-cerny commented Oct 19, 2023

Honny1 commented Oct 23, 2023

jan-cerny Oct 24, 2023

Choose a reason for hiding this comment

Honny1 Oct 24, 2023

Choose a reason for hiding this comment

jan-cerny Oct 24, 2023

Choose a reason for hiding this comment

jan-cerny Oct 24, 2023

Choose a reason for hiding this comment

jan-cerny Oct 24, 2023

Choose a reason for hiding this comment

jan-cerny Oct 24, 2023

Choose a reason for hiding this comment

Honny1 commented Oct 25, 2023

codeclimate bot commented Oct 25, 2023

jan-cerny left a comment • edited Loading

Choose a reason for hiding this comment

Honny1 commented Oct 16, 2023 •

edited

Loading

jan-cerny left a comment •

edited

Loading