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

Enable templated SCE checks #12445

Merged
merged 25 commits into from
Oct 16, 2024
Merged

Conversation

jan-cerny
Copy link
Collaborator

@jan-cerny jan-cerny commented Sep 30, 2024

Description:

The build system seems to have option to have SCE content in templates. But in reality the code is broken and this option isn't used. This PR fixes the build system to enable SCE checks in templates. Then, it adds first SCEs to templates.

This PR contains:

  • templated SCE checks for service_enabled and service_disabled checks
  • fixes to the build system to enable templated SCE checks
  • refactoring and simplification of the code
  • a small test because this wasn't tested

Most of the PR changes is actually the refactoring and simplifying to satisfy Code Climate.

Rationale:

The SCE checks for state of systemd services can be used to check service status during a container image build. In the container image build environment OVAL checks for service status don't work because they depend on DBus which isn't available there. Using SCE checks there is easier than designing and implementing new OVAL test and object types.

Review Hints:

The first 3 commits are the main, the rest is either refactoring commits or extending commits.

Build with SCE:

ADDITIONAL_CMAKE_OPTIONS=-DSSG_SCE_ENABLED=yes ./build_product --derivatives rhel9

Unfortunately, the SCE checks are hard to test at this moment because if a rule has both SCE and OVAL checks then OpenSCAP prefers using OVAL check and ignores the SCE check and there is no way to choose one of them by the user. This problem will be solved by OpenSCAP/openscap#2165. I suggest that you disable the OVAL in a selected rules temporarily for the purpose of testing.

Add a templated SCE check to the service_enabled template.
The check could be used in the "podman build" environment
where the OVAL check don't work because they depend on dbus
which isn't available in this environment. OVAL check isn't
going to be removed.
Members of the `langs` list aren't strings but are `TemplatingLang`
named tuples. Therefore, it isn't possible to write condition
`if 'sce-bash' in langs` but we need to check the `name` member
of the named tuple instances instead.

This fix will make building templated SCE content from templates working.
We will rename the check `variable`.  The `check` variable was shadowed
later when OVAL check is inserted. If a rule had both SCE and OVAL
check, the element referencing the SCE check was overwritten by the
element referencing the OVAL check.  This will allow rules to have both
SCE and OVAL at the same time.
Reduce code complexity of to_xml_element method by extracting the
code responsible for creating the SCE check reference element to
a new method _add_sce_check_element.
Reduce code complexity by extracting code responsible for inserting
the element that references the OVAL check to a new method
_add_oval_check_element.
Reduce the complexity of the to_xml_element method by moving out
the code responsible for creating XCCDF check element referencing
OCIL to a new method.
Refactor code by extracting it to a new method
build_templated_sce_check.
This variable is set and incremented but never used.
The `SCE_DIR` argument of the `/build-scripts/build_sce.py` was
set to `/shared/checks/sce` and `${product}/checks/sce`. These
directories don't exist and it wouldn't make sense to create them.
We will remove the code working with these directories, this code
is effectively unused.
This commit converts `checks()` function to a new class `SCEBuilder`.
This change will allow us future refactoring and simplification.
Extracts code that processes static SCE checks to a new method
_build_static_sce_check.
Transform function `build_templated_sce_check` to a method
of the `SCEBuilder` class `_build_templated_sce_check`.
Simplify code in _build_templated_sce_check to prevent having
too many levels.
Move variable local_env_yaml to method where it is used.
Avoid passing it as a parameter if this isn't necessary.
Move code responsible for assembling a list of directories that should
be interated over.
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Sep 30, 2024
Copy link

openshift-ci bot commented Sep 30, 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

github-actions bot commented Sep 30, 2024

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

rhel8 (from CTF) Environment (using Fedora as testing environment)
Open in Gitpod

Fedora Testing Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

Copy link

github-actions bot commented Sep 30, 2024

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

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:12445

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

Extracting some code to a new method `_get_rule_sce_lang` allows
us to get rid of the `for` loop because there is always at most
1 item with name "sce-bash".

Addressing:
Function _build_templated_sce_check has a Cognitive Complexity of 8
(exceeds 7 allowed). Consider refactoring.
Extract code from the `_add_sce_check_element` method to multiple new
methods in order to reduce complexity.

Addressing:
Function _add_sce_check_element has a Cognitive Complexity of 16
(exceeds 7 allowed). Consider refactoring.
Move duplicate code to a new function.

Addressing:
Identical blocks of code found in 2 locations. Consider refactoring.
Addressing:
Expected 2 blank lines, found 1
@jan-cerny jan-cerny added the Infrastructure Our content build system label Oct 3, 2024
@jan-cerny jan-cerny added this to the 0.1.75 milestone Oct 3, 2024
Copy link

codeclimate bot commented Oct 9, 2024

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

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

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

View more on Code Climate.

@jan-cerny jan-cerny marked this pull request as ready for review October 11, 2024 09:45
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Oct 11, 2024
@ggbecker ggbecker self-assigned this Oct 14, 2024
@jan-cerny jan-cerny added the Image Mode Bootable containers and Image Mode RHEL label Oct 15, 2024
Copy link
Member

@ggbecker ggbecker left a comment

Choose a reason for hiding this comment

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

After running tests as suggested by the author, I was able to verify that the SCE content coming from templates is in the datastream and OpenSCAP is able to use them (with the openscap-engine-sce package installed) correctly. I made sure to disable the OVAL check from the template data for the rule I was testing so OpenSCAP uses SCE only as suggested by the author.

The code looks code to me.

The CI failures are not related to these changes.

@ggbecker ggbecker merged commit 53ce958 into ComplianceAsCode:master Oct 16, 2024
98 of 100 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Image Mode Bootable containers and Image Mode RHEL Infrastructure Our content build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants