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

Remove Support for OVAL 5.10 #9604

Merged
merged 8 commits into from
Oct 11, 2022
Merged

Conversation

Mab879
Copy link
Member

@Mab879 Mab879 commented Sep 30, 2022

Description:

This PR removes OVAL 5.10 support from the project.

Rationale:

Implements #9451

@Mab879 Mab879 added Infrastructure Our content build system Highlight This PR/Issue should make it to the featured changelog. labels Sep 30, 2022
@Mab879 Mab879 added this to the 0.1.65 milestone Sep 30, 2022
@github-actions
Copy link

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

@matejak matejak self-assigned this Oct 3, 2022
@matejak
Copy link
Member

matejak commented Oct 3, 2022

It looks like that the datastream diff failed in this PR - there was a change in the OVAL. Any ideas what happened?

@matejak
Copy link
Member

matejak commented Oct 3, 2022

The systemd-related test will fail in Automatus, because the container environment doesn't work well with this systemd-related rule.

@matejak
Copy link
Member

matejak commented Oct 3, 2022

One more thing - the build_product script offers the possibility to select the OVAL version - either the 5.10 should be removed, or the whole option could be removed.

@ggbecker
Copy link
Member

ggbecker commented Oct 5, 2022

What about these occurrences of 5.10 in the project? Do we need to remove them?

linux_os/guide/services/obsolete/r_services/no_host_based_files/oval/shared.xml:
  1: <def-group oval_version="5.10">

linux_os/guide/services/obsolete/r_services/no_user_host_based_files/oval/shared.xml:
  1: <def-group oval_version="5.10">

linux_os/guide/system/software/integrity/endpoint_security_software/mcafee_security_software/mcafee_antivirus_definitions_updated/oval/shared.xml:
  1: <def-group oval_version="5.10">

linux_os/guide/system/software/integrity/fips/enable_fips_mode/oval/rhcos4.xml:
  1: <def-group oval_version="5.10">

linux_os/guide/system/software/integrity/fips/enable_fips_mode/oval/shared.xml:
  1: <def-group oval_version="5.10">

linux_os/guide/system/software/integrity/fips/etc_system_fips_exists/oval/shared.xml:
  1: <def-group oval_version="5.10">

linux_os/guide/system/software/integrity/fips/grub2_enable_fips_mode/oval/shared.xml:
  1: <def-group oval_version="5.10">

linux_os/guide/system/software/updating/clean_components_post_updating/oval/shared.xml:
  1: <def-group oval_version="5.10">

linux_os/guide/system/software/updating/clean_components_post_updating/oval/sle12.xml:
  1: <def-group oval_version="5.10">

linux_os/guide/system/software/updating/clean_components_post_updating/oval/sle15.xml:
  1: <def-group oval_version="5.10">

Similarly for "5.11", do we need to explicitly state the oval_version attribute?

@Mab879
Copy link
Member Author

Mab879 commented Oct 5, 2022

What about these occurrences of 5.10 in the project? Do we need to remove them?

It might be a good idea and I will remove those references.

Similarly for "5.11", do we need to explicitly state the oval_version attribute?

The build system will automatically add it.

CMakeLists.txt Outdated
@@ -145,11 +145,12 @@ cmake_dependent_option(ENABLE_PYTHON_COVERAGE "Enable Python tests with coverage
find_package(OpenSCAP REQUIRED)

if (SSG_TARGET_OVAL_MAJOR_VERSION EQUAL "5" AND SSG_TARGET_OVAL_VERSION_MINOR EQUAL "11" AND NOT "${OSCAP_V_OUTPUT}" MATCHES "OVAL Version: 5.11")
message(FATAL_ERROR "Your version of OpenSCAP does not support OVAL 5.11, please switch the OVAL target version to 5.10 or lower. $ cmake -DSSG_TARGET_OVAL_MINOR_VERSION=10 ../")
message(FATAL_ERROR "Your version of OpenSCAP does not support OVAL 5.11, please upgrade to a newer version of OpenSCAP.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can go even further and remove this check, because the "OVAL Version: 5.11" has been introduced in OpenSCAP 1.2.2 so the error happens on RHEL 7.1 and older and I don't think anybody uses these old systems for content development.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems reasonable to me. Some basic research shows that OpenSCAP 1.2.2+ is in Fedora, RHEL, Ubuntu, Debian, and OpenSUSE.

@@ -85,7 +85,7 @@ print_help()
printf '%s\n' "Wipes out contents of the 'build' directory and builds only and only the given products."
printf 'Usage: %s [-o|--oval <VERSION>] [-b|--builder <BUILDER>] [-j|--jobs <arg>] [--(no-)debug] [--(no-)derivatives] [--(no-)ansible-playbooks] [--(no-)bash-scripts] [-d|--(no-)datastream-only] [-p|--(no-)profiling] [-h|--help] [<product-1>] ... [<product-n>] ...\n' "$0"
printf '\t%s\n' "<product>: Products to build, ALL means all products (defaults for <product>: 'ALL')"
printf '\t%s\n' "-o, --oval: OVAL version. Can be one of: '5.10', '5.11' and 'auto' (default: 'auto')"
printf '\t%s\n' "-o, --oval: OVAL version. Can be one of: '5.11' or 'auto' (default: 'auto')"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it make sense to keep the option here when it basically has no effect?

Copy link
Member Author

Choose a reason for hiding this comment

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

Main reason I kept was for backword compability you still set 5.11 if you want to. If we don't see this as a valid reason and don't see OVAL 5.12 coming out, we may want to remove this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok

@codeclimate
Copy link

codeclimate bot commented Oct 6, 2022

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

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

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

View more on Code Climate.

@openshift-ci
Copy link

openshift-ci bot commented Oct 6, 2022

@Mab879: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-rhcos4-high b165f2d link true /test e2e-aws-rhcos4-high

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@jan-cerny jan-cerny requested a review from matejak October 7, 2022 12:18
Copy link
Member

@matejak matejak left a comment

Choose a reason for hiding this comment

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

LGTM, and it seems to work, good riddance.

Coverage difference can be disregarded, as there are minor changes to files that don't have test coverage.

@matejak matejak merged commit 7f96de4 into ComplianceAsCode:master Oct 11, 2022
@Mab879 Mab879 deleted the remove_oval_5_10 branch October 11, 2022 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Highlight This PR/Issue should make it to the featured changelog. Infrastructure Our content build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants