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

Fix tests for file_permissions, file_owner, file_groupowner #11814

Merged
merged 3 commits into from
Apr 29, 2024

Conversation

mpurg
Copy link
Contributor

@mpurg mpurg commented Apr 12, 2024

Description:

  • Fixed tests for file_permissions, file_owner, file_groupowner templates

Rationale:

  • Several tests did not fail when run on a compliant system because the permissions/ownership were not modified

@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Apr 12, 2024
Copy link

openshift-ci bot commented Apr 12, 2024

Hi @mpurg. 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.

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 12, 2024

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

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

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

@mpurg mpurg changed the title Fix test for template file_permissions Fix tests for file_permissions, file_owner, file_groupowner Apr 12, 2024
@jan-cerny
Copy link
Collaborator

@mpurg

Do you have any rule that uses these templates with the file_regex parameter and where these templated scenarios aren't shadowed? I found multiple rules using these templates with the file_regex parameter but all of them shadow the templated test scenarios. Do you know where the changes are seen in practice? Also, do you plan to remove the shadowing files?

What about other templated test scenarios that also have the echo "Create specific tests for this rule because of regex" part? Can they be also changed?

@mpurg
Copy link
Contributor Author

mpurg commented Apr 16, 2024

Hm... I was implementing new rules for the Ubuntu 22.04 STIG (to be PR-ed) and noticed that these tests were failing. There weren't any custom tests in the rules I was using as a template (file_owner/groupowner/permissions_system_journal), and I didn't quite understand why this can't be solved by applying the solution from the remediation, so I adapted the tests.
I can implement the shadow tests if that's considered the correct approach.

Regarding the other test scenarios, I guess I didn't trip over them because they are pass tests and will thus succeed on a system where ownership/permissions are already compliant.

@dodys dodys requested a review from a team April 17, 2024 10:58
@dodys dodys self-assigned this Apr 17, 2024
@dodys dodys added Test Suite Update in Test Suite. Ubuntu Ubuntu product related. ok-to-test Used by openshift-ci bot. STIG STIG Benchmark related. and removed needs-ok-to-test Used by openshift-ci bot. labels Apr 17, 2024
@dodys
Copy link
Contributor

dodys commented Apr 17, 2024

/packit retest-failed

@jan-cerny
Copy link
Collaborator

@mpurg thanks for explanation!

@marcusburghardt
Copy link
Member

marcusburghardt commented Apr 18, 2024

@mpurg , could you rebase the PR so the sssd_enable_pam_services no longer fails in testing-farm tests, please?

@mpurg mpurg force-pushed the fix_test_file_permissions branch from 7f6c76e to 3b4c25b Compare April 18, 2024 10:23
@mpurg
Copy link
Contributor Author

mpurg commented Apr 18, 2024

@marcusburghardt done

@dodys
Copy link
Contributor

dodys commented Apr 26, 2024

/packit build

Copy link
Contributor

@dodys dodys left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

lenient_permissions.fail.sh test did not fail when run on a
compliant system because the permissions were not modified.
incorrect_owner.fail.sh test did not fail when run on a
compliant system because the ownership was not modified.

(cherry picked from commit 68664db)
incorrect_groupowner.fail.sh test did not fail when run on a
compliant system because the group ownership was not modified.

(cherry picked from commit 328c2ae)
@mpurg mpurg force-pushed the fix_test_file_permissions branch from 3b4c25b to eda77ab Compare April 26, 2024 18:20
Copy link

codeclimate bot commented Apr 26, 2024

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

View more on Code Climate.

@dodys
Copy link
Contributor

dodys commented Apr 29, 2024

/packit retest-failed

@dodys dodys merged commit 9e91c4d into ComplianceAsCode:master Apr 29, 2024
113 checks passed
@Mab879 Mab879 added this to the 0.1.73 milestone Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Used by openshift-ci bot. STIG STIG Benchmark related. Test Suite Update in Test Suite. Ubuntu Ubuntu product related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants