-
Notifications
You must be signed in to change notification settings - Fork 685
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
Added a new rule firewalld_service_disabled #9941
Added a new rule firewalld_service_disabled #9941
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. |
7db0030
to
5930f5e
Compare
@rumch-se please rebase. |
Please rename the rule to |
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.
Thank you for the new rule. It needs just some small adjustments. Please see comments.
title: 'Verify firewalld service disabled' | ||
|
||
description: |- | ||
firewalld (Dynamic Firewall Manager) provides a dynamically managed firewall with |
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.
firewalld (Dynamic Firewall Manager) provides a dynamically managed firewall with | |
Firewalld (Dynamic Firewall Manager) provides a dynamically managed firewall with |
{{{ describe_service_disable(service="firewalld") }}} | ||
|
||
rationale: |- | ||
Running both <b>nftables.service</b> and <b>firewalld.service</b> may lead to conflict and |
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.
I would remove the mention of nftables and write something like... running Firewalld along other firewall solution may cause problems.
The reason is to keep the rule generic.
5930f5e
to
1b7eb04
Compare
Hello @vojtapolasek |
controls/cis_sle15.yml
Outdated
@@ -1104,6 +1105,7 @@ controls: | |||
status: automated | |||
rules: | |||
- package_firewalld_removed | |||
- firewalld_service_disabled |
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.
This line also needs the new rule name.
Also I see that you are adding some CCE identifiers into the pool... this should not be part of this PR. |
Hello @vojtapolasek I did not added these CCE numbers to the pull. I think that this is because of rebase and they are related to another (rules). Because of last changes which use CCE numbers and because validation of each PRs has some time lag we will have this effect of discrepancy. To me more clear - this PR should "inherit" available numbers from previous PR which is 9937, but 9937 has to "inherit" available CCE numbers from 9932, 9932 from 9931, 9931 from 9930, 9930 from 9927, and 9927 from 9900. Unfortunately in this chain of PRs there are some PRs approved /9932 and 9937/ and the rest not - i.e. we have something like gaps related to the usage of available numbers. Have a nice day. |
Hello, |
Code Climate has analyzed commit 1ae99ad 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 49.8% (1.0% change). View more on Code Climate. |
Automatus is failing because the rule is sle15 only. Thanks for updates, merging. |
One of @ComplianceAsCode/suse-maintainers has to review this and accept. |
Description:
Rationale:
(Automated)
Review Hints:
Review hints here. Replace this text. Don't use the italics format!
Use this optional section to give any relevant information which could help the reviewer to more quickly and assertively understand and test the changes.
Good examples are useful commands, if it is better to review all commits together or in a suggested sequence, any relevant discussion in other PRs or issues, etc.