-
Notifications
You must be signed in to change notification settings - Fork 718
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
New SLE 12/15 rule all_apparmor_profiles_in_enforce_complain_mode whi… #10064
New SLE 12/15 rule all_apparmor_profiles_in_enforce_complain_mode whi… #10064
Conversation
Hi @rumch-se. 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 Once the patch is verified, the new status will be reflected by the 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few fixes needed
...x_os/guide/system/apparmor/all_apparmor_profiles_in_enforce_complain_mode/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/apparmor/all_apparmor_profiles_in_enforce_complain_mode/oval/shared.xml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/apparmor/all_apparmor_profiles_in_enforce_complain_mode/oval/shared.xml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/apparmor/all_apparmor_profiles_in_enforce_complain_mode/oval/shared.xml
Outdated
Show resolved
Hide resolved
<linux:state state_ref="no_processes_which_are_unconfined" /> | ||
</linux:apparmorstatus_test> | ||
|
||
<linux:apparmorstatus_object id="apparmor_exists_on_our_system" version="1"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can consolidate this is one line:
<linux:apparmorstatus_object id="apparmor_exists_on_our_system" version="1" />
linux_os/guide/system/apparmor/all_apparmor_profiles_in_enforce_complain_mode/oval/shared.xml
Outdated
Show resolved
Hide resolved
fyi, since we don't have a SUSE maintainer and this rule also is needed for Ubuntu, I've assigned it to me. |
This rule is not buildable in Ubuntu (and Debian), as apparmor schema was only added in OVAL Format 5.11.2 and Ubuntu now only has OVAL Format 5.11.1. A separate OVAL will be sent after this is merged to make it work in Ubuntu. @rumch-se can you confirm that this is also the case for OpenSUSE, as the logs are showing that it doesn't recognize the apparmor schema. |
Hello @dodys |
Hey @rumch-se, I meant please check the OVAL format version in opensuse, I believe you have OVAL Format < 5.11.2 and therefore that's why the build/test is failing here. For that you must run: |
Hello @dodys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
@marcusburghardt this rule will fail to build in a few of the containers because of the OVAL version, as apparmor tests were added to OVAL Format 5.11.2 only. Can we merge it as-is?
...x_os/guide/system/apparmor/all_apparmor_profiles_in_enforce_complain_mode/ansible/shared.yml
Outdated
Show resolved
Hide resolved
...x_os/guide/system/apparmor/all_apparmor_profiles_in_enforce_complain_mode/ansible/shared.yml
Outdated
Show resolved
Hide resolved
...x_os/guide/system/apparmor/all_apparmor_profiles_in_enforce_complain_mode/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/apparmor/all_apparmor_profiles_in_enforce_complain_mode/oval/shared.xml
Outdated
Show resolved
Hide resolved
Is the package already available but the containers are not updated? I am not sure about the impact of CI tests if we merge it. @mildas , maybe you can help us on this? |
Good morning @marcusburghardt and @dodys |
I have talked to @mildas offline and he confirmed my concern of merging this PR now. It would impact the CI tests and cause failures in other PRs. The ideal would be to ensure the OVAL Format 5.11.2 support before merging. |
Should we rename the oval to be sle15.xml so it won't get used in other platforms? Would that fix this issue temporarily? |
Interesting idea @dodys . I believe this should work. What do you think @yuumasato ? |
@rumch-se could you please rename the oval file from |
Hello @dodys |
Thanks! |
@marcusburghardt ok, so the renaming of the oval file will not prevent tests to fail. |
Hi @rumch-se, could you please rebase this PR and remove the commit to add the OVAL. Since your OVAL is dependent of OVAL format 5.11.2 and most of the CI only has OVAL format 5.11.1, we won't be able to merge it as this will break the CI. |
…ch covers CIS recommendation
4e10996
to
bb7dd28
Compare
Hello @dodys |
Code Climate has analyzed commit c04426f 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 51.7% (0.0% change). View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm thanks!
The Automatus tests are failing because the rule is restricted to |
Although I don't have a system to properly test Therefore, I am overriding CODEOWNERS since a SUSE approver is not currently available. |
…ch covers CIS recommendation
Description:
Rationale: