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

Improve service_disabled template #10026

Merged
merged 3 commits into from
Jan 6, 2023

Conversation

marcusburghardt
Copy link
Member

Description:

This rule is a rebase for the #9468 plus some additional improvements in conditionals.
In alignment to the remediation, conditionals for systemd units were included in the two test scenario scripts by the initial PR.

After the rebase, I improved the conditionals in test scenarios and bash remediation to remove unnecessary grep from the command.

Rationale:

Better alignment between remediation and test scenarios.
Simpler conditionals with faster commands.

Review Hints:

For testing this changes I chose some random rules which uses the service_disabled template.
This should be enough for testing the changes.

maage and others added 3 commits January 5, 2023 14:28
They have service like: <name>@.service and are only meant to be used
via socket activation.

Also there is considerable speed difference between slow:
	systemctl list-unit-files
and fast:
	systemctl list-unit-files <name>.<type>

Use fast solution here.
Since the unit is now informed in the for the sysctl list-unit-files,
the return code will already be 0 if the unit exists and 1 otherwise.
So, this | grep is no longer necessary.
@marcusburghardt marcusburghardt added Test Suite Update in Test Suite. Bash Bash remediation update. labels Jan 6, 2023
@marcusburghardt marcusburghardt added this to the 0.1.66 milestone Jan 6, 2023
@github-actions
Copy link

github-actions bot commented Jan 6, 2023

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

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@codeclimate
Copy link

codeclimate bot commented Jan 6, 2023

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

View more on Code Climate.

@jan-cerny jan-cerny self-assigned this Jan 6, 2023
@jan-cerny
Copy link
Collaborator

@marcusburghardt

For testing this changes I chose some random rules which uses the service_disabled template.

Can you give an example of a rule that had problems problems before and is fixed by this PR?

Also, my understanding of the original pr #9468 was that there was a problem with service_telnet_disabled but this rule still fails the automatus's tests even with this PR. I think there might be a problem related to the fact the the telnet service isn't provided by the telnet package but by the telnet-server package and the rule rule doesn't reflect this. Are the rules related to telnet service related to this PR and are they expected to be fixed here?

@marcusburghardt
Copy link
Member Author

@marcusburghardt

For testing this changes I chose some random rules which uses the service_disabled template.

Can you give an example of a rule that had problems problems before and is fixed by this PR?

Also, my understanding of the original pr #9468 was that there was a problem with service_telnet_disabled but this rule still fails the automatus's tests even with this PR. I think there might be a problem related to the fact the the telnet service isn't provided by the telnet package but by the telnet-server package and the rule rule doesn't reflect this. Are the rules related to telnet service related to this PR and are they expected to be fixed here?

Actually, this PR is not fixing any issue, but only improving the conditional. In practice, the result is the same.
Regarding the issue with the service_telnet_disabled, you are correct. It is because the rule is using the wrong package name telnet (which is just a client) instead telnet-server. I saw this during my tests but I will update the rule in a separate PR.

So, in short, neither this PR or the old one are not related to the service_telnet_disabled issue but just the conditional improvement.

@marcusburghardt
Copy link
Member Author

Once this PR is merged, I will send another PR fixing the service_telnet_disabled rule.

@marcusburghardt
Copy link
Member Author

/retest

@jan-cerny
Copy link
Collaborator

thanks for clarification!

@jan-cerny jan-cerny merged commit ed0bd2c into ComplianceAsCode:master Jan 6, 2023
@marcusburghardt marcusburghardt deleted the rebase_9468 branch January 9, 2023 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bash Bash remediation update. Test Suite Update in Test Suite.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants