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

Update sysctl template to check(and not fix) /usr/lib/sysctl.d directory #10637

Merged
merged 4 commits into from
Jul 14, 2023

Conversation

Xeicker
Copy link
Contributor

@Xeicker Xeicker commented May 25, 2023

Description:

  • Update the template to look into /usr/lib/sysctl.d directory

Rationale:

  • This PR: Don't scan dir with preconfigured sysctls #8718 removed the check of /usr/lib/sysctl.d for RHEL and OL. The rationale was that the files in there shouldn't be modified. So adding the check again, but taking into account precedence, that means that the rule will pass if the configuration is correctly set in /usr/lib/sysctl.d and not overriden, but wont fail in case the configuration there is wrong but overriden correctly somewhere else

Review Hints:

  • Tests should reflect this new behavior.
  • The criteria to pass/fail were obtained from the next table:
    ____| _ LC _ | _ LW _ | _ LM _ |
    NC _ | _ 1 __ | _ 1 __ | __ 1 __ |
    NW _| _ 0 __ | _ 0 __ | __ 0 __ |
    NM _| _ 1 __ | _ 0 __ | _ 0 (1) _ |

Correct: C
Wrong: W
Missing M
in /usr/lib/sysctl.d : L
not in /usr/lib/sysctl.d: N

rule passes: 1
rule fails: 0
rule result when missing_parameter_pass is set to true, if different: (x)

There are configurations set by packages in /usr/lib, so it is possible
to find there the expected configuration, but it is not recommended to
modify those files in case of a non compliant configuration. So
modified OVAL to check those files in a way that not touching them
would fix any non compliant scenario.

This means that the rule can pass if the expected conf is included in a
file in /usr/lib. But also if there is a non compliant value there, and
it is overwritten by a configuration in a different file.

Signed-off-by: Edgar Aguilar <edgar.aguilar@oracle.com>
Signed-off-by: Edgar Aguilar <edgar.aguilar@oracle.com>
@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label May 25, 2023
@openshift-ci
Copy link

openshift-ci bot commented May 25, 2023

Hi @Xeicker. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@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

@yuumasato yuumasato self-assigned this May 26, 2023
Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

Hi @Xeicker, sorry for the delay.

Nice work and thank you for the improvement.

shared/templates/sysctl/oval.template Outdated Show resolved Hide resolved
shared/templates/sysctl/oval.template Outdated Show resolved Hide resolved
shared/templates/sysctl/oval.template Show resolved Hide resolved
Adding test wrong_usr_lib_wrong_etc.fail.sh, to complement
wrong_usr_lib_correct_etc.pass.sh

Signed-off-by: Edgar Aguilar <edgar.aguilar@oracle.com>
- Fix comments in OVAL tests
- Remove OVAL test whith jinja when the criterion is also removed

Signed-off-by: Edgar Aguilar <edgar.aguilar@oracle.com>
@codeclimate
Copy link

codeclimate bot commented Jun 13, 2023

Code Climate has analyzed commit b39c007 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 52.8% (0.0% change).

View more on Code Climate.

@Xeicker Xeicker requested a review from yuumasato June 30, 2023 15:36
Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

@Xeicker Thanks for the update!

@yuumasato yuumasato added this to the 0.1.69 milestone Jul 14, 2023
@yuumasato yuumasato merged commit eb1f81a into ComplianceAsCode:master Jul 14, 2023
33 checks passed
@Mab879 Mab879 added the OVAL OVAL update. Related to the systems assessments. label Jul 14, 2023
mpurg added a commit to mpurg/ComplianceAsCode that referenced this pull request Apr 20, 2024
On Ubuntu /lib is symlinked to /usr/lib, thus /lib/sysctl.d
contains package-managed configs, which should not be
modified and can be incorrect if overriden elsewhere (see ComplianceAsCode#10637).
mpurg added a commit to mpurg/ComplianceAsCode that referenced this pull request Apr 22, 2024
On Ubuntu /lib is symlinked to /usr/lib, thus /lib/sysctl.d
contains package-managed configs, which should not be
modified and can be incorrect if overriden elsewhere (see ComplianceAsCode#10637).
mpurg added a commit to mpurg/ComplianceAsCode that referenced this pull request Apr 29, 2024
On Ubuntu /lib is symlinked to /usr/lib, thus /lib/sysctl.d
contains package-managed configs, which should not be
modified and can be incorrect if overriden elsewhere (see ComplianceAsCode#10637).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ok-to-test Used by openshift-ci bot. OVAL OVAL update. Related to the systems assessments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants