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

Add a test for pcre2 compatibility #11022

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

jan-cerny
Copy link
Collaborator

This test will ensure that the regular expressions used in OVAL checks are compatible with the pcre2 library. This will be important when OpenSCAP will move to use pcre2 internally.

The test collects regular expressions used in SCAP source data streams and tries to compile these regular expressions using the code from pcre2 library.

Unfortunately, a large amount of regular expressions used in our OVAL checks are composed dynamically using data collected during the scan from the evaluated system. These regular expressions won't be tested by this test that performs a static analysis. I think that possible problems with these expressions could be caught by Automatus test scenarios eventually.

Related to: https://bugzilla.redhat.com/show_bug.cgi?id=2128342

Review Hints:

  1. sudo pip3 install pcre2
  2. break some regular expression in an OVAL check in your favorite rule to make the regular expression invalid
  3. build the content
  4. cd build && ctest --verbose -R check-pcre2-compatibility-ssg-rhel9-ds.xml

@jan-cerny jan-cerny added the Test Suite Update in Test Suite. label Aug 24, 2023
@jan-cerny jan-cerny added this to the 0.1.70 milestone Aug 24, 2023
@github-actions
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

@Mab879 Mab879 self-assigned this Aug 24, 2023
Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I'm okay with not having this checked with mypy as the prce2 library doesn't support types.

I was able to get the test to fail by modifying a regex.

Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

Looks like we need to add --no-use-pep517 to the Debian 10 pip install.

@jan-cerny
Copy link
Collaborator Author

@Mab879 Where should I put this option? To pip command in .github/workflows/gate.yaml ?

@Mab879
Copy link
Member

Mab879 commented Aug 25, 2023

@Mab879 Where should I put this option? To pip command in .github/workflows/gate.yaml ?

Yes

@jan-cerny
Copy link
Collaborator Author

it seems that pip on Debian doesn't know this option

@Mab879
Copy link
Member

Mab879 commented Aug 25, 2023

I think I found a work around:

Add libpcre2-dev to the apt-get line.

Use python3 -m pip vs pip3. That worked in a container on my box.

@jan-cerny
Copy link
Collaborator Author

workaround doesn't work 😿

@Mab879
Copy link
Member

Mab879 commented Aug 25, 2023

It seems that the Python package cmake doesn't work with the older version of pip in Debian. We could upgrade pip using itself, but I will leave that up to you.

This test will ensure that the regular expressions used in OVAL
checks are compatible with the pcre2 library. This will be important
when OpenSCAP will move to use pcre2 internally.

The test collects regular expressions used in SCAP source data streams
and tries to compile these regular expressions using the code from
pcre2 library.

Unfortunately, a large amount of regular expressions used in our
OVAL checks are composed dynamically using data collected during
the scan from the evaluated system. These regular expressions won't
be tested by this test that performs a static analysis. I think
that possible problems with these expressions could be caught
by Automatus test scenarios eventually.

Related to: https://bugzilla.redhat.com/show_bug.cgi?id=2128342
@codeclimate
Copy link

codeclimate bot commented Aug 29, 2023

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

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

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

View more on Code Climate.

@jan-cerny
Copy link
Collaborator Author

I have made the test running only on the Fedora worker. I have rebased this PR on the top of the latest upstream master branch.

Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and the work to get the CI work!

@Mab879 Mab879 merged commit d817d80 into ComplianceAsCode:master Aug 29, 2023
37 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Test Suite Update in Test Suite.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants