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

Removing unused variables from the datastream #11858

Merged
merged 4 commits into from
Apr 26, 2024

Conversation

Honny1
Copy link
Collaborator

@Honny1 Honny1 commented Apr 19, 2024

Description:

This PR removes unused variables in the datastream. It also fixes missing variables for thin data streams. The problem was caused by removing XCCDF groups without rules because the removed groups contained variables that were used in rules from other groups.

The problem can be replicated by building a thin ds for the enable_fips_mode rule.

./build_product fedora --rule-id enable_fips_mode
oscap ds sds-validate ./build/ssg-fedora-ds.xml

To solve this problem, an in-memory XCCDF benchmark is created that contains all the rules from which information is obtained about which rule uses which variable. This step slows down the build.

Review Hints:

Verify that the problem does not occur with a single rule:

./build_product fedora --rule-id enable_fips_mode
oscap ds sds-validate ./build/ssg-fedora-ds.xml

For all thin ds, you can run a test from the test suite. (A validation test has been implemented for this problem)

./build_product fedora --thin
python3 -m pytest -n auto tests/test_thin_ds.py

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Apr 19, 2024
Copy link

openshift-ci bot commented Apr 19, 2024

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

Copy link

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

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

Copy link

github-actions bot commented Apr 19, 2024

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:11858
This image was built from commit: c904b62

Click here to see how to deploy it

If you alread have Compliance Operator deployed:
utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:11858

Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and:
CONTENT_IMAGE=ghcr.io/complianceascode/k8scontent:11858 make deploy-local

@Honny1 Honny1 force-pushed the variables-clean branch 6 times, most recently from 4e55df6 to 6a9e5cd Compare April 22, 2024 09:16
@marcusburghardt marcusburghardt added the Infrastructure Our content build system label Apr 22, 2024
@Honny1 Honny1 marked this pull request as ready for review April 22, 2024 13:26
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Apr 22, 2024
@jan-cerny
Copy link
Collaborator

@Honny1 I have built the rhel9 product from this PR's branch and I have noticed a problem in the build/ssg-rhel9-ds.xml. There are some variables that are referenced in refine-value elements in Profile elements but there is no corresponding Value element with that ID, so these references are dangling. For example, var_sudo_dedicated_group or var_sudo_umask that are refined in ANSSI High profile but aren't defined in the benchmark.

@jan-cerny jan-cerny self-assigned this Apr 23, 2024
@Honny1
Copy link
Collaborator Author

Honny1 commented Apr 25, 2024

@jan-cerny
I added variables present in the profile to the variable selection. So now the variables should be present in the Datastream.

Copy link

codeclimate bot commented Apr 25, 2024

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

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

This pull request will bring the total coverage in the repository to 59.4% (0.1% change).

View more on Code Climate.

@Honny1
Copy link
Collaborator Author

Honny1 commented Apr 25, 2024

/packit rebuild-failed

@Honny1 Honny1 requested a review from jan-cerny April 25, 2024 12:21
Copy link
Collaborator

@jan-cerny jan-cerny left a comment

Choose a reason for hiding this comment

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

I like the PR. I have checked the review hints. I have also compared the built data streams for RHEL 8 and 9 products. I haven't found any problem.

sub_el.get("idref").replace("xccdf_org.ssgproject.content_value_", "")
)
out_var_ids[
rule.get("id").replace("xccdf_org.ssgproject.content_rule_", "")
Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be better to extract this code to a variable

@marcusburghardt marcusburghardt added this to the 0.1.73 milestone Apr 26, 2024
@jan-cerny jan-cerny merged commit 3fddb5e into ComplianceAsCode:master Apr 26, 2024
113 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Our content build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants