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 missing_file_pass to owner/group-owner rules #8194

Closed
wants to merge 1 commit into from

Conversation

dodys
Copy link
Contributor

@dodys dodys commented Feb 10, 2022

Description:

With commit fe36b35 file_groupowner and file_owner started to filter/
exclude symlinks. Unfortunately the rules touched here, have some main
paths that are actually a symlink (e.g. /lib64 on Ubuntu) and the filter
together with the all_exist rule in their OVAL, make the rule evaluation
to fail. By adding missing_file_pass, we change it to any_exist.

With commit fe36b35 file_groupowner and file_owner started to filter/
exclude symlinks. Unfortunately the rules touched here, have some main
paths that are actually a symlink (e.g. /lib64 on Ubuntu) and the filter
together with the all_exist rule in their OVAL, make the rule evaluation
to fail. By adding missing_file_pass, we change it to any_exist.
@openshift-ci
Copy link

openshift-ci bot commented Feb 10, 2022

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

@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Feb 10, 2022
@github-actions
Copy link

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

Open in Gitpod

@Mab879
Copy link
Member

Mab879 commented Feb 10, 2022

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Used by openshift-ci bot. and removed needs-ok-to-test Used by openshift-ci bot. labels Feb 10, 2022
@ggbecker
Copy link
Member

I believe that in this case we should provide missing_file_pass test scenarios to override the templated test.

@jan-cerny
Copy link
Collaborator

Who will provide them?

@ggbecker
Copy link
Member

Who will provide them?

let's ping @dodys and see if he replies in a couple of days... otherwise we can submit some tests to get this merged.

@dodys
Copy link
Contributor Author

dodys commented Feb 23, 2022

Who will provide them?

let's ping @dodys and see if he replies in a couple of days... otherwise we can submit some tests to get this merged.

sorry, I haven't had the time to take a look at it.
Also, I'm not sure what kind of tests you think it would be good as the missing_file_test.pass.sh template test is just doing a rm -rf

@ggbecker
Copy link
Member

ggbecker commented Mar 8, 2022

Who will provide them?

let's ping @dodys and see if he replies in a couple of days... otherwise we can submit some tests to get this merged.

sorry, I haven't had the time to take a look at it. Also, I'm not sure what kind of tests you think it would be good as the missing_file_test.pass.sh template test is just doing a rm -rf

The logs have:

+ rm -f /lib/
rm: cannot remove '/lib/': Is a directory
+ rm -f /lib64/
rm: cannot remove '/lib64/': Is a directory
+ rm -f /usr/lib/
rm: cannot remove '/usr/lib/': Is a directory
+ rm -f /usr/lib64/
rm: cannot remove '/usr/lib64/': Is a directory

I believe the templated scenario should be updated to consider if the path is a directory, but apart from that... it's probably not a good idea to remove those directories even in a test, it might break the system. So a custom test scenario might help in this case.

@dodys
Copy link
Contributor Author

dodys commented Mar 31, 2022

This PR doesn't seem needed anymore with the fixes done in PR #8456
As soon as that PR makes to master I will re-run the tests and confirm it here.

@dodys
Copy link
Contributor Author

dodys commented Apr 4, 2022

This PR doesn't seem needed anymore with the fixes done in PR #8456 As soon as that PR makes to master I will re-run the tests and confirm it here.

Confirmed and closing this PR.

@dodys dodys closed this Apr 4, 2022
@dodys dodys deleted the fix-ownership-rules branch March 31, 2023 17:21
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants